From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 6FAAC3858403 for ; Tue, 1 Nov 2022 19:51:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6FAAC3858403 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=us.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=us.ibm.com Received: from pps.filterd (m0098410.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 2A1JkmFs021614 for ; Tue, 1 Nov 2022 19:51:06 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : from : to : cc : date : in-reply-to : references : content-type : mime-version : content-transfer-encoding : subject; s=pp1; bh=9m8yF3zDSzsMEUqZzOvmAMnTyvaDMl/bT//65pP5jDc=; b=OE8PfSNmT7Hw34ftzCYsjwyX3Omw6FRNN0TNyAf5lcztm8b9R2++t/ybj96m1YxBFoh1 F62FIqWqOndmC42JGQh94KoK9yYlmEWxvO7NujOCRwU4IvkhS69PbluB5OuJsv6E7uZc CxtmYwQ29vV7ERSC87Nh1vuMzeu3QWHg+kNqrGB9oYPlLIlsAiay99Jxd6Y3FvP1ZKFU fLFYZSb6pcBgbQsdnA4ZBvoOfbOOUI6heu6r30j7zI2q/iwr5xT32B6Y5G0T5viU8lTs SLwYv50DescGmFzhKAbWw9BixrfNwMIYHpuiC/gDHaiaK2HzbJoGrAiABdF7V7yUFfF0 5w== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3kjufh5xud-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 01 Nov 2022 19:51:06 +0000 Received: from m0098410.ppops.net (m0098410.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 2A1JRmlc022804 for ; Tue, 1 Nov 2022 19:51:05 GMT Received: from ppma02wdc.us.ibm.com (aa.5b.37a9.ip4.static.sl-reverse.com [169.55.91.170]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3kjufh5xtr-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 01 Nov 2022 19:51:05 +0000 Received: from pps.filterd (ppma02wdc.us.ibm.com [127.0.0.1]) by ppma02wdc.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 2A1JZlVw024817; Tue, 1 Nov 2022 19:51:04 GMT Received: from b01cxnp22035.gho.pok.ibm.com (b01cxnp22035.gho.pok.ibm.com [9.57.198.25]) by ppma02wdc.us.ibm.com with ESMTP id 3kguta645b-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 01 Nov 2022 19:51:04 +0000 Received: from smtpav03.wdc07v.mail.ibm.com ([9.208.128.112]) by b01cxnp22035.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 2A1Jp3dH3867190 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 1 Nov 2022 19:51:04 GMT Received: from smtpav03.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 767F65805D; Tue, 1 Nov 2022 19:51:03 +0000 (GMT) Received: from smtpav03.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id CC6135805C; Tue, 1 Nov 2022 19:51:02 +0000 (GMT) Received: from li-e362e14c-2378-11b2-a85c-87d605f3c641.ibm.com (unknown [9.163.10.231]) by smtpav03.wdc07v.mail.ibm.com (Postfix) with ESMTP; Tue, 1 Nov 2022 19:51:02 +0000 (GMT) Message-ID: <9cbed9664acd4483eb8ec5a5d1b30a4f44f56ecf.camel@us.ibm.com> From: Carl Love To: Bruno Larsen , cel@us.ibm.com Cc: "gdb-patches@sourceware.org" , Ulrich Weigand , Will Schmidt Date: Tue, 01 Nov 2022 12:51:02 -0700 In-Reply-To: <711495ea-57f4-276d-db50-067b29c14de6@redhat.com> References: <711495ea-57f4-276d-db50-067b29c14de6@redhat.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-18.el8) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: JgzZoENeE9Ak9aZgUnfcNcr1d0uZgCB0 X-Proofpoint-ORIG-GUID: 63MF4I1V5HDtwnwcdzlAMiT3WMnyc4zw Subject: RE: Question: [PATCH] Change calculation of frame_id by amd64 epilogue unwinder X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-11-01_09,2022-11-01_02,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 clxscore=1015 mlxscore=0 spamscore=0 lowpriorityscore=0 malwarescore=0 bulkscore=0 impostorscore=0 priorityscore=1501 adultscore=0 mlxlogscore=999 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000 definitions=main-2211010140 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,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 Tue, 2022-11-01 at 11:20 +0100, Bruno Larsen wrote: > On 31/10/2022 21:16, Carl Love wrote: > > Bruno: > > > > Sorry for the slow response, I was out all of last week. > > > > It looks like you committed the patch: > > > > commit 49d7cd733a7f1b87aa1d40318b3d7c2b65aca5ac > > Author: Bruno Larsen > > Date: Fri Aug 19 15:11:28 2022 +0200 > > > > Change calculation of frame_id by amd64 epilogue unwinder > > > > When GDB is stopped at a ret instruction and no debug > > information is > > available for unwinding, GDB defaults to the amd64 epilogue > > unwinder, to > > be able to generate a decent backtrace. However, when > > calculating the > > frame id, the epilogue unwinder generates information as if > > the return > > instruction was the whole frame. > > > > This was an issue especially when attempting to reverse debug, > > as GDB > > would place a step_resume_breakpoint from the epilogue of a > > function if > > we were to attempt to skip that function, and this breakpoint > > should > > ideally have the current function's frame_id to avoid other > > problems > > such as PR record/16678. > > > > This commit changes the frame_id calculation for the amd64 > > epilogue, > > so that it is always the same as the dwarf2 unwinder's > > frame_id. > > > > It also adds a test to confirm that the frame_id will be the > > same, > > regardless of using the epilogue unwinder or not, thanks to > > Andrew > > Burgess. > > > > Co-Authored-By: Andrew Burgess > > > > a week or so ago. The test is generating hundreds of new errors on > > the > > various PowerPC processors. From the discussion above, it looks > > like > > you were trying to fix an issue with the amd64 epilogue. > > Hi Carl, > > The main point of the patch was fixing the issue with amd64 epilogue > unwinding, but the issue itself was that frame IDs aren't consistent > throughout the function. With my change in the following patch, if > the > frame ID is different at any point, using "reverse next" might > consume > all the history and stop at the start of the recording, instead of > going > a single line. This would be applicable to all architectures. > > > The patch introduced a new patch to test the amd64 epilogue > > changes. > > Do you expect this new test to run on all other platforms? > > Yes, we did. See above rationale as to why it is important, but also > all > instructions in a function should have the same frame id, otherwise > all > sorts of internal GDB and user confusion can happen. > > > In addition > > to the 575 new errors on Power 10, I see a lots of warnings: > > > > WARNING: While evaluating expression in gdb_assert: missing > > operand at _@_ > > in expression " _@_== 0x7fffffffc920" > > > > where the hex value changes for each warning. Still working on > > sorting > > out where the messages come from. > I don't think this would be a regression from this patch. Either > this > was already a problem (just not being triggered) or it was another > patch > that regressed this. If I clear out my regression backlog, I'll try > to > help you figure out what is going on. > > The other thing I notice in the gdb/testsuite/gdb.base/unwind-on- > > each- > > insn.exp file is that you set a break point in function foo with > > the > > command: gdb_breakpoint "*foo" > > > > The use of breakpoint on *function is problematic on Power as > > discussed > > earlier. The use of *foo will set the breakpoint on the remote > > breakpoint which will not get hit in this test on PowerPC. I don't > > see > > why you chose to use *foo instead of foo? I don't see anything in > > the > > test that relies on the prolog where you would need the address of > > the > > function before the prolog. I only see references to the epilog of > > the > > function. > Right, that is a problem with the test. We do want to test around > the > epilogue because all instructions have to show the same frame ID, > but > what could be done instead is breaking on line 23 and using stepi > once. > > Anyway, I am trying to figure out how to fix this test to work on > > PowerPC. Not sure if there needs to be some similar changes to the > > rs6000-tdep.c file similar to the changes in amd64-tdep.c? Any > > thoughts you have on the test that would be helpful to getting it > > to > > work on PowerPC, if that is appropriate, or having PowerPC skip > > this > > test would be appreciated. Thanks. Bruno: I found the issue after chasing down a few dead ends... Basically, the test disassembles function foo and then scans thru the disassembly looking for the address of the last instruction which is assumed to be right before the line "End of assembler dump". The PowerPC disassembly has three addresses containing a .long after the last instruction in the function. So, the test gets the wrong final address for function foo. Basicaly, the test works fine until you walk past the end of the function. I added a condition to the test gdb_test_multiple "disassemble foo" to ignore the lines with the .long entries so the correct address for the last instruction is recorded. This fixes all of the errors and warnings. The break foo* does not seem to be a problem on this test as I was concerned that it would be. I have tested the patch on PowerPC and Intel. Please take a look and see what you think. If it looks OK to you, I will formally post it to the mailing list for review and approval. Thanks. Carl ----------------------------------- >From 764f4476f16fa222e95ab2a7087355dfae818502 Mon Sep 17 00:00:00 2001 From: Carl Love Date: Tue, 1 Nov 2022 15:33:17 -0400 Subject: [PATCH] Powerpc fix for gdb.base/unwind-on-each-insn.exp The test disassembles function foo and searches for the line "End of assembler dump" to determing the last address in the function. The assumption is the last instruction will be given right before the line "End of assembler dump". This assumption fails on PowerPC. The PowerPC disassembly of the function foo looks like: Dump of assembler code for function foo: # => 0x00000000100006dc <+0>: std r31,-8(r1) # 0x00000000100006e0 <+4>: stdu r1,-48(r1) # 0x00000000100006e4 <+8>: mr r31,r1 # 0x00000000100006e8 <+12>: nop # 0x00000000100006ec <+16>: addi r1,r31,48 # 0x00000000100006f0 <+20>: ld r31,-8(r1) # 0x00000000100006f4 <+24>: blr # 0x00000000100006f8 <+28>: .long 0x0 # 0x00000000100006fc <+32>: .long 0x0 # 0x0000000010000700 <+36>: .long 0x1000180 # End of assembler dump. The blr instruction is the last instruction in function foo. The lines with .long follwing the blr instruction need to be ignored. This patch adds a new condition to the gdb_test_multiple "disassemble foo" test to ignore the lines with the .long. The patch has been tested on PowerPC and Intel X86-64. --- .../gdb.base/unwind-on-each-insn.exp | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp index 3b48805cff8..c908a4b838e 100644 --- a/gdb/testsuite/gdb.base/unwind-on-each-insn.exp +++ b/gdb/testsuite/gdb.base/unwind-on-each-insn.exp @@ -41,7 +41,6 @@ if ![runto_main] then { proc get_sp_and_fba { testname } { with_test_prefix "get \$sp and frame base $testname" { set sp [get_hexadecimal_valueof "\$sp" "*UNKNOWN*"] - set fba "" gdb_test_multiple "info frame" "" { -re -wrap ".*Stack level ${::decimal}, frame at ($::hex):.*" { @@ -75,6 +74,23 @@ gdb_continue_to_breakpoint "enter foo" # Figure out the range of addresses covered by this function. set last_addr_in_foo "" + +# The disassembly of foo on PowerPC looks like: +# Dump of assembler code for function foo: +# => 0x00000000100006dc <+0>: std r31,-8(r1) +# 0x00000000100006e0 <+4>: stdu r1,-48(r1) +# 0x00000000100006e4 <+8>: mr r31,r1 +# 0x00000000100006e8 <+12>: nop +# 0x00000000100006ec <+16>: addi r1,r31,48 +# 0x00000000100006f0 <+20>: ld r31,-8(r1) +# 0x00000000100006f4 <+24>: blr +# 0x00000000100006f8 <+28>: .long 0x0 +# 0x00000000100006fc <+32>: .long 0x0 +# 0x0000000010000700 <+36>: .long 0x1000180 +# End of assembler dump. +# +# The last instruction in function foo is blr. Ignore the .long +# entries following the blr instruction. gdb_test_multiple "disassemble foo" "" { -re "^disassemble foo\r\n" { exp_continue @@ -84,7 +100,11 @@ gdb_test_multiple "disassemble foo" "" { exp_continue } - -re "^...($hex) \[^\r\n\]+\r\n" { + -re "^...($hex) \[<>+0-9:\s\t\]*\.long\[\s\t\]*\[^\r\n\]*\r\n" { + exp_continue + } + + -re "^...($hex)\[^\r\n\]+\r\n" { set last_addr_in_foo $expect_out(1,string) exp_continue } @@ -137,6 +157,7 @@ for { set i_count 1 } { true } { incr i_count } { gdb_test "frame 0" ".*" set pc [get_hexadecimal_valueof "\$pc" "*UNKNOWN*"] + if { $pc == $last_addr_in_foo } { break } -- 2.37.2