src: log warning on process.binding('natives')

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<int>* Environment::internal_script_ids() {
+  return &internal_script_ids_;
+}
+
 inline bool Environment::using_domains() const {
   return using_domains_;
 }

src/env.cc

@@ -11,14 +11,14 @@
 #endif
 
 #include <stdio.h>
+#include <algorithm>
 
 namespace node {
 
 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<v8::StackTrace> 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<StackFrame> 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<O>(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<v8::StackFrame> 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 : "<anonymous>",
+            *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);
 }
 

src/env.h

@@ -16,6 +16,8 @@
 #include "v8.h"
 
 #include <stdint.h>
+#include <stdio.h>
+#include <vector>
 
 // 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 +453,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<int>* internal_script_ids();
+
   inline bool using_domains() const;
   inline void set_using_domains(bool value);
 
@@ -507,6 +512,13 @@ class Environment {
 
   inline v8::Local<v8::Object> 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)
@@ -578,6 +590,7 @@ class Environment {
   uint32_t* heap_space_statistics_buffer_ = nullptr;
 
   char* http_parser_buffer_;
+  std::vector<int> internal_script_ids_;
 
 #define V(PropertyName, TypeName)                                             \
   v8::Persistent<TypeName> PropertyName ## _;

src/node.cc

@@ -123,6 +123,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 +1660,6 @@ static void ReportException(Environment* env, const TryCatch& try_catch) {
 }
 
 
-// Executes a str within the current v8 context.
-static Local<Value> ExecuteString(Environment* env,
-                                  Local<String> source,
-                                  Local<String> 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<v8::Script> script =
-      v8::Script::Compile(env->context(), source, &origin);
-  if (script.IsEmpty()) {
-    ReportException(env, try_catch);
-    exit(3);
-  }
-
-  Local<Value> result = script.ToLocalChecked()->Run();
-  if (result.IsEmpty()) {
-    ReportException(env, try_catch);
-    exit(4);
-  }
-
-  return scope.Escape(result);
-}
-
-
 static void GetActiveRequests(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
@@ -2543,6 +2515,15 @@ 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;
+}
+
+
 static void Binding(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
@@ -2581,9 +2562,19 @@ static void Binding(const FunctionCallbackInfo<Value>& args) {
     DefineConstants(env->isolate(), exports);
     cache->Set(module, exports);
   } else if (!strcmp(*module_v, "natives")) {
+    auto caller_script_id = GetCallerScriptId(env->isolate());
+    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.
+      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());
     DefineJavaScript(env, exports);
-    cache->Set(module, exports);
   } else {
     char errmsg[1024];
     snprintf(errmsg,
@@ -3377,14 +3368,32 @@ void LoadEnvironment(Environment* env) {
   // 'internal_bootstrap_node_native' is the string containing that source code.
   Local<String> script_name = FIXED_ONE_BYTE_STRING(env->isolate(),
                                                     "bootstrap_node.js");
-  Local<Value> f_value = ExecuteString(env, MainSource(env), script_name);
+  ScriptOrigin origin(script_name);
+  Local<v8::Script> 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<Value> 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<Function> f = Local<Function>::Cast(f_value);
+  CHECK(result->IsFunction());
+  Local<Function> f = Local<Function>::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

src/node_contextify.cc

@@ -515,20 +515,18 @@ class ContextifyScript : public BaseObject {
     else if (produce_cached_data)
       compile_options = ScriptCompiler::kProduceCodeCache;
 
-    MaybeLocal<UnboundScript> v8_script = ScriptCompiler::CompileUnboundScript(
-        env->isolate(),
-        &source,
-        compile_options);
-
-    if (v8_script.IsEmpty()) {
+    Local<UnboundScript> 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());
+    }
   }
 
 

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