From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 71496 invoked by alias); 29 Dec 2017 18:05:24 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 71087 invoked by uid 89); 29 Dec 2017 18:05:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.6 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,KAM_SHORT,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=nicolas, Nicolas, opposed, HX-Proofpoint-Spam-Details:1011 X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0a-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.156.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 29 Dec 2017 18:05:20 +0000 Received: from pps.filterd (m0098404.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id vBTI5176113352 for ; Fri, 29 Dec 2017 13:05:19 -0500 Received: from e14.ny.us.ibm.com (e14.ny.us.ibm.com [129.33.205.204]) by mx0a-001b2d01.pphosted.com with ESMTP id 2f5sqqsa8y-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Fri, 29 Dec 2017 13:05:18 -0500 Received: from localhost by e14.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 29 Dec 2017 13:05:15 -0500 Received: from b01cxnp22035.gho.pok.ibm.com (9.57.198.25) by e14.ny.us.ibm.com (146.89.104.201) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 29 Dec 2017 13:05:12 -0500 Received: from b01ledav005.gho.pok.ibm.com (b01ledav005.gho.pok.ibm.com [9.57.199.110]) by b01cxnp22035.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id vBTI5CD749938532; Fri, 29 Dec 2017 18:05:12 GMT Received: from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A1964AE043; Fri, 29 Dec 2017 13:06:18 -0500 (EST) Received: from pedro.localdomain (unknown [9.80.221.254]) by b01ledav005.gho.pok.ibm.com (Postfix) with ESMTP id 19D93AE04E; Fri, 29 Dec 2017 13:06:18 -0500 (EST) Received: by pedro.localdomain (Postfix, from userid 1000) id B75B53C4FEB; Fri, 29 Dec 2017 16:05:08 -0200 (-02) From: Pedro Franco de Carvalho To: Nikola Prica , Kevin Buettner Cc: gdb-patches@sourceware.org, "Ananthakrishna Sowda \(asowda\)" , "Ivan Baev \(ibaev\)" , "'Nemanja Popov'" , Djordje Todorovic , Ulrich.Weigand@de.ibm.com Subject: Re: [PING][PATCH] Fix for prologue processing on PowerPC In-Reply-To: References: <20171108095850.394a48ca@pinnacle.lan> <8bf0014c-e83c-5988-4d06-173572f21186@rt-rk.com> <7ba16b14-9384-34d9-937e-531a2192842a@linux.vnet.ibm.com> Date: Fri, 29 Dec 2017 18:05:00 -0000 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-TM-AS-GCONF: 00 x-cbid: 17122918-0052-0000-0000-000002986FCF X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00008282; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000244; SDB=6.00967360; UDB=6.00489701; IPR=6.00747333; BA=6.00005762; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00018773; XFM=3.00000015; UTC=2017-12-29 18:05:15 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17122918-0053-0000-0000-0000530B9209 Message-Id: <87608p4dgr.fsf@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-12-29_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=7 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1712290254 X-IsSubscribed: yes X-SW-Source: 2017-12/txt/msg00519.txt.bz2 Hello Nikola, > > - I think it's more clear to only set lr_register when needed (pc > > reaches the limit), as opposed to resetting it to -1 if pc didn't reach > > the limit. > > The body of condition that becomes visitable after this patch=20 > invalidates lr_reg by setting it to -2 before reaching the limit. Sorry, I was referring to to fdata->lr_register. What I meant is that it seems better to me to only set fdata->lr_register to lr_reg >> 21 at the end of the function (as Kevin had suggested), as opposed to setting it to -1 at the end of the function, in this part: > + if (pc !=3D lim_pc) > + fdata->lr_register =3D -1; In any case, for this part to be correct, shouldn't the condition be the full complement of the original condition? That is, if (pc !=3D lim_pc || lr_reg < 0) Which is the complement of: if (pc =3D=3D lim_pc && lr_reg >=3D 0) Instead of: > + if (pc !=3D lim_pc) Previously if there was a mflr/stw sequence that invalidated lr_reg, fdata->lr_register would be -1 when the function exited, even if pc =3D=3D lim_pc (because of the second term of the condition). Still, I think it's safer to keep fdata->lr_register =3D=3D -1 and set it at the end if needed. I wasn't able to apply your patch directly, I think there are some extra whitespaces around there. And thank you for providing a testcase. There's just a few comments: > +if {![istarget "powerpc*"] } { > + verbose "Skipping powerpc back trace test." > + return > +} > + > + > +set main_testfile ppc-prolog-frame > + > +if {![istarget "powerpc*-*-*"]} then { > + verbose "Skipping PowerPC instructions disassembly." > + return -1 > +} There is an extra target check here. Also, because the assembly file uses the 32-bit abi I think the target selection should be: "istarget "powerpc-*-*"", this way it excludes 64-bit targets. The main_testfile name doesn't matche the test file name. In any case, you = can use the standard_testfile macro like this: standard_testfile .c .S It will set $srcfile and $srcfile2 to powerpc-prologue-frame.c/.S, respectively. > ${srcdir}/${subdir}/$main_testfile.S] \ > + [standard_output_file ${main_testfile}] \ > + executable {debug}] !=3D ""} { The option flag should be empty ({}), because we want to test the case without debug information. I tried the test and the regular expression didn't seem to match. This is probably because of the "at" parts: > +gdb_test "bt" \ > + "#0 \[x0-9a-f\]* bar \\(\\) at .*#1 \[x0-9a-f in\]* foo \\(\\)= =20 > at .*#2 \[x0-9a-f in\]* main \\(\\) at .*" \ Does your gdb output "at" instead of in? I tried a recent version and it ou= tputs "in". I adapted one from the powerpc-prologue.exp test test that seemed to work: gdb_test "bt" \ "#0\[ \t\]*$hex in bar.*\r\n#1\[ \t\]*$hex in foo.*\r\n#2\[ \t\]*$hex i= n main.*" \ "Backtrace to the main frame" It might be good to also remove unecessary info from the assembly file for inclusion in the testcase, e.g. the .ident directive. I'm not sure of what are the minimum necessary directives according to the abi, but looking at another test, powerpc-stackless.exp, the corresponding assembly source reads: #include FUNC_START(main) li sp,0 mtlr sp blr FUNC_END(main) These macros could be useful here (in this case the extension should be .S so that gcc will preprocess the file). See below for a diff that incorporate these comments. Feel free to include the changes in your patch. Thanks, Pedro diff --git a/gdb/testsuite/gdb.arch/powerpc-prologue-frame.S b/gdb/testsuit= e/gdb.arch/powerpc-prologue-frame.S new file mode 100644 index 0000000000..8697213934 --- /dev/null +++ b/gdb/testsuite/gdb.arch/powerpc-prologue-frame.S @@ -0,0 +1,35 @@ +/* This test is part of GDB, the GNU debugger. + + Copyright 2017 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . = */ + +#include + +FUNC_START(foo) + stwu 1,-32(1) + mflr 3 + stw 3,36(1) + stw 31,28(1) + mr 31,1 + bl bar + mr 9,3 + mr 3,9 + addi 11,31,32 + lwz 0,4(11) + mtlr 0 + lwz 31,-4(11) + mr 1,11 + blr +FUNC_END(foo) diff --git a/gdb/testsuite/gdb.arch/powerpc-prologue-frame.c b/gdb/testsuit= e/gdb.arch/powerpc-prologue-frame.c new file mode 100644 index 0000000000..7a743aff8e --- /dev/null +++ b/gdb/testsuite/gdb.arch/powerpc-prologue-frame.c @@ -0,0 +1,31 @@ +/* This test is part of GDB, the GNU debugger. + + Copyright 2017 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . = */ + +int +bar (void) +{ + return 0; +} + +int +foo (void); + +int +main (void) +{ + return foo (); +} diff --git a/gdb/testsuite/gdb.arch/powerpc-prologue-frame.exp b/gdb/testsu= ite/gdb.arch/powerpc-prologue-frame.exp new file mode 100644 index 0000000000..f124717169 --- /dev/null +++ b/gdb/testsuite/gdb.arch/powerpc-prologue-frame.exp @@ -0,0 +1,43 @@ +# Copyright 2017 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see + +# Test the PowerPC prologue analyzer when a register rx other than r0 is u= sed +# to save the link register in the mflr rx/stw rx sequence. + +if {![istarget "powerpc*-*-*"]} then { + verbose "Skipping PowerPC prologue tests." + return +} + +standard_testfile .c .S + +set binfile [standard_output_file ${testfile}] + +if {[gdb_compile [list "${srcdir}/${subdir}/${srcfile}" "${srcdir}/${subdi= r}/${srcfile2}"] \ + "${binfile}" executable {}] !=3D ""} { + untested "Failed to build ${binfile}" + return -1 +} + +clean_restart ${binfile} + +if ![runto bar] { + untested "could not run to bar" + return -1 +} + +gdb_test "bt" \ + "#0\[ \t\]*$hex in bar.*\r\n#1\[ \t\]*$hex in foo.*\r\n#2\[ \t\]*$hex = in main.*" \ + "Backtrace to the main frame" Nikola Prica writes: > Hello Pedro and Kevi, > > I've created and tested example based on your example. Thank you for that. > > > - I think it's more clear to only set lr_register when needed (pc > > reaches the limit), as opposed to resetting it to -1 if pc didn't reach > > the limit. > > The body of condition that becomes visitable after this patch=20 > invalidates lr_reg by setting it to -2 before reaching the limit. > > else if (lr_reg >=3D 0 && > /* std Rx, NUM(r1) || stdu Rx, NUM(r1) */ > (((op & 0xffff0000) =3D=3D (lr_reg | 0xf8010000)) || > /* stw Rx, NUM(r1) */ > ((op & 0xffff0000) =3D=3D (lr_reg | 0x90010000)) || > /* stwu Rx, NUM(r1) */ > ((op & 0xffff0000) =3D=3D (lr_reg | 0x94010000)))) > { /* where Rx =3D=3D lr */ > fdata->lr_offset =3D offset; > fdata->nosavedpc =3D 0; > /* Invalidate lr_reg, but don't set it to -1. > That would mean that it had never been set. */ > lr_reg =3D -2; > ... > > Thanks, > > Nikola > > From 9aaddf9670d9f4cb7f088499febd1fa9c6a7076c Mon Sep 17 00:00:00 2001 > From: Prica > Date: Tue, 19 Dec 2017 14:29:09 +0100 > Subject: [PATCH] Fix for prologue processing on PowerPc > > One of conditions in skip_prologue() is never visited because it > expects non shifted `lr_reg`. That condtition is supposed to set PC > offset. When body of this condition is visited PC offset is set and > there will be no need to look for it in next frames nor to use frame > unwind directives. > > gdb/ChangeLog: > *rs600-tdep.c (skip_prologue): Remove shifting for lr_reg and assign > shifted lr_reg to fdata->lr_register when lr_reg is set. If > iteration do not hit lim_pc lr_register is set as -1. > *testsuite/gdb.arch/ppc-prologue-frame.s: New file. > *testsuite/gdb.arch/ppc-prologue-frame.c: Likewise. > *testsuite/gdb.arch/ppr-prologue-frame.exp: Likewise. > --- > gdb/rs6000-tdep.c | 14 ++++--- > gdb/testsuite/gdb.arch/powerpc-prologue-frame.c | 28 +++++++++++++ > gdb/testsuite/gdb.arch/powerpc-prologue-frame.exp | 48=20 > +++++++++++++++++++++++ > gdb/testsuite/gdb.arch/powerpc-prologue-frame.s | 40 ++++++++++++++++= +++ > 4 files changed, 125 insertions(+), 5 deletions(-) > create mode 100644 gdb/testsuite/gdb.arch/powerpc-prologue-frame.c > create mode 100644 gdb/testsuite/gdb.arch/powerpc-prologue-frame.exp > create mode 100644 gdb/testsuite/gdb.arch/powerpc-prologue-frame.s > > diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c > index 456dbcc..f0d2781 100644 > --- a/gdb/rs6000-tdep.c > +++ b/gdb/rs6000-tdep.c > @@ -1655,9 +1655,13 @@ skip_prologue (struct gdbarch *gdbarch, CORE_ADDR= =20 > pc, CORE_ADDR lim_pc, > remember just the first one, but skip over additional > ones. */ > if (lr_reg =3D=3D -1) > - lr_reg =3D (op & 0x03e00000) >> 21; > - if (lr_reg =3D=3D 0) > - r0_contains_arg =3D 0; > + { > + lr_reg =3D (op & 0x03e00000); > + fdata->lr_register =3D lr_reg >> 21; > + } > + if (lr_reg =3D=3D 0) > + r0_contains_arg =3D 0; > + > continue; > } > else if ((op & 0xfc1fffff) =3D=3D 0x7c000026) > @@ -2180,8 +2184,8 @@ skip_prologue (struct gdbarch *gdbarch, CORE_ADDR=20 > pc, CORE_ADDR lim_pc, > } > #endif /* 0 */ > > - if (pc =3D=3D lim_pc && lr_reg >=3D 0) > - fdata->lr_register =3D lr_reg; > + if (pc !=3D lim_pc) > + fdata->lr_register =3D -1; > > fdata->offset =3D -fdata->offset; > return last_prologue_pc; > diff --git a/gdb/testsuite/gdb.arch/powerpc-prologue-frame.c=20 > b/gdb/testsuite/gdb.arch/powerpc-prologue-frame.c > new file mode 100644 > index 0000000..f59210a > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/powerpc-prologue-frame.c > @@ -0,0 +1,28 @@ > +/* This test is part of GDB, the GNU debugger. > + > + Copyright 2017 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see=20 > . */ > + > +int bar() > +{ > + return 0; > +} > + > +int foo(); > + > +int main(void) > +{ > + return foo(); > +} > diff --git a/gdb/testsuite/gdb.arch/powerpc-prologue-frame.exp=20 > b/gdb/testsuite/gdb.arch/powerpc-prologue-frame.exp > new file mode 100644 > index 0000000..e90a8c1 > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/powerpc-prologue-frame.exp > @@ -0,0 +1,48 @@ > +# Copyright 2017 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see > + > +if {![istarget "powerpc*"] } { > + verbose "Skipping powerpc back trace test." > + return > +} > + > + > +set main_testfile ppc-prolog-frame > + > +if {![istarget "powerpc*-*-*"]} then { > + verbose "Skipping PowerPC instructions disassembly." > + return -1 > +} > + > + > +if {[gdb_compile \ > + [list ${srcdir}/${subdir}/$main_testfile.c=20 > ${srcdir}/${subdir}/$main_testfile.S] \ > + [standard_output_file ${main_testfile}] \ > + executable {debug}] !=3D ""} { > + untested "failed to build $main_testfile" > + return -1 > +} > + > + > +clean_restart ${main_testfile} > + > +if ![runto bar] { > + untested "could not run to bar" > + return -1 > +} > + > +gdb_test "bt" \ > + "#0 \[x0-9a-f\]* bar \\(\\) at .*#1 \[x0-9a-f in\]* foo \\(\\)= =20 > at .*#2 \[x0-9a-f in\]* main \\(\\) at .*" \ > + "Backtrace to the main frame" > diff --git a/gdb/testsuite/gdb.arch/powerpc-prologue-frame.s=20 > b/gdb/testsuite/gdb.arch/powerpc-prologue-frame.s > new file mode 100644 > index 0000000..16cd7e2 > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/powerpc-prologue-frame.s > @@ -0,0 +1,40 @@ > +/* This test is part of GDB, the GNU debugger. > + > + Copyright 2017 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see=20 > . */ > + > + .file "foo.c" > + .section ".text" > + .align 2 > + .globl foo > + .type foo, @function > +foo: > + stwu 1,-32(1) > + mflr 3 > + stw 3,36(1) > + stw 31,28(1) > + mr 31,1 > + bl bar > + mr 9,3 > + mr 3,9 > + addi 11,31,32 > + lwz 0,4(11) > + mtlr 0 > + lwz 31,-4(11) > + mr 1,11 > + blr > + .size foo,.-foo > + .ident "GCC: (Ubuntu 4.8.2-19ubuntu1) 4.8.2" > + .section .note.GNU-stack,"",@progbits > --=20 > > > On 01.12.2017. 20:36, pedromfc wrote: >> Hello Nikola, Kevin, >>=20 >> Thank you for providing these patches. >>=20 >> I tested both patches (Nicola's second patch and Kevin's patch) on=20 >> ppc64le, and there were no regressions, except for some additional=20 >> expected failures in gdb.threads/attach-many-short-lived-threads.exp. >>=20 >> Some comments: >>=20 >> - Nikola's patch moves "if(lr_reg =3D=3D 0)/r0_contains_arg =3D 0;" to w= ithin=20 >> the first if. This is useful for the case when a second mflr Rx with Rx= =20 >> !=3D 0 is detected. Previously this would cause r0_contains_arg to be=20 >> reset, despite Rx not being 0. However, if the second mflr has Rx =3D=3D= 0,=20 >> it would make sense to reset r0_contains_arg (and this would work=20 >> accidentally before the patch, assuming the first mflr also had Rx =3D= =3D=20 >> 0). Maybe the best solution here is to always check the Rx contained in= =20 >> the opcode and clear r0_contains_arg if it is 0, regardless of lr_reg,=20 >> or leave it as before since it's a separate issue. >>=20 >> - Kevin's patch assigns lr_reg =3D op & 0x03e00000, but lr_reg is an int= ,=20 >> and op is an unsigned long. Will the unshifted reg always fit in a int? >>=20 >> - I think it's more clear to only set lr_register when needed (pc=20 >> reaches the limit), as opposed to resetting it to -1 if pc didn't reach= =20 >> the limit. >>=20 >> - I wasn't able to directly apply Nikola's patch, I did so manually. >>=20 >> I've also attached a reproducer (prologue.c/foo.S) to check if the=20 >> patches fixed the issue. I had to alter a generated assembly file so=20 >> that mflr would use a register other than R0 (in which case the old code= =20 >> does work). >>=20 >> gcc -g0 -O0 -o prologue prologue.c foo.S >>=20 >> (gdb) file prologue >> Reading symbols from prologue...done. >> (gdb) break bar >> Breakpoint 1 at 0x100005b8 >> (gdb) run >>=20 >> Starting program: /home/pedromfc/prologue >>=20 >> Before the patch: >>=20 >> Breakpoint 1, 0x00000000100005b8 in bar () >> (gdb) bt >> #0=C2=A0 0x00000000100005b8 in bar () >> #1=C2=A0 0x0000000010000644 in foo () >> Backtrace stopped: frame did not save the PC >>=20 >> After (both patches): >>=20 >> Breakpoint 1, 0x00000000100005b8 in bar () >> (gdb) bt >> #0=C2=A0 0x00000000100005b8 in bar () >> #1=C2=A0 0x0000000010000644 in foo () >> #2=C2=A0 0x00000000100005f8 in main () >>=20 >> (gdb) info f 1 >> Stack frame at 0x7fffffffeee0: >> =C2=A0pc =3D 0x10000644 in foo; saved pc =3D 0x100005f8 >>=20 >> Thanks! >> Pedro >>=20 >> On 11/09/2017 04:15 PM, Nikola Prica wrote: >>> Hi Kevin, >>> >>> lr_reg could be also set to -2 in part of code which is reachable=20 >>> after shifting removal. >>> >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* Invalidate lr_reg, but don't set it t= o -1. >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 That would mean that i= t had never been set.=C2=A0 */ >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 lr_reg =3D -2; >>> >>> This part of the code which depends of non shifted lr_reg, and the=20 >>> part where shifting is removed are only two places where lr_reg is=20 >>> changed. As so, I've added last condition to set fdata->lr_register on= =20 >>> -1 if lim_pc is not reached. >>> >>> If it seems fine now could you pleas commit it because I don't have=20 >>> rights to do it. >>> >>> Thanks, >>> >>> Nikola Prica >>> >>> >>> From: Prica >>> Date: Thu, 9 Nov 2017 13:10:48 +0100 >>> Subject: Fix for prologue processing on PowerPC >>> >>> One of conditions in skip_prologue() is never visited because it >>> expects non shifted `lr_reg`.=C2=A0 That condition is supposed to set PC >>> offset.=C2=A0 When body of this condition is visited PC offset is set a= nd >>> there will be no need to look for it in next frames nor to use frame >>> unwind directives. >>> >>> gdb/ChangeLog: >>> >>> =C2=A0=C2=A0=C2=A0=C2=A0*rs6000-tdep.c (skip_prologue): Remove shifting= for lr_reg >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 and assign shifted lr_reg to fdata->lr_r= egister when lr_reg is >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 set. If iteration do not hit lim_pc lr_r= egister is set as -1. >>> --- >>> =C2=A0gdb/rs6000-tdep.c | 13 ++++++++----- >>> =C2=A01 file changed, 8 insertions(+), 5 deletions(-) >>> >>> diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c >>> index 6c44995..6f05ef5 100644 >>> --- a/gdb/rs6000-tdep.c >>> +++ b/gdb/rs6000-tdep.c >>> @@ -1655,9 +1655,12 @@ skip_prologue (struct gdbarch *gdbarch,=20 >>> CORE_ADDR pc, CORE_ADDR lim_pc, >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 remember just th= e first one, but skip over additional >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ones.=C2=A0 */ >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (lr_reg =3D=3D -1) >>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 lr_reg =3D (op & 0x03e00000= ) >> 21; >>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (lr_reg =3D= =3D 0) >>> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 r0_= contains_arg =3D 0; >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 { >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 lr_reg =3D (op & 0x03e00000= ); >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 fdata->lr_register =3D lr_r= eg >> 21; >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (lr_reg =3D=3D 0) >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 r0_contains_arg= =3D 0; >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 continue; >>> =C2=A0=C2=A0=C2=A0=C2=A0 } >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 else if ((op & 0xfc1fffff) =3D=3D = 0x7c000026) >>> @@ -2180,8 +2183,8 @@ skip_prologue (struct gdbarch *gdbarch,=20 >>> CORE_ADDR pc, CORE_ADDR lim_pc, >>> =C2=A0=C2=A0=C2=A0=C2=A0 } >>> =C2=A0#endif /* 0 */ >>> >>> -=C2=A0 if (pc =3D=3D lim_pc && lr_reg >=3D 0) >>> -=C2=A0=C2=A0=C2=A0 fdata->lr_register =3D lr_reg; >>> +=C2=A0 if (pc !=3D lim_pc) >>> +=C2=A0=C2=A0=C2=A0 fdata->lr_register =3D -1; >>> >>> =C2=A0=C2=A0 fdata->offset =3D -fdata->offset; >>> =C2=A0=C2=A0 return last_prologue_pc; >>=20