Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: reject per-process isolate flags in workers #53780

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,7 @@ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
isolate->SetWasmStreamingCallback(wasm_web_api::StartStreamingCompilation);
}

if (per_process::cli_options->get_per_isolate_options()
->experimental_shadow_realm) {
if (per_process::cli_options->experimental_shadow_realm) {
isolate->SetHostCreateShadowRealmContextCallback(
shadow_realm::HostCreateShadowRealmContextCallback);
}
Expand Down
104 changes: 55 additions & 49 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
AddOption("--experimental-report", "", NoOp{}, kAllowedInEnvvar);
AddOption(
"--experimental-wasi-unstable-preview1", "", NoOp{}, kAllowedInEnvvar);
AddOption("--expose-gc", "expose gc extension", V8Option{}, kAllowedInEnvvar);
AddOption("--expose-internals", "", &EnvironmentOptions::expose_internals);
AddOption("--frozen-intrinsics",
"experimental frozen intrinsics support",
Expand Down Expand Up @@ -565,9 +564,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"preserve symbolic links when resolving the main module",
&EnvironmentOptions::preserve_symlinks_main,
kAllowedInEnvvar);
AddOption("--prof",
"Generate V8 profiler output.",
V8Option{});
AddOption("--prof-process",
"process V8 profiler output generated using --prof",
&EnvironmentOptions::prof_process);
Expand Down Expand Up @@ -820,42 +816,14 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {

PerIsolateOptionsParser::PerIsolateOptionsParser(
const EnvironmentOptionsParser& eop) {
// Generally, V8 flags are saved in per-process storage and should be added
// to PerProcessOptionsParser.

AddOption("--track-heap-objects",
"track heap object allocations for heap snapshots",
&PerIsolateOptions::track_heap_objects,
kAllowedInEnvvar);

// Explicitly add some V8 flags to mark them as allowed in NODE_OPTIONS.
AddOption("--abort-on-uncaught-exception",
"aborting instead of exiting causes a core file to be generated "
"for analysis",
V8Option{},
kAllowedInEnvvar);
AddOption("--interpreted-frames-native-stack",
"help system profilers to translate JavaScript interpreted frames",
V8Option{},
kAllowedInEnvvar);
AddOption("--max-old-space-size", "", V8Option{}, kAllowedInEnvvar);
AddOption("--max-semi-space-size", "", V8Option{}, kAllowedInEnvvar);
AddOption("--perf-basic-prof", "", V8Option{}, kAllowedInEnvvar);
AddOption(
"--perf-basic-prof-only-functions", "", V8Option{}, kAllowedInEnvvar);
AddOption("--perf-prof", "", V8Option{}, kAllowedInEnvvar);
AddOption("--perf-prof-unwinding-info", "", V8Option{}, kAllowedInEnvvar);
AddOption("--stack-trace-limit", "", V8Option{}, kAllowedInEnvvar);
AddOption("--disallow-code-generation-from-strings",
"disallow eval and friends",
V8Option{},
kAllowedInEnvvar);
AddOption("--huge-max-old-generation-size",
"increase default maximum heap size on machines with 16GB memory "
"or more",
V8Option{},
kAllowedInEnvvar);
AddOption("--jitless",
"disable runtime allocation of executable memory",
V8Option{},
kAllowedInEnvvar);
AddOption("--report-uncaught-exception",
"generate diagnostic report on uncaught exceptions",
&PerIsolateOptions::report_uncaught_exception,
Expand All @@ -870,21 +838,7 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
&PerIsolateOptions::report_signal,
kAllowedInEnvvar);
Implies("--report-signal", "--report-on-signal");
AddOption("--enable-etw-stack-walking",
"provides heap data to ETW Windows native tracing",
V8Option{},
kAllowedInEnvvar);

AddOption("--experimental-top-level-await", "", NoOp{}, kAllowedInEnvvar);

AddOption("--experimental-shadow-realm",
"",
&PerIsolateOptions::experimental_shadow_realm,
kAllowedInEnvvar);
AddOption("--harmony-shadow-realm", "", V8Option{});
Implies("--experimental-shadow-realm", "--harmony-shadow-realm");
Implies("--harmony-shadow-realm", "--experimental-shadow-realm");
ImpliesNot("--no-harmony-shadow-realm", "--experimental-shadow-realm");
AddOption("--build-snapshot",
"Generate a snapshot blob when the process exits.",
&PerIsolateOptions::build_snapshot,
Expand Down Expand Up @@ -1092,6 +1046,58 @@ PerProcessOptionsParser::PerProcessOptionsParser(
"performance.",
&PerProcessOptions::disable_wasm_trap_handler,
kAllowedInEnvvar);

// Explicitly add some V8 flags to mark them as allowed in NODE_OPTIONS.
// V8 flags are be per-process options and should not be modified with
// NODE_OPTIONS or execArgv with a Worker constructor.
AddOption("--abort-on-uncaught-exception",
"aborting instead of exiting causes a core file to be generated "
"for analysis",
V8Option{},
kAllowedInEnvvar);
AddOption("--expose-gc", "expose gc extension", V8Option{}, kAllowedInEnvvar);
AddOption("--interpreted-frames-native-stack",
"help system profilers to translate JavaScript interpreted frames",
V8Option{},
kAllowedInEnvvar);
AddOption("--max-old-space-size", "", V8Option{}, kAllowedInEnvvar);
AddOption("--max-semi-space-size", "", V8Option{}, kAllowedInEnvvar);
AddOption("--perf-basic-prof", "", V8Option{}, kAllowedInEnvvar);
AddOption(
"--perf-basic-prof-only-functions", "", V8Option{}, kAllowedInEnvvar);
AddOption("--perf-prof", "", V8Option{}, kAllowedInEnvvar);
AddOption("--perf-prof-unwinding-info", "", V8Option{}, kAllowedInEnvvar);
AddOption("--prof", "Generate V8 profiler output.", V8Option{});
AddOption("--stack-trace-limit", "", V8Option{}, kAllowedInEnvvar);
AddOption("--disallow-code-generation-from-strings",
"disallow eval and friends",
V8Option{},
kAllowedInEnvvar);
AddOption("--huge-max-old-generation-size",
"increase default maximum heap size on machines with 16GB memory "
"or more",
V8Option{},
kAllowedInEnvvar);
AddOption("--jitless",
"disable runtime allocation of executable memory",
V8Option{},
kAllowedInEnvvar);
AddOption("--enable-etw-stack-walking",
"provides heap data to ETW Windows native tracing",
V8Option{},
kAllowedInEnvvar);

AddOption("--experimental-top-level-await", "", NoOp{}, kAllowedInEnvvar);

AddOption("--experimental-shadow-realm",
"",
&PerProcessOptions::experimental_shadow_realm,
kAllowedInEnvvar);
AddOption("--harmony-shadow-realm", "", V8Option{});

Implies("--experimental-shadow-realm", "--harmony-shadow-realm");
Implies("--harmony-shadow-realm", "--experimental-shadow-realm");
ImpliesNot("--no-harmony-shadow-realm", "--experimental-shadow-realm");
}

inline std::string RemoveBrackets(const std::string& host) {
Expand Down
2 changes: 1 addition & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,6 @@ class PerIsolateOptions : public Options {
bool track_heap_objects = false;
bool report_uncaught_exception = false;
bool report_on_signal = false;
bool experimental_shadow_realm = false;
std::string report_signal = "SIGUSR2";
bool build_snapshot = false;
std::string build_snapshot_config;
Expand Down Expand Up @@ -289,6 +288,7 @@ class PerProcessOptions : public Options {
bool print_v8_help = false;
bool print_version = false;
std::string experimental_sea_config;
bool experimental_shadow_realm = false;
std::string run;

#ifdef NODE_HAVE_I18N_SUPPORT
Expand Down
45 changes: 23 additions & 22 deletions test/parallel/test-cli-node-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,6 @@ expectNoWorker('--trace-event-file-pattern {pid}-${rotation}.trace_events ' +
'--trace-event-categories node.async_hooks', 'B\n');
expect('--unhandled-rejections=none', 'B\n');

if (common.isLinux) {
expect('--perf-basic-prof', 'B\n');
expect('--perf-basic-prof-only-functions', 'B\n');

if (['arm', 'x64'].includes(process.arch)) {
expect('--perf-prof', 'B\n');
expect('--perf-prof-unwinding-info', 'B\n');
}
}

if (common.hasCrypto) {
expectNoWorker('--use-openssl-ca', 'B\n');
expectNoWorker('--use-bundled-ca', 'B\n');
Expand All @@ -67,20 +57,31 @@ if (common.hasCrypto) {
}

// V8 options
expect('--abort_on-uncaught_exception', 'B\n');
expect('--disallow-code-generation-from-strings', 'B\n');
expect('--expose-gc', 'B\n');
expect('--huge-max-old-generation-size', 'B\n');
expect('--jitless', 'B\n');
expect('--max-old-space-size=0', 'B\n');
expect('--max-semi-space-size=0', 'B\n');
expect('--stack-trace-limit=100',
/(\s*at f \(\[(eval|worker eval)\]:1:\d*\)\r?\n)/,
'(function f() { f(); })();',
true);
expectNoWorker('--abort_on-uncaught_exception', 'B\n');
expectNoWorker('--disallow-code-generation-from-strings', 'B\n');
expectNoWorker('--expose-gc', 'B\n');
expectNoWorker('--huge-max-old-generation-size', 'B\n');
expectNoWorker('--jitless', 'B\n');
expectNoWorker('--max-old-space-size=0', 'B\n');
expectNoWorker('--max-semi-space-size=0', 'B\n');
expectNoWorker('--stack-trace-limit=100',
/(\s*at f \(\[eval\]:1:\d*\)\r?\n)/,
'(function f() { f(); })();',
true);
// Unsupported on arm. See https://crbug.com/v8/8713.
if (!['arm', 'arm64'].includes(process.arch))
expect('--interpreted-frames-native-stack', 'B\n');
expectNoWorker('--interpreted-frames-native-stack', 'B\n');

// Linux only V8 options
if (common.isLinux) {
expectNoWorker('--perf-basic-prof', 'B\n');
expectNoWorker('--perf-basic-prof-only-functions', 'B\n');

if (['arm', 'x64'].includes(process.arch)) {
expectNoWorker('--perf-prof', 'B\n');
expectNoWorker('--perf-prof-unwinding-info', 'B\n');
}
}

// Workers can't eval as ES Modules. https://github.com/nodejs/node/issues/30682
expectNoWorker('--input-type=module',
Expand Down
Loading