Skip to content

Commit 121f967

Browse files
hyjorc1hyjorc1
committed
[JSC][WASM][Debugger] Fix active thread selection in multi-VM stops and add step tests
rdar://174187067 https://bugs.webkit.org/show_bug.cgi?id=311594 Reviewed by Keith Miller. LLDB always selected the wrong thread when multiple VMs stopped simultaneously. Root cause: LLDB resets m_selected_tid on every stop and falls back to m_threads[0] (lowest IndexID). Passive threads had StopInfoSignal(SIGSTOP) with ShouldSelect()=true, causing the selection loop to be skipped. Fix by sending signal 0 for passive threads so ShouldSelect()=false and the event thread wins selection. Also inline collectAllStoppedThreads into sendStopReplyForThread with event-thread-first ordering, fix a null-deref before vm null check, fix off-by-1 WASM offset comments in multi-vm-same-module-different-funcs.js, add step test coverage for multi-VM test cases, and remove the stale known issue from README.md. Tests: * JSTests/wasm/debugger/tests/tests.py: (MultiVMSameModuleSameFunctionTestCase.stepTest) (MultiVMSameModuleDifferentFunctionsTestCase.stepTest) Canonical link: https://commits.webkit.org/310847@main
1 parent 71df8be commit 121f967

4 files changed

Lines changed: 103 additions & 59 deletions

File tree

‎JSTests/wasm/debugger/resources/wasm/multi-vm-same-module-different-funcs.js‎

Lines changed: 30 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -24,46 +24,46 @@ const wasm = new Uint8Array([
2424
// Export function 2 as "func3"
2525
0x05, 0x66, 0x75, 0x6e, 0x63, 0x33, 0x00, 0x02,
2626

27-
// [0x30] Code section
27+
// [0x2f] Code section
2828
0x0a, // section id=10
2929
0x22, // section size=34
3030
0x03, // 3 functions
3131

3232
// Function 0: func1 (body size=10)
3333
0x0a, // body size
3434
0x00, // 0 local declarations
35-
0x01, // [0x35] nop
36-
0x41, 0x0a, // [0x36] i32.const 10
37-
0x1a, // [0x38] drop
38-
0x01, // [0x39] nop
39-
0x41, 0x14, // [0x3a] i32.const 20
40-
0x1a, // [0x3c] drop
41-
0x0b, // [0x3d] end
35+
0x01, // [0x34] nop
36+
0x41, 0x0a, // [0x35] i32.const 10
37+
0x1a, // [0x37] drop
38+
0x01, // [0x38] nop
39+
0x41, 0x14, // [0x39] i32.const 20
40+
0x1a, // [0x3b] drop
41+
0x0b, // [0x3c] end
4242

4343
// Function 1: func2 (body size=10)
4444
0x0a, // body size
4545
0x00, // 0 local declarations
46-
0x01, // [0x40] nop
47-
0x41, 0x1e, // [0x41] i32.const 30
48-
0x1a, // [0x43] drop
49-
0x01, // [0x44] nop
50-
0x41, 0x28, // [0x45] i32.const 40
51-
0x1a, // [0x47] drop
52-
0x0b, // [0x48] end
46+
0x01, // [0x3f] nop
47+
0x41, 0x1e, // [0x40] i32.const 30
48+
0x1a, // [0x42] drop
49+
0x01, // [0x43] nop
50+
0x41, 0x28, // [0x44] i32.const 40
51+
0x1a, // [0x46] drop
52+
0x0b, // [0x47] end
5353

5454
// Function 2: func3 (body size=10)
5555
0x0a, // body size
5656
0x00, // 0 local declarations
57-
0x01, // [0x4b] nop
58-
0x41, 0x32, // [0x4c] i32.const 50
59-
0x1a, // [0x4e] drop
60-
0x01, // [0x4f] nop
61-
0x41, 0x3c, // [0x50] i32.const 60
62-
0x1a, // [0x52] drop
63-
0x0b // [0x53] end
57+
0x01, // [0x4a] nop
58+
0x41, 0x32, // [0x4b] i32.const 50
59+
0x1a, // [0x4d] drop
60+
0x01, // [0x4e] nop
61+
0x41, 0x3c, // [0x4f] i32.const 60
62+
0x1a, // [0x51] drop
63+
0x0b // [0x52] end
6464
]);
6565

66-
const NUM_WORKERS = 2;
66+
const NUM_WORKERS = 1;
6767
const PRINT_INTERVAL = 1e7;
6868

6969
print("=== Multi-VM Different Functions Test ===");
@@ -88,20 +88,19 @@ const workerScript = (workerId, funcName) => `
8888
}
8989
`;
9090

91-
// Start workers, each running a different function
92-
const functions = ['func2', 'func3'];
93-
for (let i = 0; i < NUM_WORKERS; i++) {
94-
$.agent.start(workerScript(i + 1, functions[i]));
95-
print(`Started worker ${i + 1}/${NUM_WORKERS} running ${functions[i]}`);
96-
}
97-
9891
// Main thread runs func1
9992
print("Main thread starting func1 execution...");
100-
print("");
10193
const mainModule = new WebAssembly.Module(wasm);
10294
const mainInstance = new WebAssembly.Instance(mainModule, {});
10395
const func1 = mainInstance.exports.func1;
10496

97+
// Worker runs func2 (different from main thread's func1)
98+
const functions = ['func2'];
99+
for (let i = 0; i < NUM_WORKERS; i++) {
100+
$.agent.start(workerScript(i + 1, functions[i]));
101+
print(`Started worker ${i + 1}/${NUM_WORKERS} running ${functions[i]}`);
102+
}
103+
105104
let iteration = 0;
106105
for (; ;) {
107106
func1();

‎JSTests/wasm/debugger/tests/tests.py‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,7 +1567,17 @@ def execute(self):
15671567
raise Exception(f"Test failed: {e}")
15681568

15691569
def stepTest(self):
1570-
# FIXME: Add tests when thread select is full supported
1570+
self.send_lldb_command_or_raise("th list", patterns=["thread #1", "thread #2", "thread #3"])
1571+
self.send_lldb_command_or_raise("b 0x4000000100000024")
1572+
self.send_lldb_command_or_raise("c", patterns=["-> 0x4000000100000024: nop"])
1573+
patterns = [
1574+
["-> 0x4000000100000025: i32.const 42"],
1575+
["-> 0x4000000100000027: drop"],
1576+
["-> 0x4000000100000028: nop"],
1577+
["-> 0x4000000100000029: end"],
1578+
]
1579+
for pattern in patterns:
1580+
self.send_lldb_command_or_raise("si", patterns=pattern)
15711581
return
15721582

15731583

@@ -1589,7 +1599,40 @@ def execute(self):
15891599
raise Exception(f"Test failed: {e}")
15901600

15911601
def stepTest(self):
1592-
# FIXME: Add tests when thread select is full supported
1602+
self.send_lldb_command_or_raise("th list", patterns=["thread #1", "thread #2"])
1603+
1604+
self.send_lldb_command_or_raise("b 0x4000000000000034")
1605+
self.send_lldb_command_or_raise("c", patterns=["-> 0x4000000000000034: nop"])
1606+
patterns = [
1607+
["-> 0x4000000000000035: i32.const 10"],
1608+
["-> 0x4000000000000037: drop"],
1609+
["-> 0x4000000000000038: nop"],
1610+
["-> 0x4000000000000039: i32.const 20"],
1611+
["-> 0x400000000000003b: drop"],
1612+
["-> 0x400000000000003c: end"],
1613+
]
1614+
for pattern in patterns:
1615+
self.send_lldb_command_or_raise("si", patterns=pattern)
1616+
self.send_lldb_command_or_raise(
1617+
"br del -f", patterns=["All breakpoints removed. (1 breakpoint)"]
1618+
)
1619+
1620+
self.send_lldb_command_or_raise("b 0x400000010000003f")
1621+
self.send_lldb_command_or_raise("c", patterns=["-> 0x400000010000003f: nop"])
1622+
patterns = [
1623+
["-> 0x4000000100000040: i32.const 30"],
1624+
["-> 0x4000000100000042: drop"],
1625+
["-> 0x4000000100000043: nop"],
1626+
["-> 0x4000000100000044: i32.const 40"],
1627+
["-> 0x4000000100000046: drop"],
1628+
["-> 0x4000000100000047: end"],
1629+
]
1630+
for pattern in patterns:
1631+
self.send_lldb_command_or_raise("si", patterns=pattern)
1632+
self.send_lldb_command_or_raise(
1633+
"br del -f", patterns=["All breakpoints removed. (1 breakpoint)"]
1634+
)
1635+
15931636
return
15941637

15951638

‎Source/JavaScriptCore/wasm/debugger/README.md‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,11 +196,6 @@ See [RWI_ARCHITECTURE.md](./RWI_ARCHITECTURE.md) for complete setup instructions
196196
- **Location**: `WasmDebugServer.cpp:348-349`
197197
- **Solution**: Introduce various stop states and proper termination handling
198198

199-
### Multi-Thread Display in LLDB
200-
201-
- **Issue**: Thread select and stop reply protocol handlers need improvement to correctly display multi-VM data in LLDB
202-
- **Current Status**: Multi-VM stop-the-world is implemented, but thread information may not display correctly in LLDB UI
203-
204199
### X86_64 Support
205200

206201
- **Issue**: The WebAssembly debugger is currently restricted to ARM64 platforms only

‎Source/JavaScriptCore/wasm/debugger/WasmExecutionHandler.cpp‎

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -752,22 +752,6 @@ struct ThreadInfo {
752752
StringView stopReason;
753753
};
754754

755-
static Vector<ThreadInfo> collectAllStoppedThreads()
756-
{
757-
Vector<ThreadInfo> threads;
758-
VMManager::forEachVM([&](VM& vm) {
759-
auto* state = vm.debugState();
760-
if (!state->isStopped())
761-
return IterationStatus::Continue;
762-
763-
uint64_t threadId = ExecutionHandler::threadId(vm);
764-
auto stopInfo = stopReasonToInfo(*state);
765-
threads.append({ threadId, getStopPC(*state), getThreadName(*state, threadId), stopInfo.reasonSuffix });
766-
return IterationStatus::Continue;
767-
});
768-
return threads;
769-
}
770-
771755
void ExecutionHandler::sendStopReply(AbstractLocker& locker) WTF_REQUIRES_LOCK(m_lock)
772756
{
773757
sendStopReplyForThread(locker, threadId(*m_debuggee));
@@ -776,20 +760,43 @@ void ExecutionHandler::sendStopReply(AbstractLocker& locker) WTF_REQUIRES_LOCK(m
776760
void ExecutionHandler::sendStopReplyForThread(AbstractLocker& locker, uint64_t threadId) WTF_REQUIRES_LOCK(m_lock)
777761
{
778762
VM* vm = findVM(threadId);
763+
if (!vm) {
764+
dataLogLnIf(Options::verboseWasmDebugger(), "[Debugger] sendStopReplyForThread: thread ", threadId, " not found");
765+
sendErrorReply(ProtocolError::InvalidAddress);
766+
return;
767+
}
768+
779769
DebugState* state = vm->debugState();
780-
if (!vm || !state) {
770+
if (!state) {
781771
dataLogLnIf(Options::verboseWasmDebugger(), "[Debugger] sendStopReplyForThread: thread ", threadId, " not found");
782772
sendErrorReply(ProtocolError::InvalidAddress);
783773
return;
784774
}
785775

786776
RELEASE_ASSERT(state->isStopped());
787777

788-
// Gather information for the target thread
789-
Vector<ThreadInfo> allThreads = collectAllStoppedThreads();
778+
// Collect all stopped threads; swap event thread to index 0 so thread-pcs[i] aligns with threads[i].
779+
Vector<ThreadInfo> allThreads;
780+
VMManager::forEachVM([&](VM& vm) {
781+
auto* state = vm.debugState();
782+
if (!state->isStopped())
783+
return IterationStatus::Continue;
784+
uint64_t tid = ExecutionHandler::threadId(vm);
785+
allThreads.append({ tid, getStopPC(*state), getThreadName(*state, tid), stopReasonToInfo(*state).reasonSuffix });
786+
if (tid == threadId)
787+
std::swap(allThreads[0], allThreads.last());
788+
return IterationStatus::Continue;
789+
});
790790

791-
// FIXME: Report different stop reasons for active vs passive threads (currently all use same code).
792-
auto stopInfo = stopReasonToInfo(*state);
791+
// A "passive thread" is a VM that was collaterally stopped when the world was halted (its
792+
// stopReason set to Interrupted by setStopped()), as opposed to the debuggee thread that
793+
// actually triggered the stop event (breakpoint/step/trap/interrupt/new-module-load).
794+
// Passive threads get signal 0 so LLDB's ShouldSelect() returns false for them, allowing
795+
// the event thread to win thread selection in HandleProcessStateChangedEvent.
796+
bool isPassiveThread = state->stopReason == DebugState::Reason::Interrupted && m_debuggee && threadId != ExecutionHandler::threadId(*m_debuggee);
797+
auto stopInfo = isPassiveThread
798+
? StopReasonInfo { signalStopString(0), "signal"_s }
799+
: stopReasonToInfo(*state);
793800

794801
// Build packet with target thread
795802
StringBuilder reply;

0 commit comments

Comments
 (0)