Author: dlav-sc Date: 2025-09-03T19:22:55+03:00 New Revision: 3b3fc701d8f83d4ca30ee1c818fb7687336ac178
URL: https://github.com/llvm/llvm-project/commit/3b3fc701d8f83d4ca30ee1c818fb7687336ac178 DIFF: https://github.com/llvm/llvm-project/commit/3b3fc701d8f83d4ca30ee1c818fb7687336ac178.diff LOG: [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. Added: Modified: lldb/test/API/riscv/step/TestSoftwareStep.py lldb/test/API/riscv/step/branch.c lldb/test/API/riscv/step/incomplete_sequence_without_lr.c lldb/test/API/riscv/step/incomplete_sequence_without_sc.c lldb/test/API/riscv/step/main.c Removed: ################################################################################ diff --git a/lldb/test/API/riscv/step/TestSoftwareStep.py b/lldb/test/API/riscv/step/TestSoftwareStep.py index 279c4c1b797e0..0544d2102d0f9 100644 --- a/lldb/test/API/riscv/step/TestSoftwareStep.py +++ b/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. """ - diff erence = self.do_sequence_test("main", "cas") - self.assertEqual( diff erence, 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. """ - diff erence = self.do_sequence_test("branch", "branch_cas") - self.assertEqual( diff erence, 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. """ - diff erence = self.do_sequence_test( + instruction = self.do_sequence_test( "incomplete_sequence_without_lr", "incomplete_cas" ) - self.assertEqual( diff erence, 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. """ - diff erence = self.do_sequence_test( + instruction = self.do_sequence_test( "incomplete_sequence_without_sc", "incomplete_cas" ) - self.assertEqual( diff erence, 0x4) + self.assertEqual(instruction, "and a5, a2, a4") diff --git a/lldb/test/API/riscv/step/branch.c b/lldb/test/API/riscv/step/branch.c index 79bf0ca005db1..93d6c51ec75e0 100644 --- a/lldb/test/API/riscv/step/branch.c +++ b/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"); } diff --git a/lldb/test/API/riscv/step/incomplete_sequence_without_lr.c b/lldb/test/API/riscv/step/incomplete_sequence_without_lr.c index eda313c6d69c4..ceb906f7f50e9 100644 --- a/lldb/test/API/riscv/step/incomplete_sequence_without_lr.c +++ b/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" diff --git a/lldb/test/API/riscv/step/incomplete_sequence_without_sc.c b/lldb/test/API/riscv/step/incomplete_sequence_without_sc.c index 952dc7d2804e8..3f1bf64b85c54 100644 --- a/lldb/test/API/riscv/step/incomplete_sequence_without_sc.c +++ b/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" diff --git a/lldb/test/API/riscv/step/main.c b/lldb/test/API/riscv/step/main.c index 35e8aee2cae4b..6649034aa0a37 100644 --- a/lldb/test/API/riscv/step/main.c +++ b/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"); } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
