diff mbox series

[2/3] gdb/riscv: support c.ldsp and c.lwsp in prologue scanner

Message ID e26e6b46fe96d0677cda14bd496afb2bb44b81c2.1678743865.git.aburgess@redhat.com
State New
Headers
Series riscv: improve unwinding with no DWARF |

Commit Message

Andrew Burgess March 13, 2023, 9:46 p.m. UTC 
  Add support to the RISC-V prologue scanner for c.ldsp and c.lwsp
instructions.

This fixes some of the failures in gdb.base/unwind-on-each-insn.exp,
though there are further failures that are not fixed by this commit.

This change started as a wider fix that would address all the failures
in gdb.base/unwind-on-each-insn.exp, however, that wider fix needed
support for the two additional compressed instructions.

When I added support for those two compressed instructions I noticed
that some of the failures in gdb.base/unwind-on-each-insn.exp resolved
themselves!

Here's what's going on:

The reason for the failures is that GDB is trying to build the
frame-id during the last few instructions of the function.  These are
the instructions that restore the frame and stack pointers just prior
to the return instruction itself.

By the time we reach the function epilogue the stack offset that we
calculated during the prologue scan is no longer valid, and so we
calculate the wrong frame-id.

However, in the particular case of interest here, the test function
'foo', the function is so simple and short that GDB's prologue scan
could, in theory, scan every instruction of the function.

I say "could, in theory," because currently GDB stops the prologue
scan early when it hits an unknown instruction.  The unknown
instruction happens to be one of the compressed instructions that I'm
adding support for in this commit.

Now that GDB understands the compressed instructions the prologue scan
really does go from the start of the function right up to the current
program counter.  As such, GDB sees that the stack frame has been
allocated, and then deallocated, and so builds the correct frame-id.

Of course, most real functions are not as simple as the test function
'foo'.  As such, we can realistically rely on scanning right up to the
end of the function.  There are some instructions we always need to
stop at because we can't reason about how they change the inferior
state (e.g. a function call).  The test function 'bar' is just such an
example.

After this commit, we can now build the frame-id correctly for every
instruction in 'foo', but there are some tests still failing in 'bar'.
---
 gdb/riscv-tdep.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index ba7eab41ac6..5ef76555918 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1668,11 +1668,20 @@  class riscv_insn
     m_imm.s = EXTRACT_ITYPE_IMM (ival);
   }
 
-  /* Helper for DECODE, decode 16-bit compressed I-type instruction.  */
-  void decode_ci_type_insn (enum opcode opcode, ULONGEST ival)
+  /* Helper for DECODE, decode 16-bit compressed I-type instruction.  Some
+     of the CI instruction have a hard-coded rs1 register, while others
+     just use rd for both the source and destination.  RS1_REGNUM, if
+     passed, is the value to place in rs1, otherwise rd is duplicated into
+     rs1.  */
+  void decode_ci_type_insn (enum opcode opcode, ULONGEST ival,
+			    gdb::optional<int> rs1_regnum = {})
   {
     m_opcode = opcode;
-    m_rd = m_rs1 = decode_register_index (ival, OP_SH_CRS1S);
+    m_rd = decode_register_index (ival, OP_SH_CRS1S);
+    if (rs1_regnum.has_value ())
+      m_rs1 = *rs1_regnum;
+    else
+      m_rs1 = m_rd;
     m_imm.s = EXTRACT_CITYPE_IMM (ival);
   }
 
@@ -1959,6 +1968,10 @@  riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
 	decode_cl_type_insn (LD, ival);
       else if (is_c_lw_insn (ival))
 	decode_cl_type_insn (LW, ival);
+      else if (is_c_ldsp_insn (ival))
+	decode_ci_type_insn (LD, ival, RISCV_SP_REGNUM);
+      else if (is_c_lwsp_insn (ival))
+	decode_ci_type_insn (LW, ival, RISCV_SP_REGNUM);
       else
 	/* None of the other fields of INSN are valid in this case.  */
 	m_opcode = OTHER;