Automerge: [lldb][RISCV][test] make atomic region stepping test more robust (#156506)

Currently, the tests that check stepping through atomic sequences use a
hardcoded step distance, which is unreliable because this distance
depends on LLVM's codegeneration. The relocations that clang emits can
change the distance of the step.

Additionally, it was a poor idea to compute and check the step distance
because that is not what we should actually be verifying. In the tests
we already know where execution should stop after the step - for
example, at a branch instruction - therefore, it is better to check the
opcode of the instruction rather than the step distance. The step
distance itself is not important and can sometimes be misleading.

This patch rewrites the tests, so now they checks the opcode of the
instruction after the step instead of the step distance.

Author: daniilavdeev

lldb/test/API/riscv/step/TestSoftwareStep.py

@@ -26,19 +26,26 @@ def do_sequence_test(self, filename, bkpt_name):
             substrs=["stopped", "stop reason = instruction step into"],
         )
 
-        pc = cur_thread.GetFrameAtIndex(0).GetPC()
+        # Get the instruction we stopped at
+        pc = cur_thread.GetFrameAtIndex(0).GetPCAddress()
+        inst = target.ReadInstructions(pc, 1).GetInstructionAtIndex(0)
 
-        return pc - entry_pc
+        inst_mnemonic = inst.GetMnemonic(target)
+        inst_operands = inst.GetOperands(target)
+        if not inst_operands:
+            return inst_mnemonic
 
-    @skipIf(archs=no_match("^rv.*"))
+        return f"{inst_mnemonic} {inst_operands}"
+
+    @skipIf(archs=no_match("^riscv.*"))
     def test_cas(self):
         """
         This test verifies LLDB instruction step handling of a proper lr/sc pair.
         """
-        difference = self.do_sequence_test("main", "cas")
-        self.assertEqual(difference, 0x1A)
+        instruction = self.do_sequence_test("main", "cas")
+        self.assertEqual(instruction, "nop")
 
-    @skipIf(archs=no_match("^rv.*"))
+    @skipIf(archs=no_match("^riscv.*"))
     def test_branch_cas(self):
         """
         LLDB cannot predict the actual state of registers within a critical section (i.e., inside an atomic
@@ -51,29 +58,29 @@ def test_branch_cas(self):
         test is nearly identical to the previous one, except for the branch condition, which is inverted and
         will result in a taken jump.
         """
-        difference = self.do_sequence_test("branch", "branch_cas")
-        self.assertEqual(difference, 0x1A)
+        instruction = self.do_sequence_test("branch", "branch_cas")
+        self.assertEqual(instruction, "ret")
 
-    @skipIf(archs=no_match("^rv.*"))
+    @skipIf(archs=no_match("^riscv.*"))
     def test_incomplete_sequence_without_lr(self):
         """
         This test verifies the behavior of a standalone sc instruction without a preceding lr. Since the sc
         lacks the required lr pairing, LLDB should treat it as a non-atomic store rather than part of an
         atomic sequence.
         """
-        difference = self.do_sequence_test(
+        instruction = self.do_sequence_test(
             "incomplete_sequence_without_lr", "incomplete_cas"
         )
-        self.assertEqual(difference, 0x4)
+        self.assertEqual(instruction, "and a5, a2, a4")
 
-    @skipIf(archs=no_match("^rv.*"))
+    @skipIf(archs=no_match("^riscv.*"))
     def test_incomplete_sequence_without_sc(self):
         """
         This test checks the behavior of a standalone lr instruction without a subsequent sc. Since the lr
         lacks its required sc counterpart, LLDB should treat it as a non-atomic load rather than part of an
         atomic sequence.
         """
-        difference = self.do_sequence_test(
+        instruction = self.do_sequence_test(
             "incomplete_sequence_without_sc", "incomplete_cas"
         )
-        self.assertEqual(difference, 0x4)
+        self.assertEqual(instruction, "and a5, a2, a4")

lldb/test/API/riscv/step/branch.c

@@ -11,6 +11,7 @@ void __attribute__((naked)) branch_cas(int *a, int *b) {
                "xor a5, a2, a5\n\t"
                "sc.w a5, a1, (a3)\n\t"
                "beqz a5, 1b\n\t"
+               "nop\n\t"
                "2:\n\t"
                "ret\n\t");
 }

lldb/test/API/riscv/step/incomplete_sequence_without_lr.c

@@ -1,7 +1,7 @@
 void __attribute__((naked)) incomplete_cas(int *a, int *b) {
   // Stop at the first instruction (an sc without a corresponding lr), then make
   // a step instruction and ensure that execution stops at the next instruction
-  // (and).
+  // (and a5, a2, a4).
   asm volatile("1:\n\t"
                "sc.w a5, a1, (a3)\n\t"
                "and a5, a2, a4\n\t"

lldb/test/API/riscv/step/incomplete_sequence_without_sc.c

@@ -1,7 +1,7 @@
 void __attribute__((naked)) incomplete_cas(int *a, int *b) {
   // Stop at the first instruction (an lr without a corresponding sc), then make
   // a step instruction and ensure that execution stops at the next instruction
-  // (and).
+  // (and a5, a2, a4).
   asm volatile("1:\n\t"
                "lr.w a2, (a0)\n\t"
                "and a5, a2, a4\n\t"

lldb/test/API/riscv/step/main.c

@@ -1,7 +1,7 @@
 void __attribute__((naked)) cas(int *a, int *b) {
   // This atomic sequence implements a copy-and-swap function. This test should
-  // at the first instruction, and after step instruction, we should stop at the
-  // end of the sequence (on the ret instruction).
+  // stop at the first instruction, and after step instruction, we should stop
+  // at the end of the sequence (on the ret instruction).
   asm volatile("1:\n\t"
                "lr.w a2, (a0)\n\t"
                "and a5, a2, a4\n\t"
@@ -11,6 +11,7 @@ void __attribute__((naked)) cas(int *a, int *b) {
                "xor a5, a2, a5\n\t"
                "sc.w a5, a1, (a3)\n\t"
                "beqz a5, 1b\n\t"
+               "nop\n\t"
                "2:\n\t"
                "ret\n\t");
 }