Re: [PATCH v3] Return the regnum for PC (32) on aarch64

[Yichao Yu <yyc1992@gmail.com>]

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.
>