Revert "Prevent more compiler optimization of mockable functions"

This reverts commit e4b980c8.

When a binary links against a .a archive (as opposed to a shared library),
any symbols which are marked as 'weak' get silently dropped. As a result
when the binary later runs, those 'weak' functions have an address of
0x0 and thus crash when run.

This happened with virtlogd and virtlockd because they don't link to
libvirt.so, but instead just libvirt_util.a and libvirt_rpc.a. The
virRandomBits symbols was weak and so left out of the virtlogd &
virtlockd binaries, despite being required by virHashTable functions.

Various other binaries like libvirt_lxc, libvirt_iohelper, etc also
link directly to .a files instead of libvirt.so, so are potentially
at risk of dropping symbols leading to a later runtime crash.

This is normal linker behaviour because a weak symbol is not treated
as undefined, so nothing forces it to be pulled in from the .a You
have to force the linker to pull in weak symbols using -u$SYMNAME
which is not a practical approach.

This risk is silent bad linkage that affects runtime behaviour is
not acceptable for a fix that was merely trying to fix the test
suite. So stop using __weak__ again.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

build-aux/mock-noinline.pl

@@ -43,7 +43,7 @@ sub scan_annotations {
         } elsif (/^\s*$/) {
             $func = undef;
         }
-        if (/ATTRIBUTE_MOCKABLE/) {
+        if (/ATTRIBUTE_NOINLINE/) {
             if (defined $func) {
                 $noninlined{$func} = 1;
             }

src/check-symfile.pl

@@ -52,7 +52,7 @@ foreach my $elflib (@elflibs) {
     open NM, "-|", "nm", $elflib or die "cannot run 'nm $elflib': $!";
 
     while (<NM>) {
-        next unless /^\S+\s(?:[TBDW])\s(\S+)\s*$/;
+        next unless /^\S+\s(?:[TBD])\s(\S+)\s*$/;
 
         $gotsyms{$1} = 1;
     }

src/internal.h

@@ -113,26 +113,13 @@
 # endif
 
 /**
- * ATTRIBUTE_MOCKABLE:
- *
- * Ensure that the symbol can be overridden in a mock
- * library preload. This implies a number of attributes
- *
- *  - noinline: prevents the body being inlined to
- *              callers,
- *  - noclone: prevents specialized copies of the
- *             function body being created for different
- *             callers
- *  - weak: prevents the compiler making optimizations
- *          such as constant return value propagation
+ * ATTRIBUTE_NOINLINE:
  *
+ * Force compiler not to inline a method. Should be used if
+ * the method need to be overridable by test mocks.
  */
-# ifndef ATTRIBUTE_MOCKABLE
-#  if __GNUC_PREREQ(4, 5)
-#   define ATTRIBUTE_MOCKABLE __attribute__((__noinline__, __noclone__, __weak__))
-#  else
-#   define ATTRIBUTE_MOCKABLE __attribute__((__noinline__, __weak__))
-#  endif
+# ifndef ATTRIBUTE_NOINLINE
+#  define ATTRIBUTE_NOINLINE __attribute__((__noinline__))
 # endif
 
 /**

src/qemu/qemu_capspriv.h

@@ -95,7 +95,7 @@ virQEMUCapsSetCPUModelInfo(virQEMUCapsPtr qemuCaps,
 virCPUDefPtr
 virQEMUCapsProbeHostCPUForEmulator(virCapsPtr caps,
                                    virQEMUCapsPtr qemuCaps,
-                                   virDomainVirtType type) ATTRIBUTE_MOCKABLE;
+                                   virDomainVirtType type) ATTRIBUTE_NOINLINE;
 
 void
 virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps,

src/rpc/virnetsocket.h

@@ -137,10 +137,10 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
                                 gid_t *gid,
                                 pid_t *pid,
                                 unsigned long long *timestamp)
-    ATTRIBUTE_MOCKABLE;
+    ATTRIBUTE_NOINLINE;
 int virNetSocketGetSELinuxContext(virNetSocketPtr sock,
                                   char **context)
-    ATTRIBUTE_MOCKABLE;
+    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) ATTRIBUTE_MOCKABLE;
+                      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) ATTRIBUTE_MOCKABLE;
+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) ATTRIBUTE_MOCKABLE;
+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) ATTRIBUTE_MOCKABLE;
+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) ATTRIBUTE_MOCKABLE;
+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) ATTRIBUTE_MOCKABLE;
+                        virMacAddrPtr addr) ATTRIBUTE_NOINLINE;
 int virMacAddrParse(const char* str,
                     virMacAddrPtr addr) ATTRIBUTE_RETURN_CHECK;
 int virMacAddrParseHex(const char* str,

src/util/virnetdev.h

@@ -156,7 +156,7 @@ int virNetDevExists(const char *brname)
 
 int virNetDevSetOnline(const char *ifname,
                        bool online)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_MOCKABLE;
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
 int virNetDevGetOnline(const char *ifname,
                       bool *online)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
@@ -164,7 +164,7 @@ int virNetDevGetOnline(const char *ifname,
 
 int virNetDevSetMAC(const char *ifname,
                     const virMacAddr *macaddr)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_MOCKABLE;
+    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;
@@ -303,8 +303,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_MOCKABLE;
+    ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
 
 int virNetDevRunEthernetScript(const char *ifname, const char *script)
-    ATTRIBUTE_MOCKABLE;
+    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_MOCKABLE;
+    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_MOCKABLE;
+    ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
 
 #endif /* __VIR_NETDEV_OPENVSWITCH_H__ */

src/util/virnetdevtap.h

@@ -39,7 +39,7 @@ int virNetDevTapCreate(char **ifname,
                        int *tapfd,
                        size_t tapfdSize,
                        unsigned int flags)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_MOCKABLE;
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
 
 int virNetDevTapDelete(const char *ifname,
                        const char *tunpath)
@@ -49,7 +49,7 @@ int virNetDevTapGetName(int tapfd, char **ifname)
     ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 
 char* virNetDevTapGetRealDeviceName(char *ifname)
-      ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_MOCKABLE;
+      ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NOINLINE;
 
 typedef enum {
    VIR_NETDEV_TAP_CREATE_NONE = 0,
@@ -89,7 +89,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_MOCKABLE;
+    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) ATTRIBUTE_MOCKABLE;
-bool virNumaIsAvailable(void) ATTRIBUTE_MOCKABLE;
-int virNumaGetMaxNode(void) ATTRIBUTE_MOCKABLE;
-bool virNumaNodeIsAvailable(int node) ATTRIBUTE_MOCKABLE;
+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) ATTRIBUTE_MOCKABLE;
+                        int *ndistances) ATTRIBUTE_NOINLINE;
 int virNumaGetNodeMemory(int node,
                          unsigned long long *memsize,
-                         unsigned long long *memfree) ATTRIBUTE_MOCKABLE;
+                         unsigned long long *memfree) ATTRIBUTE_NOINLINE;
 
 unsigned int virNumaGetMaxCPUs(void);
 
-int virNumaGetNodeCPUs(int node, virBitmapPtr *cpus) ATTRIBUTE_MOCKABLE;
+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_MOCKABLE;
+    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) ATTRIBUTE_MOCKABLE;
+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 ATTRIBUTE_MOCKABLE;
-int virRandomGenerateWWN(char **wwn, const char *virt_type) ATTRIBUTE_MOCKABLE;
+    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) ATTRIBUTE_MOCKABLE;
+                             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) ATTRIBUTE_MOCKABLE;
+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) ATTRIBUTE_MOCKABLE;
+char *virTPMCreateCancelPath(const char *devpath) ATTRIBUTE_NOINLINE;
 
 #endif /* __VIR_TPM_H__ */

src/util/virutil.h

@@ -139,8 +139,8 @@ char *virGetUserConfigDirectory(void);
 char *virGetUserCacheDirectory(void);
 char *virGetUserRuntimeDirectory(void);
 char *virGetUserShell(uid_t uid);
-char *virGetUserName(uid_t uid) ATTRIBUTE_MOCKABLE;
-char *virGetGroupName(gid_t gid) ATTRIBUTE_MOCKABLE;
+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,
@@ -201,12 +201,12 @@ verify((int)VIR_TRISTATE_BOOL_ABSENT == (int)VIR_TRISTATE_SWITCH_ABSENT);
 
 unsigned int virGetListenFDs(void);
 
-long virGetSystemPageSize(void) ATTRIBUTE_MOCKABLE;
-long virGetSystemPageSizeKB(void) ATTRIBUTE_MOCKABLE;
+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) ATTRIBUTE_MOCKABLE;
+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) ATTRIBUTE_MOCKABLE;
+int virUUIDGenerate(unsigned char *uuid) ATTRIBUTE_NOINLINE;
 
 int virUUIDParse(const char *uuidstr,
                  unsigned char *uuid)