Skip to content

Commit

Permalink
src: bootstrap prepare stack trace callback in shadow realm
Browse files Browse the repository at this point in the history
Bootstrap per-realm callbacks like `prepare_stack_trace_callback` in
the ShadowRealm. This enables stack trace decoration in the ShadowRealm.

PR-URL: nodejs/node#47107
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
sercher committed Apr 24, 2024
1 parent 4b01597 commit 110bd4b
Show file tree
Hide file tree
Showing 18 changed files with 79 additions and 69 deletions.
2 changes: 1 addition & 1 deletion graal-nodejs/.github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
/doc/api/module.md @nodejs/modules @nodejs/loaders
/doc/api/modules.md @nodejs/modules @nodejs/loaders
/doc/api/packages.md @nodejs/modules @nodejs/loaders
/lib/internal/bootstrap/loaders.js @nodejs/modules @nodejs/loaders
/lib/internal/bootstrap/realm.js @nodejs/modules @nodejs/loaders
/lib/internal/modules/* @nodejs/modules @nodejs/loaders
/lib/internal/process/esm_loader.js @nodejs/modules @nodejs/loaders
/lib/internal/process/execution.js @nodejs/modules @nodejs/loaders
Expand Down
2 changes: 1 addition & 1 deletion graal-nodejs/lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const { openSync, closeSync, readSync } = require('fs');
const { inspect } = require('internal/util/inspect');
const { isPromise, isRegExp } = require('internal/util/types');
const { EOL } = require('internal/constants');
const { BuiltinModule } = require('internal/bootstrap/loaders');
const { BuiltinModule } = require('internal/bootstrap/realm');
const { isError } = require('internal/util');

const errorCache = new SafeMap();
Expand Down
28 changes: 4 additions & 24 deletions graal-nodejs/lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Hello, and welcome to hacking node.js!
//
// This file is invoked by `Realm::BootstrapNode()` in `src/node_realm.cc`,
// This file is invoked by `Realm::BootstrapRealm()` in `src/node_realm.cc`,
// and is responsible for setting up Node.js core before main scripts
// under `lib/internal/main/` are executed.
//
Expand Down Expand Up @@ -32,9 +32,10 @@
// `DOMException` class.
// - `lib/internal/per_context/messageport.js`: JS-side components of the
// `MessagePort` implementation.
// - `lib/internal/bootstrap/loaders.js`: this sets up internal binding and
// - `lib/internal/bootstrap/realm.js`: this sets up internal binding and
// module loaders, including `process.binding()`, `process._linkedBinding()`,
// `internalBinding()` and `BuiltinModule`.
// `internalBinding()` and `BuiltinModule`, and per-realm internal states
// and bindings, including `prepare_stack_trace_callback`.
//
// The initialization done in this script is included in both the main thread
// and the worker threads. After this, further initialization is done based
Expand All @@ -52,8 +53,6 @@
// passed by `BuiltinLoader::CompileAndCall()`.
/* global process, require, internalBinding, primordials */

setupPrepareStackTrace();

const {
FunctionPrototypeCall,
JSONParse,
Expand Down Expand Up @@ -370,25 +369,6 @@ process.emitWarning = emitWarning;
// Note: only after this point are the timers effective
}

function setupPrepareStackTrace() {
const {
setEnhanceStackForFatalException,
setPrepareStackTraceCallback,
} = internalBinding('errors');
const {
prepareStackTrace,
fatalExceptionStackEnhancers: {
beforeInspector,
afterInspector,
},
} = require('internal/errors');
// Tell our PrepareStackTraceCallback passed to the V8 API
// to call prepareStackTrace().
setPrepareStackTraceCallback(prepareStackTrace);
// Set the function used to enhance the error stack for printing
setEnhanceStackForFatalException(beforeInspector, afterInspector);
}

function setupProcessObject() {
const EventEmitter = require('events');
const origProcProto = ObjectGetPrototypeOf(process);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// This file is executed in every realm that is created by Node.js, including
// the context of main thread, worker threads, and ShadowRealms.
// Only per-realm internal states and bindings should be bootstrapped in this
// file and no globals should be exposed to the user code.
//
// This file creates the internal module & binding loaders used by built-in
// modules. In contrast, user land modules are loaded using
// lib/internal/modules/cjs/loader.js (CommonJS Modules) or
Expand Down Expand Up @@ -30,7 +35,7 @@
// so they can be loaded faster without the cost of I/O. This class makes the
// lib/internal/*, deps/internal/* modules and internalBinding() available by
// default to core modules, and lets the core modules require itself via
// require('internal/bootstrap/loaders') even when this file is not written in
// require('internal/bootstrap/realm') even when this file is not written in
// CommonJS style.
//
// Other objects:
Expand Down Expand Up @@ -181,7 +186,7 @@ let internalBinding;
};
}

const loaderId = 'internal/bootstrap/loaders';
const selfId = 'internal/bootstrap/realm';
const {
builtinIds,
compileFunction,
Expand Down Expand Up @@ -238,7 +243,7 @@ class BuiltinModule {
static exposeInternals() {
for (const { 0: id, 1: mod } of BuiltinModule.map) {
// Do not expose this to user land even with --expose-internals.
if (id !== loaderId) {
if (id !== selfId) {
mod.canBeRequiredByUsers = true;
}
}
Expand Down Expand Up @@ -357,7 +362,7 @@ const loaderExports = {
};

function requireBuiltin(id) {
if (id === loaderId) {
if (id === selfId) {
return loaderExports;
}

Expand All @@ -379,5 +384,27 @@ function requireWithFallbackInDeps(request) {
return requireBuiltin(request);
}

function setupPrepareStackTrace() {
const {
setEnhanceStackForFatalException,
setPrepareStackTraceCallback,
} = internalBinding('errors');
const {
prepareStackTrace,
fatalExceptionStackEnhancers: {
beforeInspector,
afterInspector,
},
} = requireBuiltin('internal/errors');
// Tell our PrepareStackTraceCallback passed to the V8 API
// to call prepareStackTrace().
setPrepareStackTraceCallback(prepareStackTrace);
// Set the function used to enhance the error stack for printing
setEnhanceStackForFatalException(beforeInspector, afterInspector);
}

// Store the internal loaders in C++.
setInternalLoaders(internalBinding, requireBuiltin);

// Setup per-realm bindings.
setupPrepareStackTrace();
2 changes: 1 addition & 1 deletion graal-nodejs/lib/internal/main/mksnapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const {
} = primordials;

const binding = internalBinding('mksnapshot');
const { BuiltinModule } = require('internal/bootstrap/loaders');
const { BuiltinModule } = require('internal/bootstrap/realm');
const {
compileSerializeMain,
} = binding;
Expand Down
2 changes: 1 addition & 1 deletion graal-nodejs/lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ module.exports = {
initializeCJS,
};

const { BuiltinModule } = require('internal/bootstrap/loaders');
const { BuiltinModule } = require('internal/bootstrap/realm');
const {
maybeCacheSourceMap,
} = require('internal/source_map/source_map_cache');
Expand Down
2 changes: 1 addition & 1 deletion graal-nodejs/lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ class Hooks {
filename: '<preload>',
},
);
const { BuiltinModule } = require('internal/bootstrap/loaders');
const { BuiltinModule } = require('internal/bootstrap/realm');
// We only allow replacing the importMetaInitializer during preload;
// after preload is finished, we disable the ability to replace it.
//
Expand Down
2 changes: 1 addition & 1 deletion graal-nodejs/lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const {
StringPrototypeStartsWith,
} = primordials;
const internalFS = require('internal/fs/utils');
const { BuiltinModule } = require('internal/bootstrap/loaders');
const { BuiltinModule } = require('internal/bootstrap/realm');
const {
realpathSync,
statSync,
Expand Down
2 changes: 1 addition & 1 deletion graal-nodejs/lib/internal/modules/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const {
ERR_MANIFEST_DEPENDENCY_MISSING,
ERR_UNKNOWN_BUILTIN_MODULE,
} = require('internal/errors').codes;
const { BuiltinModule } = require('internal/bootstrap/loaders');
const { BuiltinModule } = require('internal/bootstrap/realm');

const { validateString } = require('internal/validators');
const path = require('path');
Expand Down
2 changes: 1 addition & 1 deletion graal-nodejs/lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ function initializeReport() {
function setupDebugEnv() {
require('internal/util/debuglog').initializeDebugEnv(process.env.NODE_DEBUG);
if (getOptionValue('--expose-internals')) {
require('internal/bootstrap/loaders').BuiltinModule.exposeInternals();
require('internal/bootstrap/realm').BuiltinModule.exposeInternals();
}
}

Expand Down
2 changes: 1 addition & 1 deletion graal-nodejs/lib/internal/util/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ const {

const assert = require('internal/assert');

const { BuiltinModule } = require('internal/bootstrap/loaders');
const { BuiltinModule } = require('internal/bootstrap/realm');
const {
validateObject,
validateString,
Expand Down
2 changes: 1 addition & 1 deletion graal-nodejs/lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ const {
globalThis,
} = primordials;

const { BuiltinModule } = require('internal/bootstrap/loaders');
const { BuiltinModule } = require('internal/bootstrap/realm');
const {
makeRequireFunction,
addBuiltinLibsToObject,
Expand Down
13 changes: 12 additions & 1 deletion graal-nodejs/src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,18 @@ MaybeLocal<Value> PrepareStackTraceCallback(Local<Context> context,
if (env == nullptr) {
return exception->ToString(context).FromMaybe(Local<Value>());
}
Local<Function> prepare = env->prepare_stack_trace_callback();
Realm* realm = Realm::GetCurrent(context);
Local<Function> prepare;
if (realm != nullptr) {
// If we are in a Realm, call the realm specific prepareStackTrace callback
// to avoid passing the JS objects (the exception and trace) across the
// realm boundary with the `Error.prepareStackTrace` override.
prepare = realm->prepare_stack_trace_callback();
} else {
// The context is created with ContextifyContext, call the principal
// realm's prepareStackTrace callback.
prepare = env->principal_realm()->prepare_stack_trace_callback();
}
if (prepare.IsEmpty()) {
return exception->ToString(context).FromMaybe(Local<Value>());
}
Expand Down
12 changes: 6 additions & 6 deletions graal-nodejs/src/node_builtins.cc
Original file line number Diff line number Diff line change
Expand Up @@ -363,9 +363,9 @@ MaybeLocal<Function> BuiltinLoader::LookupAndCompile(Local<Context> context,
std::vector<Local<String>> parameters;
Isolate* isolate = context->GetIsolate();
// Detects parameters of the scripts based on module ids.
// internal/bootstrap/loaders: process, getLinkedBinding,
// getInternalBinding, primordials
if (strcmp(id, "internal/bootstrap/loaders") == 0) {
// internal/bootstrap/realm: process, getLinkedBinding,
// getInternalBinding, primordials
if (strcmp(id, "internal/bootstrap/realm") == 0) {
parameters = {
FIXED_ONE_BYTE_STRING(isolate, "process"),
FIXED_ONE_BYTE_STRING(isolate, "getLinkedBinding"),
Expand Down Expand Up @@ -427,9 +427,9 @@ MaybeLocal<Value> BuiltinLoader::CompileAndCall(Local<Context> context,
// BuiltinLoader::LookupAndCompile().
std::vector<Local<Value>> arguments;
// Detects parameters of the scripts based on module ids.
// internal/bootstrap/loaders: process, getLinkedBinding,
// getInternalBinding, primordials
if (strcmp(id, "internal/bootstrap/loaders") == 0) {
// internal/bootstrap/realm: process, getLinkedBinding,
// getInternalBinding, primordials
if (strcmp(id, "internal/bootstrap/realm") == 0) {
Local<Value> get_linked_binding;
Local<Value> get_internal_binding;
if (!NewFunctionTemplate(isolate, binding::GetLinkedBinding)
Expand Down
10 changes: 5 additions & 5 deletions graal-nodejs/src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -954,9 +954,9 @@ void PerIsolateMessageListener(Local<Message> message, Local<Value> error) {
}

void SetPrepareStackTraceCallback(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Realm* realm = Realm::GetCurrent(args);
CHECK(args[0]->IsFunction());
env->set_prepare_stack_trace_callback(args[0].As<Function>());
realm->set_prepare_stack_trace_callback(args[0].As<Function>());
}

static void SetSourceMapsEnabled(const FunctionCallbackInfo<Value>& args) {
Expand All @@ -981,11 +981,11 @@ static void SetMaybeCacheGeneratedSourceMap(

static void SetEnhanceStackForFatalException(
const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Realm* realm = Realm::GetCurrent(args);
CHECK(args[0]->IsFunction());
CHECK(args[1]->IsFunction());
env->set_enhance_fatal_stack_before_inspector(args[0].As<Function>());
env->set_enhance_fatal_stack_after_inspector(args[1].As<Function>());
realm->set_enhance_fatal_stack_before_inspector(args[0].As<Function>());
realm->set_enhance_fatal_stack_after_inspector(args[1].As<Function>());
}

// Side effect-free stringification that will never throw exceptions.
Expand Down
22 changes: 7 additions & 15 deletions graal-nodejs/src/node_realm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,18 +183,14 @@ MaybeLocal<Value> Realm::ExecuteBootstrapper(const char* id) {
}

MaybeLocal<Value> Realm::BootstrapNode() {
EscapableHandleScope scope(isolate_);

MaybeLocal<Value> result = ExecuteBootstrapper("internal/bootstrap/node");
HandleScope scope(isolate_);

if (result.IsEmpty()) {
if (ExecuteBootstrapper("internal/bootstrap/node").IsEmpty()) {
return MaybeLocal<Value>();
}

if (!env_->no_browser_globals()) {
result = ExecuteBootstrapper("internal/bootstrap/browser");

if (result.IsEmpty()) {
if (ExecuteBootstrapper("internal/bootstrap/browser").IsEmpty()) {
return MaybeLocal<Value>();
}
}
Expand All @@ -203,19 +199,15 @@ MaybeLocal<Value> Realm::BootstrapNode() {
auto thread_switch_id =
env_->is_main_thread() ? "internal/bootstrap/switches/is_main_thread"
: "internal/bootstrap/switches/is_not_main_thread";
result = ExecuteBootstrapper(thread_switch_id);

if (result.IsEmpty()) {
if (ExecuteBootstrapper(thread_switch_id).IsEmpty()) {
return MaybeLocal<Value>();
}

auto process_state_switch_id =
env_->owns_process_state()
? "internal/bootstrap/switches/does_own_process_state"
: "internal/bootstrap/switches/does_not_own_process_state";
result = ExecuteBootstrapper(process_state_switch_id);

if (result.IsEmpty()) {
if (ExecuteBootstrapper(process_state_switch_id).IsEmpty()) {
return MaybeLocal<Value>();
}

Expand All @@ -227,7 +219,7 @@ MaybeLocal<Value> Realm::BootstrapNode() {
return MaybeLocal<Value>();
}

return scope.EscapeMaybe(result);
return v8::True(isolate_);
}

MaybeLocal<Value> Realm::RunBootstrapping() {
Expand All @@ -236,7 +228,7 @@ MaybeLocal<Value> Realm::RunBootstrapping() {
CHECK(!has_run_bootstrapping_code());

Local<Value> result;
if (!ExecuteBootstrapper("internal/bootstrap/loaders").ToLocal(&result)) {
if (!ExecuteBootstrapper("internal/bootstrap/realm").ToLocal(&result)) {
return MaybeLocal<Value>();
}

Expand Down
6 changes: 3 additions & 3 deletions graal-nodejs/test/es-module/test-loaders-hidden-from-users.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@ const assert = require('assert');

assert.throws(
() => {
require('internal/bootstrap/loaders');
require('internal/bootstrap/realm');
}, {
code: 'MODULE_NOT_FOUND',
message: /Cannot find module 'internal\/bootstrap\/loaders'/
message: /Cannot find module 'internal\/bootstrap\/realm'/
}
);

assert.throws(
() => {
const source = 'module.exports = require("internal/bootstrap/loaders")';
const source = 'module.exports = require("internal/bootstrap/realm")';
const { internalBinding } = require('internal/test/binding');
internalBinding('natives').owo = source;
require('owo');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ async function runTest() {
await session.waitForNotification((notification) => {
// The main assertion here is that we do hit the loader script first.
return notification.method === 'Debugger.scriptParsed' &&
notification.params.url === 'node:internal/bootstrap/loaders';
notification.params.url === 'node:internal/bootstrap/realm';
});

await session.waitForNotification('Debugger.paused');
Expand Down

0 comments on commit 110bd4b

Please sign in to comment.