CLang's optimizer is more aggressive at inlining functions than
gcc and so will often inline functions that our tests want to
mock-override. This causes the test to fail in bizarre ways.
We don't want to disable inlining completely, but we must at
least prevent inlining of mocked functions. Fortunately there
is a 'noinline' attribute that lets us control this per function.
A syntax check rule is added that parses tests/*mock.c to extract
the list of functions that are mocked (restricted to names starting
with 'vir' prefix). It then checks that src/*.h header file to
ensure it has a 'ATTRIBUTE_NOINLINE' annotation. This should prevent
use from bit-rotting in future.
Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
---
build-aux/mock-noinline.pl | 72 +++++++++++++++++++++++++++++++++++++++++
cfg.mk | 6 +++-
src/internal.h | 17 ++++++++++
src/qemu/qemu_capspriv.h | 2 +-
src/rpc/virnetsocket.h | 6 ++--
src/util/vircommand.h | 2 +-
src/util/vircrypto.h | 2 +-
src/util/virfile.h | 2 +-
src/util/virhostcpu.h | 4 +--
src/util/virmacaddr.h | 2 +-
src/util/virnetdev.h | 9 +++---
src/util/virnetdevip.h | 2 +-
src/util/virnetdevopenvswitch.h | 2 +-
src/util/virnetdevtap.h | 6 ++--
src/util/virnuma.h | 16 ++++-----
src/util/virrandom.h | 6 ++--
src/util/virscsi.h | 2 +-
src/util/virscsivhost.h | 2 +-
src/util/virtpm.h | 2 +-
src/util/virutil.h | 10 +++---
src/util/viruuid.h | 2 +-
21 files changed, 135 insertions(+), 39 deletions(-)
create mode 100644 build-aux/mock-noinline.pl
diff --git a/build-aux/mock-noinline.pl b/build-aux/mock-noinline.pl
new file mode 100644
index 0000000..eafe20d
--- /dev/null
+++ b/build-aux/mock-noinline.pl
@@ -0,0 +1,72 @@
+#!/usr/bin/perl
+
+my %noninlined;
+my %mocked;
+
+# Functions in public header don't get the noinline annotation
+# so whitelist them here
+$noninlined{"virEventAddTimeout"} = 1;
+
+foreach my $arg (@ARGV) {
+ if ($arg =~ /\.h$/) {
+ #print "Scan header $arg\n";
+ &scan_annotations($arg);
+ } elsif ($arg =~ /mock\.c$/) {
+ #print "Scan mock $arg\n";
+ &scan_overrides($arg);
+ }
+}
+
+my $warned = 0;
+foreach my $func (keys %mocked) {
+ next if exists $noninlined{$func};
+
+ $warned++;
+ print STDERR "$func is mocked at $mocked{$func} but missing noinline annotation\n";
+}
+
+exit $warned ? 1 : 0;
+
+
+sub scan_annotations {
+ my $file = shift;
+
+ open FH, $file or die "cannot read $file: $!";
+
+ my $func;
+ while (<FH>) {
+ if (/^\s*(\w+)\(/ || /^(?:\w+\*?\s+)+(?:\*\s*)?(\w+)\(/) {
+ my $name = $1;
+ if ($name !~ /ATTRIBUTE/) {
+ $func = $name;
+ }
+ } elsif (/^\s*$/) {
+ $func = undef;
+ }
+ if (/ATTRIBUTE_NOINLINE/) {
+ if (defined $func) {
+ $noninlined{$func} = 1;
+ }
+ }
+ }
+
+ close FH
+}
+
+sub scan_overrides {
+ my $file = shift;
+
+ open FH, $file or die "cannot read $file: $!";
+
+ my $func;
+ while (<FH>) {
+ if (/^(\w+)\(/ || /^\w+\s*(?:\*\s*)?(\w+)\(/) {
+ my $name = $1;
+ if ($name =~ /^vir/) {
+ $mocked{$name} = "$file:$.";
+ }
+ }
+ }
+
+ close FH
+}
diff --git a/cfg.mk b/cfg.mk
index cc174a3..241510e 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1062,7 +1062,7 @@ _autogen:
# regenerate HACKING as part of the syntax-check
ifneq ($(_gl-Makefile),)
syntax-check: $(top_srcdir)/HACKING spacing-check test-wrap-argv \
- prohibit-duplicate-header
+ prohibit-duplicate-header mock-noinline
endif
# Don't include duplicate header in the source (either *.c or *.h)
@@ -1076,6 +1076,10 @@ spacing-check:
{ echo '$(ME): incorrect formatting, see HACKING for rules' 1>&2; \
exit 1; }
+mock-noinline:
+ $(AM_V_GEN)files=`$(VC_LIST) | grep '\.[ch]$$'`; \
+ $(PERL) $(top_srcdir)/build-aux/mock-noinline.pl $$files
+
test-wrap-argv:
$(AM_V_GEN)files=`$(VC_LIST) | grep -E '\.(ldargs|args)'`; \
$(PERL) $(top_srcdir)/tests/test-wrap-argv.pl --check $$files
diff --git a/src/internal.h b/src/internal.h
index d64be93..b1a0556 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -153,6 +153,20 @@
# endif
/**
+ * ATTRIBUTE_NOINLINE:
+ *
+ * Force compiler not to inline a method. Should be used if
+ * the method need to be overridable by test mocks.
+ */
+# ifndef ATTRIBUTE_NOINLINE
+# if __GNUC_PREREQ (4, 0)
+# define ATTRIBUTE_NOINLINE __attribute__((__noinline__))
+# else
+# define ATTRIBUTE_NOINLINE
+# endif
+# endif
+
+/**
* ATTRIBUTE_FMT_PRINTF
*
* Macro used to check printf like functions, if compiling
@@ -236,6 +250,9 @@
# ifndef ATTRIBUTE_RETURN_CHECK
# define ATTRIBUTE_RETURN_CHECK
# endif
+# ifndef ATTRIBUTE_NOINLINE
+# define ATTRIBUTE_NOINLINE
+# endif
#
# ifndef ATTRIBUTE_FALLTHROUGH
# define ATTRIBUTE_FALLTHROUGH do {} while(0)
diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
index a7c92fd..94fa75b 100644
--- a/src/qemu/qemu_capspriv.h
+++ b/src/qemu/qemu_capspriv.h
@@ -95,7 +95,7 @@ virQEMUCapsSetCPUModelInfo(virQEMUCapsPtr qemuCaps,
virCPUDefPtr
virQEMUCapsProbeHostCPUForEmulator(virCapsPtr caps,
virQEMUCapsPtr qemuCaps,
- virDomainVirtType type);
+ virDomainVirtType type) ATTRIBUTE_NOINLINE;
void
virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps,
diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h
index 1e75ee6..de795af 100644
--- a/src/rpc/virnetsocket.h
+++ b/src/rpc/virnetsocket.h
@@ -136,9 +136,11 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
uid_t *uid,
gid_t *gid,
pid_t *pid,
- unsigned long long *timestamp);
+ unsigned long long *timestamp)
+ ATTRIBUTE_NOINLINE;
int virNetSocketGetSELinuxContext(virNetSocketPtr sock,
- char **context);
+ char **context)
+ ATTRIBUTE_NOINLINE;
int virNetSocketSetBlocking(virNetSocketPtr sock,
bool blocking);
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 99dcdeb..e7c2e51 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -58,7 +58,7 @@ enum {
void virCommandPassFD(virCommandPtr cmd,
int fd,
- unsigned int flags);
+ unsigned int flags) ATTRIBUTE_NOINLINE;
void virCommandPassListenFDs(virCommandPtr cmd);
diff --git a/src/util/vircrypto.h b/src/util/vircrypto.h
index 52ba3b3..068602f 100644
--- a/src/util/vircrypto.h
+++ b/src/util/vircrypto.h
@@ -55,6 +55,6 @@ int virCryptoEncryptData(virCryptoCipher algorithm,
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(6)
ATTRIBUTE_NONNULL(8) ATTRIBUTE_NONNULL(9) ATTRIBUTE_RETURN_CHECK;
-uint8_t *virCryptoGenerateRandom(size_t nbytes);
+uint8_t *virCryptoGenerateRandom(size_t nbytes) ATTRIBUTE_NOINLINE;
#endif /* __VIR_CRYPTO_H__ */
diff --git a/src/util/virfile.h b/src/util/virfile.h
index ba1c57c..41c25a2 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -188,7 +188,7 @@ void virFileActivateDirOverride(const char *argv0)
off_t virFileLength(const char *path, int fd) ATTRIBUTE_NONNULL(1);
bool virFileIsDir (const char *file) ATTRIBUTE_NONNULL(1);
-bool virFileExists(const char *file) ATTRIBUTE_NONNULL(1);
+bool virFileExists(const char *file) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NOINLINE;
bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1);
enum {
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
index e9c22ee..67033de 100644
--- a/src/util/virhostcpu.h
+++ b/src/util/virhostcpu.h
@@ -38,7 +38,7 @@ bool virHostCPUHasBitmap(void);
virBitmapPtr virHostCPUGetPresentBitmap(void);
virBitmapPtr virHostCPUGetOnlineBitmap(void);
int virHostCPUGetCount(void);
-int virHostCPUGetThreadsPerSubcore(virArch arch);
+int virHostCPUGetThreadsPerSubcore(virArch arch) ATTRIBUTE_NOINLINE;
int virHostCPUGetMap(unsigned char **cpumap,
unsigned int *online,
@@ -51,7 +51,7 @@ int virHostCPUGetInfo(virArch hostarch,
unsigned int *cores,
unsigned int *threads);
-int virHostCPUGetKVMMaxVCPUs(void);
+int virHostCPUGetKVMMaxVCPUs(void) ATTRIBUTE_NOINLINE;
int virHostCPUStatsAssign(virNodeCPUStatsPtr param,
const char *name,
diff --git a/src/util/virmacaddr.h b/src/util/virmacaddr.h
index ae26867..f4f5e2c 100644
--- a/src/util/virmacaddr.h
+++ b/src/util/virmacaddr.h
@@ -48,7 +48,7 @@ void virMacAddrGetRaw(const virMacAddr *src, unsigned char dst[VIR_MAC_BUFLEN]);
const char *virMacAddrFormat(const virMacAddr *addr,
char *str);
void virMacAddrGenerate(const unsigned char prefix[VIR_MAC_PREFIX_BUFLEN],
- virMacAddrPtr addr);
+ virMacAddrPtr addr) ATTRIBUTE_NOINLINE;
int virMacAddrParse(const char* str,
virMacAddrPtr addr) ATTRIBUTE_RETURN_CHECK;
int virMacAddrParseHex(const char* str,
diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
index 437a776..12a3123 100644
--- a/src/util/virnetdev.h
+++ b/src/util/virnetdev.h
@@ -121,7 +121,7 @@ int virNetDevExists(const char *brname)
int virNetDevSetOnline(const char *ifname,
bool online)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
int virNetDevGetOnline(const char *ifname,
bool *online)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
@@ -129,7 +129,7 @@ int virNetDevGetOnline(const char *ifname,
int virNetDevSetMAC(const char *ifname,
const virMacAddr *macaddr)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
int virNetDevGetMAC(const char *ifname,
virMacAddrPtr macaddr)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
@@ -263,7 +263,8 @@ int virNetDevSysfsFile(char **pf_sysfs_device_link,
const char *ifname,
const char *file)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
- ATTRIBUTE_RETURN_CHECK;
+ ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
-int virNetDevRunEthernetScript(const char *ifname, const char *script);
+int virNetDevRunEthernetScript(const char *ifname, const char *script)
+ ATTRIBUTE_NOINLINE;
#endif /* __VIR_NETDEV_H__ */
diff --git a/src/util/virnetdevip.h b/src/util/virnetdevip.h
index cc466ca..6b509ea 100644
--- a/src/util/virnetdevip.h
+++ b/src/util/virnetdevip.h
@@ -67,7 +67,7 @@ int virNetDevIPAddrAdd(const char *ifname,
virSocketAddr *addr,
virSocketAddr *peer,
unsigned int prefix)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
int virNetDevIPRouteAdd(const char *ifname,
virSocketAddrPtr addr,
unsigned int prefix,
diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h
index 7380a2d..51bb1dd 100644
--- a/src/util/virnetdevopenvswitch.h
+++ b/src/util/virnetdevopenvswitch.h
@@ -59,6 +59,6 @@ int virNetDevOpenvswitchInterfaceStats(const char *ifname,
int virNetDevOpenvswitchGetVhostuserIfname(const char *path,
char **ifname)
- ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
#endif /* __VIR_NETDEV_OPENVSWITCH_H__ */
diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h
index 6bb3b88..311f069 100644
--- a/src/util/virnetdevtap.h
+++ b/src/util/virnetdevtap.h
@@ -38,7 +38,7 @@ int virNetDevTapCreate(char **ifname,
int *tapfd,
size_t tapfdSize,
unsigned int flags)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
int virNetDevTapDelete(const char *ifname,
const char *tunpath)
@@ -48,7 +48,7 @@ int virNetDevTapGetName(int tapfd, char **ifname)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
char* virNetDevTapGetRealDeviceName(char *ifname)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
typedef enum {
VIR_NETDEV_TAP_CREATE_NONE = 0,
@@ -87,7 +87,7 @@ int virNetDevTapCreateInBridgePort(const char *brname,
unsigned int *actualMTU,
unsigned int flags)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
- ATTRIBUTE_RETURN_CHECK;
+ ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
int virNetDevTapInterfaceStats(const char *ifname,
virDomainInterfaceStatsPtr stats)
diff --git a/src/util/virnuma.h b/src/util/virnuma.h
index f3eef32..e4e1fd0 100644
--- a/src/util/virnuma.h
+++ b/src/util/virnuma.h
@@ -34,20 +34,20 @@ int virNumaSetupMemoryPolicy(virDomainNumatuneMemMode mode,
virBitmapPtr nodeset);
virBitmapPtr virNumaGetHostMemoryNodeset(void);
-bool virNumaNodesetIsAvailable(virBitmapPtr nodeset);
-bool virNumaIsAvailable(void);
-int virNumaGetMaxNode(void);
-bool virNumaNodeIsAvailable(int node);
+bool virNumaNodesetIsAvailable(virBitmapPtr nodeset) ATTRIBUTE_NOINLINE;
+bool virNumaIsAvailable(void) ATTRIBUTE_NOINLINE;
+int virNumaGetMaxNode(void) ATTRIBUTE_NOINLINE;
+bool virNumaNodeIsAvailable(int node) ATTRIBUTE_NOINLINE;
int virNumaGetDistances(int node,
int **distances,
- int *ndistances);
+ int *ndistances) ATTRIBUTE_NOINLINE;
int virNumaGetNodeMemory(int node,
unsigned long long *memsize,
- unsigned long long *memfree);
+ unsigned long long *memfree) ATTRIBUTE_NOINLINE;
unsigned int virNumaGetMaxCPUs(void);
-int virNumaGetNodeCPUs(int node, virBitmapPtr *cpus);
+int virNumaGetNodeCPUs(int node, virBitmapPtr *cpus) ATTRIBUTE_NOINLINE;
int virNumaGetPageInfo(int node,
unsigned int page_size,
@@ -59,7 +59,7 @@ int virNumaGetPages(int node,
unsigned int **pages_avail,
unsigned int **pages_free,
size_t *npages)
- ATTRIBUTE_NONNULL(5);
+ ATTRIBUTE_NONNULL(5) ATTRIBUTE_NOINLINE;
int virNumaSetPagePoolSize(int node,
unsigned int page_size,
unsigned long long page_count,
diff --git a/src/util/virrandom.h b/src/util/virrandom.h
index f457d2d..7a984ee 100644
--- a/src/util/virrandom.h
+++ b/src/util/virrandom.h
@@ -24,11 +24,11 @@
# include "internal.h"
-uint64_t virRandomBits(int nbits);
+uint64_t virRandomBits(int nbits) ATTRIBUTE_NOINLINE;
double virRandom(void);
uint32_t virRandomInt(uint32_t max);
int virRandomBytes(unsigned char *buf, size_t buflen)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
-int virRandomGenerateWWN(char **wwn, const char *virt_type);
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
+int virRandomGenerateWWN(char **wwn, const char *virt_type) ATTRIBUTE_NOINLINE;
#endif /* __VIR_RANDOM_H__ */
diff --git a/src/util/virscsi.h b/src/util/virscsi.h
index 7d88d4e..9f8b3ec 100644
--- a/src/util/virscsi.h
+++ b/src/util/virscsi.h
@@ -37,7 +37,7 @@ char *virSCSIDeviceGetSgName(const char *sysfs_prefix,
const char *adapter,
unsigned int bus,
unsigned int target,
- unsigned long long unit);
+ unsigned long long unit) ATTRIBUTE_NOINLINE;
char *virSCSIDeviceGetDevName(const char *sysfs_prefix,
const char *adapter,
unsigned int bus,
diff --git a/src/util/virscsivhost.h b/src/util/virscsivhost.h
index 6018b83..21887dd 100644
--- a/src/util/virscsivhost.h
+++ b/src/util/virscsivhost.h
@@ -61,6 +61,6 @@ void virSCSIVHostDeviceGetUsedBy(virSCSIVHostDevicePtr dev,
const char **drv_name,
const char **dom_name);
void virSCSIVHostDeviceFree(virSCSIVHostDevicePtr dev);
-int virSCSIVHostOpenVhostSCSI(int *vhostfd);
+int virSCSIVHostOpenVhostSCSI(int *vhostfd) ATTRIBUTE_NOINLINE;
#endif /* __VIR_SCSIHOST_H__ */
diff --git a/src/util/virtpm.h b/src/util/virtpm.h
index fe71307..b21fc05 100644
--- a/src/util/virtpm.h
+++ b/src/util/virtpm.h
@@ -22,6 +22,6 @@
#ifndef __VIR_TPM_H__
# define __VIR_TPM_H__
-char *virTPMCreateCancelPath(const char *devpath);
+char *virTPMCreateCancelPath(const char *devpath) ATTRIBUTE_NOINLINE;
#endif /* __VIR_TPM_H__ */
diff --git a/src/util/virutil.h b/src/util/virutil.h
index e097b77..86e9051 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -142,8 +142,8 @@ char *virGetUserConfigDirectory(void);
char *virGetUserCacheDirectory(void);
char *virGetUserRuntimeDirectory(void);
char *virGetUserShell(uid_t uid);
-char *virGetUserName(uid_t uid);
-char *virGetGroupName(gid_t gid);
+char *virGetUserName(uid_t uid) ATTRIBUTE_NOINLINE;
+char *virGetGroupName(gid_t gid) ATTRIBUTE_NOINLINE;
int virGetGroupList(uid_t uid, gid_t group, gid_t **groups)
ATTRIBUTE_NONNULL(3);
int virGetUserID(const char *name,
@@ -204,12 +204,12 @@ verify((int)VIR_TRISTATE_BOOL_ABSENT == (int)VIR_TRISTATE_SWITCH_ABSENT);
unsigned int virGetListenFDs(void);
-long virGetSystemPageSize(void);
-long virGetSystemPageSizeKB(void);
+long virGetSystemPageSize(void) ATTRIBUTE_NOINLINE;
+long virGetSystemPageSizeKB(void) ATTRIBUTE_NOINLINE;
unsigned long long virMemoryLimitTruncate(unsigned long long value);
bool virMemoryLimitIsSet(unsigned long long value);
-unsigned long long virMemoryMaxValue(bool ulong);
+unsigned long long virMemoryMaxValue(bool ulong) ATTRIBUTE_NOINLINE;
/**
* VIR_ASSIGN_IS_OVERFLOW:
diff --git a/src/util/viruuid.h b/src/util/viruuid.h
index 5790a17..1d67e9e 100644
--- a/src/util/viruuid.h
+++ b/src/util/viruuid.h
@@ -49,7 +49,7 @@ int virGetHostUUID(unsigned char *host_uuid) ATTRIBUTE_NONNULL(1);
int virUUIDIsValid(unsigned char *uuid);
-int virUUIDGenerate(unsigned char *uuid);
+int virUUIDGenerate(unsigned char *uuid) ATTRIBUTE_NOINLINE;
int virUUIDParse(const char *uuidstr,
unsigned char *uuid)
--
2.9.3
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
