[Lldb-commits] [lldb] [lldb][RISCV] Don't abort on an SC instruction outside a recognized LR/SC sequence (PR #238120)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-risc-v

Author: Jarosław Pelczar (jplcz)

<details>
<summary>Changes</summary>

`EmulateInstructionRISCV::Executor` handled SC_W and SC_D with `llvm_unreachable("should be handled in AtomicSequence")`, assuming the LR handler (AtomicSequence) always consumes the matching SC. That only holds for the exact `lr; bne exit; sc; bne entry` shape. When a function contains an LR/SC loop of another shape, `AtomicSequence` returns false, emulation continues linearly and reaches a bare SC. Unwinding then aborts (assertions enabled) or crashes with a SIGSEGV (release builds).

Return false for a lone SC_W/SC_D, like other instructions that cannot be emulated, so the unwinder falls back to the other unwind plans.

Minimal reproduction: decode and execute a bare `sc.w a3,a4,(s0)` or `sc.d a3,a4,(s0)` with `EmulateInstructionRISCV`. Before this change it hits llvm_unreachable.

Reproduction in the debugger (found with bare-metal RV64 firmware over gdb-remote):
1. Build a RISC-V function containing an LR/SC loop that is not exactly lr/bne/sc/bne (for example a compiler-generated atomic loop with other instructions in between).
2. Attach LLDB to a target stopped in a frame of that function and select the thread or run "bt".
3. UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly emulates the function. FuncUnwinders::GetUnwindPlanAtNonCallSite always computes the assembly plan, so .eh_frame/.debug_frame cannot avoid it.

Backtrace:

```
  llvm_unreachable_internal
  Executor::operator()(SC_W)
  EmulateInstructionRISCV::Execute
  UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly
  FuncUnwinders::GetAssemblyUnwindPlan
  RegisterContextUnwind::InitializeZerothFrame
```

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


2 Files Affected:

- (modified) lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp (+5-6) 
- (modified) lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp (+7) 


``````````diff
diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
index af2c1520720b45..5b5d352ab76a3f 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -1245,12 +1245,11 @@ class Executor {
   // RV32A & RV64A (The standard atomic instruction extension) //
   bool operator()(LR_W) { return AtomicSequence(m_emu); }
   bool operator()(LR_D) { return AtomicSequence(m_emu); }
-  bool operator()(SC_W) {
-    llvm_unreachable("should be handled in AtomicSequence");
-  }
-  bool operator()(SC_D) {
-    llvm_unreachable("should be handled in AtomicSequence");
-  }
+  // A well-formed LR/SC loop is consumed by AtomicSequence. An SC reached on its
+  // own (a differently shaped loop, or analysis starting mid-function) cannot be
+  // emulated, so report failure instead of aborting the debugger.
+  bool operator()(SC_W) { return false; }
+  bool operator()(SC_D) { return false; }
   bool operator()(AMOSWAP_W inst) {
     return AtomicSwap<AMOSWAP_W, uint32_t>(m_emu, inst, 4, SextW);
   }
diff --git a/lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp b/lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
index 7d865c5c49334a..79bb78a28d8388 100644
--- a/lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
+++ b/lldb/unittests/Instruction/RISCV/TestRISCVEmulator.cpp
@@ -277,6 +277,13 @@ TEST_F(RISCVEmulatorTester, TestAtomicSequence) {
   ASSERT_EQ(this->gpr.gpr[0], uint64_t(16));
 }
 
+// An SC outside the recognized LR/SC sequence must fail to emulate, not abort.
+TEST_F(RISCVEmulatorTester, TestLoneStoreConditional) {
+  this->WritePC(0x0);
+  ASSERT_FALSE(this->DecodeAndExecute(0x18e426af, false)); // sc.w a3,a4,(s0)
+  ASSERT_FALSE(this->DecodeAndExecute(0x18e436af, false)); // sc.d a3,a4,(s0)
+}
+
 struct TestDecode {
   uint32_t inst;
   RISCVInst inst_type;

``````````

</details>


https://github.com/llvm/llvm-project/pull/238120