[PATCH] gdb: Support the c.mv insn in the riscv prologue scanner.

[PATCH] gdb: Support the c.mv insn in the riscv prologue scanner.

[Lancelot SIX]

While working on other problems, I encountered situations where GDB
fails to properly unwind the stack because some functions use the C.MV
instruction in the prologue.  The prologue scanner stops when it hits
this instruction assuming its job is done at this point.  Unfortunately
the prologue is not necessarily finished yet, preventing GDB to properly
unwind.

This commit adds support for handling such instruction in
riscv_scan_prologue.

Note that C.MV is part of the compressed instruction set.  The MV
counterpart from the base ISA is a pseudo instruction that expands to
'ADDI RD,RS1,0' which is already supported.

Tested on riscv64-linux-gnu.

All feedback are welcome.
---
 gdb/riscv-tdep.c                              | 12 ++++-
 .../riscv64-unwind-prologue-with-mv.c         | 29 ++++++++++++
 .../riscv64-unwind-prologue-with-mv.exp       | 44 +++++++++++++++++
 .../riscv64-unwind-prologue-with-mv.s         | 47 +++++++++++++++++++
 4 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-mv.c
 create mode 100644 gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-mv.exp
 create mode 100644 gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-mv.s

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 8b55bc33ded..54f20f2bfaf 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1411,6 +1411,7 @@ class riscv_insn
       SW,
       LD,
       LW,
+      MV,
       /* These are needed for software breakpoint support.  */
       JAL,
       JALR,
@@ -1789,9 +1790,11 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
       else if (xlen != 4 && is_c_sdsp_insn (ival))
 	decode_css_type_insn (SD, ival, EXTRACT_CSSTYPE_SDSP_IMM (ival));
       /* C_JR and C_MV have the same opcode.  If RS2 is 0, then this is a C_JR.
-	 So must try to match C_JR first as it ahs more bits in mask.  */
+	 So must try to match C_JR first as it has more bits in mask.  */
       else if (is_c_jr_insn (ival))
 	decode_cr_type_insn (JALR, ival);
+      else if (is_c_mv_insn (ival))
+	decode_cr_type_insn (MV, ival);
       else if (is_c_j_insn (ival))
 	decode_cj_type_insn (JAL, ival);
       else if (is_c_beqz_insn (ival))
@@ -1964,6 +1967,13 @@ riscv_scan_prologue (struct gdbarch *gdbarch,
 					    insn.imm_signed ()),
 			   (insn.opcode () == riscv_insn::LW ? 4 : 8));
 	}
+      else if (insn.opcode () == riscv_insn::MV)
+	{
+	  gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
+	  gdb_assert (insn.rs2 () < RISCV_NUM_INTEGER_REGS);
+	  gdb_assert (insn.rs2 () > 0);
+	  regs[insn.rd ()] = pv_add_constant(regs[insn.rs2 ()], 0);
+	}
       else
 	{
 	  end_prologue_addr = cur_pc;
diff --git a/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-mv.c b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-mv.c
new file mode 100644
index 00000000000..bb8a8ee521e
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-mv.c
@@ -0,0 +1,29 @@
+/* Copyright 2021 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-mv.s for implementation.  */
+
+extern int bar ();
+
+/* See riscv64-unwind-prologue-with-mv.s for implementation.  */
+
+extern int foo ();
+
+int
+main ()
+{
+  return foo ();
+}
+
diff --git a/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-mv.exp b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-mv.exp
new file mode 100644
index 00000000000..47429ba1055
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-mv.exp
@@ -0,0 +1,44 @@
+# Copyright 2021 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 'c.mv' instruction in its prologue.
+
+if {![istarget "riscv64-*-*"]} {
+    verbose "Skipping ${gdb_test_file_name}."
+    return
+}
+
+standard_testfile .c .s
+if {[prepare_for_testing "failed to prepare" $testfile \
+			 "$srcfile $srcfile2"  nodebug]} {
+    return -1
+}
+
+if ![runto_main] then {
+    fail "can't run to 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"
diff --git a/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-mv.s b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-mv.s
new file mode 100644
index 00000000000..a1ec538ae2a
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-mv.s
@@ -0,0 +1,47 @@
+/* Copyright 2021 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 testcase contains a function where the 'c.mv' instruction is used in
+   the prologue.
+
+   The following functions are roughly equivalent to the following C code (with
+   prologue crafted to contain the c.mv instruction):
+
+     int bar () { return 0; }
+     int foo () { return bar (); } */
+
+	.option pic
+	.text
+	.align	1
+	.globl	bar
+	.type	bar, @function
+bar:
+	li	a0,0
+	jr	ra
+	.size	bar, .-bar
+
+	.align	1
+	.globl	foo
+	.type	foo, @function
+foo:
+	addi	sp,sp,-32
+	c.mv	t3,ra
+	sd	t3,8(sp)
+	call	bar
+	ld	t3,8(sp)
+	mv	ra,t3
+	addi	sp,sp,32
+	jr	ra
+	.size	foo, .-foo
-- 
2.30.2

Re: [PATCH] gdb: Support the c.mv insn in the riscv prologue scanner.

[Lancelot SIX]

> +      else if (insn.opcode () == riscv_insn::MV)
> +	{
> +	  gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> +	  gdb_assert (insn.rs2 () < RISCV_NUM_INTEGER_REGS);
> +	  gdb_assert (insn.rs2 () > 0);
> +	  regs[insn.rd ()] = pv_add_constant(regs[insn.rs2 ()], 0);

Actully, replying to myself, this should probably be simplified as

	regs[insn.rd ()] = regs[insn.rs2 ()];

I’ll change that.

Lancelot.

> +	}

Re: [PATCH] gdb: Support the c.mv insn in the riscv prologue scanner.

[Andrew Burgess]

* Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org> [2021-09-27 23:29:45 +0000]:

> 
> > +      else if (insn.opcode () == riscv_insn::MV)
> > +	{
> > +	  gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> > +	  gdb_assert (insn.rs2 () < RISCV_NUM_INTEGER_REGS);
> > +	  gdb_assert (insn.rs2 () > 0);
> > +	  regs[insn.rd ()] = pv_add_constant(regs[insn.rs2 ()], 0);
> 
> Actully, replying to myself, this should probably be simplified as
> 
> 	regs[insn.rd ()] = regs[insn.rs2 ()];
> 
> I’ll change that.

Looks good to me with this change made.

Thanks,
Andrew

Re: [PATCH] gdb: Support the c.mv insn in the riscv prologue scanner.

[Lancelot SIX]

> Looks good to me with this change made.
> 
> Thanks,
> Andrew

Hi,

I have pushed this patch with the mentionned change.

Best,
Lancelot.