From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 29D813858C50 for ; Tue, 22 Nov 2022 09:42:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 29D813858C50 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1669110162; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ghQF7W3xka1GlSrehs4Oz8aEZ/36mQl0/cFlo73FZ3I=; b=iF0rhkgQz0XEkf/YpsIiRrhLiCKvCBa5Qn7TzdRqHWLkacNbs0Yo9paFMhwYIxOPY0GNLJ QBKtsXjhlbiqTp/iYOCO1LGtqyP0DMTbKAN+j8XdW02Plg4Zg+iKd9rBSsjpxI8+Ubr+Aa 8X1Zl6fgp5azVMeW8znbvNxhQVTxWj0= Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-549-lxaaIsesPaSNb8FAN7CKXQ-1; Tue, 22 Nov 2022 04:42:41 -0500 X-MC-Unique: lxaaIsesPaSNb8FAN7CKXQ-1 Received: by mail-ej1-f70.google.com with SMTP id sh31-20020a1709076e9f00b007ae32b7eb51so7892535ejc.9 for ; Tue, 22 Nov 2022 01:42:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ghQF7W3xka1GlSrehs4Oz8aEZ/36mQl0/cFlo73FZ3I=; b=n6edz3dkpkEQtr/EN4ThMhH4qc2j2fHqx/AQFxlnQxlzrYeSHHUORgNCiolX4Bm2RC lkXfxMa7HP5TqzjOhAPA8nkWN5qLUgSr8f4VkQVdrhgXbxT3czErPqbSIGnTG01KWeZ4 uSllu2wJ1a+L4cnKngDDMs73uiAqA1teB2wfXMuEGK0dClacnnWzZDC8fDKbTyfvCFrm vJ5RiEu6kyJFOzNP+SSXXMweLskU9lgKOWUnNGLv/LEGrzGNAgggzTqflMqHSKfEjzH5 463WLYAiuZKH8AmM3nEgmD7pmFYijnGuazucduhKSSpf0Y+SpQImBK17YMDSsNUd/vAB tFxQ== X-Gm-Message-State: ANoB5pn4KmVN5nCOi0T05HBaWBts+idbrjUvTtK2sF/PmsttBfPWnQCv yf7+hfvHP5NVNnY+UuI3N0Onjm3Xq7fuZzwWdn5yUwWBBJoQEEl9UM9oowkPjtto/njqZsW2Bju PtJp+1HsQP29f7Pwl/d9XmQ== X-Received: by 2002:a17:906:1d14:b0:7ad:79c0:46f4 with SMTP id n20-20020a1709061d1400b007ad79c046f4mr18873554ejh.363.1669110160197; Tue, 22 Nov 2022 01:42:40 -0800 (PST) X-Google-Smtp-Source: AA0mqf6Xs6hnPf9n2Qg9HMKtd+A7UH7nvKTvigCABLQKvnUQ8pFAnHJfxjIDZI6BMEq1qsVcxj75uA== X-Received: by 2002:a17:906:1d14:b0:7ad:79c0:46f4 with SMTP id n20-20020a1709061d1400b007ad79c046f4mr18873540ejh.363.1669110159800; Tue, 22 Nov 2022 01:42:39 -0800 (PST) Received: from [10.43.2.105] (nat-pool-brq-t.redhat.com. [213.175.37.10]) by smtp.gmail.com with ESMTPSA id ku19-20020a170907789300b007ae1ab8f887sm5926561ejc.14.2022.11.22.01.42.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 22 Nov 2022 01:42:39 -0800 (PST) Message-ID: Date: Tue, 22 Nov 2022 10:42:38 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [PATCH] PowerPC, fix gdb.reverse/finish-reverse-bkpt.exp and gdb.reverse/next-reverse-bkpt-over-sr.exp To: Carl Love , "gdb-patches@sourceware.org" Cc: Ulrich Weigand , Will Schmidt References: <69b2451b-1baf-8bd4-25dd-a1b46963981f@redhat.com> From: Bruno Larsen In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 14/11/2022 22:05, Carl Love wrote: > Bruno, GDB maintainers: > > The first attempt to fix test next-reverse-bkpt-over-sr.exp basically > breaks the test per the comments from Bruno in the thread with the > subject [PATCH] Fix test next-reverse-bkpt-over-sr.exp. > > The following is a second, new approach to fixing the issue. > > PowerPC uses a Global Entry Point (GEP) and a Local Entry Point (LEP). > The GEP is located on the first instruction of the function. It sets > up register r2 before the LEP. > > : > lis r2,4098 <- GEP > addi r2,r2,32512 > mflr r0 <- LEP > std r0,16(r1) > ... > > The issue is the gdb command break *callee sets the breakpoint on the > first instruction in the function. The call to function callee enters > the function at the LEP. Thus gdb never "sees" the breakpoint at the > GEP resulting in the test failing on PowerPC. If the function is > called using a function pointer, then the linker will setup the call to > use the GEP rather instead of the LEP and the test then works as > expected on PowerPC. > > A new function pointer call to callee afollowed by a normal call to > callee is added for test next-reverse-bkpt-over-sr.exp at the end of > the main so as to not change the test behavior of step-reverse.exp. > Note, the source file step-reverse.c is used by both test next-reverse- > bkpt-over-sr.exp and test step-reverse.exp. Test next-reverse-bkpt- > over-sr.exp starts at the new call to callee at the end of the main > program. The rest of the test runs without changes. By entering via > the GEP on PowerPC, the breakpoint is seen and the test passes as > expected. > > Note, on non PowerPC systems the GEP and LEP are the same. Calling > function callee normally or with a function pointer has the same > behavior on non-PowerPC systems. This was specifically verified on > Intel X86-64. > > Test gdb.reverse/finish-reverse-bkpt.exp also fails on PowerPC for the > same reasons as discussed above. Again, this test is modified by > adding a function pointer call to void_func at the end of main. The > source code is used for both test finish-reverse-bkpt.exp and finish- > reverse.exp so the a breakpoint is added to finish-reverse-bkpt.exp to > stop before the function pointer call to void_func where the test then > starts. > > The addition of the function pointer declarations in the two source > files require an additional next statement to tests step-precsave.exp > and step-reverse.exp to keep the keep things lined up. > > Hopefully, this approach to fixing the failing tests on PowerPC is > acceptable without changing the behavior on non-PowerPC platforms. > > The patch has been tested on both PowerPC and X86-64 with no > regressions. > > Please let me know if this patch is acceptable. Thanks. > > Carl Love > > -------------------------------- > PowerPC, fix gdb.reverse/finish-reverse-bkpt.exp and gdb.reverse/next-reverse-bkpt-over-sr.exp > > The tests set a break point with the command break *func. This sets a > breakpoint on the first instruction of the function. PowerPC uses > Global Entry Points (GEP) and Local Entry Points (LEP). The first > instruction in the function is the GEP. The GEP sets up register > r2 before reaching the LEP. When the function is called with func() the > function is entered via the LEP and the test fails because GDB does not > see the breakpoint on the GEP. However, if the function is called via a > function pointer, execution begins at the GEP as the test expects. > > The tests were modified to call the function with a function pointer so > the test will work correctly on both PowerPC with a GEP and LEP as well as > on other systems. The GEP is the same as the LEP on non PowerPC systems. > > This patch fixes two PowerPC test failures in each of the tests > gdb.reverse/finish-reverse-bkpt.exp and > gdb.reverse/next-reverse-bkpt-over-sr.exp. > > Patch tested on PowerPC and Intel X86-64 with no regressions. Hi Carl, This was a great approach to solve the problem, but the location of the funcp() call in step-reverse.c is generating regressions on my machine. Currently, GDB can't record memset calls in x86_64 (at least using gcc-12.2.1), due to the instruction 0xc5. Moving the funcp and callee calls above the block that contains memset fixes the regression and should still fix the PowerPC issue. Also, style nits inlined. > --- > .../gdb.reverse/finish-reverse-bkpt.exp | 30 +++++++++++++++++++ > gdb/testsuite/gdb.reverse/finish-reverse.c | 4 +++ > .../gdb.reverse/next-reverse-bkpt-over-sr.exp | 29 ++++++++++++++++-- > gdb/testsuite/gdb.reverse/step-precsave.exp | 1 + > gdb/testsuite/gdb.reverse/step-reverse.c | 9 ++++++ > gdb/testsuite/gdb.reverse/step-reverse.exp | 1 + > 6 files changed, 71 insertions(+), 3 deletions(-) > > diff --git a/gdb/testsuite/gdb.reverse/finish-reverse-bkpt.exp b/gdb/testsuite/gdb.reverse/finish-reverse-bkpt.exp > index 2a204748d98..94bcf41dc67 100644 > --- a/gdb/testsuite/gdb.reverse/finish-reverse-bkpt.exp > +++ b/gdb/testsuite/gdb.reverse/finish-reverse-bkpt.exp > @@ -19,6 +19,27 @@ > # the functions entry would be ignored. Make sure the bug doesn't > # reappear. > > +# The test sets a breakpoint with the command break *void_func to set a > +# breakpoint on the first instruction of the function. The issue is on > +# PowerPC it uses Global Entry Points (GEP) and Local Entry Points (LEP). > +# The GEP is the first instruction in the function. It sets up register > +# r2 and then reaches the LEP. > +# > +# : > +# lis r2,4098 <- GEP > +# addi r2,r2,32512 > +# mflr r0 <- LEP > +# std r0,16(r1) > +# .... > + > +# > +# The command break *void_func sets the breakpoint on the GEP. Calling > +# the function with void_func() will enter the function via the LEP. So, > +# this test needs to use a function pointer to call void_func() so the > +# function will be entered via the GEP to work as designed on PowerPC in > +# addition to non-PowerPC systems. On non-PowerPC systems, the GEP and LEP > +# are the same. > + > if ![supports_reverse] { > return > } > @@ -38,6 +59,15 @@ if [supports_process_record] { > gdb_test_no_output "record" "turn on process record" > } > > +# Move to the function pointer call to void_func so we will use the GEP > +# to enter void_func and break. > +set breakloc [gdb_get_line_number "FUNCTION PTR" "$srcfile"] > +gdb_test "break $breakloc" \ > + "Breakpoint $decimal at .*$srcfile, line $breakloc\." \ > + "set breakpoint on funp" > +gdb_continue_to_breakpoint "funp call" ".*$srcfile:$breakloc.*" > + > +# Start the test > set breakloc [gdb_get_line_number "VOID FUNC" "$srcfile"] > gdb_test "tbreak void_func" \ > "Temporary breakpoint $decimal at .*$srcfile, line $breakloc\." \ > diff --git a/gdb/testsuite/gdb.reverse/finish-reverse.c b/gdb/testsuite/gdb.reverse/finish-reverse.c > index 316d6f6aa7e..676d960ca9c 100644 > --- a/gdb/testsuite/gdb.reverse/finish-reverse.c > +++ b/gdb/testsuite/gdb.reverse/finish-reverse.c > @@ -89,6 +89,7 @@ int main (int argc, char **argv) > float float_resultval; > double double_resultval; > int i; > + void (*funp) (void) = void_func; > > /* A "test load" that will insure that the function really returns > a ${type} (as opposed to just a truncated or part of a ${type}). */ > @@ -123,6 +124,9 @@ int main (int argc, char **argv) > testval.double_testval = 3.14159265358979323846; /* float_checkpoint */ > double_resultval = double_func (); > main_test = 1; /* double_checkpoint */ > + > + /* This call is used with finish-reverse-bkpt.exp. */ > + funp(); /* FUNCTION PTR call to void_func */ Missing space before ( > return 0; > } /* end of main */ > > diff --git a/gdb/testsuite/gdb.reverse/next-reverse-bkpt-over-sr.exp b/gdb/testsuite/gdb.reverse/next-reverse-bkpt-over-sr.exp > index 6ef56d30e7b..d78c6ac490e 100644 > --- a/gdb/testsuite/gdb.reverse/next-reverse-bkpt-over-sr.exp > +++ b/gdb/testsuite/gdb.reverse/next-reverse-bkpt-over-sr.exp > @@ -22,6 +22,25 @@ > # to get at the callee's caller. Test that a user breakpoint set at > # the same location as the step-resume breakpoint isn't ignored. > # > +# The test sets a breakpoint with the command break *callee to set a > +# breakpoint on the first instruction of the function. The issue is on > +# PowerPC it uses Global Entry Points (GEP) and Local Entry Points (LEP). > +# The GEP is the first instruction in the function. It sets up register > +# r2 and then reaches the LEP. > +# > +# : > +# lis r2,4098 <- GEP > +# addi r2,r2,32512 > +# mflr r0 <- LEP > +# std r0,16(r1) > + > +# > +# The command break *callee sets the breakpoint on the GEP. Calling > +# the function with callee() will enter the function via the LEP. So, > +# this test needs to use a function pointer to call callee() so the > +# function will be entered via the GEP to work as designed on PowerPC in > +# addition to non-PowerPC systems. On non-PowerPC systems, the GEP and LEP > +# are the same. > > if ![supports_reverse] { > return > @@ -42,8 +61,12 @@ if [supports_process_record] { > gdb_test_no_output "record" "turn on process record" > } > > -set lineno [gdb_get_line_number "STEP INTO THIS CALL"] > -gdb_test "advance $lineno" ".*STEP INTO THIS CALL.*" "get past callee call" > +# Move to the function pointer call to the callee call after the function > +# pointer call to callee to begin the test. The function pointer call to > +# callee will use the Global Entry Point on Power. > +set lineno [gdb_get_line_number "STEP INTO CALL AFTER FUNP CALL"] > +gdb_test "advance $lineno" ".*STEP INTO CALL AFTER FUNP CALL.*" \ > + "get past callee call" > > gdb_test "b \*callee" "" "set breakpoint at callee's entry" > > @@ -53,5 +76,5 @@ gdb_test "reverse-next" \ > "reverse-next over call trips user breakpoint at function entry" > > gdb_test "up" \ > - ".*NEXT OVER THIS CALL.*" \ > + ".*FUNCTION PTR CALL TO CALLEE.*" \ > "stopped at the right callee call" > diff --git a/gdb/testsuite/gdb.reverse/step-precsave.exp b/gdb/testsuite/gdb.reverse/step-precsave.exp > index 3279b6ce879..602dd7e6976 100644 > --- a/gdb/testsuite/gdb.reverse/step-precsave.exp > +++ b/gdb/testsuite/gdb.reverse/step-precsave.exp > @@ -76,6 +76,7 @@ gdb_test "record restore $precsave" \ > > # plain vanilla step/next (no count) > > +gdb_test "next" ".*BREAK AT MAIN.*" "next past funp declaration" > gdb_test "next" ".*NEXT TEST 1.*" "next test 1" > gdb_test "step" ".*STEP TEST 1.*" "step test 1" > > diff --git a/gdb/testsuite/gdb.reverse/step-reverse.c b/gdb/testsuite/gdb.reverse/step-reverse.c > index 809c7d16dc9..984fd336510 100644 > --- a/gdb/testsuite/gdb.reverse/step-reverse.c > +++ b/gdb/testsuite/gdb.reverse/step-reverse.c > @@ -54,6 +54,7 @@ large_struct_by_value (struct rhomboidal r) > int main () { > int w,x,y,z; > int a[10], b[10]; > + int (*funp) (void) = callee; > > /* Test "next" and "step" */ > w = 0; /* BREAK AT MAIN */ > @@ -90,6 +91,14 @@ int main () { > large_struct_by_value (r); /* step-test.exp: large struct by value */ > } > > + /* Test next-reverse-bkpt-over-sr.exp needs to call function callee using > + a function pointer to work correctly on PowerPC. See comments in > + next-reverse-bkpt-over-sr.exp. */ > + funp(); /* FUNCTION PTR CALL TO CALLEE */ again here > + > + /* Test that "step" doesn't */ > + callee(); /* STEP INTO CALL AFTER FUNP CALL */ and here. > + > exit (0); /* end of main */ > } > > diff --git a/gdb/testsuite/gdb.reverse/step-reverse.exp b/gdb/testsuite/gdb.reverse/step-reverse.exp > index d2975cffb5c..bce137a97ad 100644 > --- a/gdb/testsuite/gdb.reverse/step-reverse.exp > +++ b/gdb/testsuite/gdb.reverse/step-reverse.exp > @@ -40,6 +40,7 @@ if [supports_process_record] { > > # plain vanilla step/next (no count) > > +gdb_test "next" ".*BREAK AT MAIN.*" "next past funp declaration" > gdb_test "next" ".*NEXT TEST 1.*" "next test 1" > gdb_test "step" ".*STEP TEST 1.*" "step test 1" > -- Cheers, Bruno