Message ID | 20180829180259.2718-1-andrew.burgess@embecosm.com |
---|---|
State | New |
Headers | show |
>>>>> "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
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.
>>>>> "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
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
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 ()