From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id BE7E23858D37 for ; Thu, 22 Jun 2023 20:38:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BE7E23858D37 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 (m0356516.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 35MKY5x3017659; Thu, 22 Jun 2023 20:38: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 : content-transfer-encoding : mime-version : subject; s=pp1; bh=LdG1X2axek+r8/lb4a8WXc15z6NTopWNATxgdNUq2Qo=; b=MrwG5fm0PS8Y/qba5JgBJttNLrW9vWPH62Qenj5OEuTwETBWjEb/FCImVA5t0zI4FIm9 lNpa2KrqFm2zF+PIwBfVGyt5Cfz/VUAN+ohenGJKK8NiQ2+9MGTTXWaPr6GLvXNfCZK5 E8HVPi2pRp2Fj79APhy/OrXfyfJObT4mCO7w6BOg6t0VqMGI79U5Gq5fTyU0idWqyeOw xv9tALr8GakwdfA/nWu/y6NLZH1E3v7KsvCYXtYRQh6m7nnkIe2gZLcryiv0mlAjZVnu njRcicLAVzH5/5AP2UgGWpAS+iObhMwRtjlgMXxM/cDm8ExjuKSKDMX87LoXYc6kdd/l QA== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3rcuys2g3f-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 22 Jun 2023 20:38:06 +0000 Received: from m0356516.ppops.net (m0356516.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 35MKWjgu013743; Thu, 22 Jun 2023 20:38:05 GMT Received: from ppma02dal.us.ibm.com (a.bd.3ea9.ip4.static.sl-reverse.com [169.62.189.10]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3rcuys2g2x-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 22 Jun 2023 20:38:05 +0000 Received: from pps.filterd (ppma02dal.us.ibm.com [127.0.0.1]) by ppma02dal.us.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 35MJTsLH031752; Thu, 22 Jun 2023 20:38:05 GMT Received: from smtprelay04.wdc07v.mail.ibm.com ([9.208.129.114]) by ppma02dal.us.ibm.com (PPS) with ESMTPS id 3r94f6tgbt-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 22 Jun 2023 20:38:04 +0000 Received: from smtpav01.wdc07v.mail.ibm.com (smtpav01.wdc07v.mail.ibm.com [10.39.53.228]) by smtprelay04.wdc07v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 35MKc3YG35062142 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 22 Jun 2023 20:38:03 GMT Received: from smtpav01.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1E3D558065; Thu, 22 Jun 2023 20:38:03 +0000 (GMT) Received: from smtpav01.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5436158055; Thu, 22 Jun 2023 20:38:02 +0000 (GMT) Received: from li-e362e14c-2378-11b2-a85c-87d605f3c641.ibm.com (unknown [9.61.18.149]) by smtpav01.wdc07v.mail.ibm.com (Postfix) with ESMTP; Thu, 22 Jun 2023 20:38:02 +0000 (GMT) Message-ID: <93bd0e348673cf870b5fe20b5f3a2760fe5ef3a4.camel@us.ibm.com> From: Carl Love To: Simon Marchi , Bruno Larsen , gdb-patches@sourceware.org, UlrichWeigand , pedro@palves.net Cc: luis.machado@arm.com Date: Thu, 22 Jun 2023 13:38:01 -0700 In-Reply-To: <2e13271b-1f75-14a4-74da-ba1c0df59435@simark.ca> References: <74630f1ccb6e9258ae60682105ee5490726fb255.camel@us.ibm.com> <46d73c69-9168-44c6-b515-23dd893fc0eb@redhat.com> <86c65f2ad74caffc162f100e4e9c5be9062a7f59.camel@us.ibm.com> <0a2c4ebd-f01d-4b96-1b13-25d7276056a5@redhat.com> <956b8c3c9a7bdc3aa6f9a040619ec4778edc9c94.camel@us.ibm.com> <89b2fb027024f7e97de7196ee091a0ca11c0c2b3.camel@us.ibm.com> <0943e12c-049d-f8b0-c4c5-8816b1be1e45@simark.ca> <961a88a7-a820-fd32-c7ee-e707697e22a5@simark.ca> <2e13271b-1f75-14a4-74da-ba1c0df59435@simark.ca> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-18.el8) X-TM-AS-GCONF: 00 X-Proofpoint-GUID: 45iYgX5YqiB5z7r0nuh2G1RIFy3f9dHw X-Proofpoint-ORIG-GUID: hkBfRShagfNQzuA3HTBufpYPW056n-aj Content-Transfer-Encoding: 8bit X-Proofpoint-UnRewURL: 4 URL's were un-rewritten MIME-Version: 1.0 Subject: RE: [PATCH 2/2 v5] Fix reverse stepping multiple contiguous PC ranges over the line table. X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26 definitions=2023-06-22_15,2023-06-22_02,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 lowpriorityscore=0 spamscore=0 impostorscore=0 malwarescore=0 priorityscore=1501 mlxscore=0 mlxlogscore=999 clxscore=1015 adultscore=0 bulkscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2305260000 definitions=main-2306220171 X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,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.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Simon: On Mon, 2023-06-19 at 13:58 -0400, Simon Marchi wrote: > 100644 > > --- a/gdb/infrun.c > > +++ b/gdb/infrun.c > > @@ -114,6 +114,9 @@ static struct async_event_handler > > *infrun_async_inferior_event_token; > > Starts off as -1, indicating "never enabled/disabled". */ > > static int infrun_is_async = -1; > > > > +static CORE_ADDR update_line_range_start (CORE_ADDR pc, > > + struct > > execution_control_state *ecs); > > + > > This forward-declaration is not needed. I tried removing the forward-declaration and the compile fails with the message: ../../binutils-gdb-reverse-multiple-contiguous/gdb/infrun.c:6773:1: error: no previous declaration for ‘CORE_ADDR update_line_range_start(CORE_ADDR, execution_control_state*)’ [- Werror=missing-declarations] 6773 | update_line_range_start (CORE_ADDR pc, struct execution_control_state *ecs) | ^~~~~~~~~~~~~~~~~~~~~~~ cc1plus: all warnings being treated as errors make[2]: *** [Makefile:1922: infrun.o] Error 1 make[2]: Leaving directory '/home/carll/GDB/build-reverse-multiple- contiguous/gdb' make[1]: *** [Makefile:13569: all-gdb] Error 2 make[1]: Leaving directory '/home/carll/GDB/build-reverse-multiple- contiguous' make: *** [Makefile:1005: all] Error 2 Leaving the forward declaration in the code. > > > /* See infrun.h. */ > > > > void > > @@ -6769,6 +6772,25 @@ handle_signal_stop (struct > > execution_control_state *ecs) > > process_event_stop_test (ecs); > > } > > > > +CORE_ADDR > > +update_line_range_start (CORE_ADDR pc, struct > > execution_control_state *ecs) > > Please add a comment for the function. Done. > > > +{ > > + /* The line table may have multiple entries for the same source > > code line. > > + Given the PC, check the line table and return the PC that > > corresponds > > + to the line table entry for the source line that PC is > > in. */ > > + CORE_ADDR start_line_pc = ecs->event_thread- > > >control.step_range_start; > > + gdb::optional real_range_start; > > + > > + /* Call find_line_range_start to get the smallest address in the > > + linetable for multiple Line X entries in the line table. */ > > + real_range_start = find_line_range_start (pc); > > + > > + if (real_range_start.has_value ()) > > + start_line_pc = *real_range_start; > > + > > + return start_line_pc; > > When I read this, I wonder: why was control.step_range_start not set > to > the "real" range start in the first place (not only in the context of > reverse execution, every time it is set)? It would seem more robust > than patching it afterwards in some very specific spots. > > I could see some benefits for range-stepping uses cases too (relevant > when debugging remotely). Using your example here: > > Line X - [0x0 - 0x8] > Line X - [0x8 - 0x10] > Line X - [0x10 - 0x18] > > Imagine we are stopped at 0x14, and we type "next", and 0x14 is a > conditional jump to 0x5. It seems like current GDB would send a > "range > step" request to GDBserver, to step in the [0x10, 0x18[ range. When > reaching 0x5, execution would stop, and GDB would resume it again > with > the [0x0,0x8[ range. When reaching 0x8, it would stop again, GDB > would > resume it with [0x8,0x10[, and so on. If GDB could send a "range > step" > request with the [0x0,0x18[ range, it would avoid those unnecessary > intermediary stop. > > > +} > > + > > /* Come here when we've got some debug event / signal we can > > explain > > (IOW, not a random signal), and test whether it should cause a > > stop, or whether we should resume the inferior (transparently). > > @@ -7570,6 +7592,28 @@ process_event_stop_test (struct > > execution_control_state *ecs) > > > > if (stop_pc_sal.is_stmt) > > { > > + if (execution_direction == EXEC_REVERSE) > > + { > > + /* We are stepping backwards make sure we have reached > > the > > + beginning of the line. */ > > + CORE_ADDR stop_pc = ecs->event_thread->stop_pc (); > > + CORE_ADDR start_line_pc > > + = update_line_range_start (stop_pc, ecs); > > + > > + if (stop_pc != start_line_pc) > > + { > > + /* Have not reached the beginning of the source code > > line. > > + Set a step range. Execution should stop in any > > function > > + calls we execute back into before reaching the > > beginning > > + of the line. */ > > + ecs->event_thread->control.step_range_start = > > start_line_pc; > > + ecs->event_thread->control.step_range_end = stop_pc; > > + set_step_info (ecs->event_thread, frame, > > stop_pc_sal); > > + keep_going (ecs); > > + return; > > + } > > + } > > + > > /* We are at the start of a statement. > > > > So stop. Note that we don't stop if we step into the > > middle of a > > @@ -7632,6 +7676,19 @@ process_event_stop_test (struct > > execution_control_state *ecs) > > set_step_info (ecs->event_thread, frame, stop_pc_sal); > > > > infrun_debug_printf ("keep going"); > > + > > + if (execution_direction == EXEC_REVERSE) > > + { > > + CORE_ADDR stop_pc = ecs->event_thread->stop_pc (); > > + > > + /* Make sure the stop_pc is set to the beginning of the > > line. */ > > + if (stop_pc != ecs->event_thread->control.step_range_start) > > + { > > + stop_pc = update_line_range_start (stop_pc, ecs); > > + ecs->event_thread->control.step_range_start = stop_pc; > > + } > > + } > > + > > keep_going (ecs); > > } > > > > diff --git a/gdb/symtab.c b/gdb/symtab.c > > index 27611a34ec4..91d35616eb9 100644 > > --- a/gdb/symtab.c > > +++ b/gdb/symtab.c > > @@ -3282,6 +3282,55 @@ find_pc_line (CORE_ADDR pc, int notcurrent) > > return sal; > > } > > > > +/* Compare two symtab_and_line entries. Return true if both have > > + the same line number and the same symtab pointer. That means > > we > > + are dealing with two entries from the same line and from the > > same > > + source file. > > + > > + Return false otherwise. */ > > + > > +static bool > > +sal_line_symtab_matches_p (const symtab_and_line &sal1, > > + const symtab_and_line &sal2) > > +{ > > + return (sal1.line == sal2.line && sal1.symtab == sal2.symtab); > > Unnecessary parenthesis. Removed unnecessary parenthesis. > > > +} > > + > > +/* 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 (!sal_line_symtab_matches_p (prev_sal, current_sal)) > > + 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 (!sal_line_symtab_matches_p (prev_sal, current_sal)) > > + done = true; > > + } > > + > > + return prev_pc; > > Algorithmic complexity question: given that line tables are sorted by > address, would it work to start at the current line table item, and > go > look at the previous ones until we find one that is no longer > contiguous and same line? find_pc_line is somewhat heavy, so if we > don't need to do it repeatedly... > > > +} > > + > > /* See symtab.h. */ > > > > struct symtab * > > diff --git a/gdb/symtab.h b/gdb/symtab.h > > index 404d0ab30a8..f54305636da 100644 > > --- a/gdb/symtab.h > > +++ b/gdb/symtab.h > > @@ -2346,6 +2346,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. > > I think that putting this example in the comment is great. It makes > it > much more obvious what the function specifically does. > > > +*/ > > + > > +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); > > diff --git a/gdb/testsuite/gdb.reverse/func-map-to-same-line.c > > b/gdb/testsuite/gdb.reverse/func-map-to-same-line.c > > new file mode 100644 > > index 00000000000..da944874e86 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line.c > > @@ -0,0 +1,36 @@ > > +/* Copyright 2008-2023 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 < > > http://www.gnu.org/licenses/ > > >. > > + > > + This test is used to test the reverse-step and reverse-next > > instruction > > + execution for a source line that contains multiple function > > calls. */ > > + > > +void > > +func1 () > > +{ > > +} /* END FUNC1 */ > > + > > +void > > +func2 () > > +{ > > +} /* END FUNC2 */ > > + > > +int main () > > int > main (void) > Fixed. > > +{ > > + int a, b; > > + a = 1; > > + b = 2; > > + func1 (); func2 (); > > + a = a + b; /* START REVERSE TEST */ > > +} > > diff --git a/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp > > b/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp > > new file mode 100644 > > index 00000000000..89e226b0f84 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp > > @@ -0,0 +1,140 @@ > > +# Copyright 2008-2023 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 < > > http://www.gnu.org/licenses/ > > >. */ > > + > > +# This file is part of the GDB testsuite. It tests reverse > > stepping. > > +# Lots of code borrowed from "step-test.exp". > > + > > +# This test checks to make sure there is no regression failures > > for > > +# the reverse-next command when stepping back over two functions > > in > > +# the same line. > > + > > +require supports_reverse > > + > > +# This test uses the gcc no-column-info command which was added in > > gcc 7.1. > > + > > +proc run_tests {} { > > + global srcfile > > + global executable > > + > > + runto_main > > We typically check for runto_main's success: Fixed two instances of runto_main in this test case and two in the other test case. > > if { ![runto_main] } { > return > } > > runto_main logs a FAIL on failure. There are a few runto_mains in > the > patch. > > > + set target_remote [gdb_is_target_remote] > > target_remote seems unused Removed. > > > + > > + with_test_prefix "test1" { > > + gdb_test_no_output "record" "turn on process record" > > + } > > with_test_prefix with a single test in it is really just the same as: > > gdb_test_no_output "record" "test1: turn on process record" > > In fact, you have some other tests with the "test1:" or "test2:" > prefix, > I think they should be moved to the with_test_prefix. And maybe use > "next" and "step" instead of "test1" and "test2". Yup, cleaner to have the with_test_prefix cover the whole test. Changed test1 to next-test and test2 to step-next. > > > + > > + # This regression test verifies the reverse-step and reverse- > > next commands > > + # work properly when executing backwards thru a source line > > containing > > + # two function calls on the same source line, i.e. func1 (); > > func2 (); > > + # This test is compiled so the dwarf info not contain the line > > table > > + # information. > > + > > + # Test 1, reverse-next command > > + # Set breakpoint at the line after the function calls. > > + set bp_start_reverse_test [gdb_get_line_number "START REVERSE > > TEST" \ > > + $srcfile] > > + gdb_breakpoint $srcfile:$bp_start_reverse_test temporary > > + > > + # Continue to break point for reverse-next test. > > + # Command definition: reverse-next [count] > > + # Run backward to the beginning of the previous line > > executed in the > > + # current (innermost) stack frame. If the line contains > > function calls, > > + # they will be “un-executed” without stopping. Starting from > > the first > > + # line of a function, reverse-next will take you back to the > > caller of > > + # that function, before the function was called, just as the > > normal next > > + # command would take you from the last line of a function > > back to its > > + # return to its caller 2 . > > + gdb_continue_to_breakpoint \ > > + "test1: stopped at command reverse-next test start location" \ > > + ".*$srcfile:$bp_start_reverse_test\r\n.*" > > + > > + # The reverse-next should step all the way back to the > > beginning of the > > + # line, i.e. at the beginning of the func1 call. > > + gdb_test "reverse-next" ".*func1 \\(\\); func2 \\(\\);.*" \ > > + "test1: reverse-next to line with two functions" > > + > > + # We should be stopped at the first instruction of the line. A > > reverse-step > > + # should step back and stop at the beginning of the previous > > line b = 2, > > + # i.e. not in func1 (). > > + gdb_test "reverse-stepi" ".*b = 2;.*" \ > > + "test1: reverse-stepi to previous line b = 2" > > + > > + > > + # Setup for test 2 > > + clean_restart $executable > > + runto_main > > + > > + with_test_prefix "test2" { > > + gdb_test_no_output "record" "turn on process record" > > + } > > + > > + # Test 2, reverse-step command > > + # Set breakpoint at the line after the function calls. > > + gdb_breakpoint $srcfile:$bp_start_reverse_test temporary > > + > > + # Continue to the start of the reverse-step test. > > + # Command definition: reverse-step [count] > > + # Run the program backward until control reaches the start > > of a > > + # different source line; then stop it, and return control > > to gdb. > > + # Like the step command, reverse-step will only stop at the > > beginning > > + # of a source line. It “un-executes” the previously > > executed source > > + # line. If the previous source line included calls to > > debuggable > > + # functions, reverse-step will step (backward) into the > > called function, > > + # stopping at the beginning of the last statement in the > > called > > + # function (typically a return statement). Also, as with > > the step > > + # command, if non-debuggable functions are called, reverse- > > step will > > + # run thru them backward without stopping. > > + > > + gdb_continue_to_breakpoint \ > > + "test2: stopped at command reverse-step test start location" \ > > + ".*$srcfile:$bp_start_reverse_test\r\n.*" > > + > > + # The first reverse step should take us call of func2 (). > > + gdb_test "reverse-step" ".*END FUNC2.*" \ > > + "test2: reverse-step into func2 " > > + > > + # The second reverse step should take us into func1 (). > > + gdb_test "reverse-step" ".*END FUNC1.*" \ > > + "test2: reverse-step into func1 " > > + > > + # The third reverse step should take us call of func1 (). > > + gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \ > > + "test2: reverse-step to line func1(); func2(), at call for > > func1 " > > + > > + # We should be stopped at the first instruction of the line. A > > reverse > > + # stepi should take us to b = 2 (). > > + gdb_test "reverse-stepi" ".*b = 2;.*" \ > > + "test2: reverse-stepi to line b = 2 " > > +} > > + > > +set srcfile func-map-to-same-line.c > > +set executable func-map-to-same-line > > Wondering if this test should use standard_testfile (like almost > every > other tests) to set these. OK, changed to use the standard_testfile. > > > + > > +# test with and without gcc column info enabled > > +foreach_with_prefix with_column_info {yes no} { > > + if {$with_column_info == "yes"} { > > + set options [list debug column-info] > > + } else { > > + set options [list debug no-column-info] > > + } > > I didn't think of this when proposing the foreach_with_prefix, but > you > could perhaps use: > > foreach_with_prefix column_info_flag {column-info no-column-info} > > ... to avoid this boilerplate. You can then use $column_info_flag > directly when setting options. > OK, that cleans things up a bit. Changed. + + if {[build_executable "failed to prepare" $executable $srcfile \ + $options] == -1} { + return -1 + } + + clean_restart $executable clean_restart can go in run_tests. + run_tests +} diff --git a/gdb/testsuite/gdb.reverse/map-to-same-line.c b/gdb/testsuite/gdb.reverse/map-to-same-line.c new file mode 100644 index 00000000000..f20d778f40e --- /dev/null +++ b/gdb/testsuite/gdb.reverse/map-to-same-line.c @@ -0,0 +1,58 @@ +/* Copyright 2008-2023 Free Software Foundation, Inc. Just wondering if the copyright years are right. New files so yea, should just start with 2023. > + + 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 < http://www.gnu.org/licenses/ >. */ + +/* The purpose of this test is to create a DWARF line table that contains two + or more entries for the same line. When stepping (forwards or backwards), + GDB should step over the entire line and not just a particular entry in the + line table. */ + +int +main () void in the parenthesis Fixed in both test files. > +{ /* TAG: main prologue */ + asm ("main_label: .globl main_label"); + int i = 1, j = 2, k; + float f1 = 2.0, f2 = 4.1, f3; + const char *str_1 = "foo", *str_2 = "bar", *str_3; + + asm ("line1: .globl line1"); + k = i; f3 = f1; str_3 = str_1; /* TAG: line 1 */ + + asm ("line2: .globl line2"); + k = j; f3 = f2; str_3 = str_2; /* TAG: line 2 */ + + asm ("line3: .globl line3"); + k = i; f3 = f1; str_3 = str_1; /* TAG: line 3 */ + + asm ("line4: .globl line4"); + k = j; f3 = f2; str_3 = str_2; /* TAG: line 4 */ + + asm ("line5: .globl line5"); + k = i; f3 = f1; str_3 = str_1; /* TAG: line 5 */ + + asm ("line6: .globl line6"); + k = j; f3 = f2; str_3 = str_2; /* TAG: line 6 */ + + asm ("line7: .globl line7"); + k = i; f3 = f1; str_3 = str_1; /* TAG: line 7 */ + + asm ("line8: .globl line8"); + k = j; f3 = f2; str_3 = str_2; /* TAG: line 8 */ + + asm ("main_return: .globl main_return"); + k = j; f3 = f2; str_3 = str_2; /* TAG: main return */ + + asm ("end_of_sequence: .globl end_of_sequence"); + return 0; /* TAG: main return */ +} 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..16a359d90ec --- /dev/null +++ b/gdb/testsuite/gdb.reverse/map-to-same-line.exp @@ -0,0 +1,156 @@ +# Copyright 2008-2023 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 < http://www.gnu.org/licenses/ = >. + +# When stepping (forwards or backwards), GDB should step over the entire line +# and not just a particular entry in the line table. This test was added to +# verify the find_line_range_start function properly sets the step range for a +# line that consists of multiple statements, i.e. multiple entries in the line +# table. This test creates a DWARF line table that contains two entries for +# the same line to do the needed testing. + +# This test can only be run on targets which support DWARF-2 and use gas. +load_lib dwarf.exp +require dwarf2_support + +# The DWARF assembler requires the gcc compiler. +require is_c_compiler_gcc + +# This test suitable only for process that can do reverse execution +require supports_reverse + +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 distinct source lines being + # mapped to the same line entry. Line 1, 5 and 8 contain 1 statement + # each. Line 2 contains 2 statements. Line 3 contains 3 statements. + program { + DW_LNE_set_address $main_start + line [gdb_get_line_number "TAG: main prologue"] + DW_LNS_copy + DW_LNE_set_address line1 + line [gdb_get_line_number "TAG: line 1" ] + DW_LNS_copy + DW_LNE_set_address line2 + line [gdb_get_line_number "TAG: line 2" ] + DW_LNS_copy + DW_LNE_set_address line3 + line [gdb_get_line_number "TAG: line 2" ] + DW_LNS_copy + DW_LNE_set_address line4 + line [gdb_get_line_number "TAG: line 3" ] + DW_LNS_copy + DW_LNE_set_address line5 + line [gdb_get_line_number "TAG: line 3" ] + DW_LNS_copy + DW_LNE_set_address line6 + line [gdb_get_line_number "TAG: line 3" ] + DW_LNS_copy + DW_LNE_set_address line7 + line [gdb_get_line_number "TAG: line 5" ] + DW_LNS_copy + DW_LNE_set_address line8 + line [gdb_get_line_number "TAG: line 8" ] + DW_LNS_copy + DW_LNE_set_address main_return + line [gdb_get_line_number "TAG: main return"] + DW_LNS_copy + DW_LNE_set_address end_of_sequence + DW_LNE_end_sequence + } + } +} + +if { [prepare_for_testing "failed to prepare" ${testfile} \ + [list $srcfile $asm_file] {nodebug} ] } { + return -1 +} + +runto_main + +# Print the line table +gdb_test_multiple "maint info line-table ${testfile}" "" { + -re "\r\n$decimal\[ \t\]+$decimal\[ \t\]+($hex)\[ \t\]+Y\[^\r\n\]*" { + lappend is_stmt $expect_out(1,string) + exp_continue + } + -re -wrap "" { + } +} + +# Do the reverse-step test +gdb_test_no_output "record" "turn on process record" + +set bp_main_return [gdb_get_line_number "TAG: main return" $srcfile] +gdb_breakpoint $srcfile:$bp_main_return +gdb_continue_to_breakpoint "run to end of main, reverse-step test" ".*$srcfile:$bp_main_return.*" +gdb_test "display \$pc" ".*pc =.*" "display pc, reverse-step test" + +# At this point, GDB has already recorded the execution up until the return +# statement. Reverse-step and test if GDB transitions between lines in the +# expected order. It should reverse-step across lines 8, 5, 3, 2 and 1. +foreach line {8 5 3 2 1} { + gdb_test "reverse-step" ".*TAG: line $line.*" "reverse step to line $line" +} + +## Clean restart, test reverse-next command +clean_restart ${testfile} +runto_main +gdb_test_no_output "record" "turn on process record, reverst-next test" + +set bp_main_return [gdb_get_line_number "TAG: main return" $srcfile] +gdb_breakpoint $srcfile:$bp_main_return +gdb_continue_to_breakpoint "run to end of main, reverse-next test" ".*$srcfile:$bp_main_return.*" +gdb_test "display \$pc" ".*pc =.*" "display pc, reverse-next test" + +# At this point, GDB has already recorded the execution up until the return +# statement. Reverse-next and test if GDB transitions between lines in the +# expected order. It should reverse-next across lines 8, 5, 3, 2 and 1. +foreach line {8 5 3 2 1} { + gdb_test "reverse-next" ".*TAG: line $line.*" "reverse next to line $line" +} It seems like the step and next tests are identical, so I guess it could be factored out using: foreach_with_prefix method {step next} { ... } ? Yup, redid the code using the foreach_with_prefix. > Simon > yR