Skip to content

Commit bdc7d2c

Browse files
dcroussodcrousso
committed
Web Inspector: Debugger: symbolic breakpoints should work with native functions
https://bugs.webkit.org/show_bug.cgi?id=243716 <rdar://problem/98705760> Reviewed by Yusuke Suzuki. * Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.h: * Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp: (Inspector::InspectorDebuggerAgent::internalDisable): (Inspector::functionName): (Inspector::ReplacedThunk::~ReplacedThunk): Added. (Inspector::ReplacedThunk::operator==): Added. (Inspector::replacedThunks()): Added. (Inspector::InspectorDebuggerAgent::addSymbolicBreakpoint): (Inspector::InspectorDebuggerAgent::removeSymbolicBreakpoint): (Inspector::InspectorDebuggerAgent::didCreateNativeExecutable): Added. (Inspector::InspectorDebuggerAgent::willCallNativeExecutable): Added. (Inspector::InspectorDebuggerAgent::clearInspectorBreakpointState): When creating a new symbolic breakpoint (i.e. `addSymbolicBreakpoint`), walk the entire heap for all `NativeExecutable`, instrumenting any that are newly matched (i.e. it had no other matching symbolic breakpoints) as though it was just created by replacing the "call" and "construct" thunks with "debugger" copies (see below). Do the same for any newly created `NativeExecutable`. When any `NativeExecutable` is about to "call" or "construct", check first if there are any matching symbolic breakpoints, pausing immediately if so. This ensures we pause before invoking the `NativeExecutable` (whereas non-native symbolic breakpoints will pause on the first expression inside the JS function). When removing symbolic breakpoints (or closing Web Inspector), undo any replaced thunks. * Source/JavaScriptCore/debugger/Debugger.h: (JSC::Debugger::Observer::didCreateNativeExecutable): Added. (JSC::Debugger::Observer::willCallNativeExecutable): Added. (JSC::Debugger::doneProcessingDebuggerEvents const): Deleted. * Source/JavaScriptCore/debugger/Debugger.cpp: (JSC::Debugger::Debugger): (JSC::Debugger::~Debugger): (JSC::Debugger::didCreateNativeExecutable): Added. (JSC::Debugger::willCallNativeExecutable): Added. (JSC::Debugger::canDispatchFunctionToObservers const): (JSC::Debugger::dispatchFunctionToObservers): * Source/JavaScriptCore/runtime/VM.h: * Source/JavaScriptCore/runtime/VM.cpp: (JSC::VM::addDebugger): Added. (JSC::VM::removeDebugger): Added. (JSC::VM::forEachDebugger): Added. Add piping to allow other JSC objects to talk to `InspectorDebuggerAgent` without having to know about it directly. * Source/JavaScriptCore/jit/JITThunks.h: * Source/JavaScriptCore/jit/JITThunks.cpp: (JSC::JITThunks::ctiNativeCallWithDebuggerHook): Added. (JSC::JITThunks::ctiNativeConstructWithDebuggerHook): Added. * Source/JavaScriptCore/jit/ThunkGenerators.h: * Source/JavaScriptCore/jit/ThunkGenerators.cpp: (JSC::nativeForGenerator): (JSC::nativeCallWithDebuggerHookGenerator): Added. (JSC::nativeConstructWithDebuggerHookGenerator): Added. * Source/JavaScriptCore/jit/JITOperations.h: * Source/JavaScriptCore/jit/JITOperations.cpp: (JSC::operationDebuggerWillCallNativeExecutable): Added. Create "debugger" copies of the "call" and "construct" thunks that call a new JIT operation to notify the `InspectorDebuggerAgent` before invoking the native code so that it can pause if it has matching symbolic breakpoints. * Source/JavaScriptCore/jit/JITCode.h: (JSC::JITCode::canSwapCodeRefForDebugger const): Added. (JSC::NativeJITCode::canSwapCodeRefForDebugger const): Added. * Source/JavaScriptCore/jit/JITCode.cpp: (JSC::JITCode::swapCodeRefForDebugger): Added. (JSC::JITCodeWithCodeRef::swapCodeRefForDebugger): Added. * Source/JavaScriptCore/runtime/ExecutableBase.h: (JSC::ExecutableBase::generatedJITCodeWithArityCheckForCall const): Added. (JSC::ExecutableBase::generatedJITCodeWithArityCheckForConstruct const): Added. (JSC::ExecutableBase::generatedJITCodeWithArityCheckFor const): Added. (JSC::ExecutableBase::swapGeneratedJITCodeWithArityCheckForDebugger): Added. (JSC::ExecutableBase::swapGeneratedJITCodeForCallWithArityCheckForDebugger): Added. (JSC::ExecutableBase::swapGeneratedJITCodeForConstructWithArityCheckForDebugger): Added. Add utility functions to allow the `InspectorDebuggerAgent` to swap JIT code without having to regenerate the entire object. * Source/JavaScriptCore/runtime/NativeExecutable.cpp: (JSC::NativeExecutable::create): Since `NativeExeutable` are shared for all `JSGlobalObject` in a `VM`, iterate the global list of all `Debugger` to find all matching the `VM`, informing each about this new `NativeExecutable`. Each `InspectorDebuggerAgent` will similarly pull from a global list of all replaced thunks so as to avoid replacing the thunks of a single `NativeExecutable` more than once. * Source/JavaScriptCore/inspector/JSGlobalObjectDebugger.cpp: (Inspector::JSGlobalObjectDebugger::runEventLoopWhilePaused): * Source/WebCore/inspector/PageDebugger.cpp: (WebCore::PageDebugger::runEventLoopWhilePausedInternal): * Source/WebCore/inspector/WorkerDebugger.cpp: (WebCore::WorkerDebugger::runEventLoopWhilePaused): I'm not entirely sure what's going on here, but all the local testing I did had problems where `doneProcessingDebuggerEvents()` would return `false` even though `m_doneProcessingDebuggerEvents` had been set to `true`. Using `std::atomic<bool>` didn't work either (this was more of a hailmary than anything). Directly using the variable resolved this. * Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js: (WI.DebuggerManager.prototype._setSymbolicBreakpoint): (WI.DebuggerManager.prototype._handleSymbolicBreakpointActionsChanged): Drive-by: Use `this` instead of `WI.debuggerManager` since we're already inside `WI.DebuggerManager`. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-dom-exact-case-insensitive-expected.txt: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-dom-exact-case-insensitive.html: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-dom-exact-case-sensitive-expected.txt: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-dom-exact-case-sensitive.html: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-dom-regex-case-insensitive-expected.txt: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-dom-regex-case-insensitive.html: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-dom-regex-case-sensitive-expected.txt: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-dom-regex-case-sensitive.html: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-js-exact-case-insensitive-expected.txt: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-js-exact-case-insensitive.html: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-js-exact-case-sensitive-expected.txt: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-js-exact-case-sensitive.html: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-js-regex-case-insensitive-expected.txt: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-js-regex-case-insensitive.html: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-js-regex-case-sensitive-expected.txt: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-native-js-regex-case-sensitive.html: Added. * LayoutTests/inspector/debugger/symbolic-breakpoint-exact-case-insensitive.html: * LayoutTests/inspector/debugger/symbolic-breakpoint-exact-case-sensitive.html: * LayoutTests/inspector/debugger/symbolic-breakpoint-regex-case-insensitive.html: * LayoutTests/inspector/debugger/symbolic-breakpoint-regex-case-sensitive.html: Drive-by: Actually add the `WI.SymbolicBreakpoint` to the `WI.DebuggerManager` instead of relying on the event listeners added to `WI.SymbolicBreakpoint` (as opposed to a specific instance). Canonical link: https://commits.webkit.org/253791@main
1 parent 5b6026c commit bdc7d2c

40 files changed

Lines changed: 2666 additions & 37 deletions

File tree

‎LayoutTests/inspector/debugger/symbolic-breakpoint-exact-case-insensitive.html‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@
5555
InspectorTest.BreakpointOptions.addTestCases(suite, {
5656
testCaseNamePrefix: "ExactCaseInsensitive.Hit.",
5757
createBreakpoint() {
58-
return new WI.SymbolicBreakpoint("tRiGgErSyMbOlIcBrEaKpOiNt", {caseSensitive: false});
58+
let breakpoint = new WI.SymbolicBreakpoint("tRiGgErSyMbOlIcBrEaKpOiNt", {caseSensitive: false});
59+
WI.debuggerManager.addSymbolicBreakpoint(breakpoint);
60+
return breakpoint;
5961
},
6062
triggerBreakpoint,
6163
});

‎LayoutTests/inspector/debugger/symbolic-breakpoint-exact-case-sensitive.html‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@
5555
InspectorTest.BreakpointOptions.addTestCases(suite, {
5656
testCaseNamePrefix: "ExactCaseSensitive.Hit.",
5757
createBreakpoint() {
58-
return new WI.SymbolicBreakpoint("triggerSymbolicBreakpoint");
58+
let breakpoint = new WI.SymbolicBreakpoint("triggerSymbolicBreakpoint");
59+
WI.debuggerManager.addSymbolicBreakpoint(breakpoint);
60+
return breakpoint;
5961
},
6062
triggerBreakpoint,
6163
});

‎LayoutTests/inspector/debugger/symbolic-breakpoint-native-dom-exact-case-insensitive-expected.txt‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
CONSOLE MESSAGE: BREAKPOINT ACTION LOG 1
2+
CONSOLE MESSAGE: BREAKPOINT ACTION LOG 2
3+
CONSOLE MESSAGE: BREAKPOINT ACTION LOG 3
4+
CONSOLE MESSAGE: BREAKPOINT ACTION LOG 4
5+
Tests for SymbolicBreakpoint.
6+
7+
8+
== Running test suite: SymbolicBreakpoint
9+
-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Hit.Options.Condition
10+
11+
Setting condition to 'false'...
12+
13+
Triggering breakpoint...
14+
PASS: Should not pause.
15+
16+
Triggering breakpoint...
17+
PASS: Should not pause.
18+
19+
Setting condition to 'true'...
20+
21+
Triggering breakpoint...
22+
PASS: Should pause.
23+
24+
Triggering breakpoint...
25+
PASS: Should pause.
26+
27+
-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Hit.Options.Condition.ConsoleCommandLineAPI
28+
Adding saved console value 'false'...
29+
30+
Setting condition to saved console value...
31+
32+
Triggering breakpoint...
33+
PASS: Should not pause.
34+
35+
Triggering breakpoint...
36+
PASS: Should not pause.
37+
38+
Adding saved console value 'true'...
39+
Setting condition to saved console value...
40+
41+
Triggering breakpoint...
42+
PASS: Should pause.
43+
44+
Triggering breakpoint...
45+
PASS: Should pause.
46+
47+
-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Hit.Options.IgnoreCount
48+
49+
Setting ignoreCount to '2'...
50+
51+
Triggering breakpoint...
52+
PASS: Should not pause.
53+
54+
Triggering breakpoint...
55+
PASS: Should not pause.
56+
57+
Triggering breakpoint...
58+
PASS: Should pause.
59+
60+
Triggering breakpoint...
61+
PASS: Should pause.
62+
63+
-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Hit.Options.Action.Log
64+
65+
Adding log action...
66+
67+
Triggering breakpoint...
68+
PASS: Should execute breakpoint action.
69+
PASS: Should pause.
70+
71+
Editing log action...
72+
73+
Triggering breakpoint...
74+
PASS: Should execute breakpoint action.
75+
PASS: Should pause.
76+
77+
Editing log action...
78+
Enabling auto-continue...
79+
80+
Triggering breakpoint...
81+
PASS: Should execute breakpoint action.
82+
PASS: Should not pause.
83+
84+
Editing log action...
85+
86+
Triggering breakpoint...
87+
PASS: Should execute breakpoint action.
88+
PASS: Should not pause.
89+
90+
-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Hit.Options.Actions.Evaluate
91+
92+
Adding evaluate action...
93+
94+
Triggering breakpoint...
95+
PASS: Should execute breakpoint action.
96+
PASS: Should pause.
97+
98+
Editing evaluate action...
99+
100+
Triggering breakpoint...
101+
PASS: Should execute breakpoint action.
102+
PASS: Should pause.
103+
104+
Editing evaluate action...
105+
Enabling auto-continue...
106+
107+
Triggering breakpoint...
108+
PASS: Should execute breakpoint action.
109+
PASS: Should not pause.
110+
111+
Editing evaluate action...
112+
113+
Triggering breakpoint...
114+
PASS: Should execute breakpoint action.
115+
PASS: Should not pause.
116+
117+
-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Hit.Options.Actions.Evaluate.ConsoleCommandLineAPI
118+
Adding saved console value '1'...
119+
120+
Adding evaluate action using saved console value...
121+
122+
Triggering breakpoint...
123+
PASS: Should execute breakpoint action.
124+
PASS: Should pause.
125+
126+
Adding saved console value '2'...
127+
Editing evaluate action using saved console value...
128+
129+
Triggering breakpoint...
130+
PASS: Should execute breakpoint action.
131+
PASS: Should pause.
132+
133+
Adding saved console value '3'...
134+
Editing evaluate action using saved console value...
135+
Enabling auto-continue...
136+
137+
Triggering breakpoint...
138+
PASS: Should execute breakpoint action.
139+
PASS: Should not pause.
140+
141+
Adding saved console value '4'...
142+
Editing evaluate action using saved console value...
143+
144+
Triggering breakpoint...
145+
PASS: Should execute breakpoint action.
146+
PASS: Should not pause.
147+
148+
-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Miss
149+
Adding breakpoint...
150+
Triggering breakpoint...
151+
PASS: Should not pause.
152+
153+
Removing breakpoint...
154+
Triggering breakpoint...
155+
PASS: Should not pause.
156+
157+
-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Disabled
158+
Adding breakpoint...
159+
Triggering breakpoint...
160+
PASS: Should not pause.
161+
162+
Removing breakpoint...
163+
Triggering breakpoint...
164+
PASS: Should not pause.
165+

‎LayoutTests/inspector/debugger/symbolic-breakpoint-native-dom-exact-case-insensitive.html‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
5+
<script src="resources/breakpoint-options-utilities.js"></script>
6+
<script>
7+
function triggerSymbolicBreakpoint() {
8+
document.body.addEventListener("click", (event) => {
9+
event.preventDefault();
10+
TestPage.dispatchEventToFrontend("TestPage-SymbolicBreakpoint");
11+
}, {once: true});
12+
document.body.click();
13+
}
14+
15+
function test()
16+
{
17+
WI.settings.experimentalEnableSymbolicBreakpoints.value = true;
18+
19+
function triggerBreakpoint() {
20+
return Promise.all([
21+
InspectorTest.awaitEvent("TestPage-SymbolicBreakpoint"),
22+
InspectorTest.evaluateInPage(`triggerSymbolicBreakpoint()`),
23+
]);
24+
}
25+
26+
let suite = InspectorTest.createAsyncSuite("SymbolicBreakpoint");
27+
28+
InspectorTest.BreakpointOptions.addTestCases(suite, {
29+
testCaseNamePrefix: "ExactCaseInsensitive.Native.DOM.Hit.",
30+
createBreakpoint() {
31+
let breakpoint = new WI.SymbolicBreakpoint("pReVeNtDeFaUlT", {caseSensitive: false});
32+
WI.debuggerManager.addSymbolicBreakpoint(breakpoint);
33+
return breakpoint;
34+
},
35+
triggerBreakpoint,
36+
});
37+
38+
suite.addTestCase({
39+
name: "SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Miss",
40+
async test() {
41+
let pauseCount = 0;
42+
let pausedListener = WI.debuggerManager.addEventListener(WI.DebuggerManager.Event.Paused, (event) => {
43+
++pauseCount;
44+
WI.debuggerManager.resume();
45+
});
46+
47+
InspectorTest.log("Adding breakpoint...");
48+
let breakpoint = new WI.SymbolicBreakpoint("pReVeNt", {caseSensitive: false});
49+
WI.debuggerManager.addSymbolicBreakpoint(breakpoint);
50+
InspectorTest.assert(!WI.debuggerManager.symbolicBreakpointsForSymbol("preventDefault").length, "Should not match breakpoint.");
51+
52+
InspectorTest.log("Triggering breakpoint...");
53+
await triggerBreakpoint();
54+
InspectorTest.expectEqual(pauseCount, 0, "Should not pause.");
55+
56+
InspectorTest.newline();
57+
58+
InspectorTest.log("Removing breakpoint...");
59+
breakpoint.remove();
60+
61+
InspectorTest.log("Triggering breakpoint...");
62+
await triggerBreakpoint();
63+
InspectorTest.expectEqual(pauseCount, 0, "Should not pause.");
64+
65+
WI.debuggerManager.removeEventListener(WI.DebuggerManager.Event.Paused, pausedListener);
66+
},
67+
});
68+
69+
suite.addTestCase({
70+
name: "SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Disabled",
71+
async test() {
72+
let pauseCount = 0;
73+
let pausedListener = WI.debuggerManager.addEventListener(WI.DebuggerManager.Event.Paused, (event) => {
74+
++pauseCount;
75+
WI.debuggerManager.resume();
76+
});
77+
78+
InspectorTest.log("Adding breakpoint...");
79+
let breakpoint = new WI.SymbolicBreakpoint("pReVeNtDeFaUlT", {caseSensitive: false, disabled: true});
80+
WI.debuggerManager.addSymbolicBreakpoint(breakpoint);
81+
InspectorTest.assert(!WI.debuggerManager.symbolicBreakpointsForSymbol("preventDefault").length, "Should not match breakpoint.");
82+
83+
InspectorTest.log("Triggering breakpoint...");
84+
await triggerBreakpoint();
85+
InspectorTest.expectEqual(pauseCount, 0, "Should not pause.");
86+
87+
InspectorTest.newline();
88+
89+
InspectorTest.log("Removing breakpoint...");
90+
breakpoint.remove();
91+
92+
InspectorTest.log("Triggering breakpoint...");
93+
await triggerBreakpoint();
94+
InspectorTest.expectEqual(pauseCount, 0, "Should not pause.");
95+
96+
WI.debuggerManager.removeEventListener(WI.DebuggerManager.Event.Paused, pausedListener);
97+
},
98+
});
99+
100+
suite.runTestCasesAndFinish();
101+
}
102+
</script>
103+
</head>
104+
<body onload="runTest()">
105+
<p>Tests for SymbolicBreakpoint.</p>
106+
</body>
107+
</html>

0 commit comments

Comments
 (0)