Merge pull request #26528 from keszybz/valgrind-simplification

Drop the -Dvalgrind configuration option

README

@@ -403,12 +403,12 @@ WARNINGS and TAINT FLAGS:
         See org.freedesktop.systemd1(5) for more information.
 
 VALGRIND:
-        To run systemd under valgrind, compile with meson option
+        To run systemd under valgrind, compile systemd with the valgrind
-        -Dvalgrind=true and have valgrind development headers installed
+        development headers available (i.e. valgrind-devel or equivalent).
-        (i.e. valgrind-devel or equivalent). Otherwise, false positives will be
+        Otherwise, false positives will be triggered by code which violates
-        triggered by code which violates some rules but is actually safe. Note
+        some rules but is actually safe. Note that valgrind generates nice
-        that valgrind generates nice output only on exit(), hence on shutdown
+        output only on exit(), hence on shutdown we don't execve()
-        we don't execve() systemd-shutdown.
+        systemd-shutdown.
 
 STABLE BRANCHES AND BACKPORTS:
         Stable branches with backported patches are available in the

meson.build

@@ -1020,8 +1020,6 @@ endforeach
 conf.set10('ENABLE_DEBUG_HASHMAP', enable_debug_hashmap)
 conf.set10('ENABLE_DEBUG_MMAP_CACHE', enable_debug_mmap_cache)
 conf.set10('ENABLE_DEBUG_SIPHASH', enable_debug_siphash)
-
-conf.set10('VALGRIND', get_option('valgrind'))
 conf.set10('LOG_TRACE', get_option('log-trace'))
 
 default_user_path = get_option('user-path')
@@ -4609,7 +4607,6 @@ foreach tuple : [
         ['debug hashmap'],
         ['debug mmap cache'],
         ['debug siphash'],
-        ['valgrind',               conf.get('VALGRIND') == 1],
         ['trace logging',          conf.get('LOG_TRACE') == 1],
         ['install tests',          install_tests],
         ['link-udev-shared',       get_option('link-udev-shared')],

meson_options.txt

@@ -79,8 +79,6 @@ option('bump-proc-sys-fs-file-max', type : 'boolean',
        description : 'bump /proc/sys/fs/file-max to LONG_MAX')
 option('bump-proc-sys-fs-nr-open', type : 'boolean',
        description : 'bump /proc/sys/fs/nr_open to INT_MAX')
-option('valgrind', type : 'boolean', value : false,
-       description : 'do extra operations to avoid valgrind warnings')
 option('log-trace', type : 'boolean', value : false,
        description : 'enable low level debug logging')
 option('user-path', type : 'string',

src/basic/hashmap.c

@@ -5,6 +5,9 @@
 #include <pthread.h>
 #include <stdint.h>
 #include <stdlib.h>
+#if HAVE_VALGRIND_VALGRIND_H
+#  include <valgrind/valgrind.h>
+#endif
 
 #include "alloc-util.h"
 #include "fileio.h"
@@ -295,10 +298,11 @@ void hashmap_trim_pools(void) {
         mempool_trim(&ordered_hashmap_pool);
 }
 
-#if VALGRIND
+#if HAVE_VALGRIND_VALGRIND_H
 _destructor_ static void cleanup_pools(void) {
         /* Be nice to valgrind */
-        hashmap_trim_pools();
+        if (RUNNING_ON_VALGRIND)
+                hashmap_trim_pools();
 }
 #endif
 

src/core/main.c

@@ -12,7 +12,7 @@
 #include <seccomp.h>
 #endif
 #if HAVE_VALGRIND_VALGRIND_H
-#include <valgrind/valgrind.h>
+#  include <valgrind/valgrind.h>
 #endif
 
 #include "sd-bus.h"
@@ -1866,8 +1866,8 @@ static int do_reexecute(
                 assert(i <= args_size);
 
                 /*
-                 * We want valgrind to print its memory usage summary before reexecution.  Valgrind won't do
+                 * We want valgrind to print its memory usage summary before reexecution. Valgrind won't do
-                 * this is on its own on exec(), but it will do it on exit().  Hence, to ensure we get a
+                 * this is on its own on exec(), but it will do it on exit(). Hence, to ensure we get a
                  * summary here, fork() off a child, let it exit() cleanly, so that it prints the summary,
                  * and wait() for it in the parent, before proceeding into the exec().
                  */

src/libsystemd-network/test-dhcp-client.c

@@ -9,6 +9,9 @@
 #include <stdio.h>
 #include <sys/socket.h>
 #include <unistd.h>
+#if HAVE_VALGRIND_VALGRIND_H
+#  include <valgrind/valgrind.h>
+#endif
 
 #include "sd-dhcp-client.h"
 #include "sd-event.h"
@@ -546,11 +549,12 @@ int main(int argc, char *argv[]) {
         test_discover_message(e);
         test_addr_acq(e);
 
-#if VALGRIND
+#if HAVE_VALGRIND_VALGRIND_H
         /* Make sure the async_close thread has finished.
          * valgrind would report some of the phread_* structures
          * as not cleaned up properly. */
-        sleep(1);
+        if (RUNNING_ON_VALGRIND)
+                sleep(1);
 #endif
 
         return 0;

src/libsystemd/sd-journal/journal-send.c

@@ -7,7 +7,7 @@
 #include <sys/un.h>
 #include <unistd.h>
 #if HAVE_VALGRIND_VALGRIND_H
-#include <valgrind/valgrind.h>
+#  include <valgrind/valgrind.h>
 #endif
 
 #define SD_JOURNAL_SUPPRESS_LOCATION
@@ -77,9 +77,9 @@ int journal_fd_nonblock(bool nonblock) {
         return fd_nonblock(r, nonblock);
 }
 
-#if VALGRIND
 void close_journal_fd(void) {
-        /* Be nice to valgrind. This is not atomic. This must be used only in tests. */
+#if HAVE_VALGRIND_VALGRIND_H
+        /* Be nice to valgrind. This is not atomic, so it is useful mainly for debugging. */
 
         if (!RUNNING_ON_VALGRIND)
                 return;
@@ -92,8 +92,8 @@ void close_journal_fd(void) {
 
         safe_close(fd_plus_one - 1);
         fd_plus_one = 0;
-}
 #endif
+}
 
 _public_ int sd_journal_print(int priority, const char *format, ...) {
         int r;

src/libsystemd/sd-journal/journal-send.h

@@ -4,9 +4,4 @@
 #include <stdbool.h>
 
 int journal_fd_nonblock(bool nonblock);
-
-#if VALGRIND
 void close_journal_fd(void);
-#else
-static inline void close_journal_fd(void) {}
-#endif

src/libsystemd/sd-journal/lookup3.c

@@ -4,6 +4,12 @@
 
 #include "lookup3.h"
 
+#if HAVE_VALGRIND_VALGRIND_H
+#  include <valgrind/valgrind.h>
+#else
+#  define RUNNING_ON_VALGRIND 0
+#endif
+
 /*
 -------------------------------------------------------------------------------
 lookup3.c, by Bob Jenkins, May 2006, Public Domain.
@@ -320,27 +326,28 @@ uint32_t jenkins_hashlittle( const void *key, size_t length, uint32_t initval)
      * still catch it and complain.  The masking trick does make the hash
      * noticeably faster for short strings (like English words).
      */
-#if !VALGRIND && !HAS_FEATURE_ADDRESS_SANITIZER && !HAS_FEATURE_MEMORY_SANITIZER
+#define VALGRIND_LIKE (_unlikely_(HAS_FEATURE_ADDRESS_SANITIZER ||      \
+                                  HAS_FEATURE_MEMORY_SANITIZER ||       \
+                                  RUNNING_ON_VALGRIND))
 
-    switch(length)
+    if (!VALGRIND_LIKE) {
-    {
+      switch(length)
-    case 12: c+=k[2]; b+=k[1]; a+=k[0]; break;
+      {
-    case 11: c+=k[2]&0xffffff; b+=k[1]; a+=k[0]; break;
+      case 12: c+=k[2]; b+=k[1]; a+=k[0]; break;
-    case 10: c+=k[2]&0xffff; b+=k[1]; a+=k[0]; break;
+      case 11: c+=k[2]&0xffffff; b+=k[1]; a+=k[0]; break;
-    case 9 : c+=k[2]&0xff; b+=k[1]; a+=k[0]; break;
+      case 10: c+=k[2]&0xffff; b+=k[1]; a+=k[0]; break;
-    case 8 : b+=k[1]; a+=k[0]; break;
+      case 9 : c+=k[2]&0xff; b+=k[1]; a+=k[0]; break;
-    case 7 : b+=k[1]&0xffffff; a+=k[0]; break;
+      case 8 : b+=k[1]; a+=k[0]; break;
-    case 6 : b+=k[1]&0xffff; a+=k[0]; break;
+      case 7 : b+=k[1]&0xffffff; a+=k[0]; break;
-    case 5 : b+=k[1]&0xff; a+=k[0]; break;
+      case 6 : b+=k[1]&0xffff; a+=k[0]; break;
-    case 4 : a+=k[0]; break;
+      case 5 : b+=k[1]&0xff; a+=k[0]; break;
-    case 3 : a+=k[0]&0xffffff; break;
+      case 4 : a+=k[0]; break;
-    case 2 : a+=k[0]&0xffff; break;
+      case 3 : a+=k[0]&0xffffff; break;
-    case 1 : a+=k[0]&0xff; break;
+      case 2 : a+=k[0]&0xffff; break;
-    case 0 : return c;              /* zero length strings require no mixing */
+      case 1 : a+=k[0]&0xff; break;
-    }
+      case 0 : return c;              /* zero length strings require no mixing */
-
+      }
-#else /* make valgrind happy */
+    } else {
-    {
       const uint8_t *k8 = (const uint8_t *) k;
 
       switch(length)
@@ -361,8 +368,6 @@ uint32_t jenkins_hashlittle( const void *key, size_t length, uint32_t initval)
       }
     }
 
-#endif /* !valgrind */
-
   } else if (HASH_LITTLE_ENDIAN && ((u.i & 0x1) == 0)) {
     const uint16_t *k = (const uint16_t *)key;         /* read 16-bit chunks */
     const uint8_t  *k8;
@@ -505,29 +510,26 @@ void jenkins_hashlittle2(
      * still catch it and complain.  The masking trick does make the hash
      * noticeably faster for short strings (like English words).
      */
-#if !VALGRIND && !HAS_FEATURE_ADDRESS_SANITIZER && !HAS_FEATURE_MEMORY_SANITIZER
+    if (!VALGRIND_LIKE) {
-
+      switch(length)
-    switch(length)
+      {
-    {
+      case 12: c+=k[2]; b+=k[1]; a+=k[0]; break;
-    case 12: c+=k[2]; b+=k[1]; a+=k[0]; break;
+      case 11: c+=k[2]&0xffffff; b+=k[1]; a+=k[0]; break;
-    case 11: c+=k[2]&0xffffff; b+=k[1]; a+=k[0]; break;
+      case 10: c+=k[2]&0xffff; b+=k[1]; a+=k[0]; break;
-    case 10: c+=k[2]&0xffff; b+=k[1]; a+=k[0]; break;
+      case 9 : c+=k[2]&0xff; b+=k[1]; a+=k[0]; break;
-    case 9 : c+=k[2]&0xff; b+=k[1]; a+=k[0]; break;
+      case 8 : b+=k[1]; a+=k[0]; break;
-    case 8 : b+=k[1]; a+=k[0]; break;
+      case 7 : b+=k[1]&0xffffff; a+=k[0]; break;
-    case 7 : b+=k[1]&0xffffff; a+=k[0]; break;
+      case 6 : b+=k[1]&0xffff; a+=k[0]; break;
-    case 6 : b+=k[1]&0xffff; a+=k[0]; break;
+      case 5 : b+=k[1]&0xff; a+=k[0]; break;
-    case 5 : b+=k[1]&0xff; a+=k[0]; break;
+      case 4 : a+=k[0]; break;
-    case 4 : a+=k[0]; break;
+      case 3 : a+=k[0]&0xffffff; break;
-    case 3 : a+=k[0]&0xffffff; break;
+      case 2 : a+=k[0]&0xffff; break;
-    case 2 : a+=k[0]&0xffff; break;
+      case 1 : a+=k[0]&0xff; break;
-    case 1 : a+=k[0]&0xff; break;
+      case 0 : *pc=c; *pb=b; return;  /* zero length strings require no mixing */
-    case 0 : *pc=c; *pb=b; return;  /* zero length strings require no mixing */
+      }
-    }
+    } else {
-
-#else /* make valgrind happy */
-
-    {
       const uint8_t *k8 = (const uint8_t *)k;
+
       switch(length)
       {
       case 12: c+=k[2]; b+=k[1]; a+=k[0]; break;
@@ -546,8 +548,6 @@ void jenkins_hashlittle2(
       }
     }
 
-#endif /* !valgrind */
-
   } else if (HASH_LITTLE_ENDIAN && ((u.i & 0x1) == 0)) {
     const uint16_t *k = (const uint16_t *)key;         /* read 16-bit chunks */
     const uint8_t  *k8;
@@ -681,29 +681,27 @@ uint32_t jenkins_hashbig( const void *key, size_t length, uint32_t initval)
      * still catch it and complain.  The masking trick does make the hash
      * noticeably faster for short strings (like English words).
      */
-#if !VALGRIND && !HAS_FEATURE_ADDRESS_SANITIZER && !HAS_FEATURE_MEMORY_SANITIZER
 
-    switch(length)
+    if (!VALGRIND_LIKE) {
-    {
+      switch(length)
-    case 12: c+=k[2]; b+=k[1]; a+=k[0]; break;
+      {
-    case 11: c+=k[2]&0xffffff00; b+=k[1]; a+=k[0]; break;
+      case 12: c+=k[2]; b+=k[1]; a+=k[0]; break;
-    case 10: c+=k[2]&0xffff0000; b+=k[1]; a+=k[0]; break;
+      case 11: c+=k[2]&0xffffff00; b+=k[1]; a+=k[0]; break;
-    case 9 : c+=k[2]&0xff000000; b+=k[1]; a+=k[0]; break;
+      case 10: c+=k[2]&0xffff0000; b+=k[1]; a+=k[0]; break;
-    case 8 : b+=k[1]; a+=k[0]; break;
+      case 9 : c+=k[2]&0xff000000; b+=k[1]; a+=k[0]; break;
-    case 7 : b+=k[1]&0xffffff00; a+=k[0]; break;
+      case 8 : b+=k[1]; a+=k[0]; break;
-    case 6 : b+=k[1]&0xffff0000; a+=k[0]; break;
+      case 7 : b+=k[1]&0xffffff00; a+=k[0]; break;
-    case 5 : b+=k[1]&0xff000000; a+=k[0]; break;
+      case 6 : b+=k[1]&0xffff0000; a+=k[0]; break;
-    case 4 : a+=k[0]; break;
+      case 5 : b+=k[1]&0xff000000; a+=k[0]; break;
-    case 3 : a+=k[0]&0xffffff00; break;
+      case 4 : a+=k[0]; break;
-    case 2 : a+=k[0]&0xffff0000; break;
+      case 3 : a+=k[0]&0xffffff00; break;
-    case 1 : a+=k[0]&0xff000000; break;
+      case 2 : a+=k[0]&0xffff0000; break;
-    case 0 : return c;              /* zero length strings require no mixing */
+      case 1 : a+=k[0]&0xff000000; break;
-    }
+      case 0 : return c;              /* zero length strings require no mixing */
-
+      }
-#else  /* make valgrind happy */
+    } else {
-
-    {
       const uint8_t *k8 = (const uint8_t *)k;
+
       switch(length)                   /* all the case statements fall through */
       {
       case 12: c+=k[2]; b+=k[1]; a+=k[0]; break;
@@ -722,8 +720,6 @@ uint32_t jenkins_hashbig( const void *key, size_t length, uint32_t initval)
       }
     }
 
-#endif /* !VALGRIND */
-
   } else {                        /* need to read the key one byte at a time */
     const uint8_t *k = (const uint8_t *)key;
 

src/test/test-set.c

@@ -5,8 +5,6 @@
 #include "strv.h"
 #include "tests.h"
 
-const bool mempool_use_allowed = VALGRIND;
-
 TEST(set_steal_first) {
         _cleanup_set_free_ Set *m = NULL;
         int seen[3] = {};
@@ -37,10 +35,9 @@ static void item_seen(Item *item) {
 TEST(set_free_with_destructor) {
         Set *m;
         struct Item items[4] = {};
-        unsigned i;
 
         assert_se(m = set_new(NULL));
-        for (i = 0; i < ELEMENTSOF(items) - 1; i++)
+        for (size_t i = 0; i < ELEMENTSOF(items) - 1; i++)
                 assert_se(set_put(m, items + i) == 1);
 
         m = set_free_with_destructor(m, item_seen);
@@ -55,10 +52,9 @@ DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(item_hash_ops, void, trivial_hash_
 TEST(set_free_with_hash_ops) {
         Set *m;
         struct Item items[4] = {};
-        unsigned i;
 
         assert_se(m = set_new(&item_hash_ops));
-        for (i = 0; i < ELEMENTSOF(items) - 1; i++)
+        for (size_t i = 0; i < ELEMENTSOF(items) - 1; i++)
                 assert_se(set_put(m, items + i) == 1);
 
         m = set_free(m);

tmpfiles.d/systemd-nologin.conf

@@ -6,6 +6,5 @@
 #  (at your option) any later version.
 
 # See tmpfiles.d(5), systemd-user-sessions.service(8) and pam_nologin(8).
-# This file has special suffix so it is not run by mistake.
 
 F! /run/nologin 0644 - - - "System is booting up. Unprivileged users are not permitted to log in yet. Please come back later. For technical details, see pam_nologin(8)."