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

[Yichao Yu <yyc1992@gmail.com>]

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