[patch/idea] Add register scrambling to testsuite

  [Note: I tried to add a special case for the bug noted below, but ran
out of time while trying to learn enough ppc64/vsx opcodery]

Allow for target-specific register "scrambling" - loading arbitrary
values into all registers that need not be call-saved.  These values
should be non-zero and invalid addresses, to help catch inadvertent
uses of otherwise uninitialized registers.

Intended to help prevent bugs such as those fixed by
0218463dd8265ed937622f88ac68c7d984fe0cfc
  

On Fri, Jun 10, 2022 at 8:53 PM DJ Delorie via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
> [Note: I tried to add a special case for the bug noted below, but ran
> out of time while trying to learn enough ppc64/vsx opcodery]
>
> Allow for target-specific register "scrambling" - loading arbitrary
> values into all registers that need not be call-saved.  These values
> should be non-zero and invalid addresses, to help catch inadvertent
> uses of otherwise uninitialized registers.
>
> Intended to help prevent bugs such as those fixed by
> 0218463dd8265ed937622f88ac68c7d984fe0cfc
>
> diff --git a/support/Makefile b/support/Makefile
> index 9b50eac117..91b940c379 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -76,6 +76,7 @@ libsupport-routines = \
>    support_quote_string \
>    support_record_failure \
>    support_run_diff \
> +  support_scramble_registers \
>    support_select_modifies_timeout \
>    support_select_normalizes_timeout \
>    support_set_small_thread_stack_size \
> diff --git a/support/support.h b/support/support.h
> index d20051da4d..3d049575d0 100644
> --- a/support/support.h
> +++ b/support/support.h
> @@ -233,6 +233,11 @@ void support_stack_free (struct support_stack *stack);
>     The returned value is the lowest file descriptor number.  */
>  int support_open_dev_null_range (int num, int flags, mode_t mode);
>
> +/* Write arbitrary values to all registers that can be written do, to
> +   avoid assumptions about initial register contents in test
> +   cases.  */
> +void support_scramble_registers (void);
> +
>  __END_DECLS
>
>  #endif /* SUPPORT_H */
> diff --git a/support/support_scramble_registers.c b/support/support_scramble_registers.c
> new file mode 100644
> index 0000000000..d5e2d3fd6d
> --- /dev/null
> +++ b/support/support_scramble_registers.c
> @@ -0,0 +1,29 @@
> +/* scramble any call-not-preserved registers
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/support.h>
> +
> +#include "scramble-regs.h"
> +
> +void
> +support_scramble_registers(void)
> +{
> +#ifdef SCRAMBLE_REGS
> +  SCRAMBLE_REGS;
> +#endif
> +}
> diff --git a/support/support_test_main.c b/support/support_test_main.c
> index 60307fd68e..0ccb182791 100644
> --- a/support/support_test_main.c
> +++ b/support/support_test_main.c
> @@ -269,6 +269,8 @@ adjust_exit_status (int status)
>  int
>  support_test_main (int argc, char **argv, const struct test_config *config)
>  {
> +  support_scramble_registers();
> +
>    if (test_main_called)
>      {
>        printf ("error: test_main called for a second time\n");
> diff --git a/sysdeps/generic/scramble-regs.h b/sysdeps/generic/scramble-regs.h
> new file mode 100644
> index 0000000000..7ac55d1bfc
> --- /dev/null
> +++ b/sysdeps/generic/scramble-regs.h
> @@ -0,0 +1,36 @@
> +/* scramble any call-not-preserved registers, target portion.
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* Example target-specific usage:
> +
> +   #define SCRAMBLE_REGS \
> +     asm volatile ("movl %0, %%eax" :: "i" (1234)); \
> +     asm volatile ("movl %0, %%edx" :: "i" (5678));
> +
> +   Targets are encouraged to create their own target-specific sub-definitions, like
> +
> +   #ifndef SCRAMBLE_REGS_FPU
> +   #define SCRAMBLE_REGS_FPU
> +   #endif
> +   #define SCRAMBLE_REGS \
> +     SCRAMBLE_REGS_FPU \
> +     asm volatile ("..."); \
> +
> +*/
> +
> +/* #define SCRAMBLE_REGS */
> diff --git a/sysdeps/powerpc/scramble-regs.h b/sysdeps/powerpc/scramble-regs.h
> new file mode 100644
> index 0000000000..9400b2ed6b
> --- /dev/null
> +++ b/sysdeps/powerpc/scramble-regs.h
> @@ -0,0 +1,20 @@
> +/* scramble any call-not-preserved registers, powerpc version
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#define SCRAMBLE_REGS                 \
> +  asm volatile ("li 0, %0" :: "i" (0x1235));
> diff --git a/sysdeps/x86_64/scramble-regs.h b/sysdeps/x86_64/scramble-regs.h
> new file mode 100644
> index 0000000000..66ffab3c8b
> --- /dev/null
> +++ b/sysdeps/x86_64/scramble-regs.h
> @@ -0,0 +1,31 @@
> +/* scramble any call-not-preserved registers, x86_64 version
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* SysV ABI: preserve EBX, ESP, EBP and R12-R15.  */
> +
> +#define SCRAMBLE_REGS                 \
> +  asm volatile ("movl %0, %%eax" :: "i" (0x12345679)); \
> +  asm volatile ("movl %0, %%ecx" :: "i" (0x12345679)); \
> +  asm volatile ("movl %0, %%edx" :: "i" (0x12345679)); \
> +  asm volatile ("movl %0, %%esi" :: "i" (0x12345679)); \
> +  asm volatile ("movl %0, %%edi" :: "i" (0x12345679)); \
> +  asm volatile ("mov %0, %%r8"  :: "i" (0x12345679));  \
> +  asm volatile ("mov %0, %%r9"  :: "i" (0x12345679));  \
> +  asm volatile ("mov %0, %%r10" :: "i" (0x12345679));  \
> +  asm volatile ("mov %0, %%r11" :: "i" (0x12345679));  \
Do these statements need to specify the register they are clobbering?
> +
>
  

On 6/10/2022 9:52 PM, DJ Delorie via Libc-alpha wrote:
> [Note: I tried to add a special case for the bug noted below, but ran
> out of time while trying to learn enough ppc64/vsx opcodery]
>
> Allow for target-specific register "scrambling" - loading arbitrary
> values into all registers that need not be call-saved.  These values
> should be non-zero and invalid addresses, to help catch inadvertent
> uses of otherwise uninitialized registers.
>
> Intended to help prevent bugs such as those fixed by
> 0218463dd8265ed937622f88ac68c7d984fe0cfc
If you wanted to get nasty you could intercept all calls going through 
the PLT and do something similar there as well.

Jeff
  

* Noah Goldstein via Libc-alpha:

> Do these statements need to specify the register they are clobbering?

Thet do.  This funcationality is likely more easily written as an
assembler routine that expects a function pointer and closure argument,
performs the register clobbers, and then calls the function with the
closure argument.

Thanks,
Florian
  

On 11/06/2022 09:22, DJ Delorie via Libc-alpha wrote:
> 
> [Note: I tried to add a special case for the bug noted below, but ran
> out of time while trying to learn enough ppc64/vsx opcodery]
> 
> Allow for target-specific register "scrambling" - loading arbitrary
> values into all registers that need not be call-saved.  These values
> should be non-zero and invalid addresses, to help catch inadvertent
> uses of otherwise uninitialized registers.
> 
> Intended to help prevent bugs such as those fixed by
> 0218463dd8265ed937622f88ac68c7d984fe0cfc

+1 to the idea, although as Jeff pointed out, PLT boundaries may be a 
more effective place, otherwise it relies on dummy values surviving 
multiple calls.  If that's the route we take, we may also want to 
measure the execution time overhead on the testsuite and if it's too 
high, put this behind a build option, e.g. make SCRAMBLE_REGS=1 check.

Also I wonder if in the longer term (i.e. when glibc can be built with 
some sanitizers enabled) this would be better suited as a 
compiler/sanitizer flag that clobbers regs at function entry instead of 
glibc trying to do the interception.  ISTM that may have broader impact 
than simply catching the ppc64le oddball case.  The performance impact 
may be insane though...

Sid
  

Jeff Law via Libc-alpha <libc-alpha@sourceware.org> writes:
> If you wanted to get nasty you could intercept all calls going through 
> the PLT and do something similar there as well.

The LD_AUDIT interface may or may not be able to do that, if the backend
doesn't preserve all registers "for our convenience" :-)

Would it be too much to ask for a "gcc -fscramble-regs" that writes
something to every call-clobbered reg in every prologue?  It wouldn't
have to be random, it would just have to be something that's obviously
wrong for most uses.
  

Noah Goldstein <goldstein.w.n@gmail.com> writes:
>> +  asm volatile ("mov %0, %%r11" :: "i" (0x12345679));  \
> Do these statements need to specify the register they are clobbering?

Er, yes.  Despite being in an isolated function and volatile, I keep
forgetting that LTO invalidates those considerations.

I'm also hoping that the platform maintainers will step up and provide
the appropriate clobber lists anyway ;-)
  

Florian Weimer <fweimer@redhat.com> writes:
> This funcationality is likely more easily written as an assembler
> routine that expects a function pointer and closure argument, performs
> the register clobbers, and then calls the function with the closure
> argument.

I thought of putting the scramble just before the do_test calls, but
there were more than one of those, and for the cases I'm trying to
avoid, it doesn't matter when the regs are clobbered.

Calling scramble once is enough to more accurately simulate a "busy app"
where gcc would have eventually filled all the registers with something,
which is what happened in the bug I referenced.  A closure-based asm
routine would make the work much more complicated, for no real benefit.

I've seen testsuites where each call-under-test was wrapped in a routine
that called it multiple times, permuting the "untouched" registers each
time.  I don't think we need to go that far.
  

Siddhesh Poyarekar <siddhesh@gotplt.org> writes:
> otherwise it relies on dummy values surviving multiple calls.

Most of our test cases are short enough that the more likely case is
that a zero value in a register remains throughout, and zeros tend to
hide bugs.  If our dummy value doesn't survive multiple calls, it's
likely to be replaced by some other dummy value, so... win ;-)

> The performance impact may be insane though...

I've written checkers like that before, where the thing to do was
"bracket the bug with these calls, invoke bug, go to lunch, get results
when you return."  An hour of computing time to find a bug is better
than days of engineer time, but yeah, we don't want to make every test
do this every time.
  

Hi DJ,

DJ Delorie via Libc-alpha <libc-alpha@sourceware.org> writes:

> Florian Weimer <fweimer@redhat.com> writes:
>> This funcationality is likely more easily written as an assembler
>> routine that expects a function pointer and closure argument, performs
>> the register clobbers, and then calls the function with the closure
>> argument.
>
> I thought of putting the scramble just before the do_test calls, but
> there were more than one of those, and for the cases I'm trying to
> avoid, it doesn't matter when the regs are clobbered.
>

I was actually working on a patch similar to this one, but looks like
you beat me to it =). And you version seems cleaner.

You could embed your call to support_scramble_registers() in the CALL
macro on test-string.h:

#define CALL(impl, ...) \
  ({ support_scramble_registers (); (* (proto_t) (impl)->fn) (__VA_ARGS__); })

This way it would apply for all optimized function calls without
requiring many more changes. It also wouldn't affect the string
benchtests, since they use a separate CALL definition from
bench-string.h

> Calling scramble once is enough to more accurately simulate a "busy app"
> where gcc would have eventually filled all the registers with something,
> which is what happened in the bug I referenced.  A closure-based asm
> routine would make the work much more complicated, for no real benefit.
>
> I've seen testsuites where each call-under-test was wrapped in a routine
> that called it multiple times, permuting the "untouched" registers each
> time.  I don't think we need to go that far.

--
Matheus Castanho
  

Matheus Castanho <msc@linux.ibm.com> writes:
> You could embed your call to support_scramble_registers() in the CALL
> macro on test-string.h:

Yeah, there's probably lots of places we could tuck this, I was trying
to get the easy wins :-)
  

On Fri, Jun 10, 2022 at 8:53 PM DJ Delorie via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
> [Note: I tried to add a special case for the bug noted below, but ran
> out of time while trying to learn enough ppc64/vsx opcodery]
>
> Allow for target-specific register "scrambling" - loading arbitrary
> values into all registers that need not be call-saved.  These values
> should be non-zero and invalid addresses, to help catch inadvertent
> uses of otherwise uninitialized registers.
>
> Intended to help prevent bugs such as those fixed by
> 0218463dd8265ed937622f88ac68c7d984fe0cfc
>
> diff --git a/support/Makefile b/support/Makefile
> index 9b50eac117..91b940c379 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -76,6 +76,7 @@ libsupport-routines = \
>    support_quote_string \
>    support_record_failure \
>    support_run_diff \
> +  support_scramble_registers \
>    support_select_modifies_timeout \
>    support_select_normalizes_timeout \
>    support_set_small_thread_stack_size \
> diff --git a/support/support.h b/support/support.h
> index d20051da4d..3d049575d0 100644
> --- a/support/support.h
> +++ b/support/support.h
> @@ -233,6 +233,11 @@ void support_stack_free (struct support_stack *stack);
>     The returned value is the lowest file descriptor number.  */
>  int support_open_dev_null_range (int num, int flags, mode_t mode);
>
> +/* Write arbitrary values to all registers that can be written do, to
> +   avoid assumptions about initial register contents in test
> +   cases.  */
> +void support_scramble_registers (void);
> +
>  __END_DECLS
>
>  #endif /* SUPPORT_H */
> diff --git a/support/support_scramble_registers.c b/support/support_scramble_registers.c
> new file mode 100644
> index 0000000000..d5e2d3fd6d
> --- /dev/null
> +++ b/support/support_scramble_registers.c
> @@ -0,0 +1,29 @@
> +/* scramble any call-not-preserved registers
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/support.h>
> +
> +#include "scramble-regs.h"
> +
> +void
> +support_scramble_registers(void)
> +{
> +#ifdef SCRAMBLE_REGS
> +  SCRAMBLE_REGS;
> +#endif
> +}
> diff --git a/support/support_test_main.c b/support/support_test_main.c
> index 60307fd68e..0ccb182791 100644
> --- a/support/support_test_main.c
> +++ b/support/support_test_main.c
> @@ -269,6 +269,8 @@ adjust_exit_status (int status)
>  int
>  support_test_main (int argc, char **argv, const struct test_config *config)
>  {
> +  support_scramble_registers();
> +
>    if (test_main_called)
>      {
>        printf ("error: test_main called for a second time\n");
> diff --git a/sysdeps/generic/scramble-regs.h b/sysdeps/generic/scramble-regs.h
> new file mode 100644
> index 0000000000..7ac55d1bfc
> --- /dev/null
> +++ b/sysdeps/generic/scramble-regs.h
> @@ -0,0 +1,36 @@
> +/* scramble any call-not-preserved registers, target portion.
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* Example target-specific usage:
> +
> +   #define SCRAMBLE_REGS \
> +     asm volatile ("movl %0, %%eax" :: "i" (1234)); \
> +     asm volatile ("movl %0, %%edx" :: "i" (5678));
> +
> +   Targets are encouraged to create their own target-specific sub-definitions, like
> +
> +   #ifndef SCRAMBLE_REGS_FPU
> +   #define SCRAMBLE_REGS_FPU
> +   #endif
> +   #define SCRAMBLE_REGS \
> +     SCRAMBLE_REGS_FPU \
> +     asm volatile ("..."); \
> +
> +*/
> +
> +/* #define SCRAMBLE_REGS */
> diff --git a/sysdeps/powerpc/scramble-regs.h b/sysdeps/powerpc/scramble-regs.h
> new file mode 100644
> index 0000000000..9400b2ed6b
> --- /dev/null
> +++ b/sysdeps/powerpc/scramble-regs.h
> @@ -0,0 +1,20 @@
> +/* scramble any call-not-preserved registers, powerpc version
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#define SCRAMBLE_REGS                 \
> +  asm volatile ("li 0, %0" :: "i" (0x1235));
> diff --git a/sysdeps/x86_64/scramble-regs.h b/sysdeps/x86_64/scramble-regs.h
> new file mode 100644
> index 0000000000..66ffab3c8b
> --- /dev/null
> +++ b/sysdeps/x86_64/scramble-regs.h
> @@ -0,0 +1,31 @@
> +/* scramble any call-not-preserved registers, x86_64 version
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* SysV ABI: preserve EBX, ESP, EBP and R12-R15.  */
> +
> +#define SCRAMBLE_REGS                 \
> +  asm volatile ("movl %0, %%eax" :: "i" (0x12345679)); \
> +  asm volatile ("movl %0, %%ecx" :: "i" (0x12345679)); \
> +  asm volatile ("movl %0, %%edx" :: "i" (0x12345679)); \
> +  asm volatile ("movl %0, %%esi" :: "i" (0x12345679)); \
> +  asm volatile ("movl %0, %%edi" :: "i" (0x12345679)); \
> +  asm volatile ("mov %0, %%r8"  :: "i" (0x12345679));  \
> +  asm volatile ("mov %0, %%r9"  :: "i" (0x12345679));  \
> +  asm volatile ("mov %0, %%r10" :: "i" (0x12345679));  \
> +  asm volatile ("mov %0, %%r11" :: "i" (0x12345679));  \
> +
>

Should we also scramble XMM/YMM/ZMM registers?
  

On Mon, Jun 13, 2022 at 8:42 PM H.J. Lu via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> On Fri, Jun 10, 2022 at 8:53 PM DJ Delorie via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> >
> > [Note: I tried to add a special case for the bug noted below, but ran
> > out of time while trying to learn enough ppc64/vsx opcodery]
> >
> > Allow for target-specific register "scrambling" - loading arbitrary
> > values into all registers that need not be call-saved.  These values
> > should be non-zero and invalid addresses, to help catch inadvertent
> > uses of otherwise uninitialized registers.
> >
> > Intended to help prevent bugs such as those fixed by
> > 0218463dd8265ed937622f88ac68c7d984fe0cfc
> >
> > diff --git a/support/Makefile b/support/Makefile
> > index 9b50eac117..91b940c379 100644
> > --- a/support/Makefile
> > +++ b/support/Makefile
> > @@ -76,6 +76,7 @@ libsupport-routines = \
> >    support_quote_string \
> >    support_record_failure \
> >    support_run_diff \
> > +  support_scramble_registers \
> >    support_select_modifies_timeout \
> >    support_select_normalizes_timeout \
> >    support_set_small_thread_stack_size \
> > diff --git a/support/support.h b/support/support.h
> > index d20051da4d..3d049575d0 100644
> > --- a/support/support.h
> > +++ b/support/support.h
> > @@ -233,6 +233,11 @@ void support_stack_free (struct support_stack *stack);
> >     The returned value is the lowest file descriptor number.  */
> >  int support_open_dev_null_range (int num, int flags, mode_t mode);
> >
> > +/* Write arbitrary values to all registers that can be written do, to
> > +   avoid assumptions about initial register contents in test
> > +   cases.  */
> > +void support_scramble_registers (void);
> > +
> >  __END_DECLS
> >
> >  #endif /* SUPPORT_H */
> > diff --git a/support/support_scramble_registers.c b/support/support_scramble_registers.c
> > new file mode 100644
> > index 0000000000..d5e2d3fd6d
> > --- /dev/null
> > +++ b/support/support_scramble_registers.c
> > @@ -0,0 +1,29 @@
> > +/* scramble any call-not-preserved registers
> > +   Copyright (C) 2022 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#include <support/support.h>
> > +
> > +#include "scramble-regs.h"
> > +
> > +void
> > +support_scramble_registers(void)
> > +{
> > +#ifdef SCRAMBLE_REGS
> > +  SCRAMBLE_REGS;
> > +#endif
> > +}
> > diff --git a/support/support_test_main.c b/support/support_test_main.c
> > index 60307fd68e..0ccb182791 100644
> > --- a/support/support_test_main.c
> > +++ b/support/support_test_main.c
> > @@ -269,6 +269,8 @@ adjust_exit_status (int status)
> >  int
> >  support_test_main (int argc, char **argv, const struct test_config *config)
> >  {
> > +  support_scramble_registers();
> > +
> >    if (test_main_called)
> >      {
> >        printf ("error: test_main called for a second time\n");
> > diff --git a/sysdeps/generic/scramble-regs.h b/sysdeps/generic/scramble-regs.h
> > new file mode 100644
> > index 0000000000..7ac55d1bfc
> > --- /dev/null
> > +++ b/sysdeps/generic/scramble-regs.h
> > @@ -0,0 +1,36 @@
> > +/* scramble any call-not-preserved registers, target portion.
> > +   Copyright (C) 2022 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +/* Example target-specific usage:
> > +
> > +   #define SCRAMBLE_REGS \
> > +     asm volatile ("movl %0, %%eax" :: "i" (1234)); \
> > +     asm volatile ("movl %0, %%edx" :: "i" (5678));
> > +
> > +   Targets are encouraged to create their own target-specific sub-definitions, like
> > +
> > +   #ifndef SCRAMBLE_REGS_FPU
> > +   #define SCRAMBLE_REGS_FPU
> > +   #endif
> > +   #define SCRAMBLE_REGS \
> > +     SCRAMBLE_REGS_FPU \
> > +     asm volatile ("..."); \
> > +
> > +*/
> > +
> > +/* #define SCRAMBLE_REGS */
> > diff --git a/sysdeps/powerpc/scramble-regs.h b/sysdeps/powerpc/scramble-regs.h
> > new file mode 100644
> > index 0000000000..9400b2ed6b
> > --- /dev/null
> > +++ b/sysdeps/powerpc/scramble-regs.h
> > @@ -0,0 +1,20 @@
> > +/* scramble any call-not-preserved registers, powerpc version
> > +   Copyright (C) 2022 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#define SCRAMBLE_REGS                 \
> > +  asm volatile ("li 0, %0" :: "i" (0x1235));
> > diff --git a/sysdeps/x86_64/scramble-regs.h b/sysdeps/x86_64/scramble-regs.h
> > new file mode 100644
> > index 0000000000..66ffab3c8b
> > --- /dev/null
> > +++ b/sysdeps/x86_64/scramble-regs.h
> > @@ -0,0 +1,31 @@
> > +/* scramble any call-not-preserved registers, x86_64 version
> > +   Copyright (C) 2022 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +/* SysV ABI: preserve EBX, ESP, EBP and R12-R15.  */
> > +
> > +#define SCRAMBLE_REGS                 \
> > +  asm volatile ("movl %0, %%eax" :: "i" (0x12345679)); \
> > +  asm volatile ("movl %0, %%ecx" :: "i" (0x12345679)); \
> > +  asm volatile ("movl %0, %%edx" :: "i" (0x12345679)); \
> > +  asm volatile ("movl %0, %%esi" :: "i" (0x12345679)); \
> > +  asm volatile ("movl %0, %%edi" :: "i" (0x12345679)); \
> > +  asm volatile ("mov %0, %%r8"  :: "i" (0x12345679));  \
> > +  asm volatile ("mov %0, %%r9"  :: "i" (0x12345679));  \
> > +  asm volatile ("mov %0, %%r10" :: "i" (0x12345679));  \
> > +  asm volatile ("mov %0, %%r11" :: "i" (0x12345679));  \
> > +
> >
>
> Should we also scramble XMM/YMM/ZMM registers?

Yes. But I think the idea is the commit is just to introduce the concept and
let the arch maintainers fill in all the holes.

Once this is in we can just throw in a `vzeroall`.
>
> --
> H.J.
  

"H.J. Lu" <hjl.tools@gmail.com> writes:
> Should we also scramble XMM/YMM/ZMM registers?

If we can, yes!  The ppc64 bug was caused by accidentally using the
wrong register in a vector-optimized routine, so including all the
vector registers would be good.

I'm hoping the platform maintainers will, like you did, continue to
provide this information ;-)
  

Noah Goldstein <goldstein.w.n@gmail.com> writes:
> Once this is in we can just throw in a `vzeroall`.

As long as it doesn't zero all the registers, which is what blinded the
testcase that triggered this.  Oh wait...

Ideally we'd want to fill all registers with a large-ish constant that
isn't a valid pointer, like 0x13579bdf.  "obviously wrong" is the goal.
  

On 13/06/2022 22:22, DJ Delorie wrote:
> Siddhesh Poyarekar <siddhesh@gotplt.org> writes:
>> otherwise it relies on dummy values surviving multiple calls.
> 
> Most of our test cases are short enough that the more likely case is
> that a zero value in a register remains throughout, and zeros tend to
> hide bugs.  If our dummy value doesn't survive multiple calls, it's
> likely to be replaced by some other dummy value, so... win ;-)

Trouble is that it could be a zero dummy value, which ends up masking an 
issue.  In any case it's something we can iterate on over time.  This is 
a good start.

>> The performance impact may be insane though...
> 
> I've written checkers like that before, where the thing to do was
> "bracket the bug with these calls, invoke bug, go to lunch, get results
> when you return."  An hour of computing time to find a bug is better
> than days of engineer time, but yeah, we don't want to make every test
> do this every time.

Yeah, the performance issue only comes into play if we decide to do this 
for every test every time with, e.g. a -fscramble-regs or sanitizer.

Sid
  

Siddhesh Poyarekar <siddhesh@gotplt.org> writes:
>>> The performance impact may be insane though...
>> 
>> I've written checkers like that before, where the thing to do was
>> "bracket the bug with these calls, invoke bug, go to lunch, get results
>> when you return."  An hour of computing time to find a bug is better
>> than days of engineer time, but yeah, we don't want to make every test
>> do this every time.
>
> Yeah, the performance issue only comes into play if we decide to do this 
> for every test every time with, e.g. a -fscramble-regs or sanitizer.

To be clear, I think we're talking about two different cases...

In the case of "scramble registers at every call prolog in our
testsuite" (i.e. -fscramble-regs), I think the cost is insignificant and
the value is high, and would argue that we should encourage such
adoption.  This first patch is a subset of that, scrambling once per
test.

The checker I wrote in the past was, literally, "do a full heap
validation analysis on entry and exit of each malloc ABI function" and
caused real applications to run 100 to 1000 times slower.  While this is
a useful feature to have, and certainly faster than a human debugger,
it's not something that should ever be enabled by default.

Finding the fine line between these two is, of course, the interesting
discussion :-)
  

On 2022-06-13, DJ Delorie via Libc-alpha wrote:
>Jeff Law via Libc-alpha <libc-alpha@sourceware.org> writes:
>> If you wanted to get nasty you could intercept all calls going through
>> the PLT and do something similar there as well.
>
>The LD_AUDIT interface may or may not be able to do that, if the backend
>doesn't preserve all registers "for our convenience" :-)
>
>Would it be too much to ask for a "gcc -fscramble-regs" that writes
>something to every call-clobbered reg in every prologue?  It wouldn't
>have to be random, it would just have to be something that's obviously
>wrong for most uses.
>

(2020-10) gcc x86, (recent) clang x86, and (recent) clang aarch64
support -fzero-call-used-regs which is similar.  The option uses zero
though, not what we desire here :)