Re: [PATCH] gdb: Ensure compiler doesn't optimise variable out in test

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Palmer Dabbelt <palmer@sifive.com> [2018-08-29 17:21:58 -0700]:

> On Wed, 29 Aug 2018 11:02:59 PDT (-0700), andrew.burgess@embecosm.com wrote:
> > In the test gdb.base/funcargs.exp, there's this function:
> > 
> >     void recurse (SVAL a, int depth)
> >     {
> >       a.s = a.i = a.l = --depth;
> >       if (depth == 0)
> >         hitbottom ();
> >       else
> >         recurse (a, depth);
> >     }
> > 
> > The test script places a breakpoint in hitbottom, and runs the
> > executable which calls recurse with an initial depth of 4.
> > 
> > When GDB hits the breakpoint in hitbottom the testscript performs a
> > backtrace, and examines 'a' at each level.
> > 
> > The problem is that 'a' is not live after either the call to
> > 'hitbottom' or the call to 'recurse', and as a result the test fails.
> > 
> > In the particular case I was looking at GCC for RISC-V 32-bit, the
> > variable 'a' is on the stack and GCC selects the register $ra (the
> > return address register) to hold the pointer to 'a'.  This is fine,
> > because, by the time the $ra register is needed to hold a return
> > address (calling hitbottom or recurse) then 'a' is dead.
> > 
> > In this patch I propose that a use of 'a' is added after the calls to
> > hitbottom and recurse, this should cause the compiler to keep 'a'
> > around, which should ensure GDB can find it.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> > 	* gdb.base/funcargs.c (use_a): New function.
> > 	(recurse): Call use_a.
> > ---
> >  gdb/testsuite/ChangeLog           | 5 +++++
> >  gdb/testsuite/gdb.base/funcargs.c | 7 +++++++
> >  2 files changed, 12 insertions(+)
> > 
> > diff --git a/gdb/testsuite/gdb.base/funcargs.c b/gdb/testsuite/gdb.base/funcargs.c
> > index 600792f0a7e..515631f5491 100644
> > --- a/gdb/testsuite/gdb.base/funcargs.c
> > +++ b/gdb/testsuite/gdb.base/funcargs.c
> > @@ -424,6 +424,10 @@ void hitbottom ()
> >  {
> >  }
> > 
> > +void use_a (SVAL a)
> > +{
> > +}
> > +
> >  void recurse (SVAL a, int depth)
> >  {
> >    a.s = a.i = a.l = --depth;
> > @@ -431,6 +435,9 @@ void recurse (SVAL a, int depth)
> >      hitbottom ();
> >    else
> >      recurse (a, depth);
> > +
> > +  /* Ensure A is not discarded after the above calls.  */
> > +  use_a (a);
> >  }
> > 
> >  void test_struct_args ()
> 
> Isn't the compiler still free to kill "a" here because it can see into
> use_a() and therefor inline it?  I'd expected it to choose to inline
> use_a(), as doing nothing is always cheaper than calling a function.

Like Tom said, the test is compiled without optimisation, so I'd be
surprised if GCC inlined use_a and optimised accordingly.  I'm pretty
sure that if GCC did start to optimise that aggressively at O0 then
this whole test would fail, given that hitbottom is also empty.

Still, I aim to please, so, thanks to Ruslan for the suggestion,
there's a revised patch below.

Thanks,
Andrew

--

gdb: Ensure compiler doesn't optimise variable out in test

In the test gdb.base/funcargs.exp, there's this function:

    void recurse (SVAL a, int depth)
    {
      a.s = a.i = a.l = --depth;
      if (depth == 0)
        hitbottom ();
      else
        recurse (a, depth);
    }

The test script places a breakpoint in hitbottom, and runs the
executable which calls recurse with an initial depth of 4.

When GDB hits the breakpoint in hitbottom the testscript performs a
backtrace, and examines 'a' at each level.

The problem is that 'a' is not live after either the call to
'hitbottom' or the call to 'recurse', and as a result the test fails.

In the particular case I was looking at GCC for RISC-V 32-bit, the
variable 'a' is on the stack and GCC selects the register $ra (the
return address register) to hold the pointer to 'a'.  This is fine,
because, by the time the $ra register is needed to hold a return
address (calling hitbottom or recurse) then 'a' is dead.

In this patch I propose that a use of 'a' is added after the calls to
hitbottom and recurse, this should cause the compiler to keep 'a'
around, which should ensure GDB can find it.

gdb/testsuite/ChangeLog:

        * gdb.base/funcargs.c (use_a): New function.
        (recurse): Call use_a.

diff --git a/gdb/testsuite/gdb.base/funcargs.c b/gdb/testsuite/gdb.base/funcargs.c
index 600792f0a7e..d5ff19218c6 100644
--- a/gdb/testsuite/gdb.base/funcargs.c
+++ b/gdb/testsuite/gdb.base/funcargs.c
@@ -424,6 +424,12 @@ void hitbottom ()
 {
 }
 
+void use_a (SVAL a)
+{
+  /* Trick the compiler into thinking A is important.  */
+  volatile SVAL dummy = a;
+}
+
 void recurse (SVAL a, int depth)
 {
   a.s = a.i = a.l = --depth;
@@ -431,6 +437,9 @@ void recurse (SVAL a, int depth)
     hitbottom ();
   else
     recurse (a, depth);
+
+  /* Ensure A is not discarded after the above calls.  */
+  use_a (a);
 }
 
 void test_struct_args ()