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 A9FCF3858C53 for ; Mon, 4 Apr 2022 16:55:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A9FCF3858C53 Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 234FGxq4003634; Mon, 4 Apr 2022 16:55:42 GMT Received: from ppma03dal.us.ibm.com (b.bd.3ea9.ip4.static.sl-reverse.com [169.62.189.11]) by mx0a-001b2d01.pphosted.com with ESMTP id 3f8369b6dn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 04 Apr 2022 16:55:42 +0000 Received: from pps.filterd (ppma03dal.us.ibm.com [127.0.0.1]) by ppma03dal.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 234Gqtwv009570; Mon, 4 Apr 2022 16:55:41 GMT Received: from b01cxnp23034.gho.pok.ibm.com (b01cxnp23034.gho.pok.ibm.com [9.57.198.29]) by ppma03dal.us.ibm.com with ESMTP id 3f6e49d2vn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 04 Apr 2022 16:55:41 +0000 Received: from b01ledav006.gho.pok.ibm.com (b01ledav006.gho.pok.ibm.com [9.57.199.111]) by b01cxnp23034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 234GterL27918824 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 4 Apr 2022 16:55:40 GMT Received: from b01ledav006.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 788AEAC060; Mon, 4 Apr 2022 16:55:40 +0000 (GMT) Received: from b01ledav006.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 2ECF3AC064; Mon, 4 Apr 2022 16:55:40 +0000 (GMT) Received: from lexx (unknown [9.160.28.151]) by b01ledav006.gho.pok.ibm.com (Postfix) with ESMTP; Mon, 4 Apr 2022 16:55:40 +0000 (GMT) Message-ID: Subject: Re: [PATCH, v2] Fix reverse stepping multiple contiguous PC ranges over the line table From: will schmidt To: Luis Machado , gdb-patches@sourceware.org Date: Mon, 04 Apr 2022 11:55:39 -0500 In-Reply-To: <20220331135246.7913-1-luis.machado@arm.com> References: <7a429c919395db6ec4642803badca5dbb97bff66.camel@us.ibm.com> <20220331135246.7913-1-luis.machado@arm.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: QAYAdWoXsV5miiZkoLoTnco9sYWOlRpU X-Proofpoint-ORIG-GUID: QAYAdWoXsV5miiZkoLoTnco9sYWOlRpU X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.850,Hydra:6.0.425,FMLib:17.11.64.514 definitions=2022-04-04_06,2022-03-31_01,2022-02-23_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 priorityscore=1501 phishscore=0 lowpriorityscore=0 mlxscore=0 malwarescore=0 bulkscore=0 impostorscore=0 spamscore=0 mlxlogscore=999 adultscore=0 clxscore=1011 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000 definitions=main-2204040094 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Apr 2022 16:55:48 -0000 On Thu, 2022-03-31 at 14:52 +0100, Luis Machado via Gdb-patches wrote: > v2: > - Check if both the line and symtab match for a particular line table entry. > > -- > > When running GDB's testsuite on aarch64-linux/Ubuntu 20.04 (also spotted on > the ppc backend), I noticed some failures in gdb.reverse/solib-precsave.exp > and gdb.reverse/solib-reverse.exp. > > The failure happens around the following code: > > 38 b[1] = shr2(17); /* middle part two */ > 40 b[0] = 6; b[1] = 9; /* generic statement, end part two */ > 42 shr1 ("message 1\n"); /* shr1 one */ > > Normal execution: > > - step from line 38 will land on line 40. > - step from line 40 will land on line 42. > > Reverse execution: > > - step from line 42 will land on line 40. > - step from line 40 will land on line 40. > - step from line 40 will land on line 38. > > The problem here is that line 40 contains two contiguous but distinct > PC ranges in the line table, like so: > > Line 40 - [0x7ec ~ 0x7f4] > Line 40 - [0x7f4 ~ 0x7fc] > I'm not particularly happy with how we go back in the ranges (using "pc - 1"). > Feedback would be welcome. I suppose there could be a loop of some sort that iterates backwards to a valid line; though I'd think pc - 1 is sufficient? > > Validated on aarch64-linux/Ubuntu 20.04/18.04. Carl Love has verified that it > does fix a similar issue on ppc. > > Ubuntu 18.04 doesn't actually run into these failures because the compiler > doesn't generate distinct PC ranges for the same line. > > I see similar failures on x86_64 in the gdb.reverse tests > (gdb.reverse/step-reverse.exp and gdb.reverse/step-reverse.exp). Those are > also fixed by this patch, although Carl's testcase doesn't fail for x86_64. > > There seems to be a corner case where a line table has only one instruction, > and while stepping that doesn't take the same path through infrun. I think it > needs some more investigation. Therefore this is a tentative patch for now. Are you (or Carl) continuing to pursue that edge case? Unless you think the contents here would be significantly invalidated, may be worth moving forrward with this as at least an incremental improvement. > > Co-authored-by: Carl Love > --- > gdb/infrun.c | 51 +++++++- > gdb/symtab.c | 49 ++++++++ > gdb/symtab.h | 16 +++ > gdb/testsuite/gdb.reverse/map-to-same-line.c | 30 +++++ > .../gdb.reverse/map-to-same-line.exp | 114 ++++++++++++++++++ Code changes all seem reasonable. Just one comment on the commentary here. > diff --git a/gdb/testsuite/gdb.reverse/map-to-same-line.exp b/gdb/testsuite/gdb.reverse/map-to-same-line.exp > new file mode 100644 > index 00000000000..da407b50e94 > --- /dev/null > +++ b/gdb/testsuite/gdb.reverse/map-to-same-line.exp > @@ -0,0 +1,114 @@ > + > +# The purpose of this test is to create a DWARF line table that says the > +# two assignment statements are on the same line. The expect test checks > +# to make sure gdb reverse steps over each statement individually, i.e. > +# not over the line containing both assignments. */ > + > +load_lib dwarf.exp > + > +# This test can only be run on targets which support DWARF-2 and use gas. > +if {![dwarf2_support]} { > + unsupported "dwarf2 support required for this test" > + return 0 > +} > + > +if [get_compiler_info] { > + return -1 > +} > + > +# The DWARF assembler requires the gcc compiler. > +if {!$gcc_compiled} { > + unsupported "gcc is required for this test" > + return 0 > +} > + > +standard_testfile .c .S > + > +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { > + return -1 > +} > + > +set asm_file [standard_output_file $srcfile2] > +Dwarf::assemble $asm_file { > + global srcdir subdir srcfile > + declare_labels integer_label L > + > + # Find start address and length of program > + lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \ > + main_start main_len > + set main_end "$main_start + $main_len" > + > + cu {} { > + compile_unit { > + {language @DW_LANG_C} > + {name map-to-same-line.c} > + {stmt_list $L DW_FORM_sec_offset} > + {low_pc 0 addr} > + } { > + subprogram { > + {external 1 flag} > + {name main} > + {low_pc $main_start addr} > + {high_pc $main_len DW_FORM_data4} > + } > + } > + } > + > + lines {version 2 default_is_stmt 1} L { > + include_dir "${srcdir}/${subdir}" > + file_name "$srcfile" 1 > + > + # Generate the line table program with the assignments to j and k > + # are listed on the same source line in the table. > + program { > + {DW_LNE_set_address $main_start} > + {line [gdb_get_line_number "TAG: main prologue"]} > + {DW_LNS_copy} > + > + {DW_LNE_set_address i_assignment} > + {line [gdb_get_line_number "TAG: assignment i"]} > + {DW_LNS_copy} > + > + {DW_LNE_set_address j_assignment} > + {line [gdb_get_line_number "TAG: assignment j"]} > + {DW_LNS_copy} > + > + {DW_LNE_set_address k_assignment} THough it's covered in the section comment, a comment here to clarify "Force the Dwarf line number for k_assignment to the j assignment here." may be useful. I was explicitly looking for it, and missed it in my first scan through. :-) Thanks, -Will > + {line [gdb_get_line_number "TAG: assignment j"]} > + {DW_LNS_copy} > + > + {DW_LNE_set_address l_assignment} > + {line [gdb_get_line_number "TAG: assignment l"]} > + {DW_LNS_copy} > + > + {DW_LNE_set_address m_assignment} > + {line [gdb_get_line_number "TAG: assignment m"]} > + {DW_LNS_copy} > + > + {DW_LNE_end_sequence} > + } > + } > +} > + > +if { [prepare_for_testing "failed to prepare" ${testfile} \ > + [list $srcfile $asm_file] {nodebug} ] } { > + return -1 > +} > + > +if ![runto_main] { > + return -1 > +} > + > +if [supports_process_record] { > + # Activate process record/replay > + gdb_test_no_output "record" "turn on process record" > +} > + > +set break_at_l [gdb_get_line_number "TAG: assignment l" ] > + > +gdb_test "tbreak $break_at_l" "Temporary breakpoint .*" "breakpoint l = 10" > + > +gdb_test "continue" "Temporary breakpoint .*" "run to l = 10" > +# j = 3 and k = 4 are on the same line. The reverse step stops at j = 3 > +gdb_test "reverse-step" ".* j = 3;.*" "reverse-step to j = 3" > +gdb_test "reverse-step" ".* i = 1;.*" "reverse-step to i = 1"