From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 5E4083858C54 for ; Wed, 7 Jun 2023 09:55:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5E4083858C54 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-out2.suse.de (Postfix) with ESMTPS id 80C641FDAD; Wed, 7 Jun 2023 09:55:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1686131701; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MxsHWFrcSby64nL2fGplir5ZPmpHz2GjU0bi0Kcupzc=; b=AqRiQwiFuWdX/GWuAHpGr7QWlzjE8UBnVgiYs4k09nAcjwNdxZyHWGrR2PoZZwxd0IAgsD I39tZcieOZg/xGpRMVBAiKske2NUdCdWKRKbD9E6wJ+QtQJ8RJPPuewKofbbWryRKwqhx4 wMWMMGztt727TaysLC83AYy1pEAa9iQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1686131701; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MxsHWFrcSby64nL2fGplir5ZPmpHz2GjU0bi0Kcupzc=; b=Vv7Q3czfLp/8wYHJNpMCBHaG6ycN2fvyPs9PdViXdkIwZIgNqBE28v3y1HUk/9iaEc3xOd tJFqSy/qLi64sFBQ== 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 5D39C13776; Wed, 7 Jun 2023 09:55:01 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id Q49jFfVTgGREbgAAMHmgww (envelope-from ); Wed, 07 Jun 2023 09:55:01 +0000 Message-ID: Date: Wed, 7 Jun 2023 11:55:06 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH] Fix PR30369 regression on aarch64/arm (PR30506) Content-Language: en-US To: Luis Machado , gdb-patches@sourceware.org References: <20230607094010.57892-1-luis.machado@arm.com> From: Tom de Vries In-Reply-To: <20230607094010.57892-1-luis.machado@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,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 6/7/23 11:40, Luis Machado wrote: > From: Tom de Vries > > The gdb.dwarf2/dw2-prologue-end-2.exp test was failing for both AArch64 and > Arm. > > As Tom pointed out here (https://inbox.sourceware.org/gdb-patches/6663707c-4297-c2f2-a0bd-f3e84fc62aad@suse.de/), > there are issues with both the prologue skipper for AArch64 and Arm and an > incorrect assumption by the testcase. > > This patch fixes both of AArch64's and Arm's prologue skippers to not skip past > the end of a function. It also incorporates a fix to the testcase so it > doesn't assume the prologue skipper will stop at the first instruction of the > functions/labels. > > Regression-tested on aarch64-linux/arm-linux Ubuntu 20.04/22.04 and > x86_64-linux Ubuntu 20.04. > Hi Luis, thanks for picking this up. LGTM. Thanks, - Tom > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30506 > > Co-Authored-By: Tom de Vries > Co-Authored-By: Luis Machado > --- > gdb/aarch64-tdep.c | 10 +++++++-- > gdb/arm-tdep.c | 21 ++++++++++++++++--- > .../gdb.dwarf2/dw2-prologue-end-2.exp | 10 ++++----- > 3 files changed, 31 insertions(+), 10 deletions(-) > > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > index d8603c45fd3..84a90b63b55 100644 > --- a/gdb/aarch64-tdep.c > +++ b/gdb/aarch64-tdep.c > @@ -917,12 +917,15 @@ 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); > @@ -942,6 +945,9 @@ aarch64_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc) > if (limit_pc == 0) > limit_pc = pc + 128; /* Magic. */ > > + limit_pc > + = func_end_addr == 0? 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/arm-tdep.c b/gdb/arm-tdep.c > index d5128754f02..a0f59557072 100644 > --- a/gdb/arm-tdep.c > +++ b/gdb/arm-tdep.c > @@ -1769,12 +1769,18 @@ arm_skip_stack_protector(CORE_ADDR pc, struct gdbarch *gdbarch) > static CORE_ADDR > arm_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); > + > + /* Whether the function is thumb mode or not. */ > + bool func_is_thumb = false; > + > + if (func_addr_found) > { > CORE_ADDR post_prologue_pc > = skip_prologue_using_sal (gdbarch, func_addr); > @@ -1811,7 +1817,8 @@ arm_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc) > associate prologue code with the opening brace; so this > lets us skip the first line if we think it is the opening > brace. */ > - if (arm_pc_is_thumb (gdbarch, func_addr)) > + func_is_thumb = arm_pc_is_thumb (gdbarch, func_addr); > + if (func_is_thumb) > analyzed_limit = thumb_analyze_prologue (gdbarch, func_addr, > post_prologue_pc, NULL); > else > @@ -1837,6 +1844,14 @@ arm_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc) > if (limit_pc == 0) > limit_pc = pc + 64; /* Magic. */ > > + /* Set the correct adjustment based on whether the function is thumb mode or > + not. We use it to get the address of the last instruction in the > + function (as opposed to the first address of the next function). */ > + CORE_ADDR adjustment = func_is_thumb? 2 : 4; > + > + limit_pc > + = func_end_addr == 0? limit_pc : std::min (limit_pc, > + func_end_addr - adjustment); > > /* Check if this is Thumb code. */ > if (arm_pc_is_thumb (gdbarch, pc)) > 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 }