From f51b9438e51fa699eb4e8c7c9768c7a6a949de43 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 25 Aug 2016 11:39:56 +0200 Subject: [PATCH 1/3] src: log warning on process.binding('natives') This is used by old versions of graceful-fs but will stop working in the not too distant future. Print a warning so people know to upgrade. --- lib/_debugger.js | 2 +- lib/internal/bootstrap_node.js | 2 +- lib/internal/v8_prof_processor.js | 2 +- src/node.cc | 19 ++++++++++++++++++- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/_debugger.js b/lib/_debugger.js index 738eb9871bb2b6..906abd218447fc 100644 --- a/lib/_debugger.js +++ b/lib/_debugger.js @@ -180,7 +180,7 @@ Client.prototype._addHandle = function(desc) { }; -const natives = process.binding('natives'); +const natives = process.binding('$natives'); Client.prototype._addScript = function(desc) { diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 17e34f40361235..663586ba0dcfd7 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -401,7 +401,7 @@ this.loading = false; } - NativeModule._source = process.binding('natives'); + NativeModule._source = process.binding('$natives'); NativeModule._cache = {}; NativeModule.require = function(id) { diff --git a/lib/internal/v8_prof_processor.js b/lib/internal/v8_prof_processor.js index 86f06629e1b025..c243c61e209dc1 100644 --- a/lib/internal/v8_prof_processor.js +++ b/lib/internal/v8_prof_processor.js @@ -15,7 +15,7 @@ const scriptFiles = [ var script = ''; scriptFiles.forEach(function(s) { - script += process.binding('natives')[s] + '\n'; + script += process.binding('$natives')[s] + '\n'; }); var tickArguments = []; diff --git a/src/node.cc b/src/node.cc index 5cab485bbd7c40..a295233f86505b 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2580,7 +2580,24 @@ static void Binding(const FunctionCallbackInfo& args) { exports = Object::New(env->isolate()); DefineConstants(env->isolate(), exports); cache->Set(module, exports); - } else if (!strcmp(*module_v, "natives")) { + } else if (!strcmp(*module_v, "$natives") || !strcmp(*module_v, "natives")) { + if (!strcmp(*module_v, "natives")) { + // graceful-fs < 4 evals process.binding('natives').fs, which prohibits + // using internal modules in that module. Encourage people to upgrade. + auto pid = getpid(); + fprintf(stderr, + "(node:%d) process.binding('natives') is deprecated.\n", pid); + fprintf(stderr, + "(node:%d) If you use graceful-fs < 4, please update.\n", pid); + auto stack_trace = v8::StackTrace::CurrentStackTrace(env->isolate(), 12); + for (int i = 0, n = stack_trace->GetFrameCount(); i < n; i += 1) { + Local frame = stack_trace->GetFrame(i); + node::Utf8Value name(env->isolate(), frame->GetScriptName()); + fprintf(stderr, + "(node:%d) at %s:%d\n", pid, *name, frame->GetLineNumber()); + } + fflush(stderr); + } exports = Object::New(env->isolate()); DefineJavaScript(env, exports); cache->Set(module, exports); From 35a62c5272c6c2c9b7ade23a8ee3680de1235c0f Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 25 Aug 2016 14:02:55 +0200 Subject: [PATCH 2/3] squash! alternative take without $natives --- lib/_debugger.js | 2 +- lib/internal/bootstrap_node.js | 2 +- lib/internal/v8_prof_processor.js | 2 +- src/env-inl.h | 9 ++++ src/env.h | 5 ++ src/node.cc | 78 +++++++++++++++++-------------- src/node_contextify.cc | 20 ++++---- src/node_internals.h | 4 ++ 8 files changed, 75 insertions(+), 47 deletions(-) diff --git a/lib/_debugger.js b/lib/_debugger.js index 906abd218447fc..738eb9871bb2b6 100644 --- a/lib/_debugger.js +++ b/lib/_debugger.js @@ -180,7 +180,7 @@ Client.prototype._addHandle = function(desc) { }; -const natives = process.binding('$natives'); +const natives = process.binding('natives'); Client.prototype._addScript = function(desc) { diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 663586ba0dcfd7..17e34f40361235 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -401,7 +401,7 @@ this.loading = false; } - NativeModule._source = process.binding('$natives'); + NativeModule._source = process.binding('natives'); NativeModule._cache = {}; NativeModule.require = function(id) { diff --git a/lib/internal/v8_prof_processor.js b/lib/internal/v8_prof_processor.js index c243c61e209dc1..86f06629e1b025 100644 --- a/lib/internal/v8_prof_processor.js +++ b/lib/internal/v8_prof_processor.js @@ -15,7 +15,7 @@ const scriptFiles = [ var script = ''; scriptFiles.forEach(function(s) { - script += process.binding('$natives')[s] + '\n'; + script += process.binding('natives')[s] + '\n'; }); var tickArguments = []; diff --git a/src/env-inl.h b/src/env-inl.h index bdd3a0585ce026..1fd7da5ad502c6 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -277,6 +277,15 @@ inline uint64_t Environment::timer_base() const { return timer_base_; } +inline int Environment::bootstrap_script_id() const { + CHECK(!internal_script_ids_.empty()); + return internal_script_ids_[0]; +} + +inline std::vector* Environment::internal_script_ids() { + return &internal_script_ids_; +} + inline bool Environment::using_domains() const { return using_domains_; } diff --git a/src/env.h b/src/env.h index d8b6b8bb5fdfa2..295d13ee960fff 100644 --- a/src/env.h +++ b/src/env.h @@ -16,6 +16,7 @@ #include "v8.h" #include +#include // Caveat emptor: we're going slightly crazy with macros here but the end // hopefully justifies the means. We have a lot of per-context properties @@ -451,6 +452,9 @@ class Environment { inline node_ares_task_list* cares_task_list(); inline IsolateData* isolate_data() const; + inline int bootstrap_script_id() const; + inline std::vector* internal_script_ids(); + inline bool using_domains() const; inline void set_using_domains(bool value); @@ -578,6 +582,7 @@ class Environment { uint32_t* heap_space_statistics_buffer_ = nullptr; char* http_parser_buffer_; + std::vector internal_script_ids_; #define V(PropertyName, TypeName) \ v8::Persistent PropertyName ## _; diff --git a/src/node.cc b/src/node.cc index a295233f86505b..70df68ec68ae59 100644 --- a/src/node.cc +++ b/src/node.cc @@ -59,6 +59,7 @@ #include #include +#include #include #include @@ -123,6 +124,7 @@ using v8::PromiseRejectMessage; using v8::PropertyCallbackInfo; using v8::ScriptOrigin; using v8::SealHandleScope; +using v8::StackTrace; using v8::String; using v8::TryCatch; using v8::Uint32Array; @@ -1659,35 +1661,6 @@ static void ReportException(Environment* env, const TryCatch& try_catch) { } -// Executes a str within the current v8 context. -static Local ExecuteString(Environment* env, - Local source, - Local filename) { - EscapableHandleScope scope(env->isolate()); - TryCatch try_catch(env->isolate()); - - // try_catch must be nonverbose to disable FatalException() handler, - // we will handle exceptions ourself. - try_catch.SetVerbose(false); - - ScriptOrigin origin(filename); - MaybeLocal script = - v8::Script::Compile(env->context(), source, &origin); - if (script.IsEmpty()) { - ReportException(env, try_catch); - exit(3); - } - - Local result = script.ToLocalChecked()->Run(); - if (result.IsEmpty()) { - ReportException(env, try_catch); - exit(4); - } - - return scope.Escape(result); -} - - static void GetActiveRequests(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -2543,6 +2516,21 @@ void ClearFatalExceptionHandlers(Environment* env) { } +int GetCallerScriptId(v8::Isolate* isolate) { + auto options = StackTrace::kScriptId; + auto stack_trace = StackTrace::CurrentStackTrace(isolate, 1, options); + if (stack_trace->GetFrameCount() > 0) + return stack_trace->GetFrame(0)->GetScriptId(); + return Message::kNoScriptIdInfo; +} + + +inline bool IsInternalScriptId(Environment* env, int script_id) { + auto ids = env->internal_script_ids(); + return ids->end() != std::find(ids->begin(), ids->end(), script_id); +} + + static void Binding(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -2580,8 +2568,9 @@ static void Binding(const FunctionCallbackInfo& args) { exports = Object::New(env->isolate()); DefineConstants(env->isolate(), exports); cache->Set(module, exports); - } else if (!strcmp(*module_v, "$natives") || !strcmp(*module_v, "natives")) { - if (!strcmp(*module_v, "natives")) { + } else if (!strcmp(*module_v, "natives")) { + auto caller_script_id = GetCallerScriptId(env->isolate()); + if (!IsInternalScriptId(env, caller_script_id)) { // graceful-fs < 4 evals process.binding('natives').fs, which prohibits // using internal modules in that module. Encourage people to upgrade. auto pid = getpid(); @@ -2589,7 +2578,7 @@ static void Binding(const FunctionCallbackInfo& args) { "(node:%d) process.binding('natives') is deprecated.\n", pid); fprintf(stderr, "(node:%d) If you use graceful-fs < 4, please update.\n", pid); - auto stack_trace = v8::StackTrace::CurrentStackTrace(env->isolate(), 12); + auto stack_trace = StackTrace::CurrentStackTrace(env->isolate(), 12); for (int i = 0, n = stack_trace->GetFrameCount(); i < n; i += 1) { Local frame = stack_trace->GetFrame(i); node::Utf8Value name(env->isolate(), frame->GetScriptName()); @@ -2600,7 +2589,6 @@ static void Binding(const FunctionCallbackInfo& args) { } exports = Object::New(env->isolate()); DefineJavaScript(env, exports); - cache->Set(module, exports); } else { char errmsg[1024]; snprintf(errmsg, @@ -3394,14 +3382,32 @@ void LoadEnvironment(Environment* env) { // 'internal_bootstrap_node_native' is the string containing that source code. Local script_name = FIXED_ONE_BYTE_STRING(env->isolate(), "bootstrap_node.js"); - Local f_value = ExecuteString(env, MainSource(env), script_name); + ScriptOrigin origin(script_name); + Local script; + auto maybe_script = + v8::Script::Compile(env->context(), MainSource(env), &origin); + if (!maybe_script.ToLocal(&script)) { + ReportException(env, try_catch); + exit(3); + } + + auto internal_script_ids = env->internal_script_ids(); + CHECK(internal_script_ids->empty()); + internal_script_ids->push_back(script->GetUnboundScript()->GetId()); + + Local result = script->Run(); + if (result.IsEmpty()) { + ReportException(env, try_catch); + exit(4); + } + if (try_catch.HasCaught()) { ReportException(env, try_catch); exit(10); } // The bootstrap_node.js file returns a function 'f' - CHECK(f_value->IsFunction()); - Local f = Local::Cast(f_value); + CHECK(result->IsFunction()); + Local f = Local::Cast(result); // Now we call 'f' with the 'process' variable that we've built up with // all our bindings. Inside bootstrap_node.js we'll take care of diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 091b2a5ee3f711..05524b65c63317 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -515,20 +515,18 @@ class ContextifyScript : public BaseObject { else if (produce_cached_data) compile_options = ScriptCompiler::kProduceCodeCache; - MaybeLocal v8_script = ScriptCompiler::CompileUnboundScript( - env->isolate(), - &source, - compile_options); - - if (v8_script.IsEmpty()) { + Local script; + auto maybe_script = + ScriptCompiler::CompileUnboundScript(env->isolate(), &source, + compile_options); + if (!maybe_script.ToLocal(&script)) { if (display_errors) { DecorateErrorStack(env, try_catch); } try_catch.ReThrow(); return; } - contextify_script->script_.Reset(env->isolate(), - v8_script.ToLocalChecked()); + contextify_script->script_.Reset(env->isolate(), script); if (compile_options == ScriptCompiler::kConsumeCodeCache) { args.This()->Set( @@ -548,6 +546,12 @@ class ContextifyScript : public BaseObject { env->cached_data_produced_string(), Boolean::New(env->isolate(), cached_data_produced)); } + + auto caller_script_id = GetCallerScriptId(env->isolate()); + if (caller_script_id == env->bootstrap_script_id()) { + // Called by NativeModule.require(). + env->internal_script_ids()->push_back(script->GetId()); + } } diff --git a/src/node_internals.h b/src/node_internals.h index e908da37b1f565..c77a00ae9a75df 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -151,6 +151,10 @@ void SetupProcessObject(Environment* env, int exec_argc, const char* const* exec_argv); +// Returns the script id of the top-most JS frame or v8::Message::kNoScriptId +// if no such frame exists or if the frame cannot be mapped to a script. +int GetCallerScriptId(v8::Isolate* isolate); + enum Endianness { kLittleEndian, // _Not_ LITTLE_ENDIAN, clashes with endian.h. kBigEndian From dc8e0e672aa746662d1d177ac9742b4ab3383963 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 26 Aug 2016 02:44:06 +0200 Subject: [PATCH 3/3] squash! do not print internal stack frames --- src/env.cc | 75 +++++++++++++++++++++++++---------------------------- src/env.h | 8 ++++++ src/node.cc | 26 +++++-------------- 3 files changed, 50 insertions(+), 59 deletions(-) diff --git a/src/env.cc b/src/env.cc index 8efe13816c0ee9..00aed1535959f8 100644 --- a/src/env.cc +++ b/src/env.cc @@ -11,6 +11,7 @@ #endif #include +#include namespace node { @@ -18,7 +19,6 @@ using v8::Context; using v8::FunctionTemplate; using v8::HandleScope; using v8::Local; -using v8::Message; using v8::StackFrame; using v8::StackTrace; @@ -108,47 +108,44 @@ void Environment::StopProfilerIdleNotifier() { uv_check_stop(&idle_check_handle_); } -void Environment::PrintSyncTrace() const { - if (!trace_sync_io_) - return; +bool Environment::IsInternalScriptId(int script_id) const { + auto ids = internal_script_ids_; + return ids.end() != std::find(ids.begin(), ids.end(), script_id); +} +void Environment::PrintStackTrace(FILE* stream, const char* prefix, + PrintStackTraceMode mode) const { HandleScope handle_scope(isolate()); - Local stack = - StackTrace::CurrentStackTrace(isolate(), 10, StackTrace::kDetailed); - - fprintf(stderr, "(node:%d) WARNING: Detected use of sync API\n", getpid()); - - for (int i = 0; i < stack->GetFrameCount() - 1; i++) { - Local stack_frame = stack->GetFrame(i); - node::Utf8Value fn_name_s(isolate(), stack_frame->GetFunctionName()); - node::Utf8Value script_name(isolate(), stack_frame->GetScriptName()); - const int line_number = stack_frame->GetLineNumber(); - const int column = stack_frame->GetColumn(); - - if (stack_frame->IsEval()) { - if (stack_frame->GetScriptId() == Message::kNoScriptIdInfo) { - fprintf(stderr, " at [eval]:%i:%i\n", line_number, column); - } else { - fprintf(stderr, - " at [eval] (%s:%i:%i)\n", - *script_name, - line_number, - column); - } - break; - } - - if (fn_name_s.length() == 0) { - fprintf(stderr, " at %s:%i:%i\n", *script_name, line_number, column); - } else { - fprintf(stderr, - " at %s (%s:%i:%i)\n", - *fn_name_s, - *script_name, - line_number, - column); - } + // kExposeFramesAcrossSecurityOrigins is currently implied but let's be + // explicit so it won't break after a V8 upgrade. + // If you include kColumnOffset here, you should ensure that the offset of + // the first line for non-eval'd code compensates for the module wrapper. + using O = StackTrace::StackTraceOptions; + auto options = static_cast(StackTrace::kExposeFramesAcrossSecurityOrigins | + StackTrace::kFunctionName | + StackTrace::kLineNumber | + StackTrace::kScriptId | + StackTrace::kScriptName); + auto stack_trace = StackTrace::CurrentStackTrace(isolate(), 12, options); + for (int i = 0, n = stack_trace->GetFrameCount(); i < n; i += 1) { + Local frame = stack_trace->GetFrame(i); + if (mode == kNoInternalScripts && IsInternalScriptId(frame->GetScriptId())) + continue; + node::Utf8Value function_name(isolate(), frame->GetFunctionName()); + node::Utf8Value script_name(isolate(), frame->GetScriptName()); + fprintf(stream, "%s at %s (%s:%d)\n", prefix, + **function_name ? *function_name : "", + *script_name, frame->GetLineNumber()); } +} + +void Environment::PrintSyncTrace() const { + if (!trace_sync_io_) + return; + char prefix[32]; + snprintf(prefix, sizeof(prefix), "(node:%d) ", getpid()); + fprintf(stderr, "%sWARNING: Detected use of sync API\n", prefix); + PrintStackTrace(stderr, prefix); fflush(stderr); } diff --git a/src/env.h b/src/env.h index 295d13ee960fff..23ac36c4ae1355 100644 --- a/src/env.h +++ b/src/env.h @@ -16,6 +16,7 @@ #include "v8.h" #include +#include #include // Caveat emptor: we're going slightly crazy with macros here but the end @@ -511,6 +512,13 @@ class Environment { inline v8::Local NewInternalFieldObject(); + bool IsInternalScriptId(int script_id) const; + + enum PrintStackTraceMode { kInternalScripts, kNoInternalScripts }; + + void PrintStackTrace(FILE* stream, const char* prefix = "", + PrintStackTraceMode mode = kInternalScripts) const; + // Strings and private symbols are shared across shared contexts // The getters simply proxy to the per-isolate primitive. #define VP(PropertyName, StringValue) V(v8::Private, PropertyName, StringValue) diff --git a/src/node.cc b/src/node.cc index 70df68ec68ae59..34baea3815be1a 100644 --- a/src/node.cc +++ b/src/node.cc @@ -59,7 +59,6 @@ #include #include -#include #include #include @@ -2525,12 +2524,6 @@ int GetCallerScriptId(v8::Isolate* isolate) { } -inline bool IsInternalScriptId(Environment* env, int script_id) { - auto ids = env->internal_script_ids(); - return ids->end() != std::find(ids->begin(), ids->end(), script_id); -} - - static void Binding(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -2570,21 +2563,14 @@ static void Binding(const FunctionCallbackInfo& args) { cache->Set(module, exports); } else if (!strcmp(*module_v, "natives")) { auto caller_script_id = GetCallerScriptId(env->isolate()); - if (!IsInternalScriptId(env, caller_script_id)) { + if (!env->IsInternalScriptId(caller_script_id)) { // graceful-fs < 4 evals process.binding('natives').fs, which prohibits // using internal modules in that module. Encourage people to upgrade. - auto pid = getpid(); - fprintf(stderr, - "(node:%d) process.binding('natives') is deprecated.\n", pid); - fprintf(stderr, - "(node:%d) If you use graceful-fs < 4, please update.\n", pid); - auto stack_trace = StackTrace::CurrentStackTrace(env->isolate(), 12); - for (int i = 0, n = stack_trace->GetFrameCount(); i < n; i += 1) { - Local frame = stack_trace->GetFrame(i); - node::Utf8Value name(env->isolate(), frame->GetScriptName()); - fprintf(stderr, - "(node:%d) at %s:%d\n", pid, *name, frame->GetLineNumber()); - } + char prefix[32]; + snprintf(prefix, sizeof(prefix), "(node:%d) ", getpid()); + fprintf(stderr, "%sprocess.binding('natives') is deprecated.\n", prefix); + fprintf(stderr, "%sIf you use graceful-fs < 4, please update.\n", prefix); + env->PrintStackTrace(stderr, prefix, Environment::kNoInternalScripts); fflush(stderr); } exports = Object::New(env->isolate());