Skip to content
Permalink
Browse files
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/253999@main
  • Loading branch information
dcrousso committed Aug 31, 2022
1 parent 922bc7c commit f0c85afe24ed57997d439232c58979a19481b93b
Show file tree
Hide file tree
Showing 40 changed files with 2,666 additions and 37 deletions.
@@ -55,7 +55,9 @@
InspectorTest.BreakpointOptions.addTestCases(suite, {
testCaseNamePrefix: "ExactCaseInsensitive.Hit.",
createBreakpoint() {
return new WI.SymbolicBreakpoint("tRiGgErSyMbOlIcBrEaKpOiNt", {caseSensitive: false});
let breakpoint = new WI.SymbolicBreakpoint("tRiGgErSyMbOlIcBrEaKpOiNt", {caseSensitive: false});
WI.debuggerManager.addSymbolicBreakpoint(breakpoint);
return breakpoint;
},
triggerBreakpoint,
});
@@ -55,7 +55,9 @@
InspectorTest.BreakpointOptions.addTestCases(suite, {
testCaseNamePrefix: "ExactCaseSensitive.Hit.",
createBreakpoint() {
return new WI.SymbolicBreakpoint("triggerSymbolicBreakpoint");
let breakpoint = new WI.SymbolicBreakpoint("triggerSymbolicBreakpoint");
WI.debuggerManager.addSymbolicBreakpoint(breakpoint);
return breakpoint;
},
triggerBreakpoint,
});
@@ -0,0 +1,165 @@
CONSOLE MESSAGE: BREAKPOINT ACTION LOG 1
CONSOLE MESSAGE: BREAKPOINT ACTION LOG 2
CONSOLE MESSAGE: BREAKPOINT ACTION LOG 3
CONSOLE MESSAGE: BREAKPOINT ACTION LOG 4
Tests for SymbolicBreakpoint.


== Running test suite: SymbolicBreakpoint
-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Hit.Options.Condition

Setting condition to 'false'...

Triggering breakpoint...
PASS: Should not pause.

Triggering breakpoint...
PASS: Should not pause.

Setting condition to 'true'...

Triggering breakpoint...
PASS: Should pause.

Triggering breakpoint...
PASS: Should pause.

-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Hit.Options.Condition.ConsoleCommandLineAPI
Adding saved console value 'false'...

Setting condition to saved console value...

Triggering breakpoint...
PASS: Should not pause.

Triggering breakpoint...
PASS: Should not pause.

Adding saved console value 'true'...
Setting condition to saved console value...

Triggering breakpoint...
PASS: Should pause.

Triggering breakpoint...
PASS: Should pause.

-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Hit.Options.IgnoreCount

Setting ignoreCount to '2'...

Triggering breakpoint...
PASS: Should not pause.

Triggering breakpoint...
PASS: Should not pause.

Triggering breakpoint...
PASS: Should pause.

Triggering breakpoint...
PASS: Should pause.

-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Hit.Options.Action.Log

Adding log action...

Triggering breakpoint...
PASS: Should execute breakpoint action.
PASS: Should pause.

Editing log action...

Triggering breakpoint...
PASS: Should execute breakpoint action.
PASS: Should pause.

Editing log action...
Enabling auto-continue...

Triggering breakpoint...
PASS: Should execute breakpoint action.
PASS: Should not pause.

Editing log action...

Triggering breakpoint...
PASS: Should execute breakpoint action.
PASS: Should not pause.

-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Hit.Options.Actions.Evaluate

Adding evaluate action...

Triggering breakpoint...
PASS: Should execute breakpoint action.
PASS: Should pause.

Editing evaluate action...

Triggering breakpoint...
PASS: Should execute breakpoint action.
PASS: Should pause.

Editing evaluate action...
Enabling auto-continue...

Triggering breakpoint...
PASS: Should execute breakpoint action.
PASS: Should not pause.

Editing evaluate action...

Triggering breakpoint...
PASS: Should execute breakpoint action.
PASS: Should not pause.

-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Hit.Options.Actions.Evaluate.ConsoleCommandLineAPI
Adding saved console value '1'...

Adding evaluate action using saved console value...

Triggering breakpoint...
PASS: Should execute breakpoint action.
PASS: Should pause.

Adding saved console value '2'...
Editing evaluate action using saved console value...

Triggering breakpoint...
PASS: Should execute breakpoint action.
PASS: Should pause.

Adding saved console value '3'...
Editing evaluate action using saved console value...
Enabling auto-continue...

Triggering breakpoint...
PASS: Should execute breakpoint action.
PASS: Should not pause.

Adding saved console value '4'...
Editing evaluate action using saved console value...

Triggering breakpoint...
PASS: Should execute breakpoint action.
PASS: Should not pause.

-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Miss
Adding breakpoint...
Triggering breakpoint...
PASS: Should not pause.

Removing breakpoint...
Triggering breakpoint...
PASS: Should not pause.

-- Running test case: SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Disabled
Adding breakpoint...
Triggering breakpoint...
PASS: Should not pause.

Removing breakpoint...
Triggering breakpoint...
PASS: Should not pause.

@@ -0,0 +1,107 @@
<!DOCTYPE html>
<html>
<head>
<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
<script src="resources/breakpoint-options-utilities.js"></script>
<script>
function triggerSymbolicBreakpoint() {
document.body.addEventListener("click", (event) => {
event.preventDefault();
TestPage.dispatchEventToFrontend("TestPage-SymbolicBreakpoint");
}, {once: true});
document.body.click();
}

function test()
{
WI.settings.experimentalEnableSymbolicBreakpoints.value = true;

function triggerBreakpoint() {
return Promise.all([
InspectorTest.awaitEvent("TestPage-SymbolicBreakpoint"),
InspectorTest.evaluateInPage(`triggerSymbolicBreakpoint()`),
]);
}

let suite = InspectorTest.createAsyncSuite("SymbolicBreakpoint");

InspectorTest.BreakpointOptions.addTestCases(suite, {
testCaseNamePrefix: "ExactCaseInsensitive.Native.DOM.Hit.",
createBreakpoint() {
let breakpoint = new WI.SymbolicBreakpoint("pReVeNtDeFaUlT", {caseSensitive: false});
WI.debuggerManager.addSymbolicBreakpoint(breakpoint);
return breakpoint;
},
triggerBreakpoint,
});

suite.addTestCase({
name: "SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Miss",
async test() {
let pauseCount = 0;
let pausedListener = WI.debuggerManager.addEventListener(WI.DebuggerManager.Event.Paused, (event) => {
++pauseCount;
WI.debuggerManager.resume();
});

InspectorTest.log("Adding breakpoint...");
let breakpoint = new WI.SymbolicBreakpoint("pReVeNt", {caseSensitive: false});
WI.debuggerManager.addSymbolicBreakpoint(breakpoint);
InspectorTest.assert(!WI.debuggerManager.symbolicBreakpointsForSymbol("preventDefault").length, "Should not match breakpoint.");

InspectorTest.log("Triggering breakpoint...");
await triggerBreakpoint();
InspectorTest.expectEqual(pauseCount, 0, "Should not pause.");

InspectorTest.newline();

InspectorTest.log("Removing breakpoint...");
breakpoint.remove();

InspectorTest.log("Triggering breakpoint...");
await triggerBreakpoint();
InspectorTest.expectEqual(pauseCount, 0, "Should not pause.");

WI.debuggerManager.removeEventListener(WI.DebuggerManager.Event.Paused, pausedListener);
},
});

suite.addTestCase({
name: "SymbolicBreakpoint.ExactCaseInsensitive.Native.DOM.Disabled",
async test() {
let pauseCount = 0;
let pausedListener = WI.debuggerManager.addEventListener(WI.DebuggerManager.Event.Paused, (event) => {
++pauseCount;
WI.debuggerManager.resume();
});

InspectorTest.log("Adding breakpoint...");
let breakpoint = new WI.SymbolicBreakpoint("pReVeNtDeFaUlT", {caseSensitive: false, disabled: true});
WI.debuggerManager.addSymbolicBreakpoint(breakpoint);
InspectorTest.assert(!WI.debuggerManager.symbolicBreakpointsForSymbol("preventDefault").length, "Should not match breakpoint.");

InspectorTest.log("Triggering breakpoint...");
await triggerBreakpoint();
InspectorTest.expectEqual(pauseCount, 0, "Should not pause.");

InspectorTest.newline();

InspectorTest.log("Removing breakpoint...");
breakpoint.remove();

InspectorTest.log("Triggering breakpoint...");
await triggerBreakpoint();
InspectorTest.expectEqual(pauseCount, 0, "Should not pause.");

WI.debuggerManager.removeEventListener(WI.DebuggerManager.Event.Paused, pausedListener);
},
});

suite.runTestCasesAndFinish();
}
</script>
</head>
<body onload="runTest()">
<p>Tests for SymbolicBreakpoint.</p>
</body>
</html>

0 comments on commit f0c85af

Please sign in to comment.