[PATCH] explicit_bzero-tests: robustify a bit

[Paul Eggert]

This is mostly to suppress GCC warnings, though I suppose it
might also improve the test.
* tests/test-explicit_bzero.c: Do not ignore -Wmaybe-uninitialized,
as this warning should no longer be generated given the other changes.
(test_heap): ASSERT that malloc succeeds, to suppress any GCC
warning about relying on malloc succeeding.  Declare addr to be
volatile, so that GCC does not deduce that it is heapbuf in
disguise and diagnose using it after freeing it.
(do_secret_stuff): Declare more things to be volatile, to avoid
optimizations that might disturb the stack and/or mess up the test
in other ways.  Pass last_stackbuf address as a parameter rather
than having last_stackbuf be static, to avoid GCC warning about
assigning address of a local to a static; all callers changed.
---
 ChangeLog                   | 17 +++++++++++++++++
 tests/test-explicit_bzero.c | 37 +++++++++++++++++--------------------
 2 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8b3945834c..730460b591 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2022-09-17  Paul Eggert  <eggert@cs.ucla.edu>
+
+       explicit_bzero-tests: robustify a bit
+       This is mostly to suppress GCC warnings, though I suppose it
+       might also improve the test.
+       * tests/test-explicit_bzero.c: Do not ignore -Wmaybe-uninitialized,
+       as this warning should no longer be generated given the other changes.
+       (test_heap): ASSERT that malloc succeeds, to suppress any GCC
+       warning about relying on malloc succeeding.  Declare addr to be
+       volatile, so that GCC does not deduce that it is heapbuf in
+       disguise and diagnose using it after freeing it.
+       (do_secret_stuff): Declare more things to be volatile, to avoid
+       optimizations that might disturb the stack and/or mess up the test
+       in other ways.  Pass last_stackbuf address as a parameter rather
+       than having last_stackbuf be static, to avoid GCC warning about
+       assigning address of a local to a static; all callers changed.
+
 2022-09-17  Bruno Haible  <bruno@clisp.org>
 
        wchar: Fix compilation errors in C++ mode on Solaris 11.3.
diff --git a/tests/test-explicit_bzero.c b/tests/test-explicit_bzero.c
index 113f713577..42d9d3cb60 100644
--- a/tests/test-explicit_bzero.c
+++ b/tests/test-explicit_bzero.c
@@ -31,12 +31,6 @@ SIGNATURE_CHECK (explicit_bzero, void, (void *, size_t));
 #include "vma-iter.h"
 #include "macros.h"
 
-/* Suppress GCC warning that do_secret_stuff (2) reads uninitialized
-   local storage.  */
-#if 4 < __GNUC__ + (3 <= __GNUC_MINOR__)
-# pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
-#endif
-
 #define SECRET "xyzzy1729"
 #define SECRET_SIZE 9
 
@@ -111,10 +105,12 @@ static void
 test_heap (void)
 {
   char *heapbuf = (char *) malloc (SECRET_SIZE);
-  uintptr_t addr = (uintptr_t) heapbuf;
+  ASSERT (heapbuf);
+  uintptr_t volatile addr = (uintptr_t) heapbuf;
   memcpy (heapbuf, SECRET, SECRET_SIZE);
   explicit_bzero (heapbuf, SECRET_SIZE);
   free (heapbuf);
+  heapbuf = (char *) addr;
   if (is_range_mapped (addr, addr + SECRET_SIZE))
     {
       /* some implementation could override freed memory by canaries so
@@ -132,28 +128,28 @@ test_heap (void)
      1. Put a secret in memory and invoke explicit_bzero on it.
      2. Verify that the memory has been erased.
    Implement them in the same function, so that they access the same memory
-   range on the stack.  That way, the test verifies that the compiler
+   range on the stack.  Declare the local scalars to be volatile so they
+   are not optimized away.  That way, the test verifies that the compiler
    does not eliminate a call to explicit_bzero, even if data flow analysis
    reveals that the stack area is dead at the end of the function.  */
-static int _GL_ATTRIBUTE_NOINLINE
-do_secret_stuff (volatile int pass)
+static bool _GL_ATTRIBUTE_NOINLINE
+do_secret_stuff (int volatile pass, char *volatile *volatile last_stackbuf)
 {
-  static char *last_stackbuf;
   char stackbuf[SECRET_SIZE];
   if (pass == 1)
     {
       memcpy (stackbuf, SECRET, SECRET_SIZE);
       explicit_bzero (stackbuf, SECRET_SIZE);
-      last_stackbuf = stackbuf;
-      return 0;
+      *last_stackbuf = stackbuf;
+      return false;
     }
   else /* pass == 2 */
     {
-      /* Use last_stackbuf here, because stackbuf may be allocated at a
-         different address than last_stackbuf.  This can happen
+      /* Use *last_stackbuf here, because stackbuf may be allocated at a
+         different address than *last_stackbuf.  This can happen
          when the compiler splits this function into different functions,
          one for pass == 1 and one for pass != 1.  */
-      return memcmp (zero, last_stackbuf, SECRET_SIZE) != 0;
+      return memcmp (zero, *last_stackbuf, SECRET_SIZE) != 0;
     }
 }
 
@@ -162,18 +158,19 @@ test_stack (void)
 {
   int count = 0;
   int repeat;
+  char *volatile last_stackbuf;
 
   for (repeat = 2 * 1000; repeat > 0; repeat--)
     {
       /* This odd way of writing two consecutive statements
-           do_secret_stuff (1);
-           count += do_secret_stuff (2);
+           do_secret_stuff (1, &last_stackbuf);
+           count += do_secret_stuff (2, &last_stackbuf);
          ensures that the two do_secret_stuff calls are performed with the same
          stack pointer value, on m68k.  */
       if ((repeat % 2) == 0)
-        do_secret_stuff (1);
+        do_secret_stuff (1, &last_stackbuf);
       else
-        count += do_secret_stuff (2);
+        count += do_secret_stuff (2, &last_stackbuf);
     }
   /* If explicit_bzero works, count is near 0.  (It may be > 0 if there were
      some asynchronous signal invocations between the two calls of
-- 
2.37.3