This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] AArch64: Allow additional sizes in prologue
- From: Alan Hayward <Alan dot Hayward at arm dot com>
- To: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Cc: Tom Tromey <tom at tromey dot com>, nd <nd at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>
- Date: Wed, 14 Aug 2019 15:03:25 +0000
- Subject: Re: [PATCH] AArch64: Allow additional sizes in prologue
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=KYwli2rCuh0LXKTD4elSkWoEjrexF+6DbGc4MQfgBL0=; b=aIQKkq1jCrKZVKPG626eWy2t7omm1COkPCs1opHoGWuFVnX80jmiyV+4b5TQn5j9YQH8Bd40WRAHQuOrZawPHcdPUOKV5L9nTlzOj6+2jEjkM/75AUpzQlbsXNLCnLSCcVfG4uTDUyH0Ci+MWvwc2u4Uy+qlg8xHVm+YIvES7f2criwXuMBEayk331k5FCnQEjzGq0ncX+PDMV+E7rsvDb4b9C0uN+UCY+FTJf7WvKUT3jP+MRWoAbbUSDtsGR4m/aswzYFRgKF16cTKCJ1Ot2BCS+VLFTVqZHzagipIcsO2Mp/5UMFaiskY19lsSu77N+XrBPYjfRfqGeKTznLYqA==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UaqCykeSUiRm/54cxmy9k2oeZwFStcMTqydPVhtDERMc4dLq3kf/JynqoGN8hBv3ahWZbXOs9RC7LZjs/6eq6K2bcIzkAfiqW0cBlc2i57LkW4zbPE0IkHwb6dQ91mIeBUrnfdHXkcSVGQsKRUWXWQFxl2g9evYg161iS+W6kpV09SC8oqGPhtI8U48uS5Wagyz25xcCJzQCEA6aabXBtdeTAYSsCB40ecIgUiH3pu4fPPc+B7rMCadYHS0DgNMMbxN+f17fiIni/QoFW6E4PWaRiEy66OLIEeStCF7SGQUkIkjYTfAOymtMmwwOM4v+nBwsElEM0WCEQYjM1wuiYA==
- Original-authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan dot Hayward at arm dot com;
- References: <20190805124152.26127-1-alan.hayward@arm.com> <8736ifcxu8.fsf@tromey.com> <16866921-5116-49AE-A00E-9D738CE212BD@arm.com> <793a518d-be9c-75f0-96a5-9acdc90d7a62@arm.com> <B0728864-B442-461B-8E1F-38AFE5BF9AEE@arm.com>
Patch pushed with the formatting fixed.
Pasted to end of email for reference.
> On 6 Aug 2019, at 13:18, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
>
>
>> On 6 Aug 2019, at 10:56, Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:
>>
>> On 06/08/2019 09:58, Alan Hayward wrote:
>>>> On 5 Aug 2019, at 18:43, Tom Tromey <tom@tromey.com> wrote:
>>>>
>>>>>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:
>>>>
>>>> Alan> When saving registers to the stack at the start of a function, not all state
>>>> Alan> needs to be saved. For example, only the first 64bits of float registers need
>>>> Alan> saving. However, a program may choose to store extra state if it wishes,
>>>> Alan> there is nothing preventing it doing so.
>>>>
>>>> Alan> The aarch64_analyze_prologue will error if it detects extra state being
>>>> Alan> stored. Relex this restriction.
>>>>
>>>> I don't know anything about AArch64, so I can't really comment on the
>>>> content of the patch, but I did happen to see a nit:
>>>>
>>>> Alan> stack.store (pv_add_constant (regs[rn],
>>>> Alan> inst.operands[1].addr.offset.imm),
>>>> Alan> - is64 ? 8 : 4, regs[rt]);
>>>> Alan> + size, regs[rt]);
>>>>
>>>> ...this addition looked mis-indented to me.
>>> Thanks.
>>> I probably missed that because the correct formatting looks horrible:
>>> stack.store (pv_add_constant (regs[rn],
>>> inst.operands[1].addr.offset.imm),
>>> size, regs[rt]);
>>> Instead, I can update it to the following
>>> stack.store (
>>> pv_add_constant (regs[rn], inst.operands[1].addr.offset.imm),
>>> size, regs[rt]);
>>
>> I think the coding convention in that case is to write:
>>
>>
>> stack.store
>> (pv_add_constant (regs[rn], inst.operands[1].addr.offset.imm),
>> size, regs[rt]);
>>
>> R.
>
> Ok, will use that way.
>
> Alan.
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 15904da82c..6f7474f282 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2019-08-14 Alan Hayward <alan.hayward@arm.com>
+
+ * aarch64-tdep.c (aarch64_analyze_prologue): Allow any valid
+ register sizes.
+
2019-08-14 Tom Tromey <tromey@adacore.com>
* darwin-nat.c: Include gdbarch.h.
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 9b6324f0fc..5e9f7b8ee0 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -387,17 +387,16 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
{
unsigned rt = inst.operands[0].reg.regno;
unsigned rn = inst.operands[1].addr.base_regno;
- int is64
- = (aarch64_get_qualifier_esize (inst.operands[0].qualifier) == 8);
+ int size = aarch64_get_qualifier_esize (inst.operands[0].qualifier);
gdb_assert (aarch64_num_of_operands (inst.opcode) == 2);
gdb_assert (inst.operands[0].type == AARCH64_OPND_Rt);
gdb_assert (inst.operands[1].type == AARCH64_OPND_ADDR_SIMM9);
gdb_assert (!inst.operands[1].addr.offset.is_reg);
- stack.store (pv_add_constant (regs[rn],
- inst.operands[1].addr.offset.imm),
- is64 ? 8 : 4, regs[rt]);
+ stack.store
+ (pv_add_constant (regs[rn], inst.operands[1].addr.offset.imm),
+ size, regs[rt]);
}
else if ((inst.opcode->iclass == ldstpair_off
|| (inst.opcode->iclass == ldstpair_indexed
@@ -409,6 +408,7 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
unsigned rt2;
unsigned rn = inst.operands[2].addr.base_regno;
int32_t imm = inst.operands[2].addr.offset.imm;
+ int size = aarch64_get_qualifier_esize (inst.operands[0].qualifier);
gdb_assert (inst.operands[0].type == AARCH64_OPND_Rt
|| inst.operands[0].type == AARCH64_OPND_Ft);
@@ -430,17 +430,12 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
rt2 = inst.operands[1].reg.regno;
if (inst.operands[0].type == AARCH64_OPND_Ft)
{
- /* Only bottom 64-bit of each V register (D register) need
- to be preserved. */
- gdb_assert (inst.operands[0].qualifier == AARCH64_OPND_QLF_S_D);
rt1 += AARCH64_X_REGISTER_COUNT;
rt2 += AARCH64_X_REGISTER_COUNT;
}
- stack.store (pv_add_constant (regs[rn], imm), 8,
- regs[rt1]);
- stack.store (pv_add_constant (regs[rn], imm + 8), 8,
- regs[rt2]);
+ stack.store (pv_add_constant (regs[rn], imm), size, regs[rt1]);
+ stack.store (pv_add_constant (regs[rn], imm + size), size, regs[rt2]);
if (inst.operands[2].addr.writeback)
regs[rn] = pv_add_constant (regs[rn], imm);
@@ -457,21 +452,14 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
unsigned int rt = inst.operands[0].reg.regno;
int32_t imm = inst.operands[1].addr.offset.imm;
unsigned int rn = inst.operands[1].addr.base_regno;
- bool is64
- = (aarch64_get_qualifier_esize (inst.operands[0].qualifier) == 8);
+ int size = aarch64_get_qualifier_esize (inst.operands[0].qualifier);
gdb_assert (inst.operands[0].type == AARCH64_OPND_Rt
|| inst.operands[0].type == AARCH64_OPND_Ft);
if (inst.operands[0].type == AARCH64_OPND_Ft)
- {
- /* Only bottom 64-bit of each V register (D register) need
- to be preserved. */
- gdb_assert (inst.operands[0].qualifier == AARCH64_OPND_QLF_S_D);
- rt += AARCH64_X_REGISTER_COUNT;
- }
+ rt += AARCH64_X_REGISTER_COUNT;
- stack.store (pv_add_constant (regs[rn], imm),
- is64 ? 8 : 4, regs[rt]);
+ stack.store (pv_add_constant (regs[rn], imm), size, regs[rt]);
if (inst.operands[1].addr.writeback)
regs[rn] = pv_add_constant (regs[rn], imm);
}
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index e4b9616dde..6c80a69e06 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-08-14 Alan Hayward <alan.hayward@arm.com>
+
+ * gdb.arch/aarch64-prologue.c: New test.
+ * gdb.arch/aarch64-prologue.exp: New file.
+
2019-08-13 Tom de Vries <tdevries@suse.de>
* gdb.gdb/selftest.exp (send ^C to child process again): Accept also
diff --git a/gdb/testsuite/gdb.arch/aarch64-prologue.c b/gdb/testsuite/gdb.arch/aarch64-prologue.c
new file mode 100644
index 0000000000..aa664f704f
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-prologue.c
@@ -0,0 +1,83 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2019 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/>. */
+
+#include <stdio.h>
+
+extern void excessiveprologue ();
+
+void
+innerfunc ()
+{
+ printf ("inner\n");
+}
+
+/* excessiveprologue saves to the stack in multiple ways. */
+
+asm ("\t.section .gnu.sgstubs,\"ax\",%progbits\n"
+ "\t.global excessiveprologue\n"
+ "\t.type excessiveprologue, %function\n"
+ "excessiveprologue:\n"
+ "\tstp x29, x30, [sp, #-208]!\n"
+ "\tmov x29, sp\n"
+ "\tstp w0,w1,[sp,16]\n"
+ "\tstp x2,x3,[sp,24]\n"
+ "\tstr w4,[sp,40]\n"
+ "\tstr x5,[sp,48]\n"
+ "\tstur w6,[sp,52]\n"
+ "\tstur x7,[sp,56]\n"
+ "\tstp s0,s1,[sp,64]\n"
+ "\tstp d2,d3,[sp,72]\n"
+ "\tstp q4,q5,[sp,96]\n"
+ "\tstr b6,[sp,128]\n"
+ "\tstr h7,[sp,132]\n"
+ "\tstr s8,[sp,136]\n"
+ "\tstr d9,[sp,140]\n"
+ "\tstr q10,[sp,148]\n"
+ "\tstur b11,[sp,164]\n"
+ "\tstur h12,[sp,160]\n"
+ "\tstur s13,[sp,172]\n"
+ "\tstur d14,[sp,176]\n"
+ "\tstur q15,[sp,184]\n"
+ "\tbl innerfunc\n"
+ "\tldp w0,w1,[sp,16]\n"
+ "\tldp x2,x3,[sp,24]\n"
+ "\tldr w4,[sp,40]\n"
+ "\tldr x5,[sp,48]\n"
+ "\tldur w6,[sp,52]\n"
+ "\tldur x7,[sp,56]\n"
+ "\tldp s0,s1,[sp,64]\n"
+ "\tldp d2,d3,[sp,72]\n"
+ "\tldp q4,q5,[sp,96]\n"
+ "\tldr b6,[sp,128]\n"
+ "\tldr h7,[sp,132]\n"
+ "\tldr s8,[sp,136]\n"
+ "\tldr d9,[sp,140]\n"
+ "\tldr q10,[sp,148]\n"
+ "\tldur b11,[sp,164]\n"
+ "\tldur h12,[sp,160]\n"
+ "\tldur s13,[sp,172]\n"
+ "\tldur d14,[sp,176]\n"
+ "\tldur q15,[sp,184]\n"
+ "\tldp x29, x30, [sp], #208\n"
+ "ret\n");
+
+int
+main (void)
+{
+ excessiveprologue ();
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/aarch64-prologue.exp b/gdb/testsuite/gdb.arch/aarch64-prologue.exp
new file mode 100644
index 0000000000..e31261ad25
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-prologue.exp
@@ -0,0 +1,36 @@
+# Copyright 2019 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.
+
+if {![is_aarch64_target]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return 1
+}
+
+standard_testfile
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile {nodebug}]} {
+ return -1
+}
+
+if ![runto_main] {
+ untested "could not run to main"
+ return -1
+}
+
+# Ensure gdb can break at excessiveprologue then continue.
+gdb_breakpoint "excessiveprologue"
+gdb_continue_to_breakpoint "excessiveprologue"
+gdb_continue_to_end "excessiveprologue" "continue" 1