Skip to content

Commit

Permalink
src: parse --stack-trace-limit and use it in --trace-* flags
Browse files Browse the repository at this point in the history
Previously, --trace-exit and --trace-sync-io doesn't take care
of --stack-trace-limit and always print a stack trace with maximum
size of 10. This patch parses --stack-trace-limit during
initialization and use the value in --trace-* flags.

PR-URL: #55121
Fixes: #55100
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
  • Loading branch information
joyeecheung authored and targos committed Oct 4, 2024
1 parent a57c8ba commit b229083
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 9 deletions.
4 changes: 4 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,10 @@ inline double Environment::get_default_trigger_async_id() {
return default_trigger_async_id;
}

inline int64_t Environment::stack_trace_limit() const {
return isolate_data_->options()->stack_trace_limit;
}

inline std::shared_ptr<EnvironmentOptions> Environment::options() {
return options_;
}
Expand Down
16 changes: 10 additions & 6 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1252,9 +1252,11 @@ void Environment::PrintSyncTrace() const {

fprintf(
stderr, "(node:%d) WARNING: Detected use of sync API\n", uv_os_getpid());
PrintStackTrace(isolate(),
StackTrace::CurrentStackTrace(
isolate(), stack_trace_limit(), StackTrace::kDetailed));
PrintStackTrace(
isolate(),
StackTrace::CurrentStackTrace(isolate(),
static_cast<int>(stack_trace_limit()),
StackTrace::kDetailed));
}

MaybeLocal<Value> Environment::RunSnapshotSerializeCallback() const {
Expand Down Expand Up @@ -1856,9 +1858,11 @@ void Environment::Exit(ExitCode exit_code) {
fprintf(stderr,
"WARNING: Exited the environment with code %d\n",
static_cast<int>(exit_code));
PrintStackTrace(isolate(),
StackTrace::CurrentStackTrace(
isolate(), stack_trace_limit(), StackTrace::kDetailed));
PrintStackTrace(
isolate(),
StackTrace::CurrentStackTrace(isolate(),
static_cast<int>(stack_trace_limit()),
StackTrace::kDetailed));
}
process_exit_handler_(this, exit_code);
}
Expand Down
2 changes: 1 addition & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -981,7 +981,7 @@ class Environment final : public MemoryRetainer {
inline std::shared_ptr<EnvironmentOptions> options();
inline std::shared_ptr<ExclusiveAccess<HostPort>> inspector_host_port();

inline int32_t stack_trace_limit() const { return 10; }
inline int64_t stack_trace_limit() const;

#if HAVE_INSPECTOR
void set_coverage_connection(
Expand Down
7 changes: 6 additions & 1 deletion src/node_options-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -447,9 +447,14 @@ void OptionsParser<Options>::Parse(
case kBoolean:
*Lookup<bool>(info.field, options) = !is_negation;
break;
case kInteger:
case kInteger: {
// Special case to pass --stack-trace-limit down to V8.
if (name == "--stack-trace-limit") {
v8_args->push_back(arg);
}
*Lookup<int64_t>(info.field, options) = std::atoll(value.c_str());
break;
}
case kUInteger:
*Lookup<uint64_t>(info.field, options) =
std::strtoull(value.c_str(), nullptr, 10);
Expand Down
10 changes: 9 additions & 1 deletion src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -921,7 +921,10 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
"--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("--stack-trace-limit",
"",
&PerIsolateOptions::stack_trace_limit,
kAllowedInEnvvar);
AddOption("--disallow-code-generation-from-strings",
"disallow eval and friends",
V8Option{},
Expand Down Expand Up @@ -1313,6 +1316,11 @@ void GetCLIOptionsValues(const FunctionCallbackInfo<Value>& args) {
if (item.first == "--abort-on-uncaught-exception") {
value = Boolean::New(isolate,
s.original_per_env->abort_on_uncaught_exception);
} else if (item.first == "--stack-trace-limit") {
value =
Number::New(isolate,
static_cast<double>(
*_ppop_instance.Lookup<int64_t>(field, opts)));
} else {
value = undefined_value;
}
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ class PerIsolateOptions : public Options {
bool report_uncaught_exception = false;
bool report_on_signal = false;
bool experimental_shadow_realm = false;
int64_t stack_trace_limit = 10;
std::string report_signal = "SIGUSR2";
bool build_snapshot = false;
std::string build_snapshot_config;
Expand Down
15 changes: 15 additions & 0 deletions test/fixtures/deep-exit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

// This is meant to be run with --trace-exit.

const depth = parseInt(process.env.STACK_DEPTH) || 30;
let counter = 1;
function recurse() {
if (counter++ < depth) {
recurse();
} else {
process.exit(0);
}
}

recurse();
42 changes: 42 additions & 0 deletions test/parallel/test-trace-exit-stack-limit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
'use strict';

// This tests that --stack-trace-limit can be used to tweak the stack trace size of --trace-exit.
require('../common');
const fixture = require('../common/fixtures');
const { spawnSyncAndAssert } = require('../common/child_process');
const assert = require('assert');

// When the stack trace limit is bigger than the stack trace size, it should output them all.
spawnSyncAndAssert(
process.execPath,
['--trace-exit', '--stack-trace-limit=50', fixture.path('deep-exit.js')],
{
env: {
...process.env,
STACK_DEPTH: 30
}
},
{
stderr(output) {
const matches = [...output.matchAll(/at recurse/g)];
assert.strictEqual(matches.length, 30);
}
});

// When the stack trace limit is smaller than the stack trace size, it should truncate the stack size.
spawnSyncAndAssert(
process.execPath,
['--trace-exit', '--stack-trace-limit=30', fixture.path('deep-exit.js')],
{
env: {
...process.env,
STACK_DEPTH: 30
}
},
{
stderr(output) {
const matches = [...output.matchAll(/at recurse/g)];
// The top frame is process.exit(), so one frame from recurse() is truncated.
assert.strictEqual(matches.length, 29);
}
});

0 comments on commit b229083

Please sign in to comment.