From patchwork Thu Aug 30 08:12:00 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 29127 Received: (qmail 52838 invoked by alias); 30 Aug 2018 08:12:06 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 51814 invoked by uid 89); 30 Aug 2018 08:12: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=Trick, Hx-languages-length:4683 X-HELO: mail-wr1-f68.google.com Received: from mail-wr1-f68.google.com (HELO mail-wr1-f68.google.com) (209.85.221.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 30 Aug 2018 08:12:04 +0000 Received: by mail-wr1-f68.google.com with SMTP id u12-v6so7158797wrr.4 for ; Thu, 30 Aug 2018 01:12:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=r5cIZ0Iph9HyobTJ8MYQNgN70VLvLebS63TdHZ8ddkY=; b=ggGo2Tq4TU1gMWPJeezE63jqfCuoFGcLsNCElpGaKJbFBZShFLATkB/LeuXXMhMEW6 06RkEso4FF2cgyWm9/tFZbIwye/DBbxi1tuy9gsvtGI+bNkOcRqEbbg+4+YqHEvsHlhv sdMiqm8V0pgSX/RAPOomqjl10/0Ix/9cidiGQmp3HNJAtdjFSznAhYnnfwQwqjFDZhMF eLEPoOTDwBTLqd0chbFjf8JoCm5bN4syH5LAaIfTqPSYGoYa6Oepz8s3Em+6BKc3kAXK qyLa87xhjD4CAHn2Hxex/fjTB0nTDD8wKFWEQjCy/YSfBgEjuY8RCfw6Guu94V8+ylrK xHvQ== Return-Path: Received: from localhost (host86-164-85-29.range86-164.btcentralplus.com. [86.164.85.29]) by smtp.gmail.com with ESMTPSA id j66-v6sm10057673wrj.28.2018.08.30.01.12.00 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 30 Aug 2018 01:12:00 -0700 (PDT) Date: Thu, 30 Aug 2018 09:12:00 +0100 From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Palmer Dabbelt , Ruslan Kabatsayev Subject: Re: [PATCH] gdb: Ensure compiler doesn't optimise variable out in test Message-ID: <20180830081200.GP32506@embecosm.com> References: <20180829180259.2718-1-andrew.burgess@embecosm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Fortune: Decreasing electron flux X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes * Palmer Dabbelt [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 ()