Skip to content

Commit 3b3fc70

Browse files
authored
[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.
1 parent 5545a92 commit 3b3fc70

File tree

5 files changed

+27
-18
lines changed

5 files changed

+27
-18
lines changed

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

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,26 @@ def do_sequence_test(self, filename, bkpt_name):
2626
substrs=["stopped", "stop reason = instruction step into"],
2727
)
2828

29-
pc = cur_thread.GetFrameAtIndex(0).GetPC()
29+
# Get the instruction we stopped at
30+
pc = cur_thread.GetFrameAtIndex(0).GetPCAddress()
31+
inst = target.ReadInstructions(pc, 1).GetInstructionAtIndex(0)
3032

31-
return pc - entry_pc
33+
inst_mnemonic = inst.GetMnemonic(target)
34+
inst_operands = inst.GetOperands(target)
35+
if not inst_operands:
36+
return inst_mnemonic
3237

33-
@skipIf(archs=no_match("^rv.*"))
38+
return f"{inst_mnemonic} {inst_operands}"
39+
40+
@skipIf(archs=no_match("^riscv.*"))
3441
def test_cas(self):
3542
"""
3643
This test verifies LLDB instruction step handling of a proper lr/sc pair.
3744
"""
38-
difference = self.do_sequence_test("main", "cas")
39-
self.assertEqual(difference, 0x1A)
45+
instruction = self.do_sequence_test("main", "cas")
46+
self.assertEqual(instruction, "nop")
4047

41-
@skipIf(archs=no_match("^rv.*"))
48+
@skipIf(archs=no_match("^riscv.*"))
4249
def test_branch_cas(self):
4350
"""
4451
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):
5158
test is nearly identical to the previous one, except for the branch condition, which is inverted and
5259
will result in a taken jump.
5360
"""
54-
difference = self.do_sequence_test("branch", "branch_cas")
55-
self.assertEqual(difference, 0x1A)
61+
instruction = self.do_sequence_test("branch", "branch_cas")
62+
self.assertEqual(instruction, "ret")
5663

57-
@skipIf(archs=no_match("^rv.*"))
64+
@skipIf(archs=no_match("^riscv.*"))
5865
def test_incomplete_sequence_without_lr(self):
5966
"""
6067
This test verifies the behavior of a standalone sc instruction without a preceding lr. Since the sc
6168
lacks the required lr pairing, LLDB should treat it as a non-atomic store rather than part of an
6269
atomic sequence.
6370
"""
64-
difference = self.do_sequence_test(
71+
instruction = self.do_sequence_test(
6572
"incomplete_sequence_without_lr", "incomplete_cas"
6673
)
67-
self.assertEqual(difference, 0x4)
74+
self.assertEqual(instruction, "and a5, a2, a4")
6875

69-
@skipIf(archs=no_match("^rv.*"))
76+
@skipIf(archs=no_match("^riscv.*"))
7077
def test_incomplete_sequence_without_sc(self):
7178
"""
7279
This test checks the behavior of a standalone lr instruction without a subsequent sc. Since the lr
7380
lacks its required sc counterpart, LLDB should treat it as a non-atomic load rather than part of an
7481
atomic sequence.
7582
"""
76-
difference = self.do_sequence_test(
83+
instruction = self.do_sequence_test(
7784
"incomplete_sequence_without_sc", "incomplete_cas"
7885
)
79-
self.assertEqual(difference, 0x4)
86+
self.assertEqual(instruction, "and a5, a2, a4")

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ void __attribute__((naked)) branch_cas(int *a, int *b) {
1111
"xor a5, a2, a5\n\t"
1212
"sc.w a5, a1, (a3)\n\t"
1313
"beqz a5, 1b\n\t"
14+
"nop\n\t"
1415
"2:\n\t"
1516
"ret\n\t");
1617
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
void __attribute__((naked)) incomplete_cas(int *a, int *b) {
22
// Stop at the first instruction (an sc without a corresponding lr), then make
33
// a step instruction and ensure that execution stops at the next instruction
4-
// (and).
4+
// (and a5, a2, a4).
55
asm volatile("1:\n\t"
66
"sc.w a5, a1, (a3)\n\t"
77
"and a5, a2, a4\n\t"

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
void __attribute__((naked)) incomplete_cas(int *a, int *b) {
22
// Stop at the first instruction (an lr without a corresponding sc), then make
33
// a step instruction and ensure that execution stops at the next instruction
4-
// (and).
4+
// (and a5, a2, a4).
55
asm volatile("1:\n\t"
66
"lr.w a2, (a0)\n\t"
77
"and a5, a2, a4\n\t"

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
void __attribute__((naked)) cas(int *a, int *b) {
22
// This atomic sequence implements a copy-and-swap function. This test should
3-
// at the first instruction, and after step instruction, we should stop at the
4-
// end of the sequence (on the ret instruction).
3+
// stop at the first instruction, and after step instruction, we should stop
4+
// at the end of the sequence (on the ret instruction).
55
asm volatile("1:\n\t"
66
"lr.w a2, (a0)\n\t"
77
"and a5, a2, a4\n\t"
@@ -11,6 +11,7 @@ void __attribute__((naked)) cas(int *a, int *b) {
1111
"xor a5, a2, a5\n\t"
1212
"sc.w a5, a1, (a3)\n\t"
1313
"beqz a5, 1b\n\t"
14+
"nop\n\t"
1415
"2:\n\t"
1516
"ret\n\t");
1617
}

0 commit comments

Comments
 (0)