Message ID | 20180829180259.2718-1-andrew.burgess@embecosm.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 35429 invoked by alias); 29 Aug 2018 18:03:06 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 35416 invoked by uid 89); 29 Aug 2018 18:03:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-wm0-f42.google.com Received: from mail-wm0-f42.google.com (HELO mail-wm0-f42.google.com) (74.125.82.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 29 Aug 2018 18:03:04 +0000 Received: by mail-wm0-f42.google.com with SMTP id f21-v6so6165945wmc.5 for <gdb-patches@sourceware.org>; Wed, 29 Aug 2018 11:03:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id; bh=NeiXTd0ux2E4pi6s4ChDlHb1d6rPvfmFTIGY54XHr+g=; b=QKxy/rMPSFGQctuQ6cKozbKL1aXoWN98T+r/OVytW+A1BaDeQcdPKsHm2cthzRpa1L CA1zoskMs4Sw4vDL4ExLPI0FFMtMf0GjhtLb2U0O9BQdlAv58JUEjC5iTx97qcICBwle EatzpU7XK/FNqgobzmkLp36YGpohWSb3iWXl1JGNmxjlZjb5seD8obXST3zXbBeFeKL5 UEl4JLH7khLP3glzMXoRoFOdIrNjldhhqQVvo3DJoGbMndcsKP1EW4RJogk9v8KVt2Ro k6M+NEqqh8JdceQIlTwZ/rB6v0zuHoUokKsffMv+edAx2xHBYmwr4RXwej17XDbmtBTd xGcQ== Return-Path: <andrew.burgess@embecosm.com> Received: from localhost (host86-134-20-86.range86-134.btcentralplus.com. [86.134.20.86]) by smtp.gmail.com with ESMTPSA id a9-v6sm6446414wmf.28.2018.08.29.11.03.00 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 29 Aug 2018 11:03:01 -0700 (PDT) From: Andrew Burgess <andrew.burgess@embecosm.com> To: gdb-patches@sourceware.org Cc: Andrew Burgess <andrew.burgess@embecosm.com> Subject: [PATCH] gdb: Ensure compiler doesn't optimise variable out in test Date: Wed, 29 Aug 2018 19:02:59 +0100 Message-Id: <20180829180259.2718-1-andrew.burgess@embecosm.com> X-IsSubscribed: yes |
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
>>>>> "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 ()