OSSForks/node
1
0
Fork 0
mirror of https://github.com/zebrajr/node.git synced 2026-01-15 12:15:26 +00:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

src: fix backtrace with tail [[noreturn]] abort

Browse Source 
A function tail calls [[noreturn]] node::Abort will print an incorrect
call stack because the frame pc was advanced when calling node::Abort
to an invalid op, which may vary on different platforms.

Dumps the backtrace in the ABORT macro instead to avoid calling
backtrace in a tail [[noreturn]] call. Removes the [[noreturn]]
attribute if a function calls backtrace and may be called as a tail
statement.

[[noreturn]] attribute of public functions like `napi_fatal_error` and
`node::OnFatalError` can not be removed as compilers may complain
about no return values after the removal.

PR-URL: https://github.com/nodejs/node/pull/50849
Refs: https://github.com/nodejs/node/issues/50761
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This commit is contained in:
Chengzhong Wu
2023-12-04 00:42:57 +08:00
committed by GitHub
parent 6b03e6061b
commit 16a54794df
7 changed files with 45 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/api/environment.cc
View File

@@ -734,7 +734,7 @@ Maybe<bool> InitializeContextRuntime(Local<Context> context) {
}
} else if (per_process::cli_options->disable_proto != "") {
// Validated in ProcessGlobalArgs
OnFatalError("InitializeContextRuntime()", "invalid --disable-proto mode");
UNREACHABLE("invalid --disable-proto mode");
}
return Just(true);

6
src/inspector_agent.cc
View File

@@ -36,9 +36,6 @@
namespace node {
namespace inspector {
namespace {
using node::OnFatalError;
using v8::Context;
using v8::Function;
using v8::HandleScope;
@@ -917,8 +914,7 @@ void Agent::ToggleAsyncHook(Isolate* isolate, Local<Function> fn) {
USE(fn->Call(context, Undefined(isolate), 0, nullptr));
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
PrintCaughtException(isolate, context, try_catch);
OnFatalError("\nnode::inspector::Agent::ToggleAsyncHook",
"Cannot toggle Inspector's AsyncHook, please report this.");
UNREACHABLE("Cannot toggle Inspector's AsyncHook, please report this.");
}
}

20
src/node_errors.cc
View File

@@ -376,14 +376,7 @@ void AppendExceptionLine(Environment* env,
.FromMaybe(false));
}
[[noreturn]] void Abort() {
DumpNativeBacktrace(stderr);
DumpJavaScriptBacktrace(stderr);
fflush(stderr);
ABORT_NO_BACKTRACE();
}
[[noreturn]] void Assert(const AssertionInfo& info) {
void Assert(const AssertionInfo& info) {
std::string name = GetHumanReadableProcessName();
fprintf(stderr,
@@ -396,7 +389,7 @@ void AppendExceptionLine(Environment* env,
info.message);
fflush(stderr);
Abort();
ABORT();
}
enum class EnhanceFatalException { kEnhance, kDontEnhance };
@@ -404,7 +397,7 @@ enum class EnhanceFatalException { kEnhance, kDontEnhance };
/**
* Report the exception to the inspector, then print it to stderr.
* This should only be used when the Node.js instance is about to exit
* (i.e. this should be followed by a env->Exit() or an Abort()).
* (i.e. this should be followed by a env->Exit() or an ABORT()).
*
* Use enhance_stack = EnhanceFatalException::kDontEnhance
* when it's unsafe to call into JavaScript.
@@ -576,8 +569,7 @@ static void ReportFatalException(Environment* env,
ABORT();
}
[[noreturn]] void OOMErrorHandler(const char* location,
const v8::OOMDetails& details) {
void OOMErrorHandler(const char* location, const v8::OOMDetails& details) {
// We should never recover from this handler so once it's true it's always
// true.
is_in_oom.store(true);
@@ -1063,7 +1055,7 @@ static void TriggerUncaughtException(const FunctionCallbackInfo<Value>& args) {
if (env != nullptr && env->abort_on_uncaught_exception()) {
ReportFatalException(
env, exception, message, EnhanceFatalException::kEnhance);
Abort();
ABORT();
}
bool from_promise = args[1]->IsTrue();
errors::TriggerUncaughtException(isolate, exception, message, from_promise);
@@ -1174,7 +1166,7 @@ void TriggerUncaughtException(Isolate* isolate,
// much we can do, so we just print whatever is useful and crash.
PrintToStderrAndFlush(
FormatCaughtException(isolate, context, error, message));
Abort();
ABORT();
}
// Invoke process._fatalException() to give user a chance to handle it.

9
src/node_errors.h
View File

@@ -19,9 +19,14 @@ void AppendExceptionLine(Environment* env,
v8::Local<v8::Message> message,
enum ErrorHandlingMode mode);
// This function calls backtrace, it should have not be marked as [[noreturn]].
// But it is a public API, removing the attribute can break.
// Prefer UNREACHABLE() internally instead, it doesn't need manually set
// location.
[[noreturn]] void OnFatalError(const char* location, const char* message);
[[noreturn]] void OOMErrorHandler(const char* location,
const v8::OOMDetails& details);
// This function calls backtrace, do not mark as [[noreturn]]. Read more in the
// ABORT macro.
void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
// Helpers to construct errors similar to the ones provided by
// lib/internal/errors.js.

2
src/node_process_methods.cc
View File

@@ -64,7 +64,7 @@ Mutex umask_mutex;
#define NANOS_PER_SEC 1000000000
static void Abort(const FunctionCallbackInfo<Value>& args) {
Abort();
ABORT();
}
// For internal testing only, not exposed to userland.

2
src/node_watchdog.cc
View File

@@ -45,7 +45,7 @@ Watchdog::Watchdog(v8::Isolate* isolate, uint64_t ms, bool* timed_out)
int rc;
rc = uv_loop_init(&loop_);
if (rc != 0) {
OnFatalError("node::Watchdog::Watchdog()", "Failed to initialize uv loop.");
UNREACHABLE("Failed to initialize uv loop.");
}
rc = uv_async_init(&loop_, &async_, [](uv_async_t* signal) {

39
src/util.h
View File

@@ -113,8 +113,9 @@ struct AssertionInfo {
const char* message;
const char* function;
};
[[noreturn]] void NODE_EXTERN_PRIVATE Assert(const AssertionInfo& info);
[[noreturn]] void NODE_EXTERN_PRIVATE Abort();
// This indirectly calls backtrace so it can not be marked as [[noreturn]].
void NODE_EXTERN_PRIVATE Assert(const AssertionInfo& info);
void DumpNativeBacktrace(FILE* fp);
void DumpJavaScriptBacktrace(FILE* fp);
@@ -125,16 +126,32 @@ void DumpJavaScriptBacktrace(FILE* fp);
#define ABORT_NO_BACKTRACE() abort()
#endif
#define ABORT() node::Abort()
// Caller of this macro must not be marked as [[noreturn]]. Printing of
// backtraces may not work correctly in [[noreturn]] functions because
// when generating code for them the compiler can choose not to
// maintain the frame pointers or link registers that are necessary for
// correct backtracing.
// `ABORT` must be a macro and not a [[noreturn]] function to make sure the
// backtrace is correct.
#define ABORT() \
do { \
node::DumpNativeBacktrace(stderr); \
node::DumpJavaScriptBacktrace(stderr); \
fflush(stderr); \
ABORT_NO_BACKTRACE(); \
} while (0)
#define ERROR_AND_ABORT(expr) \
do { \
/* Make sure that this struct does not end up in inline code, but */ \
/* rather in a read-only data section when modifying this code. */ \
static const node::AssertionInfo args = { \
__FILE__ ":" STRINGIFY(__LINE__), #expr, PRETTY_FUNCTION_NAME \
}; \
node::Assert(args); \
#define ERROR_AND_ABORT(expr) \
do { \
/* Make sure that this struct does not end up in inline code, but */ \
/* rather in a read-only data section when modifying this code. */ \
static const node::AssertionInfo args = { \
__FILE__ ":" STRINGIFY(__LINE__), #expr, PRETTY_FUNCTION_NAME}; \
node::Assert(args); \
/* `node::Assert` doesn't return. Add an [[noreturn]] abort() here to */ \
/* make the compiler happy about no return value in the caller */ \
/* function when calling ERROR_AND_ABORT. */ \
ABORT_NO_BACKTRACE(); \
} while (0)
#ifdef __GNUC__