Re: [PATCH] gdb/riscv: Support c.li in prologue unwinder

[Andrew Burgess <aburgess@redhat.com>]

Andrew Burgess <aburgess@redhat.com> writes:

> I was seeing some failures in gdb.threads/omp-par-scope.exp when run
> on a riscv64 target.  It turns out the cause of the problem is that I
> didn't have debug information installed for libgomp.so, which this
> test makes use of.  The test requires GDB to backtrace through a
> libgomp function, and the riscv prologue unwinder was failing to
> unwind this particular stack frame.
>
> The reason for the failure to unwind was that the function prologue
> includes a c.li (compressed load immediate) instruction, and the riscv
> prologue scanning unwinder doesn't know what to do with this
> instruction, though the unwinder does understand c.lui (compressed
> load unsigned immediate).
>
> This commit adds support for c.li.  After this GDB is able to unwind
> through libgomp, and I no longer see any unexpected failures in
> gdb.threads/omp-par-scope.exp.
>
> I've also included a new test in gdb.arch/ which specifically checks
> for our c.li support.

I've gone ahead and pushed this.

Thanks,
Andrew


> ---
>  gdb/riscv-tdep.c                              |  8 +++-
>  .../riscv64-unwind-prologue-with-c_li-foo.s   | 47 +++++++++++++++++++
>  .../riscv64-unwind-prologue-with-c_li.c       | 29 ++++++++++++
>  .../riscv64-unwind-prologue-with-c_li.exp     | 42 +++++++++++++++++
>  4 files changed, 124 insertions(+), 2 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-c_li-foo.s
>  create mode 100644 gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-c_li.c
>  create mode 100644 gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-c_li.exp
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 8024ffed3c2..a9c5418a7d9 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -1529,6 +1529,7 @@ class riscv_insn
>        ADDW,
>        AUIPC,
>        LUI,
> +      LI,
>        SD,
>        SW,
>        LD,
> @@ -1901,6 +1902,8 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
>  	  m_rd = decode_register_index (ival, OP_SH_CRS1S);
>  	  m_imm.s = EXTRACT_CITYPE_LUI_IMM (ival);
>  	}
> +      else if (is_c_li_insn (ival))
> +	decode_ci_type_insn (LI, ival);
>        /* C_SD and C_FSW have the same opcode.  C_SD is RV64 and RV128 only,
>  	 and C_FSW is RV32 only.  */
>        else if (xlen != 4 && is_c_sd_insn (ival))
> @@ -2053,10 +2056,11 @@ riscv_scan_prologue (struct gdbarch *gdbarch,
>  	  gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
>  	  regs[insn.rd ()] = pv_constant (cur_pc + insn.imm_signed ());
>  	}
> -      else if (insn.opcode () == riscv_insn::LUI)
> +      else if (insn.opcode () == riscv_insn::LUI
> +	       || insn.opcode () == riscv_insn::LI)
>  	{
>  	  /* Handle: lui REG, n
> -	     Where REG is not gp register.  */
> +	     or:     li  REG, n  */
>  	  gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
>  	  regs[insn.rd ()] = pv_constant (insn.imm_signed ());
>  	}
> diff --git a/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-c_li-foo.s b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-c_li-foo.s
> new file mode 100644
> index 00000000000..787aacdd8f3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-c_li-foo.s
> @@ -0,0 +1,47 @@
> +/* Copyright 2023 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/>.  */
> +
> +
> +/* Simple asm function that makes use of c.li and c.lui in the
> +   function prologue before the return address and frame pointers are
> +   written the stack.  This ensures that GDB's prologue unwinder can
> +   understand these instructions.  */
> +
> +	.option pic
> +	.text
> +
> +	.globl	foo
> +	.type	foo, @function
> +foo:
> +	addi	sp,sp,-32
> +	sd	s1,8(sp)
> +
> +	c.li	s1,-4
> +	c.lui	s1,4
> +
> +	sd	fp,24(sp)
> +	sd	ra,16(sp)
> +	addi	fp,sp,32
> +
> +	call	bar@plt
> +
> +	ld	s1,8(sp)
> +	ld	ra,16(sp)
> +	ld	fp,24(sp)
> +	addi	sp,sp,32
> +	jr	ra
> +
> +	.size	foo, .-foo
> +	.section	.note.GNU-stack,"",@progbits
> diff --git a/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-c_li.c b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-c_li.c
> new file mode 100644
> index 00000000000..d3a0cb3ff40
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-c_li.c
> @@ -0,0 +1,29 @@
> +/* Copyright 2023 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/>.  */
> +
> +/* See riscv64-unwind-prologue-with-c_li-asm.s.  */
> +extern int foo (void);
> +
> +int
> +bar (void)
> +{
> +  return 0;
> +}
> +
> +int
> +main (void)
> +{
> +  return foo ();
> +}
> diff --git a/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-c_li.exp b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-c_li.exp
> new file mode 100644
> index 00000000000..069effa2b87
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-c_li.exp
> @@ -0,0 +1,42 @@
> +# Copyright 2023 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 tests GDB's ability to use the RISC-V prologue scanner in order
> +# to unwind through a function that uses the compressed 'c.li'
> +# instruction in its prologue.
> +
> +require {istarget "riscv64-*-*"}
> +
> +standard_testfile .c -foo.s
> +
> +if {[prepare_for_testing "failed to prepare" $testfile \
> +			 "$srcfile $srcfile2"  nodebug]} {
> +    return -1
> +}
> +
> +if {![runto_main]} {
> +    return 0
> +}
> +
> +gdb_breakpoint "bar"
> +gdb_continue_to_breakpoint "bar"
> +gdb_test "bt" \
> +    [multi_line \
> +	 "#0\[ \t\]*$hex in bar \\\(\\\)" \
> +	 "#1\[ \t\]*$hex in foo \\\(\\\)" \
> +	 "#2\[ \t\]*$hex in main \\\(\\\)"] \
> +    "Backtrace to the main frame"
> +gdb_test "finish" "foo \\\(\\\)" "finish bar"
> +gdb_test "finish" "main \\\(\\\)" "finish foo"
>
> base-commit: 1770eca698ad0018cd504b9744306fc9928d9a86
> -- 
> 2.25.4