[PATCH 3/3] gdb/riscv: fix regressions in gdb.base/unwind-on-each-insn.exp

[Andrew Burgess <aburgess@redhat.com>]

This commit builds on the previous one to fix all the remaining
failures in gdb.base/unwind-on-each-insn.exp for RISC-V.

The problem we have in gdb.base/unwind-on-each-insn.exp is that, when
we are in the function epilogue, the previous frame and stack pointer
values are being restored, and so, the values that we calculated
during the function prologue are no longer suitable.

Here's an example from the function 'bar' in the mentioned test.  This
was compiled for 64-bit RISC-V with compressed instruction support:

  Dump of assembler code for function bar:
     0x000000000001018a <+0>:	add	sp,sp,-32
     0x000000000001018c <+2>:	sd	ra,24(sp)
     0x000000000001018e <+4>:	sd	fp,16(sp)
     0x0000000000010190 <+6>:	add	fp,sp,32
     0x0000000000010192 <+8>:	sd	a0,-24(fp)
     0x0000000000010196 <+12>:	ld	a0,-24(fp)
     0x000000000001019a <+16>:	jal	0x10178 <foo>
     0x000000000001019e <+20>:	nop
     0x00000000000101a0 <+22>:	ld	ra,24(sp)
     0x00000000000101a2 <+24>:	ld	fp,16(sp)
     0x00000000000101a4 <+26>:	add	sp,sp,32
     0x00000000000101a6 <+28>:	ret
  End of assembler dump.

When we are at address 0x101a4 the previous instruction has restored
the frame-pointer, as such GDB's (current) preference for using the
frame-pointer as the frame base address is clearly not going to work.
We need to switch to using the stack-pointer instead.

At address 0x101a6 the previous instruction has restored the
stack-pointer value.  Currently GDB will not understand this and so
will still assume the stack has been decreased by 32 bytes in this
function.

My proposed solution is to add some additional code that scans the
instructions at the current $pc.  We're looking for this pattern:

  ld    fp,16(sp)
  add   sp,sp,32
  ret

Obviously the immediates can change, but the basic pattern indicates
that the function is in the process of restoring state before
returning.

With this implemented then gdb.base/unwind-on-each-insn.exp now fully
passes.

Obviously what I've implemented is just a heuristic.  It's not going
to work for every function.  If the compiler reorders the
instructions, or merges the epilogue back into the function body then
GDB is once again going to get the frame-id wrong.

I'm OK with that.

Remember, this is for debugging code without debug information,
and (in our imagined situation) with more aggressive levels of
optimisation being used.

Obviously GDB is going to struggle in these situations.

My thinking is, lets get something in place now.  Then, later, if
possible, we might be able to improve the logic to cover more
situations -- if there's an interest in doing so.  But I figure we
need something in place as a starting point.

After this commit gdb.base/unwind-on-each-insn.exp passes with no
failures on RV64.
---
 gdb/riscv-tdep.c | 235 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 231 insertions(+), 4 deletions(-)

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 5ef76555918..771d892df26 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1986,6 +1986,214 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
     }
 }
 
+/* Return true if INSN represents an instruction something like:
+
+   ld fp,IMMEDIATE(sp)
+
+   That is, a load from stack-pointer plus some immediate offset, with the
+   result stored into the frame pointer.  We also accept 'lw' as well as
+   'ld'.  */
+
+static bool
+is_insn_load_of_fp_from_sp (const struct riscv_insn &insn)
+{
+  return ((insn.opcode () == riscv_insn::LD
+	   || insn.opcode () == riscv_insn::LW)
+	  && insn.rd () == RISCV_FP_REGNUM
+	  && insn.rs1 () == RISCV_SP_REGNUM);
+}
+
+/* Return true if INSN represents an instruction something like:
+
+   add sp,sp,IMMEDIATE
+
+   That is, an add of an immediate to the value in the stack pointer
+   register, with the result stored back to the stack pointer register.  */
+
+static bool
+is_insn_addi_of_sp_to_sp (const struct riscv_insn &insn)
+{
+  return ((insn.opcode () == riscv_insn::ADDI
+	   || insn.opcode () == riscv_insn::ADDIW)
+	  && insn.rd () == RISCV_SP_REGNUM
+	  && insn.rs1 () == RISCV_SP_REGNUM);
+}
+
+/* Is the instruction in code memory prior to address PC a load from stack
+   instruction?  Return true if it is, otherwise, return false.
+
+   This is a best effort that is used as part of the function prologue
+   scanning logic.  With compressed instructions and arbitrary control
+   flow in the inferior, we can never be certain what the instruction
+   prior to PC is.
+
+   This function first looks for a compressed instruction, then looks for
+   a 32-bit non-compressed instruction.  */
+
+static bool
+previous_insn_is_load_fp_from_stack (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  struct riscv_insn insn;
+  insn.decode (gdbarch, pc - 2);
+  gdb_assert (insn.length () > 0);
+
+  if (insn.length () != 2 || !is_insn_load_of_fp_from_sp (insn))
+    {
+      insn.decode (gdbarch, pc - 4);
+      gdb_assert (insn.length () > 0);
+
+      if (insn.length () != 4 || !is_insn_load_of_fp_from_sp (insn))
+	return false;
+    }
+
+  riscv_unwinder_debug_printf
+    ("previous instruction at %s (length %d) was 'ld'",
+     core_addr_to_string (pc - insn.length ()), insn.length ());
+  return true;
+}
+
+/* Is the instruction in code memory prior to address PC an add of an
+   immediate to the stack pointer, with the result being written back into
+   the stack pointer?  Return true and set *PREV_PC to the address of the
+   previous instruction if we believe the previous instruction is such an
+   add, otherwise return false and *PREV_PC is undefined.
+
+   This is a best effort that is used as part of the function prologue
+   scanning logic.  With compressed instructions and arbitrary control
+   flow in the inferior, we can never be certain what the instruction
+   prior to PC is.
+
+   This function first looks for a compressed instruction, then looks for
+   a 32-bit non-compressed instruction.  */
+
+static bool
+previous_insn_is_add_imm_to_sp (struct gdbarch *gdbarch, CORE_ADDR pc,
+				CORE_ADDR *prev_pc)
+{
+  struct riscv_insn insn;
+  insn.decode (gdbarch, pc - 2);
+  gdb_assert (insn.length () > 0);
+
+  if (insn.length () != 2 || !is_insn_addi_of_sp_to_sp (insn))
+    {
+      insn.decode (gdbarch, pc - 4);
+      gdb_assert (insn.length () > 0);
+
+      if (insn.length () != 4 || !is_insn_addi_of_sp_to_sp (insn))
+	return false;
+    }
+
+  riscv_unwinder_debug_printf
+    ("previous instruction at %s (length %d) was 'add'",
+     core_addr_to_string (pc - insn.length ()), insn.length ());
+  *prev_pc = pc - insn.length ();
+  return true;
+}
+
+/* Try to spot when PC is located in an exit sequence for a particular
+   function.  Detecting an exit sequence involves a limited amount of
+   scanning backwards through the disassembly, and so, when considering
+   compressed instructions, we can never be certain that we have
+   disassembled the preceding instructions correctly.  On top of that, we
+   can't be certain that the inferior arrived at PC by passing through the
+   preceding instructions.
+
+   With all that said, we know that using prologue scanning to figure a
+   functions unwind information starts to fail when we consider returns
+   from an instruction -- we must pass through some instructions that
+   restore the previous state prior to the final return instruction, and
+   with state partially restored, our prologue derived unwind information
+   is no longer valid.
+
+   This function then, aims to spot instruction sequences like this:
+
+     ld     fp, IMM_1(sp)
+     add    sp, sp, IMM_2
+     ret
+
+   The first instruction restores the previous frame-pointer value, the
+   second restores the previous stack pointer value, and the final
+   instruction is the actual return.
+
+   We need to consider that some or all of these instructions might be
+   compressed.
+
+   This function makes the assumption that, when the inferior reaches the
+   'ret' instruction the stack pointer will have been restored to its value
+   on entry to this function.  This assumption will be true in most well
+   formed programs.
+
+   Return true if we detect that we are in such an instruction sequence,
+   that is PC points at one of the three instructions given above.  In this
+   case, set *OFFSET to IMM_2 if PC points to either of the first
+   two instructions (the 'ld' or 'add'), otherwise set *OFFSET to 0.
+
+   Otherwise, this function returns false, and the contents of *OFFSET are
+   undefined.  */
+
+static bool
+riscv_detect_end_of_function (struct gdbarch *gdbarch, CORE_ADDR pc,
+			      int *offset)
+{
+  *offset = 0;
+
+  /* We only want to scan a maximum of 3 instructions.  */
+  for (int i = 0; i < 3; ++i)
+    {
+      struct riscv_insn insn;
+      insn.decode (gdbarch, pc);
+      gdb_assert (insn.length () > 0);
+
+      if (is_insn_load_of_fp_from_sp (insn))
+	{
+	  riscv_unwinder_debug_printf ("found 'ld' instruction at %s",
+				       core_addr_to_string (pc));
+	  if (i > 0)
+	    return false;
+	  pc += insn.length ();
+	}
+      else if (is_insn_addi_of_sp_to_sp (insn))
+	{
+	  riscv_unwinder_debug_printf ("found 'add' instruction at %s",
+				       core_addr_to_string (pc));
+	  if (i > 1)
+	    return false;
+	  if (i == 0)
+	    {
+	      if (!previous_insn_is_load_fp_from_stack (gdbarch, pc))
+		return false;
+
+	      i = 1;
+	    }
+	  *offset = insn.imm_signed ();
+	  pc += insn.length ();
+	}
+      else if (insn.opcode () == riscv_insn::JALR
+	       && insn.rs1 () == RISCV_RA_REGNUM
+	       && insn.rs2 () == RISCV_ZERO_REGNUM)
+	{
+	  riscv_unwinder_debug_printf ("found 'ret' instruction at %s",
+				       core_addr_to_string (pc));
+	  gdb_assert (i != 1);
+	  if (i == 0)
+	    {
+	      CORE_ADDR prev_pc;
+	      if (!previous_insn_is_add_imm_to_sp (gdbarch, pc, &prev_pc))
+		return false;
+	      if (!previous_insn_is_load_fp_from_stack (gdbarch, prev_pc))
+		return false;
+	      i = 2;
+	    }
+
+	  pc += insn.length ();
+	}
+      else
+	return false;
+    }
+
+  return true;
+}
+
 /* The prologue scanner.  This is currently only used for skipping the
    prologue of a function when the DWARF information is not sufficient.
    However, it is written with filling of the frame cache in mind, which
@@ -2000,6 +2208,7 @@ riscv_scan_prologue (struct gdbarch *gdbarch,
 		     struct riscv_unwind_cache *cache)
 {
   CORE_ADDR cur_pc, next_pc, after_prologue_pc;
+  CORE_ADDR original_end_pc = end_pc;
   CORE_ADDR end_prologue_addr = 0;
 
   /* Find an upper limit on the function prologue using the debug
@@ -2031,10 +2240,7 @@ riscv_scan_prologue (struct gdbarch *gdbarch,
       next_pc = cur_pc + insn.length ();
 
       /* Look for common stack adjustment insns.  */
-      if ((insn.opcode () == riscv_insn::ADDI
-	   || insn.opcode () == riscv_insn::ADDIW)
-	  && insn.rd () == RISCV_SP_REGNUM
-	  && insn.rs1 () == RISCV_SP_REGNUM)
+      if (is_insn_addi_of_sp_to_sp (insn))
 	{
 	  /* Handle: addi sp, sp, -i
 	     or:     addiw sp, sp, -i  */
@@ -2171,6 +2377,27 @@ riscv_scan_prologue (struct gdbarch *gdbarch,
 	  cache->frame_base_offset = -regs[RISCV_SP_REGNUM].k;
 	}
 
+      /* Check to see if we are located near to a return instruction in
+	 this function.  If we are then the one or both of the stack
+	 pointer and frame pointer may have been restored to their previous
+	 value.  If we can spot this situation then we can adjust which
+	 register and offset we use for the frame base.  */
+      if (cache->frame_base_reg != RISCV_SP_REGNUM
+	  || cache->frame_base_offset != 0)
+	{
+	  int sp_offset;
+
+	  if (riscv_detect_end_of_function (gdbarch, original_end_pc,
+					    &sp_offset))
+	    {
+	      riscv_unwinder_debug_printf
+		("in function epilogue at %s, stack offset is %d",
+		 core_addr_to_string (original_end_pc), sp_offset);
+	      cache->frame_base_reg= RISCV_SP_REGNUM;
+	      cache->frame_base_offset = sp_offset;
+	    }
+	}
+
       /* Assign offset from old SP to all saved registers.  As we don't
 	 have the previous value for the frame base register at this
 	 point, we store the offset as the address in the trad_frame, and
-- 
2.25.4