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

Message ID 20180829180259.2718-1-andrew.burgess@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess Aug. 29, 2018, 6:02 p.m. UTC
  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(+)
  

Comments

Tom Tromey Aug. 29, 2018, 6:26 p.m. UTC | #1
>>>>> "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
  
Palmer Dabbelt Aug. 30, 2018, 12:21 a.m. UTC | #2
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.
  
Tom Tromey Aug. 30, 2018, 2:30 a.m. UTC | #3
>>>>> "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
  
Ruslan Kabatsayev Aug. 30, 2018, 7:06 a.m. UTC | #4
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
  

Patch

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