Skip to content

Commit

Permalink
src: mark fatal error functions as noreturn
Browse files Browse the repository at this point in the history
OnFatalError and OOMErrorHandler will not return control flow to the
calling function.

node::FatalError is an alias of node::OnFatalError. Replace all the
callsites with node::OnFatalError instead.

PR-URL: #47695
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
  • Loading branch information
legendecas authored and targos committed May 3, 2023
1 parent f88132f commit 2acb57b
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 20 deletions.
3 changes: 1 addition & 2 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -726,8 +726,7 @@ Maybe<bool> InitializeContextRuntime(Local<Context> context) {
}
} else if (per_process::cli_options->disable_proto != "") {
// Validated in ProcessGlobalArgs
FatalError("InitializeContextRuntime()",
"invalid --disable-proto mode");
OnFatalError("InitializeContextRuntime()", "invalid --disable-proto mode");
}

return Just(true);
Expand Down
6 changes: 3 additions & 3 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ namespace node {
namespace inspector {
namespace {

using node::FatalError;
using node::OnFatalError;

using v8::Context;
using v8::Function;
Expand Down Expand Up @@ -901,8 +901,8 @@ 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);
FatalError("\nnode::inspector::Agent::ToggleAsyncHook",
"Cannot toggle Inspector's AsyncHook, please report this.");
OnFatalError("\nnode::inspector::Agent::ToggleAsyncHook",
"Cannot toggle Inspector's AsyncHook, please report this.");
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,8 @@ NODE_EXTERN ArrayBufferAllocator* GetArrayBufferAllocator(IsolateData* data);
// a snapshot and have a main context that was read from that snapshot.
NODE_EXTERN v8::Local<v8::Context> GetMainContext(Environment* env);

NODE_EXTERN void OnFatalError(const char* location, const char* message);
[[noreturn]] NODE_EXTERN void OnFatalError(const char* location,
const char* message);
NODE_EXTERN void PromiseRejectCallback(v8::PromiseRejectMessage message);
NODE_EXTERN bool AllowWasmCodeGenerationCallback(v8::Local<v8::Context> context,
v8::Local<v8::String>);
Expand Down
2 changes: 1 addition & 1 deletion src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ NAPI_NO_RETURN void NAPI_CDECL napi_fatal_error(const char* location,
message_string.assign(const_cast<char*>(message), strlen(message));
}

node::FatalError(location_string.c_str(), message_string.c_str());
node::OnFatalError(location_string.c_str(), message_string.c_str());
}

napi_status NAPI_CDECL
Expand Down
11 changes: 3 additions & 8 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -504,13 +504,7 @@ static void ReportFatalException(Environment* env,
fflush(stderr);
}

[[noreturn]] void FatalError(const char* location, const char* message) {
OnFatalError(location, message);
// to suppress compiler warning
ABORT();
}

void OnFatalError(const char* location, const char* message) {
[[noreturn]] void OnFatalError(const char* location, const char* message) {
if (location) {
FPrintF(stderr, "FATAL ERROR: %s %s\n", location, message);
} else {
Expand All @@ -532,7 +526,8 @@ void OnFatalError(const char* location, const char* message) {
ABORT();
}

void OOMErrorHandler(const char* location, const v8::OOMDetails& details) {
[[noreturn]] void OOMErrorHandler(const char* location,
const v8::OOMDetails& details) {
const char* message =
details.is_heap_oom ? "Allocation failed - JavaScript heap out of memory"
: "Allocation failed - process out of memory";
Expand Down
6 changes: 3 additions & 3 deletions src/node_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ void AppendExceptionLine(Environment* env,
v8::Local<v8::Message> message,
enum ErrorHandlingMode mode);

[[noreturn]] void FatalError(const char* location, const char* message);
void OnFatalError(const char* location, const char* message);
void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
[[noreturn]] void OnFatalError(const char* location, const char* message);
[[noreturn]] void OOMErrorHandler(const char* location,
const v8::OOMDetails& details);

// Helpers to construct errors similar to the ones provided by
// lib/internal/errors.js.
Expand Down
3 changes: 1 addition & 2 deletions src/node_watchdog.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ Watchdog::Watchdog(v8::Isolate* isolate, uint64_t ms, bool* timed_out)
int rc;
rc = uv_loop_init(&loop_);
if (rc != 0) {
FatalError("node::Watchdog::Watchdog()",
"Failed to initialize uv loop.");
OnFatalError("node::Watchdog::Watchdog()", "Failed to initialize uv loop.");
}

rc = uv_async_init(&loop_, &async_, [](uv_async_t* signal) {
Expand Down

0 comments on commit 2acb57b

Please sign in to comment.