[lldb] Implement RISCV function unwinding using instruction emulation

[satyajanga]

We noticed this issue when processing a Meta internal RISCV coredump, currently the RISCV instruction emulation is not handling the Prologue and Epilogue instructions.

Also function unwinding using the instruction emulation is also not implemented.

This PR handles both of these issues.

NOTE: Not sure of the historic reason why this is done this way. This is done in https://reviews.llvm.org/D131759
in contrast MIPS, PPC, ARM all support this.

Test Plan:
Unfortunately there is no easy way to add testing for this. No RISCV hardware at the disposal. I welcome the suggestions.

cc: @clayborg

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (satyajanga)

Changes

We noticed this issue when processing a Meta internal RISCV coredump, currently the RISCV instruction emulation is not handling the Prologue and Epilogue instructions.

Also function unwinding using the instruction emulation is also not implemented.

This PR handles both of these issues.

NOTE: Not sure of the historic reason why this is done this way. This is done in https://reviews.llvm.org/D131759
in contrast MIPS, PPC, ARM all support this.

Test Plan:
Unfortunately there is no easy way to add testing for this. No RISCV hardware at the disposal. I welcome the suggestions.

---

Full diff: https://github.com/llvm/llvm-project/pull/147434.diff

2 Files Affected:

• (modified) lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp (+20)
• (modified) lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h (+7-4)

diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
index 2adde02aca3a1..90537587c0b23 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -1899,4 +1899,24 @@ RISCVSingleStepBreakpointLocationsPredictor::HandleAtomicSequence(
   return bp_addrs;
 }
 
+bool EmulateInstructionRISCV::CreateFunctionEntryUnwind(
+    UnwindPlan &unwind_plan) {
+  unwind_plan.Clear();
+  unwind_plan.SetRegisterKind(eRegisterKindLLDB);
+
+  UnwindPlan::Row row;
+
+  // Our previous Call Frame Address is the stack pointer
+  row.GetCFAValue().SetIsRegisterPlusOffset(gpr_sp_riscv, 0);
+  row.SetRegisterLocationToSame(gpr_fp_riscv, /*must_replace=*/false);
+
+  unwind_plan.AppendRow(std::move(row));
+  unwind_plan.SetSourceName("EmulateInstructionRISCV");
+  unwind_plan.SetSourcedFromCompiler(eLazyBoolNo);
+  unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolYes);
+  unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);
+  unwind_plan.SetReturnAddressRegister(gpr_ra_riscv);
+  return true;
+}
+
 } // namespace lldb_private
diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
index 3578a4ab03053..f5692efb03bd9 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
@@ -57,11 +57,12 @@ class EmulateInstructionRISCV : public EmulateInstruction {
 
   static bool SupportsThisInstructionType(InstructionType inst_type) {
     switch (inst_type) {
-    case eInstructionTypeAny:
-    case eInstructionTypePCModifying:
+    case lldb_private::eInstructionTypeAny:
+    case lldb_private::eInstructionTypePrologueEpilogue:
       return true;
-    case eInstructionTypePrologueEpilogue:
-    case eInstructionTypeAll:
+
+    case lldb_private::eInstructionTypePCModifying:
+    case lldb_private::eInstructionTypeAll:
       return false;
     }
     llvm_unreachable("Fully covered switch above!");
@@ -94,6 +95,8 @@ class EmulateInstructionRISCV : public EmulateInstruction {
   std::optional<RegisterInfo> GetRegisterInfo(lldb::RegisterKind reg_kind,
                                               uint32_t reg_num) override;
 
+  bool CreateFunctionEntryUnwind(UnwindPlan &unwind_plan) override;
+
   std::optional<DecodeResult> ReadInstructionAt(lldb::addr_t addr);
   std::optional<DecodeResult> Decode(uint32_t inst);
   bool Execute(DecodeResult inst, bool ignore_cond);

[DavidSpickett]

Unfortunately there is no easy way to add testing for this. No RISCV hardware at the disposal. I welcome the suggestions.

Make a core file that does not include any internal data and then it can be tested on any system. If you have to satisfy internal policies around that, consider obj2yaml-ing the file, and then either using the yaml in the test, or just using the yaml format as a way to hack out any internal data then convert it back to an object.

I know I personally ok'd a RISC-V core file change before that did not include a test, but that was a mistake and I should have pushed harder for a test case.

(for the avoidance of doubt, this next part is from the perspective of an upstream maintainer of LLDB who cares purely about the health of this project overall, and it does not represent the opinion of my employer)

Side note: If your organisation is using RISC-V tools more and more, consider supporting upstream testing of those tools. For example via Alex Bradbury's work on testing clang and llvm using qemu-system. There's only so long RISC-V LLDB can go on with spot testing, eventually it needs something comprehensive.