From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id CBA923858C83 for ; Tue, 16 May 2023 15:31:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CBA923858C83 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id F128E21D9B; Tue, 16 May 2023 15:31:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1684251074; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=CHCjKvOadi6B5ewm1f5H3noWH9gJ4yxTOSpd4AwW3WM=; b=kzNgn7Ek+6Pp+ydxwJf7dujn79/OkENNNwxzyrMIdI3iG2fJAwBzSXPrZOmAt2lJxAxdPV id5H1e+n37ovxqsz7UiISANojIGvoP+1PaV6zvc9Z+amsu2O9Ye8v641nUYohC8obVI2e9 fY58EYTGOq78GH4FN49S1xiKl6dVyQU= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1684251074; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=CHCjKvOadi6B5ewm1f5H3noWH9gJ4yxTOSpd4AwW3WM=; b=hQlrnKhDOxLVpzlBv6ZTgfL7bNWz5/03uTOwfiNyRA86QVmesuJuDxy3dWg9AUoBbAK2tZ thBlh0x2n3vZxiAA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id CEE22138F5; Tue, 16 May 2023 15:31:14 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id t4ddMcKhY2SpCQAAMHmgww (envelope-from ); Tue, 16 May 2023 15:31:14 +0000 Message-ID: Date: Tue, 16 May 2023 17:31:14 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.1 Subject: Re: [PATCH v3] gdb: Fix false match issue in skip_prologue_using_linetable Content-Language: en-US To: Luis Machado , Kevin Buettner , Tom de Vries via Gdb-patches Cc: WANG Rui References: <20230418120939.29102-1-tdevries@suse.de> <1a4148e1-1376-3bc6-e398-4c611922c58f@suse.de> <20230421110346.7ca9b163@f37-zws-nv> <276f25d7-ab32-268f-0875-70cab6180e8d@suse.de> <6663707c-4297-c2f2-a0bd-f3e84fc62aad@suse.de> From: Tom de Vries In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,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: On 5/16/23 16:19, Luis Machado wrote: > On 4/24/23 15:15, Tom de Vries wrote: >> On 4/24/23 14:53, Luis Machado wrote: >>> On 4/22/23 09:01, Tom de Vries via Gdb-patches wrote: >>>> On 4/21/23 20:03, Kevin Buettner wrote: >>>>> Hi Tom, >>>>> >>>>> On Tue, 18 Apr 2023 14:15:06 +0200 >>>>> Tom de Vries via Gdb-patches wrote: >>>>> >>>>>> On 4/18/23 14:09, Tom de Vries via Gdb-patches wrote: >>>>>>> Co-Authored-By: WANG Rui (fix, tiny change [1]) >>>>>>> Co-Authored-By: Tom de Vries (test-case) >>>>>>> >>>>>>> [1] >>>>>>> https://www.gnu.org/prep/maintain/html_node/Legally-Significant.html >>>>>> >>>>>> I'm not used to deal with these matters, so I'd appreciate some >>>>>> review/approval on this.  Is my copyright status assessment >>>>>> correct, and >>>>>> did I write it up correctly? >>>>> >>>>> I refreshed my memory via the link you provided above.  Based on what >>>>> is written there, I conclude that Wang Rui's change is not legally >>>>> signficant for copyright purposes. >>>>> >>>>> Also, I've looked over the Rui's patch as well as your test case, and >>>>> it looks good to me.  So... >>>>> >>>>> Approved-by: Kevin Buettner >>>>> >>>> >>>> Hi Kevin, >>>> >>>> Thanks for review. >>>> >>>> Committed and also backported to gdb-13-branch, because it was a 12 >>>> -> 13 regression. >>>> >>>> Thanks, >>>> - Tom >>>> >>> >>> For some reason aarch64 is grumpy with this test, and it FAIL's the >>> last comparison. >>> >>> Maybe aarch64 is broken in this regard? >> >> Hi Luis, >> >> thanks for reporting this. >> >> I could reproduce it on openSUSE Leap 15.4. >> >> I think there are two independent problems: >> - the aarch64 prologue analyzer walks past the end of the function >> - the test-case assumes that the prologue analyzer will return the first >>    insn in foo, rather that some insn in foo. >> >> The WIP patch below addresses both issues, and allows the test-case to >> pass for me. >> >> [ FWIW, alternatively using some "maint set skip-prologue" value from >> this RFC ( >> https://sourceware.org/pipermail/gdb-patches/2022-August/191343.html ) >> could also suffice to ignore the first problem. ] >> >> Thanks, >> - Tom >> >> ... >> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c >> index ec0e51bdaf7..d974595e48f 100644 >> --- a/gdb/aarch64-tdep.c >> +++ b/gdb/aarch64-tdep.c >> @@ -917,12 +917,13 @@ aarch64_analyze_prologue_test (void) >>   static CORE_ADDR >>   aarch64_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc) >>   { >> -  CORE_ADDR func_addr, limit_pc; >> +  CORE_ADDR func_addr, func_end_addr, limit_pc; >> >>     /* See if we can determine the end of the prologue via the symbol >>        table.  If so, then return either PC, or the PC after the >>        prologue, whichever is greater.  */ >> -  if (find_pc_partial_function (pc, NULL, &func_addr, NULL)) >> +  bool func_addr_found = find_pc_partial_function (pc, NULL, >> &func_addr, &func_end_addr); >> +  if (func_addr_found) >>       { >>         CORE_ADDR post_prologue_pc >>          = skip_prologue_using_sal (gdbarch, func_addr); >> @@ -941,7 +942,8 @@ aarch64_skip_prologue (struct gdbarch *gdbarch, >> CORE_ADDR pc) >>     limit_pc = skip_prologue_using_sal (gdbarch, pc); >>     if (limit_pc == 0) >>       limit_pc = pc + 128;       /* Magic.  */ >> - >> +  limit_pc = std::min (limit_pc, func_end_addr - 4); >> + >>     /* Try disassembling prologue.  */ >>     return aarch64_analyze_prologue (gdbarch, pc, limit_pc, NULL); >>   } >> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-prologue-end-2.exp >> b/gdb/testsuite/gdb.dwarf2/dw2-prologue-end-2.exp >> index 488f85f9674..c506cfd55cc 100644 >> --- a/gdb/testsuite/gdb.dwarf2/dw2-prologue-end-2.exp >> +++ b/gdb/testsuite/gdb.dwarf2/dw2-prologue-end-2.exp >> @@ -95,15 +95,15 @@ if { $break_addr == "" } { >> >>   # Get the "foo_label" address. >> >> -set foo_label_addr "" >> -gdb_test_multiple "print /x &foo_label" "" { >> +set bar_label_addr "" >> +gdb_test_multiple "print /x &bar_label" "" { >>       -re -wrap "= ($hex)" { >> -       set foo_label_addr $expect_out(1,string) >> +       set bar_label_addr $expect_out(1,string) >>          pass $gdb_test_name >>       } >>   } >> >> -if { $foo_label_addr == "" } { >> +if { $bar_label_addr == "" } { >>       return >>   } >> >> @@ -115,4 +115,4 @@ gdb_test "print &foo_end == &bar_label" " = 1" >>   # Check that the breakpoint is set at the expected address. >> Regression test >>   # for PR30369. >> >> -gdb_assert { $break_addr == $foo_label_addr } >> +gdb_assert { $break_addr < $bar_label_addr } >> ... > > Sorry, I thought I had replied to this thread. Indeed the above patch > addresses this problem with the aarch64 prologue skipper, > and it also fixes things for arm. For arm I suspect we might need the > same fix to the prologue skipper that the patch addresses for aarch64. > > I can pick it up, refresh and submit if you're happy with it as well. Hi Luis, thanks for confirming. If you want to pick this up, great, thanks. - Tom