Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-26 22:32:46 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1893611 - Add shouldAvoidSideEffects debugger flag and use it in Services resolve hook. r=iain,devtools-reviewers,nchevobbe

Browse Source 
In order to abort eager evaluation when executing a resolve hook with side-effect,
especially the Services' hook which imports ES module, which is not undo-able,
introduce a dedicate debugger flag to ask to avoid side-effectful operation in
the resolve hook.

Differential Revision: https://phabricator.services.mozilla.com/D208970
This commit is contained in:
Tooru Fujisawa 2024-05-08 10:56:39 +00:00
parent 33c3e98ee7
commit 500aa0f1c6
13 changed files with 281 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
devtools/client/webconsole/test/browser/_browser_console.toml
View File

@ -61,6 +61,9 @@ skip-if = [
["browser_console_eager_eval.js"]
["browser_console_eager_eval_resolve.js"]
skip-if = ["verify"]
["browser_console_enable_network_monitoring.js"]
skip-if = [
"verify",

64
devtools/client/webconsole/test/browser/browser_console_eager_eval_resolve.js Normal file
View File

@ -0,0 +1,64 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";
// Check evaluating eager-evaluation values.
const TEST_URI = "data:text/html;charset=utf8,<!DOCTYPE html>";
add_task(async function () {
await addTab(TEST_URI);
await pushPref("devtools.chrome.enabled", true);
info("Open the Browser Console");
const hud = await BrowserConsoleManager.toggleBrowserConsole();
await executeResolveHookWithSideEffect(hud);
});
async function executeResolveHookWithSideEffect(hud) {
// Services.droppedLinkHandler is implemented with resolve hook, which imports
// ContentAreaDropListener.sys.mjs.
//
// In order to test the resolve hook behavior, ensure the module is not yet
// loaded, which ensures the property is not yet resolved.
//
// NOTE: This test is not compatible with verify mode, given it depends on the
// initial state of the Services object and the module.
is(
Cu.isESModuleLoaded(
"resource://gre/modules/ContentAreaDropListener.sys.mjs"
),
false
);
setInputValue(hud, `Services.droppedLinkHandler`);
await wait(500);
// Eager evaluation should fail, due to the side effect in the resolve hook.
await waitForEagerEvaluationResult(hud, "");
setInputValue(hud, "");
await wait(500);
// The property should be resolved when evaluating after the eager evaluation.
await executeAndWaitForResultMessage(
hud,
`Services.droppedLinkHandler;`,
"XPCWrappedNative_NoHelper"
);
is(
Cu.isESModuleLoaded(
"resource://gre/modules/ContentAreaDropListener.sys.mjs"
),
true
);
// Eager evaluation should work after the property is resolved.
setInputValue(hud, `Services.droppedLinkHandler`);
await wait(500);
await waitForEagerEvaluationResult(hud, /XPCWrappedNative_NoHelper/);
}

2
devtools/server/actors/webconsole/eval-with-debugger.js
View File

@ -325,6 +325,7 @@ function getEvalResult(
if (noSideEffectDebugger) {
noSideEffectDebugger.removeAllDebuggees();
noSideEffectDebugger.onNativeCall = undefined;
noSideEffectDebugger.shouldAvoidSideEffects = false;
}
}
}
@ -548,6 +549,7 @@ function makeSideeffectFreeDebugger(targetActorDbg) {
// Returning null terminates the current evaluation.
return null;
};
dbg.shouldAvoidSideEffects = true;
return dbg;
}

3
js/public/Class.h
View File

@ -272,6 +272,9 @@ typedef JSString* (*JSFunToStringOp)(JSContext* cx, JS::HandleObject obj,
* JS looks for a property in an object, and if not found, tries to resolve
* the given id. *resolvedp should be set to true iff the property was defined
* on |obj|.
*
* See JS::dbg::ShouldAvoidSideEffects in Debug.h if this function has any
* other side-effect than just resolving the property.
*/
typedef bool (*JSResolveOp)(JSContext* cx, JS::HandleObject obj,
JS::HandleId id, bool* resolvedp);

30
js/public/Debug.h
View File

@ -348,6 +348,36 @@ class MOZ_STACK_CLASS JS_PUBLIC_API AutoEntryMonitor {
virtual void Exit(JSContext* cx) {}
};
// Returns true if there's any debugger attached to the given context where
// the debugger's "shouldAvoidSideEffects" property is true.
//
// This is supposed to be used by native code that performs side-effectful
// operations where the debugger cannot hook it.
//
// If this function returns true, the native function should throw an
// uncatchable exception by returning `false` without setting any pending
// exception. The debugger will handle this exception by aborting the eager
// evaluation.
//
// The native code can opt into this behavior to help the debugger performing
// the side-effect-free evaluation.
//
// Expected consumers of this API include JSClassOps.resolve hooks which have
// any side-effect other than just resolving the property.
//
// Example:
// static bool ResolveHook(JSContext* cx, HandleObject obj, HandleId id,
// bool* resolvedp) {
// *resolvedp = false;
// if (JS::dbg::ShouldAvoidSideEffects()) {
// return false;
// }
// // Resolve the property with the side-effect.
// ...
// return true;
// }
bool ShouldAvoidSideEffects(JSContext* cx);
} // namespace dbg
} // namespace JS

9
js/src/debugger/DebugAPI-inl.h
View File

@ -141,6 +141,15 @@ NativeResumeMode DebugAPI::onNativeCall(JSContext* cx, const CallArgs& args,
return NativeResumeMode::Continue;
}
/* static */
bool DebugAPI::shouldAvoidSideEffects(JSContext* cx) {
if (MOZ_UNLIKELY(cx->realm()->isDebuggee())) {
return slowPathShouldAvoidSideEffects(cx);
}
return false;
}
/* static */
bool DebugAPI::onDebuggerStatement(JSContext* cx, AbstractFramePtr frame) {
if (MOZ_UNLIKELY(cx->realm()->isDebuggee())) {

4
js/src/debugger/DebugAPI.h
View File

@ -7,6 +7,7 @@
#ifndef debugger_DebugAPI_h
#define debugger_DebugAPI_h
#include "js/Debug.h"
#include "vm/GlobalObject.h"
#include "vm/Interpreter.h"
#include "vm/JSContext.h"
@ -228,6 +229,8 @@ class DebugAPI {
const CallArgs& args,
CallReason reason);
static inline bool shouldAvoidSideEffects(JSContext* cx);
/*
* Announce to the debugger a |debugger;| statement on has been
* encountered on the youngest JS frame on |cx|. Call whatever hooks have
@ -385,6 +388,7 @@ class DebugAPI {
static NativeResumeMode slowPathOnNativeCall(JSContext* cx,
const CallArgs& args,
CallReason reason);
static bool slowPathShouldAvoidSideEffects(JSContext* cx);
[[nodiscard]] static bool slowPathOnDebuggerStatement(JSContext* cx,
AbstractFramePtr frame);
[[nodiscard]] static bool slowPathOnExceptionUnwind(JSContext* cx,

31
js/src/debugger/Debugger.cpp
View File

@ -536,6 +536,7 @@ Debugger::Debugger(JSContext* cx, NativeObject* dbg)
exclusiveDebuggerOnEval(false),
inspectNativeCallArguments(false),
collectCoverageInfo(false),
shouldAvoidSideEffects(false),
observedGCs(cx->zone()),
allocationsLog(cx),
trackingAllocationSites(false),
@ -1049,6 +1050,12 @@ NativeResumeMode DebugAPI::slowPathOnNativeCall(JSContext* cx,
return NativeResumeMode::Continue;
}
/* static */
bool DebugAPI::slowPathShouldAvoidSideEffects(JSContext* cx) {
return DebuggerExists(
cx->global(), [=](Debugger* dbg) { return dbg->shouldAvoidSideEffects; });
}
/*
* RAII class to mark a generator as "running" temporarily while running
* debugger code.
@ -4187,6 +4194,8 @@ struct MOZ_STACK_CLASS Debugger::CallData {
bool setOnEnterFrame();
bool getOnNativeCall();
bool setOnNativeCall();
bool getShouldAvoidSideEffects();
bool setShouldAvoidSideEffects();
bool getOnNewGlobalObject();
bool setOnNewGlobalObject();
bool getOnNewPromise();
@ -4405,6 +4414,22 @@ bool Debugger::CallData::setOnNativeCall() {
return true;
}
bool Debugger::CallData::getShouldAvoidSideEffects() {
args.rval().setBoolean(dbg->shouldAvoidSideEffects);
return true;
}
bool Debugger::CallData::setShouldAvoidSideEffects() {
if (!args.requireAtLeast(cx, "Debugger.set shouldAvoidSideEffects", 1)) {
return false;
}
dbg->shouldAvoidSideEffects = ToBoolean(args[0]);
args.rval().setUndefined();
return true;
}
bool Debugger::CallData::getOnNewGlobalObject() {
return getHookImpl(cx, args, *dbg, OnNewGlobalObject);
}
@ -6523,6 +6548,8 @@ const JSPropertySpec Debugger::properties[] = {
JS_DEBUG_PSGS("onPromiseSettled", getOnPromiseSettled, setOnPromiseSettled),
JS_DEBUG_PSGS("onEnterFrame", getOnEnterFrame, setOnEnterFrame),
JS_DEBUG_PSGS("onNativeCall", getOnNativeCall, setOnNativeCall),
JS_DEBUG_PSGS("shouldAvoidSideEffects", getShouldAvoidSideEffects,
setShouldAvoidSideEffects),
JS_DEBUG_PSGS("onNewGlobalObject", getOnNewGlobalObject,
setOnNewGlobalObject),
JS_DEBUG_PSGS("uncaughtExceptionHook", getUncaughtExceptionHook,
@ -7264,5 +7291,9 @@ JS_PUBLIC_API bool FireOnGarbageCollectionHook(
return true;
}
bool ShouldAvoidSideEffects(JSContext* cx) {
return DebugAPI::shouldAvoidSideEffects(cx);
}
} // namespace dbg
} // namespace JS

4
js/src/debugger/Debugger.h
View File

@ -652,6 +652,10 @@ class Debugger : private mozilla::LinkedListElement<Debugger> {
// Whether to enable code coverage on the Debuggee.
bool collectCoverageInfo;
// Whether to ask avoid side-effects in the native code.
// See JS::dbg::ShouldAvoidSideEffects.
bool shouldAvoidSideEffects;
template <typename T>
struct DebuggerLinkAccess {
static mozilla::DoublyLinkedListElement<T>& Get(T* aThis) {

7
js/src/doc/Debugger/Debugger.md
View File

@ -87,6 +87,13 @@ access to browser-level features like the `alert` function, which this
API's implementation does not, making it possible to present debugger
errors to the developer in a way suited to the context.)
### `shouldAvoidSideEffects`
A boolean value used to ask a side-effectful native code to abort.
If set to true, `JS::dbg::ShouldAvoidSideEffects(cx)` returns true.
Native code can opt into this to support debugger who wants to perform
side-effect-free evaluation.
## Debugger Handler Functions

57
js/src/jit-test/tests/debug/Debugger-shouldAvoidSideEffects.js Normal file
View File

@ -0,0 +1,57 @@
// Test shouldAvoidSideEffects flag.
const g = newGlobal({newCompartment: true});
const dbg = Debugger(g);
const gdbg = dbg.addDebuggee(g);
gdbg.executeInGlobal(`
var obj, result, reachedNextLine;
`);
dbg.shouldAvoidSideEffects = false;
assertEq(dbg.shouldAvoidSideEffects, false);
let result = gdbg.executeInGlobal(`
result = undefined;
reachedNextLine = false;
obj = createSideEffectfulResolveObject();
result = obj.test;
reachedNextLine = true;
"finished";
`);
assertEq(g.result, 42);
assertEq(g.reachedNextLine, true);
assertEq(result.return, "finished");
dbg.shouldAvoidSideEffects = true;
assertEq(dbg.shouldAvoidSideEffects, true);
result = gdbg.executeInGlobal(`
result = undefined;
reachedNextLine = false;
obj = createSideEffectfulResolveObject();
result = obj.test;
reachedNextLine = true;
"finished";
`);
assertEq(g.result, undefined);
assertEq(g.reachedNextLine, false);
assertEq(result, null);
// Resolve after abort.
dbg.shouldAvoidSideEffects = false;
assertEq(dbg.shouldAvoidSideEffects, false);
result = gdbg.executeInGlobal(`
result = undefined;
reachedNextLine = false;
result = obj.test;
reachedNextLine = true;
"finished";
`);
assertEq(g.result, 42);
assertEq(g.reachedNextLine, true);
assertEq(result.return, "finished");

66
js/src/shell/js.cpp
View File

@ -123,10 +123,10 @@
#include "js/CompilationAndEvaluation.h"
#include "js/CompileOptions.h" // JS::ReadOnlyCompileOptions, JS::CompileOptions, JS::OwningCompileOptions, JS::DecodeOptions, JS::InstantiateOptions
#include "js/ContextOptions.h" // JS::ContextOptions{,Ref}
#include "js/Debug.h"
#include "js/Equality.h" // JS::SameValue
#include "js/ErrorReport.h" // JS::PrintError
#include "js/Exception.h" // JS::StealPendingExceptionStack
#include "js/Debug.h" // JS::dbg::ShouldAvoidSideEffects
#include "js/Equality.h" // JS::SameValue
#include "js/ErrorReport.h" // JS::PrintError
#include "js/Exception.h" // JS::StealPendingExceptionStack
#include "js/experimental/CodeCoverage.h" // js::EnableCodeCoverage
#include "js/experimental/CompileScript.h" // JS::NewFrontendContext, JS::DestroyFrontendContext, JS::HadFrontendErrors, JS::ConvertFrontendErrorsToRuntimeErrors, JS::CompileGlobalScriptToStencil, JS::CompileModuleScriptToStencil
#include "js/experimental/CTypes.h" // JS::InitCTypesClass
@ -9142,6 +9142,59 @@ static bool DecompressLZ4(JSContext* cx, unsigned argc, Value* vp) {
return true;
}
static bool SideEffectfulResolveObject_enumerate(
JSContext* cx, JS::HandleObject obj, JS::MutableHandleIdVector properties,
bool enumerableOnly) {
return properties.append(NameToId(cx->names().test));
}
static bool SideEffectfulResolveObject_resolve(JSContext* cx, HandleObject obj,
HandleId id, bool* resolvedp) {
*resolvedp = false;
if (JS::dbg::ShouldAvoidSideEffects(cx)) {
return false;
}
if (id == NameToId(cx->names().test)) {
RootedValue value(cx, JS::NumberValue(42));
if (!JS_DefinePropertyById(cx, obj, id, value, JSPROP_ENUMERATE)) {
return false;
}
*resolvedp = true;
}
return true;
}
static const JSClassOps SideEffectfulResolveObject_classOps = {
nullptr, // addProperty
nullptr, // delProperty
nullptr, // enumerate
SideEffectfulResolveObject_enumerate, // newEnumerate
SideEffectfulResolveObject_resolve, // resolve
nullptr, // mayResolve
nullptr, // finalize
nullptr, // call
nullptr, // construct
nullptr,
};
static const JSClass SideEffectfulResolveObject_class = {
"SideEffectfulResolveObject", 0, &SideEffectfulResolveObject_classOps};
static bool CreateSideEffectfulResolveObject(JSContext* cx, unsigned argc,
JS::Value* vp) {
CallArgs args = CallArgsFromVp(argc, vp);
RootedObject obj(cx, JS_NewObject(cx, &SideEffectfulResolveObject_class));
if (!obj) {
return false;
}
args.rval().setObject(*obj);
return true;
}
// clang-format off
static const JSFunctionSpecWithHelp shell_functions[] = {
JS_FN_HELP("options", Options, 0, 0,
@ -9811,6 +9864,11 @@ JS_FN_HELP("createUserArrayBuffer", CreateUserArrayBuffer, 1, 0,
"decompressLZ4(bytes)",
" Return a decompressed copy of bytes using LZ4."),
JS_FN_HELP("createSideEffectfulResolveObject", CreateSideEffectfulResolveObject, 0, 0,
"createSideEffectfulResolveObject()",
" Return an object with a property 'obj.test == 42', backed by a resolve hook "
" with the Debugger shouldAvoidSideEffects flag integration."),
JS_FS_HELP_END
};
// clang-format on

5
js/xpconnect/src/JSServices.cpp
View File

@ -8,6 +8,7 @@
#include "StaticComponents.h"
#include "mozilla/ErrorResult.h"
#include "mozilla/ProfilerLabels.h"
#include "js/Debug.h" // JS::dbg::ShouldAvoidSideEffects
#include "js/PropertyAndElement.h" // JS_DefineProperty, JS_DefinePropertyById
#include "js/String.h" // JS::LinearStringHasLatin1Chars
#include "nsJSUtils.h"
@ -136,6 +137,10 @@ static JSObject* GetService(JSContext* cx, const xpcom::JSServiceEntry& service,
static bool Services_Resolve(JSContext* cx, HandleObject obj, HandleId id,
bool* resolvedp) {
*resolvedp = false;
if (JS::dbg::ShouldAvoidSideEffects(cx)) {
return false;
}
JSLinearString* name = GetNameIfLatin1(id);
if (!name) {
return true;