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 85A6E3858C83 for ; Mon, 28 Feb 2022 18:07:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 85A6E3858C83 Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 21SHd09I024614; Mon, 28 Feb 2022 18:07:47 GMT Received: from ppma02wdc.us.ibm.com (aa.5b.37a9.ip4.static.sl-reverse.com [169.55.91.170]) by mx0a-001b2d01.pphosted.com with ESMTP id 3eh2smrswv-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 28 Feb 2022 18:07:47 +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 21SHxZYT017835; Mon, 28 Feb 2022 18:02:45 GMT Received: from b01cxnp22034.gho.pok.ibm.com (b01cxnp22034.gho.pok.ibm.com [9.57.198.24]) by ppma02wdc.us.ibm.com with ESMTP id 3efbu9p8pf-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 28 Feb 2022 18:02:45 +0000 Received: from b01ledav005.gho.pok.ibm.com (b01ledav005.gho.pok.ibm.com [9.57.199.110]) by b01cxnp22034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 21SI2iej41157066 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 28 Feb 2022 18:02:44 GMT Received: from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id AB812AE064; Mon, 28 Feb 2022 18:02:44 +0000 (GMT) Received: from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 81851AE05C; Mon, 28 Feb 2022 18:02:43 +0000 (GMT) Received: from li-e362e14c-2378-11b2-a85c-87d605f3c641.ibm.com (unknown [9.211.79.233]) by b01ledav005.gho.pok.ibm.com (Postfix) with ESMTP; Mon, 28 Feb 2022 18:02:43 +0000 (GMT) Message-ID: <11a3b9c3c319452d071d10a87be45c2d8cdaa190.camel@us.ibm.com> Subject: Re: [PATCH] Updated, fix reverse stepping multiple contiguous PC ranges From: Carl Love To: gdb-patches@sourceware.org, cel@us.ibm.com Cc: Rogerio Alves , Will Schmidt , simon.marchi@polymtl.ca Date: Mon, 28 Feb 2022 10:02:42 -0800 In-Reply-To: <442684e3f81aa1df073960bd45918106acefa2b9.camel@us.ibm.com> References: <442684e3f81aa1df073960bd45918106acefa2b9.camel@us.ibm.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-ORIG-GUID: ViVqTpFCpPHkQVMzX3slHI-NwkSRMppb X-Proofpoint-GUID: ViVqTpFCpPHkQVMzX3slHI-NwkSRMppb X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.816,Hydra:6.0.425,FMLib:17.11.64.514 definitions=2022-02-28_08,2022-02-26_01,2022-02-23_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 bulkscore=0 impostorscore=0 lowpriorityscore=0 suspectscore=0 mlxlogscore=999 spamscore=0 mlxscore=0 priorityscore=1501 adultscore=0 clxscore=1015 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000 definitions=main-2202280092 X-Spam-Status: No, score=-12.4 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_NONE, 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, 28 Feb 2022 18:08:02 -0000 GDB maintainers: Just a ping on the patch. I noticed I did miss address the patch initially. This is a patch for GDB. Carl On Wed, 2022-02-23 at 08:39 -0800, Carl Love wrote: > GCC maintainers: > > The following patch was posted by Luis Machado on 2/1/2021. There > was > a little discussion on the patch but it was never fully reviewed and > approved. The email for Luis no longer > works. > > As of 2/21/2022, the patch does not compile. I made a small fix to > get > it to compile. I commented out the original line in gdb/infrun.c and > added a new line with the fix and the comment //carll fix to indicate > what I changed. Clearly, the comment needs to be removed if the > patch > is accepted but I wanted to show what I changed. > > That said, I tested the patch on Powerpc and it fixed the 5 test > failures in gdb.reverse/solib-precsave.exp and 5 test failures in > gdb.reverse/solib-reverse.exp. I tested on Intel 64-bit. The two > tests solib-precsave.exp and solib-reverse.exp both initially passed > on > Intel. No additional regression failures were seen with the patch. > > Please let me know if you have comments on the patch or if it is > acceptable as is. Thank you. > > Carl Love > ----------------------------------------------------------- > From: Luis Machado > Date: Mon, 21 Feb 2022 23:11:23 +0000 > Subject: [PATCH] Fix reverse stepping multiple contiguous PC ranges > > When running GDB's testsuite on aarch64-linux/Ubuntu 20.04, 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 1 will land on line 2. > - step from line 2 will land on line 3. > > Reverse execution: > > - step from line 3 will land on line 2. > - step from line 2 will land on line 2. > - step from line 2 will land on line 1. > > The problem here is that line 40 contains two contiguous but distinct > PC ranges, like so: > > Line 40 - [0x7ec ~ 0x7f4] > Line 40 - [0x7f4 ~ 0x7fc] > > When stepping forward from line 2, we skip both of these ranges and > land on > line 42. When stepping backward from line 3, we stop at the start PC > of the > second (or first, going backwards) range of line 40. > > This happens because we have this check in > infrun.c:process_event_stop_test: > > /* When stepping backward, stop at beginning of line range > (unless it's the function entry point, in which case > keep going back to the call point). */ > CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc; > if (stop_pc == ecs->event_thread->control.step_range_start > && stop_pc != ecs->stop_func_start > && execution_direction == EXEC_REVERSE) > end_stepping_range (ecs); > else > keep_going (ecs); > > Since we've reached ecs->event_thread->control.step_range_start, we > stop > stepping backwards. > > The right thing to do is to look for adjacent PC ranges for the same > line, > until we notice a line change. Then we take that as the start PC of > the > range. > > Another solution I thought about is to merge the contiguous ranges > when > we are reading the line tables. Though I'm not sure if we really want > to process > that data as opposed to keeping it as the compiler created, and then > working > around that. > > In any case, the following patch addresses this problem. > > I'm not particularly happy with how we go back in the ranges (using > "pc - 1"). > Feedback would be welcome. > > Validated on aarch64-linux/Ubuntu 20.04/18.04. > > Ubuntu 18.04 doesn't actually run into these failures because the > compiler > doesn't generate distinct PC ranges for the same line. > > gdb/ChangeLog: > > YYYY-MM-DD Luis Machado > > * infrun.c (process_event_stop_test): Handle backward > stepping > across multiple ranges for the same line. > * symtab.c (find_line_range_start): New function. > * symtab.h (find_line_range_start): New prototype. > > > Co-authored-by: Carl Love > --- > gdb/infrun.c | 24 +++++++++++++++++++++++- > gdb/symtab.c | 35 +++++++++++++++++++++++++++++++++++ > gdb/symtab.h | 16 ++++++++++++++++ > 3 files changed, 74 insertions(+), 1 deletion(-) > > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 376a541faf6..997042d3e45 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -6782,11 +6782,33 @@ if (ecs->event_thread- > >control.proceed_to_finish > have software watchpoints). */ > ecs->event_thread->control.may_range_step = 1; > > + /* When we are stepping inside a particular line range, in > reverse, > + and we are sitting at the first address of that range, we need > to > + check if this address also shows up in another line range as > the > + end address. > + > + If so, we need to check what line such a step range points to. > + If it points to the same line as the current step range, that > + means we need to keep going in order to reach the first > address > + of the line range. We repeat this until we eventually get to > the > + first address of a particular line we're stepping through. */ > + CORE_ADDR range_start = ecs->event_thread- > >control.step_range_start; > + if (execution_direction == EXEC_REVERSE) > + { > + gdb::optional real_range_start > + // = find_line_range_start (ecs->event_thread- > >suspend.stop_pc); > + = find_line_range_start (ecs->event_thread- > >stop_pc()); //carll fix > + > + > + if (real_range_start.has_value ()) > + range_start = *real_range_start; > + } > + > /* When stepping backward, stop at beginning of line range > (unless it's the function entry point, in which case > keep going back to the call point). */ > CORE_ADDR stop_pc = ecs->event_thread->stop_pc (); > - if (stop_pc == ecs->event_thread->control.step_range_start > + if (stop_pc == range_start > && stop_pc != ecs->stop_func_start > && execution_direction == EXEC_REVERSE) > end_stepping_range (ecs); > diff --git a/gdb/symtab.c b/gdb/symtab.c > index 1a39372aad0..c40739919d1 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -3425,6 +3425,41 @@ find_pc_line (CORE_ADDR pc, int notcurrent) > return sal; > } > > +/* See symtah.h. */ > + > +gdb::optional > +find_line_range_start (CORE_ADDR pc) > +{ > + struct symtab_and_line current_sal = find_pc_line (pc, 0); > + > + if (current_sal.line == 0) > + return {}; > + > + struct symtab_and_line prev_sal = find_pc_line (current_sal.pc - > 1, 0); > + > + /* If the previous entry is for a different line, that means we > are already > + at the entry with the start PC for this line. */ > + if (prev_sal.line != current_sal.line) > + return current_sal.pc; > + > + /* Otherwise, keep looking for entries for the same line but with > + smaller PC's. */ > + bool done = false; > + CORE_ADDR prev_pc; > + while (!done) > + { > + prev_pc = prev_sal.pc; > + > + prev_sal = find_pc_line (prev_pc - 1, 0); > + > + /* Did we notice a line change? If so, we are done with the > search. */ > + if (prev_sal.line != current_sal.line) > + done = true; > + } > + > + return prev_pc; > +} > + > /* See symtab.h. */ > > struct symtab * > diff --git a/gdb/symtab.h b/gdb/symtab.h > index d12eee6e9d8..4d893a8a3b8 100644 > --- a/gdb/symtab.h > +++ b/gdb/symtab.h > @@ -2149,6 +2149,22 @@ extern struct symtab_and_line find_pc_line > (CORE_ADDR, int); > extern struct symtab_and_line find_pc_sect_line (CORE_ADDR, > struct obj_section *, > int); > > +/* Given PC, and assuming it is part of a range of addresses that is > part of a > + line, go back through the linetable and find the starting PC of > that > + line. > + > + For example, suppose we have 3 PC ranges for line X: > + > + Line X - [0x0 - 0x8] > + Line X - [0x8 - 0x10] > + Line X - [0x10 - 0x18] > + > + If we call the function with PC == 0x14, we want to return 0x0, > as that is > + the starting PC of line X, and the ranges are contiguous. > +*/ > + > +extern gdb::optional find_line_range_start (CORE_ADDR > pc); > + > /* Wrapper around find_pc_line to just return the symtab. */ > > extern struct symtab *find_pc_line_symtab (CORE_ADDR);