[lldb][RISCV] add lr/sc step tests

daniilavdeev · daniilavdeev · commit b25477e8a693 · 2025-05-29T16:58:51.000Z
diff --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -1801,8 +1801,15 @@ RISCVSingleStepBreakpointLocationsPredictor::GetBreakpointLocations(
     return HandleAtomicSequence(*pc, status);
 
   if (FoundStoreConditional(inst->decoded)) {
-    // Ill-formed program, just set a breakpoint to the current position
-    return {*pc};
+    // Ill-formed atomic sequence (SC doesn't have corresponding LR
+    // instruction). Consider SC instruction like a non-atomic store and set a
+    // breakpoint at the next instruction.
+    Log *log = GetLog(LLDBLog::Unwind);
+    LLDB_LOGF(log,
+              "RISCVSingleStepBreakpointLocationsPredictor::%s: can't find "
+              "corresponding load reserve insturuction",
+              __FUNCTION__);
+    return {*pc + 4};
   }
 
   return SingleStepBreakpointLocationsPredictor::GetBreakpointLocations(status);
@@ -1821,7 +1828,7 @@ unsigned RISCVSingleStepBreakpointLocationsPredictor::GetBreakpointSize(
       last_instr_size)
     return *last_instr_size;
 
-  // Just place non-compressed software trap (EBREAK).
+  // Just place non-compressed software trap.
   return 4;
 }
 
@@ -1830,11 +1837,12 @@ RISCVSingleStepBreakpointLocationsPredictor::HandleAtomicSequence(
     lldb::addr_t pc, Status &error) {
   EmulateInstructionRISCV *riscv_emulator =
       static_cast<EmulateInstructionRISCV *>(m_emulator_up.get());
-  // Handle instuctions between LR and SC. According to unprivilleged
-  // RISC-V ISA there can be at most 16 instructions in the sequence
 
-  lldb::addr_t entry_pc = pc;
-  pc += 4; // add LR_W, LR_D instruction size
+  // Handle instructions between LR and SC. According to unprivilleged
+  // RISC-V ISA there can be at most 16 instructions in the sequence.
+
+  lldb::addr_t entry_pc = pc; // LR instruction address
+  pc += 4;                    // add LR_W, LR_D instruction size
 
   size_t atomic_length = 0;
   std::optional<DecodeResult> inst;
@@ -1856,20 +1864,39 @@ RISCVSingleStepBreakpointLocationsPredictor::HandleAtomicSequence(
            !FoundStoreConditional(inst->decoded));
 
   if (atomic_length >= s_max_atomic_sequence_length) {
-    // Ill-formed program, just set a breakpoint to the current position
+    // Ill-formed atomic sequence (LR doesn't have corresponding SC
+    // instruction). In this case consider LR like a non-atomic load instruction
+    // and set a breakpoint at the next after LR instruction.
     Log *log = GetLog(LLDBLog::Unwind);
     LLDB_LOGF(log,
               "RISCVSingleStepBreakpointLocationsPredictor::%s: can't find "
               "corresponding store conditional insturuction",
               __FUNCTION__);
-    return {entry_pc};
+    return {entry_pc + 4};
   }
 
-  bp_addrs.erase(
-      llvm::remove_if(bp_addrs,
-                      [pc](lldb::addr_t bp_addr) { return pc >= bp_addr; }),
-      bp_addrs.end());
-  bp_addrs.push_back(pc);
+  lldb::addr_t exit_pc = pc;
+
+  // Check if we have a branch to the start of the atomic sequence after SC
+  // instruction. If we have such branch, consider it as a part of the atomic
+  // sequence.
+  inst = riscv_emulator->ReadInstructionAt(exit_pc);
+  if (inst) {
+    B *branch = std::get_if<B>(&inst->decoded);
+    if (branch && (exit_pc + SignExt(branch->imm)) == entry_pc)
+      exit_pc += inst->is_rvc ? 2 : 4;
+  }
+
+  // Set breakpoints at the jump addresses of the forward branches that points
+  // after the end of the atomic sequence.
+  bp_addrs.erase(llvm::remove_if(bp_addrs,
+                                 [exit_pc](lldb::addr_t bp_addr) {
+                                   return exit_pc >= bp_addr;
+                                 }),
+                 bp_addrs.end());
+
+  // Set breakpoint at the end of atomic sequence.
+  bp_addrs.push_back(exit_pc);
   return bp_addrs;
 }
 
diff --git a/lldb/test/API/riscv/step/Makefile b/lldb/test/API/riscv/step/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/riscv/step/TestSoftwareStep.py b/lldb/test/API/riscv/step/TestSoftwareStep.py
@@ -0,0 +1,90 @@
+"""
+Test software step-inst
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestSoftwareStep(TestBase):
+    @skipIf(archs=no_match(re.compile("rv*")))
+    def test_cas(self):
+        self.build()
+        (target, process, cur_thread, bkpt) = lldbutil.run_to_name_breakpoint(
+            self, "cas"
+        )
+        entry_pc = cur_thread.GetFrameAtIndex(0).GetPC()
+
+        self.runCmd("thread step-inst")
+        self.expect(
+            "thread list",
+            substrs=["stopped", "stop reason = instruction step into"],
+        )
+
+        pc = cur_thread.GetFrameAtIndex(0).GetPC()
+        self.assertTrue((pc - entry_pc) > 0x10)
+
+    @skipIf(archs=no_match(re.compile("rv*")))
+    def test_branch_cas(self):
+        self.build(dictionary={"C_SOURCES": "branch.c", "EXE": "branch.x"})
+        (target, process, cur_thread, bkpt) = lldbutil.run_to_name_breakpoint(
+            self, "branch_cas", exe_name="branch.x"
+        )
+        entry_pc = cur_thread.GetFrameAtIndex(0).GetPC()
+
+        self.runCmd("thread step-inst")
+        self.expect(
+            "thread list",
+            substrs=["stopped", "stop reason = instruction step into"],
+        )
+
+        pc = cur_thread.GetFrameAtIndex(0).GetPC()
+        self.assertTrue((pc - entry_pc) > 0x10)
+
+    @skipIf(archs=no_match(re.compile("rv*")))
+    def test_incomplete_sequence_without_lr(self):
+        self.build(
+            dictionary={
+                "C_SOURCES": "incomplete_sequence_without_lr.c",
+                "EXE": "incomplete_lr.x",
+            }
+        )
+        (target, process, cur_thread, bkpt) = lldbutil.run_to_name_breakpoint(
+            self, "incomplete_cas", exe_name="incomplete_lr.x"
+        )
+        entry_pc = cur_thread.GetFrameAtIndex(0).GetPC()
+
+        self.runCmd("thread step-inst")
+
+        self.expect(
+            "thread list",
+            substrs=["stopped", "stop reason = instruction step into"],
+        )
+
+        pc = cur_thread.GetFrameAtIndex(0).GetPC()
+        self.assertTrue((pc - entry_pc) == 0x4)
+
+    @skipIf(archs=no_match(re.compile("rv*")))
+    def test_incomplete_sequence_without_sc(self):
+        self.build(
+            dictionary={
+                "C_SOURCES": "incomplete_sequence_without_sc.c",
+                "EXE": "incomplete_sc.x",
+            }
+        )
+        (target, process, cur_thread, bkpt) = lldbutil.run_to_name_breakpoint(
+            self, "incomplete_cas", exe_name="incomplete_sc.x"
+        )
+        entry_pc = cur_thread.GetFrameAtIndex(0).GetPC()
+
+        self.runCmd("thread step-inst")
+
+        self.expect(
+            "thread list",
+            substrs=["stopped", "stop reason = instruction step into"],
+        )
+
+        pc = cur_thread.GetFrameAtIndex(0).GetPC()
+        self.assertTrue((pc - entry_pc) == 0x4)
diff --git a/lldb/test/API/riscv/step/branch.c b/lldb/test/API/riscv/step/branch.c
@@ -0,0 +1,19 @@
+void __attribute__((naked)) branch_cas(int *a, int *b) {
+  asm volatile("1:\n\t"
+               "lr.w a2, (a0)\n\t"
+               "and a5, a2, a4\n\t"
+               "bne a5, a1, 2f\n\t"
+               "xor a5, a2, a0\n\t"
+               "and a5, a5, a4\n\t"
+               "xor a5, a2, a5\n\t"
+               "sc.w a5, a1, (a3)\n\t"
+               "beqz a5, 1b\n\t"
+               "2:\n\t"
+               "ret\n\t");
+}
+
+int main() {
+  int a = 4;
+  int b = 2;
+  branch_cas(&a, &b);
+}
diff --git a/lldb/test/API/riscv/step/incomplete_sequence_without_lr.c b/lldb/test/API/riscv/step/incomplete_sequence_without_lr.c
@@ -0,0 +1,19 @@
+void __attribute__((naked)) incomplete_cas(int *a, int *b) {
+  asm volatile("1:\n\t"
+               "sc.w a5, a1, (a3)\n\t"
+               "and a5, a2, a4\n\t"
+               "beq a5, a1, 2f\n\t"
+               "xor a5, a2, a0\n\t"
+               "and a5, a5, a4\n\t"
+               "xor a5, a2, a5\n\t"
+               "sc.w a5, a1, (a3)\n\t"
+               "bnez a5, 1b\n\t"
+               "2:\n\t"
+               "ret\n\t");
+}
+
+int main() {
+  int a = 4;
+  int b = 2;
+  incomplete_cas(&a, &b);
+}
diff --git a/lldb/test/API/riscv/step/incomplete_sequence_without_sc.c b/lldb/test/API/riscv/step/incomplete_sequence_without_sc.c
@@ -0,0 +1,18 @@
+void __attribute__((naked)) incomplete_cas(int *a, int *b) {
+  asm volatile("1:\n\t"
+               "lr.w a2, (a0)\n\t"
+               "and a5, a2, a4\n\t"
+               "beq a5, a1, 2f\n\t"
+               "xor a5, a2, a0\n\t"
+               "and a5, a5, a4\n\t"
+               "xor a5, a2, a5\n\t"
+               "bnez a5, 1b\n\t"
+               "2:\n\t"
+               "ret\n\t");
+}
+
+int main() {
+  int a = 4;
+  int b = 2;
+  incomplete_cas(&a, &b);
+}
diff --git a/lldb/test/API/riscv/step/main.c b/lldb/test/API/riscv/step/main.c
@@ -0,0 +1,19 @@
+void __attribute__((naked)) cas(int *a, int *b) {
+  asm volatile("1:\n\t"
+               "lr.w a2, (a0)\n\t"
+               "and a5, a2, a4\n\t"
+               "beq a5, a1, 2f\n\t"
+               "xor a5, a2, a0\n\t"
+               "and a5, a5, a4\n\t"
+               "xor a5, a2, a5\n\t"
+               "sc.w a5, a1, (a3)\n\t"
+               "beqz a5, 1b\n\t"
+               "2:\n\t"
+               "ret\n\t");
+}
+
+int main() {
+  int a = 4;
+  int b = 2;
+  cas(&a, &b);
+}

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+C_SOURCES := main.c`
	`2`	`+`
	`3`	`+include Makefile.rules`