src: parse --stack-trace-limit and use it in --trace-* flags

joyeecheung · targos · commit b22908323523 · 2024-10-04T19:20:19.000+02:00
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 <targos@protonmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
diff --git a/src/env-inl.h b/src/env-inl.h
@@ -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_;
 }
diff --git a/src/env.cc b/src/env.cc
@@ -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 {
@@ -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);
 }
diff --git a/src/env.h b/src/env.h
@@ -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(
diff --git a/src/node_options-inl.h b/src/node_options-inl.h
@@ -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);
diff --git a/src/node_options.cc b/src/node_options.cc
@@ -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{},
@@ -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;
         }
diff --git a/src/node_options.h b/src/node_options.h
@@ -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;
diff --git a/test/fixtures/deep-exit.js b/test/fixtures/deep-exit.js
@@ -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();
diff --git a/test/parallel/test-trace-exit-stack-limit.js b/test/parallel/test-trace-exit-stack-limit.js
@@ -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);
+    }
+  });

Original file line number	Diff line number	Diff line change
`@@ -458,6 +458,10 @@ inline double Environment::get_default_trigger_async_id() {`
`458`	`458`	`return default_trigger_async_id;`
`459`	`459`	`}`
`460`	`460`
	`461`	`+inline int64_t Environment::stack_trace_limit() const {`
	`462`	`+ return isolate_data_->options()->stack_trace_limit;`
	`463`	`+}`
	`464`	`+`
`461`	`465`	`inline std::shared_ptr<EnvironmentOptions> Environment::options() {`
`462`	`466`	`return options_;`
`463`	`467`	`}`