Do not use js builtins object to determine whether a function is a builtin.

We can use the script type to determine that instead. Script of type
TYPE_NATIVE are considered builtins, TYPE_NORMAL are not. The only exception
to this rule is the empty function, for which the script is TYPE_NATIVE
(observable by the debugger), but should be stringified to "function () {}"
instead of "function () { [native code] }". For this, I introduce a
hide_source flag on the script object.

We also use IsBuiltin and IsSubjectToDebugging interchangeably. For debugger,
we now use the latter, hiding the detail that only non-builtins are debuggable.

R=mstarzinger@chromium.org

Review URL: https://codereview.chromium.org/1292283004

Cr-Commit-Position: refs/heads/master@{#30285}

diff --git a/src/compiler.cc b/src/compiler.cc
index 44df5a67fa4742df56a2c853d18ba6b40068e62d..76c70acd300cf783be181663ea67b629bec32233 100644 (file)
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -1361,6 +1361,7 @@ Handle<SharedFunctionInfo> Compiler::CompileScript(
     Handle<Script> script = isolate->factory()->NewScript(source);
     if (natives == NATIVES_CODE) {
       script->set_type(Smi::FromInt(Script::TYPE_NATIVE));
+      script->set_hide_source(true);
     }
     if (!script_name.is_null()) {
       script->set_name(*script_name);

diff --git a/src/debug/debug-scopes.cc b/src/debug/debug-scopes.cc
index b9204f60500c71cea9dd687ddd2f7cffce62d29c..4703e9c3c450ad047f4c1b4e73de6fa53270b69b 100644 (file)
--- a/src/debug/debug-scopes.cc
+++ b/src/debug/debug-scopes.cc
@@ -118,7 +118,7 @@ ScopeIterator::ScopeIterator(Isolate* isolate, Handle<JSFunction> function)
       context_(function->context()),
       seen_script_scope_(false),
       failed_(false) {
-  if (function->IsBuiltin()) context_ = Handle<Context>();
+  if (!function->IsSubjectToDebugging()) context_ = Handle<Context>();
 }
 
 

diff --git a/src/debug/debug.cc b/src/debug/debug.cc
index 6a491384440a7c000e5ee70d04c7a14a7215f98e..52736d528ec96e73f45a0e5f6bcaf5bacf52ee67 100644 (file)
--- a/src/debug/debug.cc
+++ b/src/debug/debug.cc
@@ -2277,7 +2277,7 @@ void Debug::HandleDebugBreak() {
     Object* fun = it.frame()->function();
     if (fun && fun->IsJSFunction()) {
       // Don't stop in builtin functions.
-      if (JSFunction::cast(fun)->IsBuiltin()) return;
+      if (!JSFunction::cast(fun)->IsSubjectToDebugging()) return;
       GlobalObject* global = JSFunction::cast(fun)->context()->global_object();
       // Don't stop in debugger functions.
       if (IsDebugGlobal(global)) return;

diff --git a/src/isolate.cc b/src/isolate.cc
index 6ff7d47db6133f049ac7f1559536c9a425e68ec1..ee2fb7bb9741f2d8b32324ead1a0e65b86061943 100644 (file)
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -313,18 +313,13 @@ static bool IsVisibleInStackTrace(JSFunction* fun,
   }
   // Skip all frames until we've seen the caller.
   if (!(*seen_caller)) return false;
-  // Also, skip non-visible built-in functions and any call with the builtins
-  // object as receiver, so as to not reveal either the builtins object or
-  // an internal function.
+  // Functions defined in native scripts are not visible unless directly
+  // exposed, in which case the native flag is set.
   // The --builtins-in-stack-traces command line flag allows including
   // internal call sites in the stack trace for debugging purposes.
   if (!FLAG_builtins_in_stack_traces) {
     if (receiver->IsJSBuiltinsObject()) return false;
-    if (fun->IsBuiltin()) {
-      return fun->shared()->native();
-    } else if (!fun->IsSubjectToDebugging()) {
-      return false;
-    }
+    if (fun->IsBuiltin()) return fun->shared()->native();
   }
   return true;
 }

diff --git a/src/objects-inl.h b/src/objects-inl.h
index 04a47277438724bade6950f5c1bca2d7e7e0c3a9..440ce8c7872c569ed4faaf250fdd9da9923eb202 100644 (file)
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -5552,6 +5552,10 @@ void Script::set_compilation_type(CompilationType type) {
   set_flags(BooleanBit::set(flags(), kCompilationTypeBit,
       type == COMPILATION_TYPE_EVAL));
 }
+bool Script::hide_source() { return BooleanBit::get(flags(), kHideSourceBit); }
+void Script::set_hide_source(bool value) {
+  set_flags(BooleanBit::set(flags(), kHideSourceBit, value));
+}
 Script::CompilationState Script::compilation_state() {
   return BooleanBit::get(flags(), kCompilationStateBit) ?
       COMPILATION_STATE_COMPILED : COMPILATION_STATE_INITIAL;
@@ -6017,18 +6021,19 @@ void SharedFunctionInfo::set_disable_optimization_reason(BailoutReason reason) {
 }
 
 
-bool SharedFunctionInfo::IsSubjectToDebugging() {
+bool SharedFunctionInfo::IsBuiltin() {
   Object* script_obj = script();
-  if (script_obj->IsUndefined()) return false;
+  if (script_obj->IsUndefined()) return true;
   Script* script = Script::cast(script_obj);
   Script::Type type = static_cast<Script::Type>(script->type()->value());
-  return type == Script::TYPE_NORMAL;
+  return type != Script::TYPE_NORMAL;
 }
 
 
-bool JSFunction::IsBuiltin() {
-  return context()->global_object()->IsJSBuiltinsObject();
-}
+bool SharedFunctionInfo::IsSubjectToDebugging() { return !IsBuiltin(); }
+
+
+bool JSFunction::IsBuiltin() { return shared()->IsBuiltin(); }
 
 
 bool JSFunction::IsSubjectToDebugging() {

diff --git a/src/objects.h b/src/objects.h
index c8856d4c6b6f3f4772989652f5221745ae5dd821..a04ae66bb61649a19b6499083108f8729acd387e 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -5916,6 +5916,11 @@ class Script: public Struct {
   inline CompilationState compilation_state();
   inline void set_compilation_state(CompilationState state);
 
+  // [hide_source]: determines whether the script source can be exposed as
+  // function source. Encoded in the 'flags' field.
+  inline bool hide_source();
+  inline void set_hide_source(bool value);
+
   // [origin_options]: optional attributes set by the embedder via ScriptOrigin,
   // and used by the embedder to make decisions about the script. V8 just passes
   // this through. Encoded in the 'flags' field.
@@ -5988,7 +5993,8 @@ class Script: public Struct {
   // Bit positions in the flags field.
   static const int kCompilationTypeBit = 0;
   static const int kCompilationStateBit = 1;
-  static const int kOriginOptionsShift = 2;
+  static const int kHideSourceBit = 2;
+  static const int kOriginOptionsShift = 3;
   static const int kOriginOptionsSize = 3;
   static const int kOriginOptionsMask = ((1 << kOriginOptionsSize) - 1)
                                         << kOriginOptionsShift;
@@ -6398,6 +6404,9 @@ class SharedFunctionInfo: public HeapObject {
   // Tells whether this function should be subject to debugging.
   inline bool IsSubjectToDebugging();
 
+  // Whether this function is defined in native code or extensions.
+  inline bool IsBuiltin();
+
   // Check whether or not this function is inlineable.
   bool IsInlineable();
 

diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc
index 5177528f482303a6ce296472bd0ba4568319edd2..18f987c2500e2bd7b4e710bc59a5aa7f6ce59b75 100644 (file)
--- a/src/runtime/runtime-debug.cc
+++ b/src/runtime/runtime-debug.cc
@@ -533,6 +533,7 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
 
   // Get scope info and read from it for local variable information.
   Handle<JSFunction> function(JSFunction::cast(frame_inspector.GetFunction()));
+  RUNTIME_ASSERT(function->IsSubjectToDebugging());
   Handle<SharedFunctionInfo> shared(function->shared());
   Handle<ScopeInfo> scope_info(shared->scope_info());
   DCHECK(*scope_info != ScopeInfo::Empty(isolate));
@@ -712,8 +713,8 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
   // THIS MUST BE DONE LAST SINCE WE MIGHT ADVANCE
   // THE FRAME ITERATOR TO WRAP THE RECEIVER.
   Handle<Object> receiver(it.frame()->receiver(), isolate);
-  if (!receiver->IsJSObject() && is_sloppy(shared->language_mode()) &&
-      !function->IsBuiltin()) {
+  DCHECK(!function->IsBuiltin());
+  if (!receiver->IsJSObject() && is_sloppy(shared->language_mode())) {
     // If the receiver is not a JSObject and the function is not a
     // builtin or strict-mode we have hit an optimization where a
     // value object is not converted into a wrapped JS objects. To
@@ -1634,7 +1635,7 @@ RUNTIME_FUNCTION(Runtime_DebugCallbackSupportsStepping) {
   // or not even a function.
   return isolate->heap()->ToBoolean(
       callback->IsJSFunction() &&
-      (!JSFunction::cast(callback)->IsBuiltin() ||
+      (JSFunction::cast(callback)->IsSubjectToDebugging() ||
        JSFunction::cast(callback)->shared()->bound()));
 }
 

diff --git a/src/runtime/runtime-function.cc b/src/runtime/runtime-function.cc
index a368f1b14cd491057a4fd29bdc12fa92aad80c03..069bd8b548cd91e927d04d7b81b951cbcb76e253 100644 (file)
--- a/src/runtime/runtime-function.cc
+++ b/src/runtime/runtime-function.cc
@@ -192,12 +192,15 @@ RUNTIME_FUNCTION(Runtime_FunctionIsAPIFunction) {
 }
 
 
-RUNTIME_FUNCTION(Runtime_FunctionIsBuiltin) {
+RUNTIME_FUNCTION(Runtime_FunctionHidesSource) {
   SealHandleScope shs(isolate);
   DCHECK(args.length() == 1);
-
   CONVERT_ARG_CHECKED(JSFunction, f, 0);
-  return isolate->heap()->ToBoolean(f->IsBuiltin());
+
+  SharedFunctionInfo* shared = f->shared();
+  bool hide_source = !shared->script()->IsScript() ||
+                     Script::cast(shared->script())->hide_source();
+  return isolate->heap()->ToBoolean(hide_source);
 }
 
 

diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h
index 7107bdef372134c863d35c1c3b642fba02033d89..4048cfb9bbbd02bc169bcbd143be1187972b9eb3 100644 (file)
--- a/src/runtime/runtime.h
+++ b/src/runtime/runtime.h
@@ -236,7 +236,7 @@ namespace internal {
   F(FunctionSetLength, 2, 1)                                \
   F(FunctionSetPrototype, 2, 1)                             \
   F(FunctionIsAPIFunction, 1, 1)                            \
-  F(FunctionIsBuiltin, 1, 1)                                \
+  F(FunctionHidesSource, 1, 1)                              \
   F(SetCode, 2, 1)                                          \
   F(SetNativeFlag, 1, 1)                                    \
   F(ThrowStrongModeTooFewArguments, 0, 1)                   \

diff --git a/src/v8natives.js b/src/v8natives.js
index 93636d008f1b6639a79f885812549e415e24d164..dc1d34cfbcb84939fc147f2fe0cd31314188935f 100644 (file)
--- a/src/v8natives.js
+++ b/src/v8natives.js
@@ -1622,7 +1622,7 @@ function FunctionSourceString(func) {
     throw MakeTypeError(kNotGeneric, 'Function.prototype.toString');
   }
 
-  if (%FunctionIsBuiltin(func)) {
+  if (%FunctionHidesSource(func)) {
     return NativeCodeFunctionSourceString(func);
   }