From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id D6889385DC05 for ; Fri, 6 May 2022 15:05:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D6889385DC05 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-630-WVyClUfQMrWZifUG8oBk8w-1; Fri, 06 May 2022 11:05:22 -0400 X-MC-Unique: WVyClUfQMrWZifUG8oBk8w-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 55B4C8FC192; Fri, 6 May 2022 15:04:08 +0000 (UTC) Received: from [10.97.116.29] (ovpn-116-29.gru2.redhat.com [10.97.116.29]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0F0039E6F; Fri, 6 May 2022 15:04:05 +0000 (UTC) Message-ID: <9e420536-01e0-7192-d585-747c52fdf4d5@redhat.com> Date: Fri, 6 May 2022 12:04:04 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: [PATCH,v4] Fix reverse stepping multiple contiguous PC ranges over the line table To: Luis Machado , gdb-patches@sourceware.org Cc: cel@us.ibm.com, rogealve@br.ibm.com, will_schmidt@vnet.ibm.com References: <20220506085506.9184-1-luis.machado@arm.com> From: Bruno Larsen In-Reply-To: <20220506085506.9184-1-luis.machado@arm.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-14.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, 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: Fri, 06 May 2022 15:05:25 -0000 Hi Luis and Carl! Sorry about not answering about this earlier, I had a few rushed weeks, but I have some nits about the .exp file On 5/6/22 05:55, Luis Machado wrote: > v4: > - Updated testcase to make it a bit longer so it can exercise reverse-stepping > multiple times. > - Cleaned up debugging prints. > > 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..efaa60a957f > --- /dev/null > +++ b/gdb/testsuite/gdb.reverse/map-to-same-line.exp > @@ -0,0 +1,136 @@ > +# Copyright 2022 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 . > + > +# 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. > + Could you add a comment here after the copyright blurb explaining why this testcase exists? Something similar to the find_line_range_start comment should be enough, jsut so the next unrelated person who looks at this code in 2 years has some context as to what is going on. > +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 > +} There should probably be a test here for supports_reverse. Or supports_process_record. I'm not sure what the difference is between these 2 checks. > + > +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_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" > +} since we would have tested above if the target supports recording/reversing, we should be good to just use record here, without the if clause. I suggest doing it this way because there is no point in this whole test if the target doesn't support reverse execution. > + > +gdb_test "tbreak main_return" "Temporary breakpoint .*" "breakpoint at return" > +gdb_test "continue" "Temporary breakpoint .*" "run to end of main" > + > +# 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" > +} Cheers! Bruno Larsen