From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 112598 invoked by alias); 1 Dec 2017 19:37:00 -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 112574 invoked by uid 89); 1 Dec 2017 19:37:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,KB_WAM_FROM_NAME_SINGLEWORD,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=nicolas, @function X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 01 Dec 2017 19:36:58 +0000 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id vB1JXuW6074005 for ; Fri, 1 Dec 2017 14:36:56 -0500 Received: from e14.ny.us.ibm.com (e14.ny.us.ibm.com [129.33.205.204]) by mx0b-001b2d01.pphosted.com with ESMTP id 2ekd728fk9-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Fri, 01 Dec 2017 14:36:55 -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, 1 Dec 2017 14:36:55 -0500 Received: from b01cxnp22033.gho.pok.ibm.com (9.57.198.23) by e14.ny.us.ibm.com (146.89.104.201) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 1 Dec 2017 14:36:51 -0500 Received: from b01ledav005.gho.pok.ibm.com (b01ledav005.gho.pok.ibm.com [9.57.199.110]) by b01cxnp22033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id vB1JapmT40763458; Fri, 1 Dec 2017 19:36:51 GMT Received: from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id DE8E0AE03B; Fri, 1 Dec 2017 14:37:47 -0500 (EST) Received: from [9.85.155.182] (unknown [9.85.155.182]) by b01ledav005.gho.pok.ibm.com (Postfix) with ESMTP id 73EF2AE043; Fri, 1 Dec 2017 14:37:46 -0500 (EST) Subject: Re: [PING][PATCH] Fix for prologue processing on PowerPC 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 References: <20171108095850.394a48ca@pinnacle.lan> <8bf0014c-e83c-5988-4d06-173572f21186@rt-rk.com> From: pedromfc Date: Fri, 01 Dec 2017 19:37:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <8bf0014c-e83c-5988-4d06-173572f21186@rt-rk.com> Content-Type: multipart/mixed; boundary="------------F9FE553A28F5680BA62E54AD" X-TM-AS-GCONF: 00 x-cbid: 17120119-0052-0000-0000-0000028AEBC0 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00008137; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000243; SDB=6.00953995; UDB=6.00482082; IPR=6.00734069; BA=6.00005726; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00018295; XFM=3.00000015; UTC=2017-12-01 19:36:53 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17120119-0053-0000-0000-000052C9B624 Message-Id: <7ba16b14-9384-34d9-937e-531a2192842a@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-12-01_05:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 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-1712010228 X-IsSubscribed: yes X-SW-Source: 2017-12/txt/msg00031.txt.bz2 This is a multi-part message in MIME format. --------------F9FE553A28F5680BA62E54AD Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-length: 4784 Hello Nikola, Kevin, Thank you for providing these patches. I tested both patches (Nicola's second patch and Kevin's patch) on ppc64le, and there were no regressions, except for some additional expected failures in gdb.threads/attach-many-short-lived-threads.exp. Some comments: - Nikola's patch moves "if(lr_reg == 0)/r0_contains_arg = 0;" to within the first if. This is useful for the case when a second mflr Rx with Rx != 0 is detected. Previously this would cause r0_contains_arg to be reset, despite Rx not being 0. However, if the second mflr has Rx == 0, it would make sense to reset r0_contains_arg (and this would work accidentally before the patch, assuming the first mflr also had Rx == 0). Maybe the best solution here is to always check the Rx contained in the opcode and clear r0_contains_arg if it is 0, regardless of lr_reg, or leave it as before since it's a separate issue. - Kevin's patch assigns lr_reg = op & 0x03e00000, but lr_reg is an int, and op is an unsigned long. Will the unshifted reg always fit in a int? - 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. - I wasn't able to directly apply Nikola's patch, I did so manually. I've also attached a reproducer (prologue.c/foo.S) to check if the patches fixed the issue. I had to alter a generated assembly file so that mflr would use a register other than R0 (in which case the old code does work). gcc -g0 -O0 -o prologue prologue.c foo.S (gdb) file prologue Reading symbols from prologue...done. (gdb) break bar Breakpoint 1 at 0x100005b8 (gdb) run Starting program: /home/pedromfc/prologue Before the patch: Breakpoint 1, 0x00000000100005b8 in bar () (gdb) bt #0  0x00000000100005b8 in bar () #1  0x0000000010000644 in foo () Backtrace stopped: frame did not save the PC After (both patches): Breakpoint 1, 0x00000000100005b8 in bar () (gdb) bt #0  0x00000000100005b8 in bar () #1  0x0000000010000644 in foo () #2  0x00000000100005f8 in main () (gdb) info f 1 Stack frame at 0x7fffffffeee0:  pc = 0x10000644 in foo; saved pc = 0x100005f8 Thanks! Pedro 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 > after shifting removal. > >       /* Invalidate lr_reg, but don't set it to -1. >          That would mean that it had never been set.  */ >       lr_reg = -2; > > This part of the code which depends of non shifted lr_reg, and the > part where shifting is removed are only two places where lr_reg is > changed. As so, I've added last condition to set fdata->lr_register on > -1 if lim_pc is not reached. > > If it seems fine now could you pleas commit it because I don't have > 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`.  That condition 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: > >     *rs6000-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. > --- >  gdb/rs6000-tdep.c | 13 ++++++++----- >  1 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, > CORE_ADDR pc, CORE_ADDR lim_pc, >           remember just the first one, but skip over additional >           ones.  */ >        if (lr_reg == -1) > -        lr_reg = (op & 0x03e00000) >> 21; > -          if (lr_reg == 0) > -            r0_contains_arg = 0; > +      { > +        lr_reg = (op & 0x03e00000); > +        fdata->lr_register = lr_reg >> 21; > +        if (lr_reg == 0) > +          r0_contains_arg = 0; > +      } >        continue; >      } >        else if ((op & 0xfc1fffff) == 0x7c000026) > @@ -2180,8 +2183,8 @@ skip_prologue (struct gdbarch *gdbarch, > CORE_ADDR pc, CORE_ADDR lim_pc, >      } >  #endif /* 0 */ > > -  if (pc == lim_pc && lr_reg >= 0) > -    fdata->lr_register = lr_reg; > +  if (pc != lim_pc) > +    fdata->lr_register = -1; > >    fdata->offset = -fdata->offset; >    return last_prologue_pc; --------------F9FE553A28F5680BA62E54AD Content-Type: text/x-csrc; name="prologue.c" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="prologue.c" Content-length: 74 int bar() { return 0; } int foo(); int main(void) { return foo(); } --------------F9FE553A28F5680BA62E54AD Content-Type: text/plain; charset=UTF-8; name="foo.S" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="foo.S" Content-length: 659 CS5maWxlCSJmb28uYyIKCS5hYml2ZXJzaW9uIDIKCS5zZWN0aW9uCSIudGV4 dCIKCS5hbGlnbiAyCgkuZ2xvYmwgZm9vCgkudHlwZQlmb28sIEBmdW5jdGlv bgpmb286Ci5MQ0YwOgowOglhZGRpcyAyLDEyLC5UT0MuLS5MQ0YwQGhhCglh ZGRpIDIsMiwuVE9DLi0uTENGMEBsCgkubG9jYWxlbnRyeQlmb28sLi1mb28K CW1mbHIgMwoJc3RkIDMsMTYoMSkKCXN0ZCAzMSwtOCgxKQoJc3RkdSAxLC0x MTIoMSkKCW1yIDMxLDEKCWJsIGJhcgoJbm9wCgltciA5LDMKCW1yIDMsOQoJ YWRkaSAxLDMxLDExMgoJbGQgMCwxNigxKQoJbXRsciAwCglsZCAzMSwtOCgx KQoJYmxyCgkubG9uZyAwCgkuYnl0ZSAwLDAsMCwxLDEyOCwxLDAsMQoJLnNp emUJZm9vLC4tZm9vCgkuaWRlbnQJIkdDQzogKFNVU0UgTGludXgpIDcuMi4x IDIwMTcwOTAxIFtnY2MtNy1icmFuY2ggcmV2aXNpb24gMjUxNTgwXSIKCS5z ZWN0aW9uCS5ub3RlLkdOVS1zdGFjaywiIixAcHJvZ2JpdHMK --------------F9FE553A28F5680BA62E54AD--