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 DA53D3858002 for ; Wed, 9 Feb 2022 12:01:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DA53D3858002 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-100-rBcjE2i2OCuD7khieuUt7Q-1; Wed, 09 Feb 2022 07:01:48 -0500 X-MC-Unique: rBcjE2i2OCuD7khieuUt7Q-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 54DF31934100 for ; Wed, 9 Feb 2022 12:01:47 +0000 (UTC) Received: from [10.97.116.38] (ovpn-116-38.gru2.redhat.com [10.97.116.38]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0C99278DD7 for ; Wed, 9 Feb 2022 12:01:45 +0000 (UTC) Message-ID: <9a3d8abf-644a-603a-0493-728ccc00e4e3@redhat.com> Date: Wed, 9 Feb 2022 09:01:40 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: [PING] [PATCH v3] [PR gdb/17315] fix until behavior with trailing !is_stmt lines To: "gdb-patches@sourceware.org" References: <20220126130813.24716-1-blarsen@redhat.com> From: Bruno Larsen In-Reply-To: <20220126130813.24716-1-blarsen@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 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=-12.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, KAM_STOCKGEN, 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: Wed, 09 Feb 2022 12:01:53 -0000 ping On 1/26/22 10:08, Bruno Larsen wrote: > When using the command "until", it is expected that GDB will exit a > loop if the current instruction is the last one related to that loop. > However, if there were trailing non-statement instructions, "until" > would just behave as "next". This was most noticeable in clang-compiled > code, but might happen with gcc-compiled as well. PR gdb/17315 relates > to this problem, as running gdb.base/watchpoint.exp with clang > would fail for this reason. > > The current patch fixes this by adding an extra check to the > until_next_command function, going through all the following > instructions: if the next instruction relates to the same line and is > not marked as a statement, the end of the execution range is moved to > the end of this next instruction. > > This patch also adds a test case that can be run with gcc to test that > this functionality is not silently broken in future updates. > --- > gdb/infcmd.c | 23 ++++ > gdb/testsuite/gdb.base/until-trailing-insns.c | 53 ++++++++ > .../gdb.base/until-trailing-insns.exp | 118 ++++++++++++++++++ > 3 files changed, 194 insertions(+) > create mode 100644 gdb/testsuite/gdb.base/until-trailing-insns.c > create mode 100644 gdb/testsuite/gdb.base/until-trailing-insns.exp > > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index 9f4ed8bff13..5e57437a4cb 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -1346,6 +1346,29 @@ until_next_command (int from_tty) > > tp->control.step_range_start = BLOCK_ENTRY_PC (SYMBOL_BLOCK_VALUE (func)); > tp->control.step_range_end = sal.end; > + > + /* By setting the step_range_end based on the current pc, we implicitly > + assume that the last instruction for any given line must have is_stmt set. > + This is not necessarily true. Clang-13, for example, would compile > + the following code: > + > +for(int i=0; i<10; i++) > + { > + foo(); > + } > + > + with 2 entries after the last is_stmt linetable entry. To fix this, we > + iterate over the instruction related to the end PC, until we find an sal > + related to a different line, and set that pc as the step_range_end. */ > + > + struct symtab_and_line final_sal; > + final_sal = find_pc_line (tp->control.step_range_end, 0); > + > + while (final_sal.line == sal.line && !final_sal.is_stmt) > + { > + tp->control.step_range_end = final_sal.end; > + final_sal = find_pc_line (final_sal.end, 0); > + } > } > tp->control.may_range_step = 1; > > diff --git a/gdb/testsuite/gdb.base/until-trailing-insns.c b/gdb/testsuite/gdb.base/until-trailing-insns.c > new file mode 100644 > index 00000000000..68a2033f517 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/until-trailing-insns.c > @@ -0,0 +1,53 @@ > +/* 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 . */ > + > +/* This test aims to recreate clang-13 behavior in a more controllable > + environment. We want to create a linetable like so: > + > + line | code | is_stmt | > + 1 i = 0; Y > + 2 while(1){ N > + 3 if(...) Y > + 4 a = i; Y > + 5 i++; Y > + 6 } N > + > + where all lines, with the exception of line 4, all point to the loop code > + on line 2, effectively creating a "for" loop. GDB's until command had a > + problem with this setup, because when we're stopped at line 5 the range > + of execution would not include line 6 - a jump instruction still related > + to line 2, making us stop at the next instruction marked as a statement > + and not related to the current line (in this example, line 4). This test > + exercises the command "until" in this complicated situation. > + > + */ > + > +int main(){/* main prologue */ > + asm("main_label: .globl main_label"); /* This is required */ > + asm("loop_start: .globl loop_start"); > + int a, i; > + i = 0; /* loop assignment */ > + while (1) { /* loop line */ > + asm("loop_condition: .globl loop_condition"); > + if (i >= 10) break; /* loop condition */ > + asm("loop_code: .globl loop_code"); > + a = i; /* loop code */ > + asm("loop_increment: .globl loop_increment"); > + i ++; /* loop increment */ > + asm("loop_jump: .globl loop_jump"); > + } > + asm("main_return: .globl main_return"); > + return 0; /* main return */ > +} > diff --git a/gdb/testsuite/gdb.base/until-trailing-insns.exp b/gdb/testsuite/gdb.base/until-trailing-insns.exp > new file mode 100644 > index 00000000000..ea341ccd5a5 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/until-trailing-insns.exp > @@ -0,0 +1,118 @@ > +# 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 . > +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 > +} > +# dwarf assembler requires gcc compiled. > +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 > + set int_size [get_sizeof "int" 4] > + > + # Find start address and length for our functions. > + 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 until-trailing-isns.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 a line table program. This mimicks clang-13's behavior > + # of adding some !is_stmt at the end of a loop line, making until > + # not work properly. > + program { > + {DW_LNE_set_address $main_start} > + {line [gdb_get_line_number "main prologue"]} > + {DW_LNS_copy} > + {DW_LNE_set_address loop_start} > + {line [gdb_get_line_number "loop line"]} > + {DW_LNS_copy} > + {DW_LNE_set_address loop_condition} > + {line [gdb_get_line_number "loop line"]} > + {DW_LNS_negate_stmt} > + {DW_LNS_copy} > + {DW_LNE_set_address loop_code} > + {line [gdb_get_line_number "loop code"]} > + {DW_LNS_negate_stmt} > + {DW_LNS_copy} > + {DW_LNE_set_address loop_increment} > + {line [gdb_get_line_number "loop line"]} > + {DW_LNS_copy} > + {DW_LNE_set_address loop_jump} > + {line [gdb_get_line_number "loop line"]} > + {DW_LNS_negate_stmt} > + {DW_LNS_copy} > + {DW_LNE_set_address main_return} > + {line [gdb_get_line_number "main return"]} > + {DW_LNS_negate_stmt} > + {DW_LNS_copy} > + {DW_LNE_set_address $main_end} > + {line [expr [gdb_get_line_number "main return"] + 1]} > + {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 > +} > + > +gdb_test "next" ".* loop code .*" "inside the loop" > +gdb_test "next" ".* loop line .*" "ending of loop" > +gdb_test "until" ".* return 0; .*" "left loop" -- Cheers! Bruno Larsen