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

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

[Andrew Burgess]

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 ()
-- 
2.14.4

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

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

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

Andrew> gdb/testsuite/ChangeLog:

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

Thanks, this is ok.

Tom

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

[Palmer Dabbelt]

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.

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

[Tom Tromey]

>>>>> "Palmer" == Palmer Dabbelt <palmer@sifive.com> writes:

Palmer> Isn't the compiler still free to kill "a" here because it can see into
Palmer> use_a() and therefor inline it?  I'd expected it to choose to inline
Palmer> use_a(), as doing nothing is always cheaper than calling a function.

It is but in practice gdb compiles without optimization in most cases
and compilers generally don't bother in that situation.  Though if
there's a readily available, more principled fix, that would be fine
too.

Tom

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

[Ruslan Kabatsayev]

On Thu, 30 Aug 2018 at 05:31, Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Palmer" == Palmer Dabbelt <palmer@sifive.com> writes:
>
> Palmer> Isn't the compiler still free to kill "a" here because it can see into
> Palmer> use_a() and therefor inline it?  I'd expected it to choose to inline
> Palmer> use_a(), as doing nothing is always cheaper than calling a function.
>
> It is but in practice gdb compiles without optimization in most cases
> and compilers generally don't bother in that situation.  Though if
> there's a readily available, more principled fix, that would be fine
> too.

volatile is your friend when trying to make the compiler believe it
doesn't know something :)
volatile SVAL use_a=a;

>
> Tom

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

[Andrew Burgess]

* 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 ()

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

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

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

Thanks.  This is ok as well.

Tom