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: NDaniel P. Berrange <berrange@redhat.com>

build-aux/mock-noinline.pl

+#!/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
+}

cfg.mk

@@ -1069,7 +1069,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)
@@ -1083,6 +1083,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

src/internal.h

@@ -152,6 +152,20 @@
 #   endif
 #  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
  *
@@ -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)

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,

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);

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);
 

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__ */

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 {

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,

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,

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__ */

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,

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__ */

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)

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,

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__ */

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,

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__ */

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__ */

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:

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)