Re: [PATCH 6/6] Add test for explicit_bzero

[Bruno Haible]

Hi Bastien,

> Simple test

A module description file modules/explicit_bzero-tests is needed as well.

Then you can test the changed implementation through
1.
  ./gnulib-tool --create-testdir --dir=/tmp/testdir --single-configure 
explicit_bzero
2. transport the test dir to a different machine,
3. test it there.

> @@ -0,0 +1,34 @@
> +/* Test of asinl() function.

This first line is a copy&paste.

> +   Copyright (C) 2010-2020 Free Software Foundation, Inc.

You may have started with a file written in 2010. But since no
copyrightable (= significant) code is left over from it, the copyright
year range should just be 2020.

> +#include <config.h>
> +
> +#include <string.h>
> +
> +#include "macros.h"

OK.

> +char hello[] = "hello";
> +
> +int
> +main ()
> +{
> +  explicit_bzero(hello,sizeof(hello));
> +  ASSERT (hello[0]==0);

The unit test should have two purposes:
  1) Verify that the function is well defined (no link error) and
     runs (does not crash).
  2) Verify that the compiler does not eliminate it.

Re 1), your proposed test does it.

Re 2): No compiler would optimize away a memset on a global variable.
Compilers are most tempted to optimize for an array allocated on the stack,
at a point where the array is going out of scope. Like this:

   void
   do_secret_stuff (void)
   {
     char pwd[6];
     memset (pwd, "secret", 6);
     explicit_bzero (pwd, 6);
   }

But now, how to check that the array is cleared, since it's already out of
scope? I would try it like this:

   int
   do_secret_stuff (volatile int pass)
   {
     char pwd[6];
     if (pass == 1)
       {
         memset (pwd, "secret", 6);
         explicit_bzero (pwd, 6);
         return 0;
       }
     else
       {
         static char zero [6] = { 0 };
         return memcmp (pwd, zero, 6) != 0;
       }
   }

The caller will do
   do_secret_stuff (1); 
   do_secret_stuff (2);

But there's still a small probability of an interrupt that clobbers the
stack between the two calls. Therefore I would put this in a loop:

   int count = 0;
   int repeat = 1000;
   for (; repeat > 0; repeat--)
     {
       do_secret_stuff (1); 
       count += do_secret_stuff (2);
     }
   ASSERT (count < 50);

The idea is that if, in the vast majority of cases, do_secret_stuff (2)
returned 0, explicit_bzero worked as expected. If the compiler optimized
it away, you would get count > 950.

Does that sound sensible? And does it work (i.e. if you use a plain
memset instead of explicit_bzero, does the test fail)?

Bruno