[libvirt] [PATCH 1/5] annotate all mocked functions with noinline

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(a)redhat.com&gt; --- 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