* [PATCH] Return the regnum for PC (32) on aarch64
@ 2022-05-13 12:27 Yichao Yu
2022-05-16 9:05 ` Luis Machado
0 siblings, 1 reply; 17+ messages in thread
From: Yichao Yu @ 2022-05-13 12:27 UTC (permalink / raw)
To: gdb-patches, Luis Machado
This will allow the unwind info to explicitly specify a different value
for the return address than the link register. Such usage, e.g. for signal frames,
is mentioned in aadwarf64 from ARM.
Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
---
gdb/aarch64-tdep.c | 3 +
gdb/aarch64-tdep.h | 1 +
gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 +++++++++++++++
gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 63 ++++++++++++++++++++
4 files changed, 115 insertions(+)
create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 9d06ebfe27c..f9eb455232f 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
if (reg == AARCH64_DWARF_SP)
return AARCH64_SP_REGNUM;
+ if (reg == AARCH64_DWARF_PC)
+ return AARCH64_PC_REGNUM;
+
if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index e4cdebb6311..aa1bedbdac6 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -34,6 +34,7 @@ struct regset;
/* AArch64 Dwarf register numbering. */
#define AARCH64_DWARF_X0 0
#define AARCH64_DWARF_SP 31
+#define AARCH64_DWARF_PC 32
#define AARCH64_DWARF_PAUTH_RA_STATE 34
#define AARCH64_DWARF_PAUTH_DMASK 35
#define AARCH64_DWARF_PAUTH_CMASK 36
diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
new file mode 100644
index 00000000000..8589a64fd56
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
@@ -0,0 +1,48 @@
+/* Copyright 2022-2022 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+ .text
+ .globl main
+ .type main,#function
+main:
+ .cfi_startproc
+ stp x29, x30, [sp, -16]!
+ .cfi_def_cfa sp, 16
+ .cfi_offset x29, 0
+ .cfi_offset x30, 8
+ bl test_func
+ ldp x29, x30, [sp], 16
+ .cfi_restore x29
+ .cfi_restore x30
+ .cfi_def_cfa sp, 0
+ mov x0, 0
+ ret
+ .cfi_endproc
+
+ .globl test_func
+test_func:
+ .cfi_startproc
+ // Unwind x30 to a different value
+ // CFA_val_expression x30 const2u 0x1234
+ .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
+ // CFA_val_expression pc breg30 0
+ .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
+ mov x0, x30
+ .cfi_register 32, x0
+ mov x30, 0x1234
+ ret x0
+ .cfi_endproc
diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
new file mode 100644
index 00000000000..c047c7d2f1a
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
@@ -0,0 +1,63 @@
+# Copyright 2022-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 <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Test explicitly unwinding PC on aarch64
+
+if {![is_aarch64_target]} then {
+ verbose "Skipping arm displaced stepping tests."
+ return
+}
+
+standard_testfile .S
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+
+proc test_reg_vals { i } {
+ gdb_test "p \$pc - &main" "= 8" "p \$pc, $i"
+ gdb_test "p \$x30" "= 4660" "p \$x30, $i"
+}
+
+proc test_unwind_pc { i } {
+ gdb_test "si" "" "single step, $i"
+ gdb_test "backtrace" \
+ ".*#1.*in main ().*" \
+ "backtrace, $i"
+ gdb_test "up" "" "parent frame, $i"
+ test_reg_vals "$i"
+}
+
+# Ready to enter the function
+gdb_test "si" "bl test_func" "call site"
+# Step through the 3 instructions in the function to make sure that
+# we have the same unwind info throughout.
+test_unwind_pc 1
+test_unwind_pc 2
+test_unwind_pc 3
+# Check again after we returned
+gdb_test "si" "" "single step out"
+gdb_test "backtrace" \
+ "#0\[\t \]+main ().*" \
+ "backtrace, 4"
+test_reg_vals 4
+
+gdb_continue_to_end "aarch64-unwind-pc"
--
2.36.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Return the regnum for PC (32) on aarch64
2022-05-13 12:27 [PATCH] Return the regnum for PC (32) on aarch64 Yichao Yu
@ 2022-05-16 9:05 ` Luis Machado
2022-05-16 13:17 ` Yichao Yu
0 siblings, 1 reply; 17+ messages in thread
From: Luis Machado @ 2022-05-16 9:05 UTC (permalink / raw)
To: gdb-patches, Yichao Yu
Hi,
Thanks for the patch. It looks good to me. There are just some
adjustments/suggestions on the testcase itself.
> This will allow the unwind info to explicitly specify a different value
> for the return address than the link register. Such usage, e.g. for signal frames,
> is mentioned in aadwarf64 from ARM.
>
> Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
> ---
> gdb/aarch64-tdep.c | 3 +
> gdb/aarch64-tdep.h | 1 +
> gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 +++++++++++++++
> gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 63 ++++++++++++++++++++
> 4 files changed, 115 insertions(+)
> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 9d06ebfe27c..f9eb455232f 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
> if (reg == AARCH64_DWARF_SP)
> return AARCH64_SP_REGNUM;
>
> + if (reg == AARCH64_DWARF_PC)
> + return AARCH64_PC_REGNUM;
> +
> if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
> return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
>
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index e4cdebb6311..aa1bedbdac6 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -34,6 +34,7 @@ struct regset;
> /* AArch64 Dwarf register numbering. */
> #define AARCH64_DWARF_X0 0
> #define AARCH64_DWARF_SP 31
> +#define AARCH64_DWARF_PC 32
> #define AARCH64_DWARF_PAUTH_RA_STATE 34
> #define AARCH64_DWARF_PAUTH_DMASK 35
> #define AARCH64_DWARF_PAUTH_CMASK 36
> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> new file mode 100644
> index 00000000000..8589a64fd56
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> @@ -0,0 +1,48 @@
> +/* Copyright 2022-2022 Free Software Foundation, Inc.
2022-2022 -> 2022
> +
> + This file is part of GDB.
> +
> + 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/>. */
> +
> + .text
> + .globl main
> + .type main,#function
> +main:
> + .cfi_startproc
> + stp x29, x30, [sp, -16]!
> + .cfi_def_cfa sp, 16
> + .cfi_offset x29, 0
> + .cfi_offset x30, 8
> + bl test_func
> + ldp x29, x30, [sp], 16
> + .cfi_restore x29
> + .cfi_restore x30
> + .cfi_def_cfa sp, 0
> + mov x0, 0
> + ret
> + .cfi_endproc
> +
> + .globl test_func
> +test_func:
> + .cfi_startproc
> + // Unwind x30 to a different value
> + // CFA_val_expression x30 const2u 0x1234
> + .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
> + // CFA_val_expression pc breg30 0
> + .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
> + mov x0, x30
> + .cfi_register 32, x0
> + mov x30, 0x1234
> + ret x0
> + .cfi_endproc
> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> new file mode 100644
> index 00000000000..c047c7d2f1a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> @@ -0,0 +1,63 @@
> +# Copyright 2022-2022 Free Software Foundation, Inc.
2022-2022 -> 2022
> +
> +# 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.
> +
> +# Test explicitly unwinding PC on aarch64
How about...
"Test explicitly unwinding the PC DWARF register on aarch64"?
> +
> +if {![is_aarch64_target]} then {
> + verbose "Skipping arm displaced stepping tests."
> + return
> +}
> +
Copy/paste problem possibly? The usual way to report this is:
verbose "Skipping ${gdb_test_file_name}."
> +standard_testfile .S
> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> + return -1
> +}
> +
> +if ![runto_main] {
> + return -1
> +}
> +
> +proc test_reg_vals { i } {
> + gdb_test "p \$pc - &main" "= 8" "p \$pc, $i"
> + gdb_test "p \$x30" "= 4660" "p \$x30, $i"
> +}
I'd drop the argument to the proc. We can surround the proc call in a
prefix instead. See below.
> +
> +proc test_unwind_pc { i } {
> + gdb_test "si" "" "single step, $i"
Instead of passing regardless of the output, can we validate that we
really stepped things correctly?
> + gdb_test "backtrace" \
> + ".*#1.*in main ().*" \
> + "backtrace, $i"
> + gdb_test "up" "" "parent frame, $i"
> + test_reg_vals "$i"
> +}
Drop the proc argument as well and use a prefix instead. See below.
> +
> +# Ready to enter the function
> +gdb_test "si" "bl test_func" "call site"
> +# Step through the 3 instructions in the function to make sure that
> +# we have the same unwind info throughout.
> +test_unwind_pc 1
> +test_unwind_pc 2
> +test_unwind_pc 3
You can make these like so, for example...
with_test_prefix "1st stepi" {
test_unwind_pc
}
with_test_prefix "2nd stepi" {
test_unwind_pc
}
with_test_prefix "3rd stepi" {
test_unwind_pc
}
> +# Check again after we returned
> +gdb_test "si" "" "single step out"
Same as before. Instead of passing regardless of the output, can we
validate that we really stepped out? This would make the test more robust.
> +gdb_test "backtrace" \
> + "#0\[\t \]+main ().*" \
> + "backtrace, 4"
> +test_reg_vals 4
> +
Given this is the final check, do we need to number it? We could just...
+with_test_prefix "final" {
+ gdb_test "backtrace" \
+ "#0\[\t \]+main ().*" \
+ "backtrace"
+ test_reg_vals
+}
> +gdb_continue_to_end "aarch64-unwind-pc"
At this point we've already exercised what we wanted, right? Is the
above test needed?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Return the regnum for PC (32) on aarch64
2022-05-16 9:05 ` Luis Machado
@ 2022-05-16 13:17 ` Yichao Yu
2022-05-16 13:25 ` [PATCH v2] " Yichao Yu
2022-05-16 13:29 ` [PATCH] " Luis Machado
0 siblings, 2 replies; 17+ messages in thread
From: Yichao Yu @ 2022-05-16 13:17 UTC (permalink / raw)
To: Luis Machado; +Cc: gdb-patches
On Mon, May 16, 2022 at 5:05 AM Luis Machado <luis.machado@arm.com> wrote:
>
> Hi,
>
> Thanks for the patch. It looks good to me. There are just some
> adjustments/suggestions on the testcase itself.
>
> > This will allow the unwind info to explicitly specify a different value
> > for the return address than the link register. Such usage, e.g. for signal frames,
> > is mentioned in aadwarf64 from ARM.
> >
> > Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
> > ---
> > gdb/aarch64-tdep.c | 3 +
> > gdb/aarch64-tdep.h | 1 +
> > gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 +++++++++++++++
> > gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 63 ++++++++++++++++++++
> > 4 files changed, 115 insertions(+)
> > create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> > create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> >
> > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> > index 9d06ebfe27c..f9eb455232f 100644
> > --- a/gdb/aarch64-tdep.c
> > +++ b/gdb/aarch64-tdep.c
> > @@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
> > if (reg == AARCH64_DWARF_SP)
> > return AARCH64_SP_REGNUM;
> >
> > + if (reg == AARCH64_DWARF_PC)
> > + return AARCH64_PC_REGNUM;
> > +
> > if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
> > return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
> >
> > diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> > index e4cdebb6311..aa1bedbdac6 100644
> > --- a/gdb/aarch64-tdep.h
> > +++ b/gdb/aarch64-tdep.h
> > @@ -34,6 +34,7 @@ struct regset;
> > /* AArch64 Dwarf register numbering. */
> > #define AARCH64_DWARF_X0 0
> > #define AARCH64_DWARF_SP 31
> > +#define AARCH64_DWARF_PC 32
> > #define AARCH64_DWARF_PAUTH_RA_STATE 34
> > #define AARCH64_DWARF_PAUTH_DMASK 35
> > #define AARCH64_DWARF_PAUTH_CMASK 36
> > diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> > new file mode 100644
> > index 00000000000..8589a64fd56
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> > @@ -0,0 +1,48 @@
> > +/* Copyright 2022-2022 Free Software Foundation, Inc.
>
> 2022-2022 -> 2022
>
> > +
> > + This file is part of GDB.
> > +
> > + 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/>. */
> > +
> > + .text
> > + .globl main
> > + .type main,#function
> > +main:
> > + .cfi_startproc
> > + stp x29, x30, [sp, -16]!
> > + .cfi_def_cfa sp, 16
> > + .cfi_offset x29, 0
> > + .cfi_offset x30, 8
> > + bl test_func
> > + ldp x29, x30, [sp], 16
> > + .cfi_restore x29
> > + .cfi_restore x30
> > + .cfi_def_cfa sp, 0
> > + mov x0, 0
> > + ret
> > + .cfi_endproc
> > +
> > + .globl test_func
> > +test_func:
> > + .cfi_startproc
> > + // Unwind x30 to a different value
> > + // CFA_val_expression x30 const2u 0x1234
> > + .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
> > + // CFA_val_expression pc breg30 0
> > + .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
> > + mov x0, x30
> > + .cfi_register 32, x0
> > + mov x30, 0x1234
> > + ret x0
> > + .cfi_endproc
> > diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> > new file mode 100644
> > index 00000000000..c047c7d2f1a
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> > @@ -0,0 +1,63 @@
> > +# Copyright 2022-2022 Free Software Foundation, Inc.
>
> 2022-2022 -> 2022
>
> > +
> > +# 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.
> > +
> > +# Test explicitly unwinding PC on aarch64
>
> How about...
>
> "Test explicitly unwinding the PC DWARF register on aarch64"?
>
> > +
> > +if {![is_aarch64_target]} then {
> > + verbose "Skipping arm displaced stepping tests."
> > + return
> > +}
> > +
>
> Copy/paste problem possibly? The usual way to report this is:
Ooops...
> verbose "Skipping ${gdb_test_file_name}."
OK. I'll do that.
> > +standard_testfile .S
> > +
> > +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> > + return -1
> > +}
> > +
> > +if ![runto_main] {
> > + return -1
> > +}
> > +
> > +proc test_reg_vals { i } {
> > + gdb_test "p \$pc - &main" "= 8" "p \$pc, $i"
> > + gdb_test "p \$x30" "= 4660" "p \$x30, $i"
> > +}
>
> I'd drop the argument to the proc. We can surround the proc call in a
> prefix instead. See below.
>
> > +
> > +proc test_unwind_pc { i } {
> > + gdb_test "si" "" "single step, $i"
>
> Instead of passing regardless of the output, can we validate that we
> really stepped things correctly?
This is actually the main reason I included the frame number in the
backtrace test following this. I may be able to do something with the
one stepping out of the function but here each step only print the
instruction out so there's nothing common to test AFAICT.
Maybe I can pass in the expected current instruction?
> > + gdb_test "backtrace" \
> > + ".*#1.*in main ().*" \
> > + "backtrace, $i"
> > + gdb_test "up" "" "parent frame, $i"
> > + test_reg_vals "$i"
> > +}
>
> Drop the proc argument as well and use a prefix instead. See below.
>
> > +
> > +# Ready to enter the function
> > +gdb_test "si" "bl test_func" "call site"
> > +# Step through the 3 instructions in the function to make sure that
> > +# we have the same unwind info throughout.
> > +test_unwind_pc 1
> > +test_unwind_pc 2
> > +test_unwind_pc 3
>
> You can make these like so, for example...
>
> with_test_prefix "1st stepi" {
> test_unwind_pc
> }
>
> with_test_prefix "2nd stepi" {
> test_unwind_pc
> }
>
> with_test_prefix "3rd stepi" {
> test_unwind_pc
> }
>
>
> > +# Check again after we returned
> > +gdb_test "si" "" "single step out"
>
> Same as before. Instead of passing regardless of the output, can we
> validate that we really stepped out? This would make the test more robust.
>
> > +gdb_test "backtrace" \
> > + "#0\[\t \]+main ().*" \
> > + "backtrace, 4"
> > +test_reg_vals 4
> > +
>
> Given this is the final check, do we need to number it? We could just...
>
> +with_test_prefix "final" {
> + gdb_test "backtrace" \
> + "#0\[\t \]+main ().*" \
> + "backtrace"
> + test_reg_vals
> +}
>
>
> > +gdb_continue_to_end "aarch64-unwind-pc"
>
> At this point we've already exercised what we wanted, right? Is the
> above test needed?
I was just following what the other tests did here. Is this
unnecessary? (also for one earlier version of the assembly code
continuing would crash since I trashed x30 and didn't restore it, not
sure if this line would have caught that though since I didn't
actually run that version though)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] Return the regnum for PC (32) on aarch64
2022-05-16 13:17 ` Yichao Yu
@ 2022-05-16 13:25 ` Yichao Yu
2022-05-17 6:42 ` Luis Machado
2022-05-16 13:29 ` [PATCH] " Luis Machado
1 sibling, 1 reply; 17+ messages in thread
From: Yichao Yu @ 2022-05-16 13:25 UTC (permalink / raw)
To: gdb-patches, Luis Machado
This will allow the unwind info to explicitly specify a different value
for the return address than the link register. Such usage, e.g. for signal frames,
is mentioned in aadwarf64 from ARM.
Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
---
gdb/aarch64-tdep.c | 3 +
gdb/aarch64-tdep.h | 1 +
gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 +++++++++++++
gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 72 ++++++++++++++++++++
4 files changed, 124 insertions(+)
create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 9d06ebfe27c..f9eb455232f 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
if (reg == AARCH64_DWARF_SP)
return AARCH64_SP_REGNUM;
+ if (reg == AARCH64_DWARF_PC)
+ return AARCH64_PC_REGNUM;
+
if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index e4cdebb6311..aa1bedbdac6 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -34,6 +34,7 @@ struct regset;
/* AArch64 Dwarf register numbering. */
#define AARCH64_DWARF_X0 0
#define AARCH64_DWARF_SP 31
+#define AARCH64_DWARF_PC 32
#define AARCH64_DWARF_PAUTH_RA_STATE 34
#define AARCH64_DWARF_PAUTH_DMASK 35
#define AARCH64_DWARF_PAUTH_CMASK 36
diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
new file mode 100644
index 00000000000..6cc4f80e349
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
@@ -0,0 +1,48 @@
+/* Copyright 2022 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+ .text
+ .globl main
+ .type main,#function
+main:
+ .cfi_startproc
+ stp x29, x30, [sp, -16]!
+ .cfi_def_cfa sp, 16
+ .cfi_offset x29, 0
+ .cfi_offset x30, 8
+ bl test_func
+ ldp x29, x30, [sp], 16
+ .cfi_restore x29
+ .cfi_restore x30
+ .cfi_def_cfa sp, 0
+ mov x0, 0
+ ret
+ .cfi_endproc
+
+ .globl test_func
+test_func:
+ .cfi_startproc
+ // Unwind x30 to a different value
+ // CFA_val_expression x30 const2u 0x1234
+ .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
+ // CFA_val_expression pc breg30 0
+ .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
+ mov x0, x30
+ .cfi_register 32, x0
+ mov x30, 0x1234
+ ret x0
+ .cfi_endproc
diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
new file mode 100644
index 00000000000..745c286fef5
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
@@ -0,0 +1,72 @@
+# 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 <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Test explicitly unwinding the PC DWARF register on aarch64
+
+if {![is_aarch64_target]} then {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+standard_testfile .S
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+
+proc test_reg_vals {} {
+ gdb_test "p \$pc - &main" "= 8" "p \$pc"
+ gdb_test "p \$x30" "= 4660" "p \$x30"
+}
+
+proc test_unwind_pc { inst } {
+ gdb_test "si" "$inst" "single step"
+ gdb_test "backtrace" \
+ ".*#1.*in main ().*" \
+ "backtrace"
+ gdb_test "up" "" "parent frame"
+ test_reg_vals
+}
+
+# Ready to enter the function
+gdb_test "si" "bl test_func" "call site"
+# Step through the 3 instructions in the function to make sure that
+# we have the same unwind info throughout.
+with_test_prefix "1st stepi" {
+ test_unwind_pc "mov x0, x30"
+}
+with_test_prefix "2nd stepi" {
+ test_unwind_pc "mov x30, 0x1234"
+}
+with_test_prefix "3rd stepi" {
+ test_unwind_pc "ret x0"
+}
+# Check again after we returned
+with_test_prefix "final" {
+ # Check that we've stepped out (si prints out the new function name)
+ gdb_test "si" ".*main *().*" "single step out"
+ gdb_test "backtrace" \
+ "#0\[\t \]+main ().*" \
+ "backtrace"
+ test_reg_vals
+}
+
+gdb_continue_to_end "aarch64-unwind-pc"
--
2.36.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Return the regnum for PC (32) on aarch64
2022-05-16 13:17 ` Yichao Yu
2022-05-16 13:25 ` [PATCH v2] " Yichao Yu
@ 2022-05-16 13:29 ` Luis Machado
2022-05-16 14:10 ` Yichao Yu
1 sibling, 1 reply; 17+ messages in thread
From: Luis Machado @ 2022-05-16 13:29 UTC (permalink / raw)
To: Yichao Yu; +Cc: gdb-patches
On 5/16/22 14:17, Yichao Yu wrote:
> On Mon, May 16, 2022 at 5:05 AM Luis Machado <luis.machado@arm.com> wrote:
>>
>> Hi,
>>
>> Thanks for the patch. It looks good to me. There are just some
>> adjustments/suggestions on the testcase itself.
>>
>>> This will allow the unwind info to explicitly specify a different value
>>> for the return address than the link register. Such usage, e.g. for signal frames,
>>> is mentioned in aadwarf64 from ARM.
>>>
>>> Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
>>> ---
>>> gdb/aarch64-tdep.c | 3 +
>>> gdb/aarch64-tdep.h | 1 +
>>> gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 +++++++++++++++
>>> gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 63 ++++++++++++++++++++
>>> 4 files changed, 115 insertions(+)
>>> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
>>> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
>>>
>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>>> index 9d06ebfe27c..f9eb455232f 100644
>>> --- a/gdb/aarch64-tdep.c
>>> +++ b/gdb/aarch64-tdep.c
>>> @@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
>>> if (reg == AARCH64_DWARF_SP)
>>> return AARCH64_SP_REGNUM;
>>>
>>> + if (reg == AARCH64_DWARF_PC)
>>> + return AARCH64_PC_REGNUM;
>>> +
>>> if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
>>> return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
>>>
>>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
>>> index e4cdebb6311..aa1bedbdac6 100644
>>> --- a/gdb/aarch64-tdep.h
>>> +++ b/gdb/aarch64-tdep.h
>>> @@ -34,6 +34,7 @@ struct regset;
>>> /* AArch64 Dwarf register numbering. */
>>> #define AARCH64_DWARF_X0 0
>>> #define AARCH64_DWARF_SP 31
>>> +#define AARCH64_DWARF_PC 32
>>> #define AARCH64_DWARF_PAUTH_RA_STATE 34
>>> #define AARCH64_DWARF_PAUTH_DMASK 35
>>> #define AARCH64_DWARF_PAUTH_CMASK 36
>>> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
>>> new file mode 100644
>>> index 00000000000..8589a64fd56
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
>>> @@ -0,0 +1,48 @@
>>> +/* Copyright 2022-2022 Free Software Foundation, Inc.
>>
>> 2022-2022 -> 2022
>>
>>> +
>>> + This file is part of GDB.
>>> +
>>> + 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/>. */
>>> +
>>> + .text
>>> + .globl main
>>> + .type main,#function
>>> +main:
>>> + .cfi_startproc
>>> + stp x29, x30, [sp, -16]!
>>> + .cfi_def_cfa sp, 16
>>> + .cfi_offset x29, 0
>>> + .cfi_offset x30, 8
>>> + bl test_func
>>> + ldp x29, x30, [sp], 16
>>> + .cfi_restore x29
>>> + .cfi_restore x30
>>> + .cfi_def_cfa sp, 0
>>> + mov x0, 0
>>> + ret
>>> + .cfi_endproc
>>> +
>>> + .globl test_func
>>> +test_func:
>>> + .cfi_startproc
>>> + // Unwind x30 to a different value
>>> + // CFA_val_expression x30 const2u 0x1234
>>> + .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
>>> + // CFA_val_expression pc breg30 0
>>> + .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
>>> + mov x0, x30
>>> + .cfi_register 32, x0
>>> + mov x30, 0x1234
>>> + ret x0
>>> + .cfi_endproc
>>> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
>>> new file mode 100644
>>> index 00000000000..c047c7d2f1a
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
>>> @@ -0,0 +1,63 @@
>>> +# Copyright 2022-2022 Free Software Foundation, Inc.
>>
>> 2022-2022 -> 2022
>>
>>> +
>>> +# 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.
>>> +
>>> +# Test explicitly unwinding PC on aarch64
>>
>> How about...
>>
>> "Test explicitly unwinding the PC DWARF register on aarch64"?
>>
>>> +
>>> +if {![is_aarch64_target]} then {
>>> + verbose "Skipping arm displaced stepping tests."
>>> + return
>>> +}
>>> +
>>
>> Copy/paste problem possibly? The usual way to report this is:
>
> Ooops...
>
>> verbose "Skipping ${gdb_test_file_name}."
>
> OK. I'll do that.
>
>>> +standard_testfile .S
>>> +
>>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
>>> + return -1
>>> +}
>>> +
>>> +if ![runto_main] {
>>> + return -1
>>> +}
>>> +
>>> +proc test_reg_vals { i } {
>>> + gdb_test "p \$pc - &main" "= 8" "p \$pc, $i"
>>> + gdb_test "p \$x30" "= 4660" "p \$x30, $i"
>>> +}
>>
>> I'd drop the argument to the proc. We can surround the proc call in a
>> prefix instead. See below.
>>
>>> +
>>> +proc test_unwind_pc { i } {
>>> + gdb_test "si" "" "single step, $i"
>>
>> Instead of passing regardless of the output, can we validate that we
>> really stepped things correctly?
>
> This is actually the main reason I included the frame number in the
> backtrace test following this. I may be able to do something with the
> one stepping out of the function but here each step only print the
> instruction out so there's nothing common to test AFAICT.
> Maybe I can pass in the expected current instruction?
>
Passing the expected pattern containing the instruction would work I think. That way we actually test that we stepped it correctly.
Otherwise this particular test will PASS regardless of the outcome.
>>> + gdb_test "backtrace" \
>>> + ".*#1.*in main ().*" \
>>> + "backtrace, $i"
>>> + gdb_test "up" "" "parent frame, $i"
>>> + test_reg_vals "$i"
>>> +}
>>
>> Drop the proc argument as well and use a prefix instead. See below.
>>
>>> +
>>> +# Ready to enter the function
>>> +gdb_test "si" "bl test_func" "call site"
>>> +# Step through the 3 instructions in the function to make sure that
>>> +# we have the same unwind info throughout.
>>> +test_unwind_pc 1
>>> +test_unwind_pc 2
>>> +test_unwind_pc 3
>>
>> You can make these like so, for example...
>>
>> with_test_prefix "1st stepi" {
>> test_unwind_pc
>> }
>>
>> with_test_prefix "2nd stepi" {
>> test_unwind_pc
>> }
>>
>> with_test_prefix "3rd stepi" {
>> test_unwind_pc
>> }
>>
>>
>>> +# Check again after we returned
>>> +gdb_test "si" "" "single step out"
>>
>> Same as before. Instead of passing regardless of the output, can we
>> validate that we really stepped out? This would make the test more robust.
>>
>>> +gdb_test "backtrace" \
>>> + "#0\[\t \]+main ().*" \
>>> + "backtrace, 4"
>>> +test_reg_vals 4
>>> +
>>
>> Given this is the final check, do we need to number it? We could just...
>>
>> +with_test_prefix "final" {
>> + gdb_test "backtrace" \
>> + "#0\[\t \]+main ().*" \
>> + "backtrace"
>> + test_reg_vals
>> +}
>>
>>
>>> +gdb_continue_to_end "aarch64-unwind-pc"
>>
>> At this point we've already exercised what we wanted, right? Is the
>> above test needed?
>
> I was just following what the other tests did here. Is this
> unnecessary? (also for one earlier version of the assembly code
> continuing would crash since I trashed x30 and didn't restore it, not
> sure if this line would have caught that though since I didn't
> actually run that version though)
If there is a reason we should exercise continuing until we finish the program, then it makes sense to keep this. But given this is
just some custom assembly to exercise CFI/unwinding, I think the test should stop after we've verified unwinding works.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] Return the regnum for PC (32) on aarch64
2022-05-16 13:29 ` [PATCH] " Luis Machado
@ 2022-05-16 14:10 ` Yichao Yu
0 siblings, 0 replies; 17+ messages in thread
From: Yichao Yu @ 2022-05-16 14:10 UTC (permalink / raw)
To: Luis Machado; +Cc: gdb-patches
On Mon, May 16, 2022 at 9:29 AM Luis Machado <luis.machado@arm.com> wrote:
>
> On 5/16/22 14:17, Yichao Yu wrote:
> > On Mon, May 16, 2022 at 5:05 AM Luis Machado <luis.machado@arm.com> wrote:
> >>
> >> Hi,
> >>
> >> Thanks for the patch. It looks good to me. There are just some
> >> adjustments/suggestions on the testcase itself.
> >>
> >>> This will allow the unwind info to explicitly specify a different value
> >>> for the return address than the link register. Such usage, e.g. for signal frames,
> >>> is mentioned in aadwarf64 from ARM.
> >>>
> >>> Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
> >>> ---
> >>> gdb/aarch64-tdep.c | 3 +
> >>> gdb/aarch64-tdep.h | 1 +
> >>> gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 +++++++++++++++
> >>> gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 63 ++++++++++++++++++++
> >>> 4 files changed, 115 insertions(+)
> >>> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> >>> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> >>>
> >>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> >>> index 9d06ebfe27c..f9eb455232f 100644
> >>> --- a/gdb/aarch64-tdep.c
> >>> +++ b/gdb/aarch64-tdep.c
> >>> @@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
> >>> if (reg == AARCH64_DWARF_SP)
> >>> return AARCH64_SP_REGNUM;
> >>>
> >>> + if (reg == AARCH64_DWARF_PC)
> >>> + return AARCH64_PC_REGNUM;
> >>> +
> >>> if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
> >>> return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
> >>>
> >>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> >>> index e4cdebb6311..aa1bedbdac6 100644
> >>> --- a/gdb/aarch64-tdep.h
> >>> +++ b/gdb/aarch64-tdep.h
> >>> @@ -34,6 +34,7 @@ struct regset;
> >>> /* AArch64 Dwarf register numbering. */
> >>> #define AARCH64_DWARF_X0 0
> >>> #define AARCH64_DWARF_SP 31
> >>> +#define AARCH64_DWARF_PC 32
> >>> #define AARCH64_DWARF_PAUTH_RA_STATE 34
> >>> #define AARCH64_DWARF_PAUTH_DMASK 35
> >>> #define AARCH64_DWARF_PAUTH_CMASK 36
> >>> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> >>> new file mode 100644
> >>> index 00000000000..8589a64fd56
> >>> --- /dev/null
> >>> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> >>> @@ -0,0 +1,48 @@
> >>> +/* Copyright 2022-2022 Free Software Foundation, Inc.
> >>
> >> 2022-2022 -> 2022
> >>
> >>> +
> >>> + This file is part of GDB.
> >>> +
> >>> + 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/>. */
> >>> +
> >>> + .text
> >>> + .globl main
> >>> + .type main,#function
> >>> +main:
> >>> + .cfi_startproc
> >>> + stp x29, x30, [sp, -16]!
> >>> + .cfi_def_cfa sp, 16
> >>> + .cfi_offset x29, 0
> >>> + .cfi_offset x30, 8
> >>> + bl test_func
> >>> + ldp x29, x30, [sp], 16
> >>> + .cfi_restore x29
> >>> + .cfi_restore x30
> >>> + .cfi_def_cfa sp, 0
> >>> + mov x0, 0
> >>> + ret
> >>> + .cfi_endproc
> >>> +
> >>> + .globl test_func
> >>> +test_func:
> >>> + .cfi_startproc
> >>> + // Unwind x30 to a different value
> >>> + // CFA_val_expression x30 const2u 0x1234
> >>> + .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
> >>> + // CFA_val_expression pc breg30 0
> >>> + .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
> >>> + mov x0, x30
> >>> + .cfi_register 32, x0
> >>> + mov x30, 0x1234
> >>> + ret x0
> >>> + .cfi_endproc
> >>> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> >>> new file mode 100644
> >>> index 00000000000..c047c7d2f1a
> >>> --- /dev/null
> >>> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> >>> @@ -0,0 +1,63 @@
> >>> +# Copyright 2022-2022 Free Software Foundation, Inc.
> >>
> >> 2022-2022 -> 2022
> >>
> >>> +
> >>> +# 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.
> >>> +
> >>> +# Test explicitly unwinding PC on aarch64
> >>
> >> How about...
> >>
> >> "Test explicitly unwinding the PC DWARF register on aarch64"?
> >>
> >>> +
> >>> +if {![is_aarch64_target]} then {
> >>> + verbose "Skipping arm displaced stepping tests."
> >>> + return
> >>> +}
> >>> +
> >>
> >> Copy/paste problem possibly? The usual way to report this is:
> >
> > Ooops...
> >
> >> verbose "Skipping ${gdb_test_file_name}."
> >
> > OK. I'll do that.
> >
> >>> +standard_testfile .S
> >>> +
> >>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> >>> + return -1
> >>> +}
> >>> +
> >>> +if ![runto_main] {
> >>> + return -1
> >>> +}
> >>> +
> >>> +proc test_reg_vals { i } {
> >>> + gdb_test "p \$pc - &main" "= 8" "p \$pc, $i"
> >>> + gdb_test "p \$x30" "= 4660" "p \$x30, $i"
> >>> +}
> >>
> >> I'd drop the argument to the proc. We can surround the proc call in a
> >> prefix instead. See below.
> >>
> >>> +
> >>> +proc test_unwind_pc { i } {
> >>> + gdb_test "si" "" "single step, $i"
> >>
> >> Instead of passing regardless of the output, can we validate that we
> >> really stepped things correctly?
> >
> > This is actually the main reason I included the frame number in the
> > backtrace test following this. I may be able to do something with the
> > one stepping out of the function but here each step only print the
> > instruction out so there's nothing common to test AFAICT.
> > Maybe I can pass in the expected current instruction?
> >
>
> Passing the expected pattern containing the instruction would work I think. That way we actually test that we stepped it correctly.
>
> Otherwise this particular test will PASS regardless of the outcome.
Did that in v2.
>
> >>> + gdb_test "backtrace" \
> >>> + ".*#1.*in main ().*" \
> >>> + "backtrace, $i"
> >>> + gdb_test "up" "" "parent frame, $i"
> >>> + test_reg_vals "$i"
> >>> +}
> >>
> >> Drop the proc argument as well and use a prefix instead. See below.
> >>
> >>> +
> >>> +# Ready to enter the function
> >>> +gdb_test "si" "bl test_func" "call site"
> >>> +# Step through the 3 instructions in the function to make sure that
> >>> +# we have the same unwind info throughout.
> >>> +test_unwind_pc 1
> >>> +test_unwind_pc 2
> >>> +test_unwind_pc 3
> >>
> >> You can make these like so, for example...
> >>
> >> with_test_prefix "1st stepi" {
> >> test_unwind_pc
> >> }
> >>
> >> with_test_prefix "2nd stepi" {
> >> test_unwind_pc
> >> }
> >>
> >> with_test_prefix "3rd stepi" {
> >> test_unwind_pc
> >> }
> >>
> >>
> >>> +# Check again after we returned
> >>> +gdb_test "si" "" "single step out"
> >>
> >> Same as before. Instead of passing regardless of the output, can we
> >> validate that we really stepped out? This would make the test more robust.
> >>
> >>> +gdb_test "backtrace" \
> >>> + "#0\[\t \]+main ().*" \
> >>> + "backtrace, 4"
> >>> +test_reg_vals 4
> >>> +
> >>
> >> Given this is the final check, do we need to number it? We could just...
> >>
> >> +with_test_prefix "final" {
> >> + gdb_test "backtrace" \
> >> + "#0\[\t \]+main ().*" \
> >> + "backtrace"
> >> + test_reg_vals
> >> +}
> >>
> >>
> >>> +gdb_continue_to_end "aarch64-unwind-pc"
> >>
> >> At this point we've already exercised what we wanted, right? Is the
> >> above test needed?
> >
> > I was just following what the other tests did here. Is this
> > unnecessary? (also for one earlier version of the assembly code
> > continuing would crash since I trashed x30 and didn't restore it, not
> > sure if this line would have caught that though since I didn't
> > actually run that version though)
>
> If there is a reason we should exercise continuing until we finish the program, then it makes sense to keep this. But given this is
> just some custom assembly to exercise CFI/unwinding, I think the test should stop after we've verified unwinding works.
I kept this in v2, I can get rid of it in v3 if necessary.
^ permalink raw reply [flat|nested] 17+ messages in thread
* re: [PATCH v2] Return the regnum for PC (32) on aarch64
2022-05-16 13:25 ` [PATCH v2] " Yichao Yu
@ 2022-05-17 6:42 ` Luis Machado
2022-05-17 14:41 ` [PATCH v3] " Yichao Yu
2022-05-17 17:34 ` [PATCH v2] " Yichao Yu
0 siblings, 2 replies; 17+ messages in thread
From: Luis Machado @ 2022-05-17 6:42 UTC (permalink / raw)
To: gdb-patches, Yichao Yu
Hi,
> This will allow the unwind info to explicitly specify a different value
> for the return address than the link register. Such usage, e.g. for signal frames,
> is mentioned in aadwarf64 from ARM.
I'd complement the description saying that although this is uncommon, it is a valid case.
>
> Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
> ---
> gdb/aarch64-tdep.c | 3 +
> gdb/aarch64-tdep.h | 1 +
> gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 +++++++++++++
> gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 72 ++++++++++++++++++++
> 4 files changed, 124 insertions(+)
> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 9d06ebfe27c..f9eb455232f 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
> if (reg == AARCH64_DWARF_SP)
> return AARCH64_SP_REGNUM;
>
> + if (reg == AARCH64_DWARF_PC)
> + return AARCH64_PC_REGNUM;
> +
> if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
> return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
>
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index e4cdebb6311..aa1bedbdac6 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -34,6 +34,7 @@ struct regset;
> /* AArch64 Dwarf register numbering. */
> #define AARCH64_DWARF_X0 0
> #define AARCH64_DWARF_SP 31
> +#define AARCH64_DWARF_PC 32
> #define AARCH64_DWARF_PAUTH_RA_STATE 34
> #define AARCH64_DWARF_PAUTH_DMASK 35
> #define AARCH64_DWARF_PAUTH_CMASK 36
> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> new file mode 100644
> index 00000000000..6cc4f80e349
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> @@ -0,0 +1,48 @@
> +/* Copyright 2022 Free Software Foundation, Inc.
> +
> + This file is part of GDB.
> +
> + 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/>. */
> +
> + .text
> + .globl main
> + .type main,#function
> +main:
> + .cfi_startproc
> + stp x29, x30, [sp, -16]!
> + .cfi_def_cfa sp, 16
> + .cfi_offset x29, 0
> + .cfi_offset x30, 8
> + bl test_func
> + ldp x29, x30, [sp], 16
> + .cfi_restore x29
> + .cfi_restore x30
> + .cfi_def_cfa sp, 0
> + mov x0, 0
> + ret
> + .cfi_endproc
> +
> + .globl test_func
> +test_func:
> + .cfi_startproc
> + // Unwind x30 to a different value
> + // CFA_val_expression x30 const2u 0x1234
> + .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
> + // CFA_val_expression pc breg30 0
> + .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
> + mov x0, x30
> + .cfi_register 32, x0
> + mov x30, 0x1234
> + ret x0
> + .cfi_endproc
> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> new file mode 100644
> index 00000000000..745c286fef5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> @@ -0,0 +1,72 @@
> +# 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 <http://www.gnu.org/licenses/>.
> +
> +# This file is part of the gdb testsuite.
> +
> +# Test explicitly unwinding the PC DWARF register on aarch64
> +
> +if {![is_aarch64_target]} then {
> + verbose "Skipping ${gdb_test_file_name}."
> + return
> +}
> +
> +standard_testfile .S
> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> + return -1
> +}
> +
> +if ![runto_main] {
> + return -1
> +}
> +
> +proc test_reg_vals {} {
> + gdb_test "p \$pc - &main" "= 8" "p \$pc"
> + gdb_test "p \$x30" "= 4660" "p \$x30"
A suggestion is to use p/x to print x30, that way you don't have to deal with the integer value. It would print 0x1234.
> +}
> +
> +proc test_unwind_pc { inst } {
> + gdb_test "si" "$inst" "single step"
> + gdb_test "backtrace" \
> + ".*#1.*in main ().*" \
> + "backtrace"
> + gdb_test "up" "" "parent frame"
I noticed the above test also PASSes regardless of the output as well. Could you please address that?
> + test_reg_vals
> +}
> +
> +# Ready to enter the function
> +gdb_test "si" "bl test_func" "call site"
> +# Step through the 3 instructions in the function to make sure that
> +# we have the same unwind info throughout.
> +with_test_prefix "1st stepi" {
> + test_unwind_pc "mov x0, x30"
> +}
> +with_test_prefix "2nd stepi" {
> + test_unwind_pc "mov x30, 0x1234"
> +}
> +with_test_prefix "3rd stepi" {
> + test_unwind_pc "ret x0"
> +}
> +# Check again after we returned
> +with_test_prefix "final" {
> + # Check that we've stepped out (si prints out the new function name)
> + gdb_test "si" ".*main *().*" "single step out"
> + gdb_test "backtrace" \
> + "#0\[\t \]+main ().*" \
> + "backtrace"
> + test_reg_vals
> +}
> +
> +gdb_continue_to_end "aarch64-unwind-pc
I think v3 will be good to go.
Thanks!
Luis
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3] Return the regnum for PC (32) on aarch64
2022-05-17 6:42 ` Luis Machado
@ 2022-05-17 14:41 ` Yichao Yu
2022-05-17 17:34 ` [PATCH v2] " Yichao Yu
1 sibling, 0 replies; 17+ messages in thread
From: Yichao Yu @ 2022-05-17 14:41 UTC (permalink / raw)
To: gdb-patches, Luis Machado
This will allow the unwind info to explicitly specify a different value
for the return address from the link register.
Such usage, although uncommon, is valid and useful for signal frames.
It is also endorsed by aadwarf64 from ARM (Note 9 in [1]).
Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
[1] https://github.com/ARM-software/abi-aa/blob/2022Q1/aadwarf64/aadwarf64.rst#dwarf-register-names
---
gdb/aarch64-tdep.c | 3 +
gdb/aarch64-tdep.h | 1 +
gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 ++++++++++++++
gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 70 ++++++++++++++++++++
4 files changed, 122 insertions(+)
create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 9d06ebfe27c..f9eb455232f 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
if (reg == AARCH64_DWARF_SP)
return AARCH64_SP_REGNUM;
+ if (reg == AARCH64_DWARF_PC)
+ return AARCH64_PC_REGNUM;
+
if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index e4cdebb6311..aa1bedbdac6 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -34,6 +34,7 @@ struct regset;
/* AArch64 Dwarf register numbering. */
#define AARCH64_DWARF_X0 0
#define AARCH64_DWARF_SP 31
+#define AARCH64_DWARF_PC 32
#define AARCH64_DWARF_PAUTH_RA_STATE 34
#define AARCH64_DWARF_PAUTH_DMASK 35
#define AARCH64_DWARF_PAUTH_CMASK 36
diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
new file mode 100644
index 00000000000..6cc4f80e349
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
@@ -0,0 +1,48 @@
+/* Copyright 2022 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+ .text
+ .globl main
+ .type main,#function
+main:
+ .cfi_startproc
+ stp x29, x30, [sp, -16]!
+ .cfi_def_cfa sp, 16
+ .cfi_offset x29, 0
+ .cfi_offset x30, 8
+ bl test_func
+ ldp x29, x30, [sp], 16
+ .cfi_restore x29
+ .cfi_restore x30
+ .cfi_def_cfa sp, 0
+ mov x0, 0
+ ret
+ .cfi_endproc
+
+ .globl test_func
+test_func:
+ .cfi_startproc
+ // Unwind x30 to a different value
+ // CFA_val_expression x30 const2u 0x1234
+ .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
+ // CFA_val_expression pc breg30 0
+ .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
+ mov x0, x30
+ .cfi_register 32, x0
+ mov x30, 0x1234
+ ret x0
+ .cfi_endproc
diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
new file mode 100644
index 00000000000..105b9a9cc3a
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
@@ -0,0 +1,70 @@
+# 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 <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Test explicitly unwinding the PC DWARF register on aarch64
+
+if {![is_aarch64_target]} then {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+standard_testfile .S
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+
+proc test_reg_vals {} {
+ gdb_test "p \$pc - &main" "= 8" "p \$pc"
+ gdb_test "p/x \$x30" "= 0x1234" "p \$x30"
+}
+
+proc test_unwind_pc { inst } {
+ gdb_test "si" "$inst" "single step"
+ gdb_test "backtrace" \
+ ".*#1.*in main ().*" \
+ "backtrace"
+ gdb_test "up" "in main ().*" "parent frame"
+ test_reg_vals
+}
+
+# Ready to enter the function
+gdb_test "si" "bl test_func" "call site"
+# Step through the 3 instructions in the function to make sure that
+# we have the same unwind info throughout.
+with_test_prefix "1st stepi" {
+ test_unwind_pc "mov x0, x30"
+}
+with_test_prefix "2nd stepi" {
+ test_unwind_pc "mov x30, 0x1234"
+}
+with_test_prefix "3rd stepi" {
+ test_unwind_pc "ret x0"
+}
+# Check again after we returned
+with_test_prefix "final" {
+ # Check that we've stepped out (si prints out the new function name)
+ gdb_test "si" ".*main *().*" "single step out"
+ gdb_test "backtrace" \
+ "#0\[\t \]+main ().*" \
+ "backtrace"
+ test_reg_vals
+}
--
2.36.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] Return the regnum for PC (32) on aarch64
2022-05-17 6:42 ` Luis Machado
2022-05-17 14:41 ` [PATCH v3] " Yichao Yu
@ 2022-05-17 17:34 ` Yichao Yu
2022-05-17 17:36 ` [PATCH v3] " Yichao Yu
1 sibling, 1 reply; 17+ messages in thread
From: Yichao Yu @ 2022-05-17 17:34 UTC (permalink / raw)
To: Luis Machado; +Cc: gdb-patches
On Tue, May 17, 2022 at 2:42 AM Luis Machado <luis.machado@arm.com> wrote:
>
> Hi,
>
> > This will allow the unwind info to explicitly specify a different value
> > for the return address than the link register. Such usage, e.g. for signal frames,
> > is mentioned in aadwarf64 from ARM.
>
> I'd complement the description saying that although this is uncommon, it is a valid case.
>
> >
> > Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
> > ---
> > gdb/aarch64-tdep.c | 3 +
> > gdb/aarch64-tdep.h | 1 +
> > gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 +++++++++++++
> > gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 72 ++++++++++++++++++++
> > 4 files changed, 124 insertions(+)
> > create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> > create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> >
> > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> > index 9d06ebfe27c..f9eb455232f 100644
> > --- a/gdb/aarch64-tdep.c
> > +++ b/gdb/aarch64-tdep.c
> > @@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
> > if (reg == AARCH64_DWARF_SP)
> > return AARCH64_SP_REGNUM;
> >
> > + if (reg == AARCH64_DWARF_PC)
> > + return AARCH64_PC_REGNUM;
> > +
> > if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
> > return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
> >
> > diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> > index e4cdebb6311..aa1bedbdac6 100644
> > --- a/gdb/aarch64-tdep.h
> > +++ b/gdb/aarch64-tdep.h
> > @@ -34,6 +34,7 @@ struct regset;
> > /* AArch64 Dwarf register numbering. */
> > #define AARCH64_DWARF_X0 0
> > #define AARCH64_DWARF_SP 31
> > +#define AARCH64_DWARF_PC 32
> > #define AARCH64_DWARF_PAUTH_RA_STATE 34
> > #define AARCH64_DWARF_PAUTH_DMASK 35
> > #define AARCH64_DWARF_PAUTH_CMASK 36
> > diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> > new file mode 100644
> > index 00000000000..6cc4f80e349
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> > @@ -0,0 +1,48 @@
> > +/* Copyright 2022 Free Software Foundation, Inc.
> > +
> > + This file is part of GDB.
> > +
> > + 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/>. */
> > +
> > + .text
> > + .globl main
> > + .type main,#function
> > +main:
> > + .cfi_startproc
> > + stp x29, x30, [sp, -16]!
> > + .cfi_def_cfa sp, 16
> > + .cfi_offset x29, 0
> > + .cfi_offset x30, 8
> > + bl test_func
> > + ldp x29, x30, [sp], 16
> > + .cfi_restore x29
> > + .cfi_restore x30
> > + .cfi_def_cfa sp, 0
> > + mov x0, 0
> > + ret
> > + .cfi_endproc
> > +
> > + .globl test_func
> > +test_func:
> > + .cfi_startproc
> > + // Unwind x30 to a different value
> > + // CFA_val_expression x30 const2u 0x1234
> > + .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
> > + // CFA_val_expression pc breg30 0
> > + .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
> > + mov x0, x30
> > + .cfi_register 32, x0
> > + mov x30, 0x1234
> > + ret x0
> > + .cfi_endproc
> > diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> > new file mode 100644
> > index 00000000000..745c286fef5
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> > @@ -0,0 +1,72 @@
> > +# 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 <http://www.gnu.org/licenses/>.
> > +
> > +# This file is part of the gdb testsuite.
> > +
> > +# Test explicitly unwinding the PC DWARF register on aarch64
> > +
> > +if {![is_aarch64_target]} then {
> > + verbose "Skipping ${gdb_test_file_name}."
> > + return
> > +}
> > +
> > +standard_testfile .S
> > +
> > +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> > + return -1
> > +}
> > +
> > +if ![runto_main] {
> > + return -1
> > +}
> > +
> > +proc test_reg_vals {} {
> > + gdb_test "p \$pc - &main" "= 8" "p \$pc"
> > + gdb_test "p \$x30" "= 4660" "p \$x30"
>
> A suggestion is to use p/x to print x30, that way you don't have to deal with the integer value. It would print 0x1234.
>
> > +}
> > +
> > +proc test_unwind_pc { inst } {
> > + gdb_test "si" "$inst" "single step"
> > + gdb_test "backtrace" \
> > + ".*#1.*in main ().*" \
> > + "backtrace"
> > + gdb_test "up" "" "parent frame"
>
> I noticed the above test also PASSes regardless of the output as well. Could you please address that?
>
> > + test_reg_vals
> > +}
> > +
> > +# Ready to enter the function
> > +gdb_test "si" "bl test_func" "call site"
> > +# Step through the 3 instructions in the function to make sure that
> > +# we have the same unwind info throughout.
> > +with_test_prefix "1st stepi" {
> > + test_unwind_pc "mov x0, x30"
> > +}
> > +with_test_prefix "2nd stepi" {
> > + test_unwind_pc "mov x30, 0x1234"
> > +}
> > +with_test_prefix "3rd stepi" {
> > + test_unwind_pc "ret x0"
> > +}
> > +# Check again after we returned
> > +with_test_prefix "final" {
> > + # Check that we've stepped out (si prints out the new function name)
> > + gdb_test "si" ".*main *().*" "single step out"
> > + gdb_test "backtrace" \
> > + "#0\[\t \]+main ().*" \
> > + "backtrace"
> > + test_reg_vals
> > +}
> > +
> > +gdb_continue_to_end "aarch64-unwind-pc
>
> I think v3 will be good to go.
Hmmm, I thought I sent out v3 ~3 hours ago but I don't have that in
either my inbox or in the archive. Let me try again .......
>
> Thanks!
> Luis
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3] Return the regnum for PC (32) on aarch64
2022-05-17 17:34 ` [PATCH v2] " Yichao Yu
@ 2022-05-17 17:36 ` Yichao Yu
2022-05-18 1:03 ` Yichao Yu
0 siblings, 1 reply; 17+ messages in thread
From: Yichao Yu @ 2022-05-17 17:36 UTC (permalink / raw)
To: gdb-patches, Luis Machado
This will allow the unwind info to explicitly specify a different value
for the return address from the link register.
Such usage, although uncommon, is valid and useful for signal frames.
It is also supported by aadwarf64 from ARM (Note 9 in [1]).
Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
[1] https://github.com/ARM-software/abi-aa/blob/2022Q1/aadwarf64/aadwarf64.rst#dwarf-register-names
---
gdb/aarch64-tdep.c | 3 +
gdb/aarch64-tdep.h | 1 +
gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 ++++++++++++++
gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 70 ++++++++++++++++++++
4 files changed, 122 insertions(+)
create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 9d06ebfe27c..f9eb455232f 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
if (reg == AARCH64_DWARF_SP)
return AARCH64_SP_REGNUM;
+ if (reg == AARCH64_DWARF_PC)
+ return AARCH64_PC_REGNUM;
+
if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index e4cdebb6311..aa1bedbdac6 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -34,6 +34,7 @@ struct regset;
/* AArch64 Dwarf register numbering. */
#define AARCH64_DWARF_X0 0
#define AARCH64_DWARF_SP 31
+#define AARCH64_DWARF_PC 32
#define AARCH64_DWARF_PAUTH_RA_STATE 34
#define AARCH64_DWARF_PAUTH_DMASK 35
#define AARCH64_DWARF_PAUTH_CMASK 36
diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
new file mode 100644
index 00000000000..6cc4f80e349
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
@@ -0,0 +1,48 @@
+/* Copyright 2022 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+ .text
+ .globl main
+ .type main,#function
+main:
+ .cfi_startproc
+ stp x29, x30, [sp, -16]!
+ .cfi_def_cfa sp, 16
+ .cfi_offset x29, 0
+ .cfi_offset x30, 8
+ bl test_func
+ ldp x29, x30, [sp], 16
+ .cfi_restore x29
+ .cfi_restore x30
+ .cfi_def_cfa sp, 0
+ mov x0, 0
+ ret
+ .cfi_endproc
+
+ .globl test_func
+test_func:
+ .cfi_startproc
+ // Unwind x30 to a different value
+ // CFA_val_expression x30 const2u 0x1234
+ .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
+ // CFA_val_expression pc breg30 0
+ .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
+ mov x0, x30
+ .cfi_register 32, x0
+ mov x30, 0x1234
+ ret x0
+ .cfi_endproc
diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
new file mode 100644
index 00000000000..105b9a9cc3a
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
@@ -0,0 +1,70 @@
+# 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 <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Test explicitly unwinding the PC DWARF register on aarch64
+
+if {![is_aarch64_target]} then {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+standard_testfile .S
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+
+proc test_reg_vals {} {
+ gdb_test "p \$pc - &main" "= 8" "p \$pc"
+ gdb_test "p/x \$x30" "= 0x1234" "p \$x30"
+}
+
+proc test_unwind_pc { inst } {
+ gdb_test "si" "$inst" "single step"
+ gdb_test "backtrace" \
+ ".*#1.*in main ().*" \
+ "backtrace"
+ gdb_test "up" "in main ().*" "parent frame"
+ test_reg_vals
+}
+
+# Ready to enter the function
+gdb_test "si" "bl test_func" "call site"
+# Step through the 3 instructions in the function to make sure that
+# we have the same unwind info throughout.
+with_test_prefix "1st stepi" {
+ test_unwind_pc "mov x0, x30"
+}
+with_test_prefix "2nd stepi" {
+ test_unwind_pc "mov x30, 0x1234"
+}
+with_test_prefix "3rd stepi" {
+ test_unwind_pc "ret x0"
+}
+# Check again after we returned
+with_test_prefix "final" {
+ # Check that we've stepped out (si prints out the new function name)
+ gdb_test "si" ".*main *().*" "single step out"
+ gdb_test "backtrace" \
+ "#0\[\t \]+main ().*" \
+ "backtrace"
+ test_reg_vals
+}
--
2.36.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] Return the regnum for PC (32) on aarch64
2022-05-17 17:36 ` [PATCH v3] " Yichao Yu
@ 2022-05-18 1:03 ` Yichao Yu
2022-05-18 9:53 ` Luis Machado
0 siblings, 1 reply; 17+ messages in thread
From: Yichao Yu @ 2022-05-18 1:03 UTC (permalink / raw)
To: gdb-patches, Luis Machado
Sorry for the duplicated post.... I think somehow the university
network prevented the email from going out earlier today...
On Tue, May 17, 2022 at 8:45 PM Yichao Yu <yyc1992@gmail.com> wrote:
>
> This will allow the unwind info to explicitly specify a different value
> for the return address from the link register.
> Such usage, although uncommon, is valid and useful for signal frames.
> It is also supported by aadwarf64 from ARM (Note 9 in [1]).
>
> Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
>
> [1] https://github.com/ARM-software/abi-aa/blob/2022Q1/aadwarf64/aadwarf64.rst#dwarf-register-names
> ---
> gdb/aarch64-tdep.c | 3 +
> gdb/aarch64-tdep.h | 1 +
> gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 ++++++++++++++
> gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 70 ++++++++++++++++++++
> 4 files changed, 122 insertions(+)
> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 9d06ebfe27c..f9eb455232f 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
> if (reg == AARCH64_DWARF_SP)
> return AARCH64_SP_REGNUM;
>
> + if (reg == AARCH64_DWARF_PC)
> + return AARCH64_PC_REGNUM;
> +
> if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
> return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
>
> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> index e4cdebb6311..aa1bedbdac6 100644
> --- a/gdb/aarch64-tdep.h
> +++ b/gdb/aarch64-tdep.h
> @@ -34,6 +34,7 @@ struct regset;
> /* AArch64 Dwarf register numbering. */
> #define AARCH64_DWARF_X0 0
> #define AARCH64_DWARF_SP 31
> +#define AARCH64_DWARF_PC 32
> #define AARCH64_DWARF_PAUTH_RA_STATE 34
> #define AARCH64_DWARF_PAUTH_DMASK 35
> #define AARCH64_DWARF_PAUTH_CMASK 36
> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> new file mode 100644
> index 00000000000..6cc4f80e349
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> @@ -0,0 +1,48 @@
> +/* Copyright 2022 Free Software Foundation, Inc.
> +
> + This file is part of GDB.
> +
> + 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/>. */
> +
> + .text
> + .globl main
> + .type main,#function
> +main:
> + .cfi_startproc
> + stp x29, x30, [sp, -16]!
> + .cfi_def_cfa sp, 16
> + .cfi_offset x29, 0
> + .cfi_offset x30, 8
> + bl test_func
> + ldp x29, x30, [sp], 16
> + .cfi_restore x29
> + .cfi_restore x30
> + .cfi_def_cfa sp, 0
> + mov x0, 0
> + ret
> + .cfi_endproc
> +
> + .globl test_func
> +test_func:
> + .cfi_startproc
> + // Unwind x30 to a different value
> + // CFA_val_expression x30 const2u 0x1234
> + .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
> + // CFA_val_expression pc breg30 0
> + .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
> + mov x0, x30
> + .cfi_register 32, x0
> + mov x30, 0x1234
> + ret x0
> + .cfi_endproc
> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> new file mode 100644
> index 00000000000..105b9a9cc3a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> @@ -0,0 +1,70 @@
> +# 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 <http://www.gnu.org/licenses/>.
> +
> +# This file is part of the gdb testsuite.
> +
> +# Test explicitly unwinding the PC DWARF register on aarch64
> +
> +if {![is_aarch64_target]} then {
> + verbose "Skipping ${gdb_test_file_name}."
> + return
> +}
> +
> +standard_testfile .S
> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> + return -1
> +}
> +
> +if ![runto_main] {
> + return -1
> +}
> +
> +proc test_reg_vals {} {
> + gdb_test "p \$pc - &main" "= 8" "p \$pc"
> + gdb_test "p/x \$x30" "= 0x1234" "p \$x30"
> +}
> +
> +proc test_unwind_pc { inst } {
> + gdb_test "si" "$inst" "single step"
> + gdb_test "backtrace" \
> + ".*#1.*in main ().*" \
> + "backtrace"
> + gdb_test "up" "in main ().*" "parent frame"
> + test_reg_vals
> +}
> +
> +# Ready to enter the function
> +gdb_test "si" "bl test_func" "call site"
> +# Step through the 3 instructions in the function to make sure that
> +# we have the same unwind info throughout.
> +with_test_prefix "1st stepi" {
> + test_unwind_pc "mov x0, x30"
> +}
> +with_test_prefix "2nd stepi" {
> + test_unwind_pc "mov x30, 0x1234"
> +}
> +with_test_prefix "3rd stepi" {
> + test_unwind_pc "ret x0"
> +}
> +# Check again after we returned
> +with_test_prefix "final" {
> + # Check that we've stepped out (si prints out the new function name)
> + gdb_test "si" ".*main *().*" "single step out"
> + gdb_test "backtrace" \
> + "#0\[\t \]+main ().*" \
> + "backtrace"
> + test_reg_vals
> +}
> --
> 2.36.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] Return the regnum for PC (32) on aarch64
2022-05-18 1:03 ` Yichao Yu
@ 2022-05-18 9:53 ` Luis Machado
2022-05-18 13:01 ` Yichao Yu
0 siblings, 1 reply; 17+ messages in thread
From: Luis Machado @ 2022-05-18 9:53 UTC (permalink / raw)
To: Yichao Yu, gdb-patches
On 5/18/22 02:03, Yichao Yu wrote:
> Sorry for the duplicated post.... I think somehow the university
> network prevented the email from going out earlier today...
No problem. I've been having some inbound e-mail issues as well.
>
> On Tue, May 17, 2022 at 8:45 PM Yichao Yu <yyc1992@gmail.com> wrote:
>>
>> This will allow the unwind info to explicitly specify a different value
>> for the return address from the link register.
>> Such usage, although uncommon, is valid and useful for signal frames.
>> It is also supported by aadwarf64 from ARM (Note 9 in [1]).
>>
>> Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
>>
>> [1] https://github.com/ARM-software/abi-aa/blob/2022Q1/aadwarf64/aadwarf64.rst#dwarf-register-names
>> ---
>> gdb/aarch64-tdep.c | 3 +
>> gdb/aarch64-tdep.h | 1 +
>> gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 ++++++++++++++
>> gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 70 ++++++++++++++++++++
>> 4 files changed, 122 insertions(+)
>> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
>> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
>>
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index 9d06ebfe27c..f9eb455232f 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
>> if (reg == AARCH64_DWARF_SP)
>> return AARCH64_SP_REGNUM;
>>
>> + if (reg == AARCH64_DWARF_PC)
>> + return AARCH64_PC_REGNUM;
>> +
>> if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
>> return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
>>
>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
>> index e4cdebb6311..aa1bedbdac6 100644
>> --- a/gdb/aarch64-tdep.h
>> +++ b/gdb/aarch64-tdep.h
>> @@ -34,6 +34,7 @@ struct regset;
>> /* AArch64 Dwarf register numbering. */
>> #define AARCH64_DWARF_X0 0
>> #define AARCH64_DWARF_SP 31
>> +#define AARCH64_DWARF_PC 32
>> #define AARCH64_DWARF_PAUTH_RA_STATE 34
>> #define AARCH64_DWARF_PAUTH_DMASK 35
>> #define AARCH64_DWARF_PAUTH_CMASK 36
>> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
>> new file mode 100644
>> index 00000000000..6cc4f80e349
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
>> @@ -0,0 +1,48 @@
>> +/* Copyright 2022 Free Software Foundation, Inc.
>> +
>> + This file is part of GDB.
>> +
>> + 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/>. */
>> +
>> + .text
>> + .globl main
>> + .type main,#function
>> +main:
>> + .cfi_startproc
>> + stp x29, x30, [sp, -16]!
>> + .cfi_def_cfa sp, 16
>> + .cfi_offset x29, 0
>> + .cfi_offset x30, 8
>> + bl test_func
>> + ldp x29, x30, [sp], 16
>> + .cfi_restore x29
>> + .cfi_restore x30
>> + .cfi_def_cfa sp, 0
>> + mov x0, 0
>> + ret
>> + .cfi_endproc
>> +
>> + .globl test_func
>> +test_func:
>> + .cfi_startproc
>> + // Unwind x30 to a different value
>> + // CFA_val_expression x30 const2u 0x1234
>> + .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
>> + // CFA_val_expression pc breg30 0
>> + .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
>> + mov x0, x30
>> + .cfi_register 32, x0
>> + mov x30, 0x1234
>> + ret x0
>> + .cfi_endproc
>> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
>> new file mode 100644
>> index 00000000000..105b9a9cc3a
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
>> @@ -0,0 +1,70 @@
>> +# 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 <http://www.gnu.org/licenses/>.
>> +
>> +# This file is part of the gdb testsuite.
>> +
>> +# Test explicitly unwinding the PC DWARF register on aarch64
>> +
>> +if {![is_aarch64_target]} then {
>> + verbose "Skipping ${gdb_test_file_name}."
>> + return
>> +}
>> +
>> +standard_testfile .S
>> +
>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
>> + return -1
>> +}
>> +
>> +if ![runto_main] {
>> + return -1
>> +}
>> +
>> +proc test_reg_vals {} {
>> + gdb_test "p \$pc - &main" "= 8" "p \$pc"
>> + gdb_test "p/x \$x30" "= 0x1234" "p \$x30"
>> +}
>> +
>> +proc test_unwind_pc { inst } {
>> + gdb_test "si" "$inst" "single step"
>> + gdb_test "backtrace" \
>> + ".*#1.*in main ().*" \
>> + "backtrace"
>> + gdb_test "up" "in main ().*" "parent frame"
>> + test_reg_vals
>> +}
>> +
>> +# Ready to enter the function
>> +gdb_test "si" "bl test_func" "call site"
>> +# Step through the 3 instructions in the function to make sure that
>> +# we have the same unwind info throughout.
>> +with_test_prefix "1st stepi" {
>> + test_unwind_pc "mov x0, x30"
>> +}
>> +with_test_prefix "2nd stepi" {
>> + test_unwind_pc "mov x30, 0x1234"
>> +}
>> +with_test_prefix "3rd stepi" {
>> + test_unwind_pc "ret x0"
>> +}
>> +# Check again after we returned
>> +with_test_prefix "final" {
>> + # Check that we've stepped out (si prints out the new function name)
>> + gdb_test "si" ".*main *().*" "single step out"
>> + gdb_test "backtrace" \
>> + "#0\[\t \]+main ().*" \
>> + "backtrace"
>> + test_reg_vals
>> +}
>> --
>> 2.36.1
>>
Thanks. This LGTM for aarch64. Please push it.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] Return the regnum for PC (32) on aarch64
2022-05-18 9:53 ` Luis Machado
@ 2022-05-18 13:01 ` Yichao Yu
2022-05-18 13:36 ` Luis Machado
0 siblings, 1 reply; 17+ messages in thread
From: Yichao Yu @ 2022-05-18 13:01 UTC (permalink / raw)
To: Luis Machado; +Cc: gdb-patches
On Wed, May 18, 2022 at 5:54 AM Luis Machado <luis.machado@arm.com> wrote:
>
> On 5/18/22 02:03, Yichao Yu wrote:
> > Sorry for the duplicated post.... I think somehow the university
> > network prevented the email from going out earlier today...
>
> No problem. I've been having some inbound e-mail issues as well.
I assume by this you mean that someone else will commit it. (cuz I
believe I can't).
>
> >
> > On Tue, May 17, 2022 at 8:45 PM Yichao Yu <yyc1992@gmail.com> wrote:
> >>
> >> This will allow the unwind info to explicitly specify a different value
> >> for the return address from the link register.
> >> Such usage, although uncommon, is valid and useful for signal frames.
> >> It is also supported by aadwarf64 from ARM (Note 9 in [1]).
> >>
> >> Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
> >>
> >> [1] https://github.com/ARM-software/abi-aa/blob/2022Q1/aadwarf64/aadwarf64.rst#dwarf-register-names
> >> ---
> >> gdb/aarch64-tdep.c | 3 +
> >> gdb/aarch64-tdep.h | 1 +
> >> gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 ++++++++++++++
> >> gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 70 ++++++++++++++++++++
> >> 4 files changed, 122 insertions(+)
> >> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> >> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> >>
> >> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> >> index 9d06ebfe27c..f9eb455232f 100644
> >> --- a/gdb/aarch64-tdep.c
> >> +++ b/gdb/aarch64-tdep.c
> >> @@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
> >> if (reg == AARCH64_DWARF_SP)
> >> return AARCH64_SP_REGNUM;
> >>
> >> + if (reg == AARCH64_DWARF_PC)
> >> + return AARCH64_PC_REGNUM;
> >> +
> >> if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
> >> return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
> >>
> >> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> >> index e4cdebb6311..aa1bedbdac6 100644
> >> --- a/gdb/aarch64-tdep.h
> >> +++ b/gdb/aarch64-tdep.h
> >> @@ -34,6 +34,7 @@ struct regset;
> >> /* AArch64 Dwarf register numbering. */
> >> #define AARCH64_DWARF_X0 0
> >> #define AARCH64_DWARF_SP 31
> >> +#define AARCH64_DWARF_PC 32
> >> #define AARCH64_DWARF_PAUTH_RA_STATE 34
> >> #define AARCH64_DWARF_PAUTH_DMASK 35
> >> #define AARCH64_DWARF_PAUTH_CMASK 36
> >> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> >> new file mode 100644
> >> index 00000000000..6cc4f80e349
> >> --- /dev/null
> >> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> >> @@ -0,0 +1,48 @@
> >> +/* Copyright 2022 Free Software Foundation, Inc.
> >> +
> >> + This file is part of GDB.
> >> +
> >> + 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/>. */
> >> +
> >> + .text
> >> + .globl main
> >> + .type main,#function
> >> +main:
> >> + .cfi_startproc
> >> + stp x29, x30, [sp, -16]!
> >> + .cfi_def_cfa sp, 16
> >> + .cfi_offset x29, 0
> >> + .cfi_offset x30, 8
> >> + bl test_func
> >> + ldp x29, x30, [sp], 16
> >> + .cfi_restore x29
> >> + .cfi_restore x30
> >> + .cfi_def_cfa sp, 0
> >> + mov x0, 0
> >> + ret
> >> + .cfi_endproc
> >> +
> >> + .globl test_func
> >> +test_func:
> >> + .cfi_startproc
> >> + // Unwind x30 to a different value
> >> + // CFA_val_expression x30 const2u 0x1234
> >> + .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
> >> + // CFA_val_expression pc breg30 0
> >> + .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
> >> + mov x0, x30
> >> + .cfi_register 32, x0
> >> + mov x30, 0x1234
> >> + ret x0
> >> + .cfi_endproc
> >> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> >> new file mode 100644
> >> index 00000000000..105b9a9cc3a
> >> --- /dev/null
> >> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> >> @@ -0,0 +1,70 @@
> >> +# 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 <http://www.gnu.org/licenses/>.
> >> +
> >> +# This file is part of the gdb testsuite.
> >> +
> >> +# Test explicitly unwinding the PC DWARF register on aarch64
> >> +
> >> +if {![is_aarch64_target]} then {
> >> + verbose "Skipping ${gdb_test_file_name}."
> >> + return
> >> +}
> >> +
> >> +standard_testfile .S
> >> +
> >> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> >> + return -1
> >> +}
> >> +
> >> +if ![runto_main] {
> >> + return -1
> >> +}
> >> +
> >> +proc test_reg_vals {} {
> >> + gdb_test "p \$pc - &main" "= 8" "p \$pc"
> >> + gdb_test "p/x \$x30" "= 0x1234" "p \$x30"
> >> +}
> >> +
> >> +proc test_unwind_pc { inst } {
> >> + gdb_test "si" "$inst" "single step"
> >> + gdb_test "backtrace" \
> >> + ".*#1.*in main ().*" \
> >> + "backtrace"
> >> + gdb_test "up" "in main ().*" "parent frame"
> >> + test_reg_vals
> >> +}
> >> +
> >> +# Ready to enter the function
> >> +gdb_test "si" "bl test_func" "call site"
> >> +# Step through the 3 instructions in the function to make sure that
> >> +# we have the same unwind info throughout.
> >> +with_test_prefix "1st stepi" {
> >> + test_unwind_pc "mov x0, x30"
> >> +}
> >> +with_test_prefix "2nd stepi" {
> >> + test_unwind_pc "mov x30, 0x1234"
> >> +}
> >> +with_test_prefix "3rd stepi" {
> >> + test_unwind_pc "ret x0"
> >> +}
> >> +# Check again after we returned
> >> +with_test_prefix "final" {
> >> + # Check that we've stepped out (si prints out the new function name)
> >> + gdb_test "si" ".*main *().*" "single step out"
> >> + gdb_test "backtrace" \
> >> + "#0\[\t \]+main ().*" \
> >> + "backtrace"
> >> + test_reg_vals
> >> +}
> >> --
> >> 2.36.1
> >>
>
> Thanks. This LGTM for aarch64. Please push it.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] Return the regnum for PC (32) on aarch64
2022-05-18 13:01 ` Yichao Yu
@ 2022-05-18 13:36 ` Luis Machado
2022-05-18 14:01 ` Yichao Yu
0 siblings, 1 reply; 17+ messages in thread
From: Luis Machado @ 2022-05-18 13:36 UTC (permalink / raw)
To: Yichao Yu; +Cc: gdb-patches
On 5/18/22 14:01, Yichao Yu wrote:
> On Wed, May 18, 2022 at 5:54 AM Luis Machado <luis.machado@arm.com> wrote:
>>
>> On 5/18/22 02:03, Yichao Yu wrote:
>>> Sorry for the duplicated post.... I think somehow the university
>>> network prevented the email from going out earlier today...
>>
>> No problem. I've been having some inbound e-mail issues as well.
>
> I assume by this you mean that someone else will commit it. (cuz I
> believe I can't).
>
Just to confirm, do you have a copyright assignment in place with the FSF?
I see one of your previous patches has been pushed before by one of the maintainers.
>>
>>>
>>> On Tue, May 17, 2022 at 8:45 PM Yichao Yu <yyc1992@gmail.com> wrote:
>>>>
>>>> This will allow the unwind info to explicitly specify a different value
>>>> for the return address from the link register.
>>>> Such usage, although uncommon, is valid and useful for signal frames.
>>>> It is also supported by aadwarf64 from ARM (Note 9 in [1]).
>>>>
>>>> Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
>>>>
>>>> [1] https://github.com/ARM-software/abi-aa/blob/2022Q1/aadwarf64/aadwarf64.rst#dwarf-register-names
>>>> ---
>>>> gdb/aarch64-tdep.c | 3 +
>>>> gdb/aarch64-tdep.h | 1 +
>>>> gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 ++++++++++++++
>>>> gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 70 ++++++++++++++++++++
>>>> 4 files changed, 122 insertions(+)
>>>> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
>>>> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
>>>>
>>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>>>> index 9d06ebfe27c..f9eb455232f 100644
>>>> --- a/gdb/aarch64-tdep.c
>>>> +++ b/gdb/aarch64-tdep.c
>>>> @@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
>>>> if (reg == AARCH64_DWARF_SP)
>>>> return AARCH64_SP_REGNUM;
>>>>
>>>> + if (reg == AARCH64_DWARF_PC)
>>>> + return AARCH64_PC_REGNUM;
>>>> +
>>>> if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
>>>> return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
>>>>
>>>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
>>>> index e4cdebb6311..aa1bedbdac6 100644
>>>> --- a/gdb/aarch64-tdep.h
>>>> +++ b/gdb/aarch64-tdep.h
>>>> @@ -34,6 +34,7 @@ struct regset;
>>>> /* AArch64 Dwarf register numbering. */
>>>> #define AARCH64_DWARF_X0 0
>>>> #define AARCH64_DWARF_SP 31
>>>> +#define AARCH64_DWARF_PC 32
>>>> #define AARCH64_DWARF_PAUTH_RA_STATE 34
>>>> #define AARCH64_DWARF_PAUTH_DMASK 35
>>>> #define AARCH64_DWARF_PAUTH_CMASK 36
>>>> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
>>>> new file mode 100644
>>>> index 00000000000..6cc4f80e349
>>>> --- /dev/null
>>>> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
>>>> @@ -0,0 +1,48 @@
>>>> +/* Copyright 2022 Free Software Foundation, Inc.
>>>> +
>>>> + This file is part of GDB.
>>>> +
>>>> + 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/>. */
>>>> +
>>>> + .text
>>>> + .globl main
>>>> + .type main,#function
>>>> +main:
>>>> + .cfi_startproc
>>>> + stp x29, x30, [sp, -16]!
>>>> + .cfi_def_cfa sp, 16
>>>> + .cfi_offset x29, 0
>>>> + .cfi_offset x30, 8
>>>> + bl test_func
>>>> + ldp x29, x30, [sp], 16
>>>> + .cfi_restore x29
>>>> + .cfi_restore x30
>>>> + .cfi_def_cfa sp, 0
>>>> + mov x0, 0
>>>> + ret
>>>> + .cfi_endproc
>>>> +
>>>> + .globl test_func
>>>> +test_func:
>>>> + .cfi_startproc
>>>> + // Unwind x30 to a different value
>>>> + // CFA_val_expression x30 const2u 0x1234
>>>> + .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
>>>> + // CFA_val_expression pc breg30 0
>>>> + .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
>>>> + mov x0, x30
>>>> + .cfi_register 32, x0
>>>> + mov x30, 0x1234
>>>> + ret x0
>>>> + .cfi_endproc
>>>> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
>>>> new file mode 100644
>>>> index 00000000000..105b9a9cc3a
>>>> --- /dev/null
>>>> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
>>>> @@ -0,0 +1,70 @@
>>>> +# 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 <http://www.gnu.org/licenses/>.
>>>> +
>>>> +# This file is part of the gdb testsuite.
>>>> +
>>>> +# Test explicitly unwinding the PC DWARF register on aarch64
>>>> +
>>>> +if {![is_aarch64_target]} then {
>>>> + verbose "Skipping ${gdb_test_file_name}."
>>>> + return
>>>> +}
>>>> +
>>>> +standard_testfile .S
>>>> +
>>>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
>>>> + return -1
>>>> +}
>>>> +
>>>> +if ![runto_main] {
>>>> + return -1
>>>> +}
>>>> +
>>>> +proc test_reg_vals {} {
>>>> + gdb_test "p \$pc - &main" "= 8" "p \$pc"
>>>> + gdb_test "p/x \$x30" "= 0x1234" "p \$x30"
>>>> +}
>>>> +
>>>> +proc test_unwind_pc { inst } {
>>>> + gdb_test "si" "$inst" "single step"
>>>> + gdb_test "backtrace" \
>>>> + ".*#1.*in main ().*" \
>>>> + "backtrace"
>>>> + gdb_test "up" "in main ().*" "parent frame"
>>>> + test_reg_vals
>>>> +}
>>>> +
>>>> +# Ready to enter the function
>>>> +gdb_test "si" "bl test_func" "call site"
>>>> +# Step through the 3 instructions in the function to make sure that
>>>> +# we have the same unwind info throughout.
>>>> +with_test_prefix "1st stepi" {
>>>> + test_unwind_pc "mov x0, x30"
>>>> +}
>>>> +with_test_prefix "2nd stepi" {
>>>> + test_unwind_pc "mov x30, 0x1234"
>>>> +}
>>>> +with_test_prefix "3rd stepi" {
>>>> + test_unwind_pc "ret x0"
>>>> +}
>>>> +# Check again after we returned
>>>> +with_test_prefix "final" {
>>>> + # Check that we've stepped out (si prints out the new function name)
>>>> + gdb_test "si" ".*main *().*" "single step out"
>>>> + gdb_test "backtrace" \
>>>> + "#0\[\t \]+main ().*" \
>>>> + "backtrace"
>>>> + test_reg_vals
>>>> +}
>>>> --
>>>> 2.36.1
>>>>
>>
>> Thanks. This LGTM for aarch64. Please push it.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] Return the regnum for PC (32) on aarch64
2022-05-18 13:36 ` Luis Machado
@ 2022-05-18 14:01 ` Yichao Yu
2022-05-18 14:03 ` Luis Machado
0 siblings, 1 reply; 17+ messages in thread
From: Yichao Yu @ 2022-05-18 14:01 UTC (permalink / raw)
To: Luis Machado; +Cc: gdb-patches
On Wed, May 18, 2022 at 9:36 AM Luis Machado <luis.machado@arm.com> wrote:
>
> On 5/18/22 14:01, Yichao Yu wrote:
> > On Wed, May 18, 2022 at 5:54 AM Luis Machado <luis.machado@arm.com> wrote:
> >>
> >> On 5/18/22 02:03, Yichao Yu wrote:
> >>> Sorry for the duplicated post.... I think somehow the university
> >>> network prevented the email from going out earlier today...
> >>
> >> No problem. I've been having some inbound e-mail issues as well.
> >
> > I assume by this you mean that someone else will commit it. (cuz I
> > believe I can't).
> >
>
> Just to confirm, do you have a copyright assignment in place with the FSF?
>
> I see one of your previous patches has been pushed before by one of the maintainers.
Yes I have that (*.BINUTILS.pdf{,.asc} and *.GDB.pdf{,.asc}). Assuming
they don't expire.
>
> >>
> >>>
> >>> On Tue, May 17, 2022 at 8:45 PM Yichao Yu <yyc1992@gmail.com> wrote:
> >>>>
> >>>> This will allow the unwind info to explicitly specify a different value
> >>>> for the return address from the link register.
> >>>> Such usage, although uncommon, is valid and useful for signal frames.
> >>>> It is also supported by aadwarf64 from ARM (Note 9 in [1]).
> >>>>
> >>>> Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
> >>>>
> >>>> [1] https://github.com/ARM-software/abi-aa/blob/2022Q1/aadwarf64/aadwarf64.rst#dwarf-register-names
> >>>> ---
> >>>> gdb/aarch64-tdep.c | 3 +
> >>>> gdb/aarch64-tdep.h | 1 +
> >>>> gdb/testsuite/gdb.arch/aarch64-unwind-pc.S | 48 ++++++++++++++
> >>>> gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp | 70 ++++++++++++++++++++
> >>>> 4 files changed, 122 insertions(+)
> >>>> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> >>>> create mode 100644 gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> >>>>
> >>>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> >>>> index 9d06ebfe27c..f9eb455232f 100644
> >>>> --- a/gdb/aarch64-tdep.c
> >>>> +++ b/gdb/aarch64-tdep.c
> >>>> @@ -2219,6 +2219,9 @@ aarch64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
> >>>> if (reg == AARCH64_DWARF_SP)
> >>>> return AARCH64_SP_REGNUM;
> >>>>
> >>>> + if (reg == AARCH64_DWARF_PC)
> >>>> + return AARCH64_PC_REGNUM;
> >>>> +
> >>>> if (reg >= AARCH64_DWARF_V0 && reg <= AARCH64_DWARF_V0 + 31)
> >>>> return AARCH64_V0_REGNUM + reg - AARCH64_DWARF_V0;
> >>>>
> >>>> diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
> >>>> index e4cdebb6311..aa1bedbdac6 100644
> >>>> --- a/gdb/aarch64-tdep.h
> >>>> +++ b/gdb/aarch64-tdep.h
> >>>> @@ -34,6 +34,7 @@ struct regset;
> >>>> /* AArch64 Dwarf register numbering. */
> >>>> #define AARCH64_DWARF_X0 0
> >>>> #define AARCH64_DWARF_SP 31
> >>>> +#define AARCH64_DWARF_PC 32
> >>>> #define AARCH64_DWARF_PAUTH_RA_STATE 34
> >>>> #define AARCH64_DWARF_PAUTH_DMASK 35
> >>>> #define AARCH64_DWARF_PAUTH_CMASK 36
> >>>> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> >>>> new file mode 100644
> >>>> index 00000000000..6cc4f80e349
> >>>> --- /dev/null
> >>>> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.S
> >>>> @@ -0,0 +1,48 @@
> >>>> +/* Copyright 2022 Free Software Foundation, Inc.
> >>>> +
> >>>> + This file is part of GDB.
> >>>> +
> >>>> + 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/>. */
> >>>> +
> >>>> + .text
> >>>> + .globl main
> >>>> + .type main,#function
> >>>> +main:
> >>>> + .cfi_startproc
> >>>> + stp x29, x30, [sp, -16]!
> >>>> + .cfi_def_cfa sp, 16
> >>>> + .cfi_offset x29, 0
> >>>> + .cfi_offset x30, 8
> >>>> + bl test_func
> >>>> + ldp x29, x30, [sp], 16
> >>>> + .cfi_restore x29
> >>>> + .cfi_restore x30
> >>>> + .cfi_def_cfa sp, 0
> >>>> + mov x0, 0
> >>>> + ret
> >>>> + .cfi_endproc
> >>>> +
> >>>> + .globl test_func
> >>>> +test_func:
> >>>> + .cfi_startproc
> >>>> + // Unwind x30 to a different value
> >>>> + // CFA_val_expression x30 const2u 0x1234
> >>>> + .cfi_escape 0x16, 30, 0x03, 0x0a, 0x34, 0x12
> >>>> + // CFA_val_expression pc breg30 0
> >>>> + .cfi_escape 0x16, 32, 0x02, 0x8e, 0x00
> >>>> + mov x0, x30
> >>>> + .cfi_register 32, x0
> >>>> + mov x30, 0x1234
> >>>> + ret x0
> >>>> + .cfi_endproc
> >>>> diff --git a/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> >>>> new file mode 100644
> >>>> index 00000000000..105b9a9cc3a
> >>>> --- /dev/null
> >>>> +++ b/gdb/testsuite/gdb.arch/aarch64-unwind-pc.exp
> >>>> @@ -0,0 +1,70 @@
> >>>> +# 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 <http://www.gnu.org/licenses/>.
> >>>> +
> >>>> +# This file is part of the gdb testsuite.
> >>>> +
> >>>> +# Test explicitly unwinding the PC DWARF register on aarch64
> >>>> +
> >>>> +if {![is_aarch64_target]} then {
> >>>> + verbose "Skipping ${gdb_test_file_name}."
> >>>> + return
> >>>> +}
> >>>> +
> >>>> +standard_testfile .S
> >>>> +
> >>>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> >>>> + return -1
> >>>> +}
> >>>> +
> >>>> +if ![runto_main] {
> >>>> + return -1
> >>>> +}
> >>>> +
> >>>> +proc test_reg_vals {} {
> >>>> + gdb_test "p \$pc - &main" "= 8" "p \$pc"
> >>>> + gdb_test "p/x \$x30" "= 0x1234" "p \$x30"
> >>>> +}
> >>>> +
> >>>> +proc test_unwind_pc { inst } {
> >>>> + gdb_test "si" "$inst" "single step"
> >>>> + gdb_test "backtrace" \
> >>>> + ".*#1.*in main ().*" \
> >>>> + "backtrace"
> >>>> + gdb_test "up" "in main ().*" "parent frame"
> >>>> + test_reg_vals
> >>>> +}
> >>>> +
> >>>> +# Ready to enter the function
> >>>> +gdb_test "si" "bl test_func" "call site"
> >>>> +# Step through the 3 instructions in the function to make sure that
> >>>> +# we have the same unwind info throughout.
> >>>> +with_test_prefix "1st stepi" {
> >>>> + test_unwind_pc "mov x0, x30"
> >>>> +}
> >>>> +with_test_prefix "2nd stepi" {
> >>>> + test_unwind_pc "mov x30, 0x1234"
> >>>> +}
> >>>> +with_test_prefix "3rd stepi" {
> >>>> + test_unwind_pc "ret x0"
> >>>> +}
> >>>> +# Check again after we returned
> >>>> +with_test_prefix "final" {
> >>>> + # Check that we've stepped out (si prints out the new function name)
> >>>> + gdb_test "si" ".*main *().*" "single step out"
> >>>> + gdb_test "backtrace" \
> >>>> + "#0\[\t \]+main ().*" \
> >>>> + "backtrace"
> >>>> + test_reg_vals
> >>>> +}
> >>>> --
> >>>> 2.36.1
> >>>>
> >>
> >> Thanks. This LGTM for aarch64. Please push it.
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] Return the regnum for PC (32) on aarch64
2022-05-18 14:01 ` Yichao Yu
@ 2022-05-18 14:03 ` Luis Machado
2022-05-19 1:11 ` Yichao Yu
0 siblings, 1 reply; 17+ messages in thread
From: Luis Machado @ 2022-05-18 14:03 UTC (permalink / raw)
To: Yichao Yu; +Cc: gdb-patches
On 5/18/22 15:01, Yichao Yu wrote:
> On Wed, May 18, 2022 at 9:36 AM Luis Machado <luis.machado@arm.com> wrote:
>>
>> On 5/18/22 14:01, Yichao Yu wrote:
>>> On Wed, May 18, 2022 at 5:54 AM Luis Machado <luis.machado@arm.com> wrote:
>>>>
>>>> On 5/18/22 02:03, Yichao Yu wrote:
>>>>> Sorry for the duplicated post.... I think somehow the university
>>>>> network prevented the email from going out earlier today...
>>>>
>>>> No problem. I've been having some inbound e-mail issues as well.
>>>
>>> I assume by this you mean that someone else will commit it. (cuz I
>>> believe I can't).
>>>
>>
>> Just to confirm, do you have a copyright assignment in place with the FSF?
>>
>> I see one of your previous patches has been pushed before by one of the maintainers.
>
> Yes I have that (*.BINUTILS.pdf{,.asc} and *.GDB.pdf{,.asc}). Assuming
> they don't expire.
>
Great. I'll push on your behalf.
Thanks for fixing this.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3] Return the regnum for PC (32) on aarch64
2022-05-18 14:03 ` Luis Machado
@ 2022-05-19 1:11 ` Yichao Yu
0 siblings, 0 replies; 17+ messages in thread
From: Yichao Yu @ 2022-05-19 1:11 UTC (permalink / raw)
To: Luis Machado; +Cc: gdb-patches
On Wed, May 18, 2022 at 10:03 AM Luis Machado <luis.machado@arm.com> wrote:
>
> On 5/18/22 15:01, Yichao Yu wrote:
> > On Wed, May 18, 2022 at 9:36 AM Luis Machado <luis.machado@arm.com> wrote:
> >>
> >> On 5/18/22 14:01, Yichao Yu wrote:
> >>> On Wed, May 18, 2022 at 5:54 AM Luis Machado <luis.machado@arm.com> wrote:
> >>>>
> >>>> On 5/18/22 02:03, Yichao Yu wrote:
> >>>>> Sorry for the duplicated post.... I think somehow the university
> >>>>> network prevented the email from going out earlier today...
> >>>>
> >>>> No problem. I've been having some inbound e-mail issues as well.
> >>>
> >>> I assume by this you mean that someone else will commit it. (cuz I
> >>> believe I can't).
> >>>
> >>
> >> Just to confirm, do you have a copyright assignment in place with the FSF?
> >>
> >> I see one of your previous patches has been pushed before by one of the maintainers.
> >
> > Yes I have that (*.BINUTILS.pdf{,.asc} and *.GDB.pdf{,.asc}). Assuming
> > they don't expire.
> >
>
> Great. I'll push on your behalf.
>
> Thanks for fixing this.
And thanks for guiding me through the process.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-05-19 1:12 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-13 12:27 [PATCH] Return the regnum for PC (32) on aarch64 Yichao Yu
2022-05-16 9:05 ` Luis Machado
2022-05-16 13:17 ` Yichao Yu
2022-05-16 13:25 ` [PATCH v2] " Yichao Yu
2022-05-17 6:42 ` Luis Machado
2022-05-17 14:41 ` [PATCH v3] " Yichao Yu
2022-05-17 17:34 ` [PATCH v2] " Yichao Yu
2022-05-17 17:36 ` [PATCH v3] " Yichao Yu
2022-05-18 1:03 ` Yichao Yu
2022-05-18 9:53 ` Luis Machado
2022-05-18 13:01 ` Yichao Yu
2022-05-18 13:36 ` Luis Machado
2022-05-18 14:01 ` Yichao Yu
2022-05-18 14:03 ` Luis Machado
2022-05-19 1:11 ` Yichao Yu
2022-05-16 13:29 ` [PATCH] " Luis Machado
2022-05-16 14:10 ` Yichao Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).