Stack traces exposed to Javascript should omit extensions.

R=marja@chromium.org
BUG=v8:311
LOG=Y

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

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22162 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/debug.cc b/src/debug.cc
index b19b47e..8eec14a 100644 (file)
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -1226,7 +1226,7 @@ void Debug::FloodBoundFunctionWithOneShot(Handle<JSFunction> function) {
                         isolate_);
 
   if (!bindee.is_null() && bindee->IsJSFunction() &&
-      !JSFunction::cast(*bindee)->IsNative()) {
+      !JSFunction::cast(*bindee)->IsFromNativeScript()) {
     Handle<JSFunction> bindee_function(JSFunction::cast(*bindee));
     Debug::FloodWithOneShot(bindee_function);
   }
@@ -1447,7 +1447,8 @@ void Debug::PrepareStep(StepAction step_action,
       frames_it.Advance();
     }
     // Skip builtin functions on the stack.
-    while (!frames_it.done() && frames_it.frame()->function()->IsNative()) {
+    while (!frames_it.done() &&
+           frames_it.frame()->function()->IsFromNativeScript()) {
       frames_it.Advance();
     }
     // Step out: If there is a JavaScript caller frame, we need to
@@ -1534,7 +1535,7 @@ void Debug::PrepareStep(StepAction step_action,
         Handle<JSFunction> js_function(JSFunction::cast(fun));
         if (js_function->shared()->bound()) {
           Debug::FloodBoundFunctionWithOneShot(js_function);
-        } else if (!js_function->IsNative()) {
+        } else if (!js_function->IsFromNativeScript()) {
           // Don't step into builtins.
           // It will also compile target function if it's not compiled yet.
           FloodWithOneShot(js_function);
@@ -1676,7 +1677,7 @@ void Debug::HandleStepIn(Handle<JSFunction> function,
     if (function->shared()->bound()) {
       // Handle Function.prototype.bind
       Debug::FloodBoundFunctionWithOneShot(function);
-    } else if (!function->IsNative()) {
+    } else if (!function->IsFromNativeScript()) {
       // Don't allow step into functions in the native context.
       if (function->shared()->code() ==
           isolate->builtins()->builtin(Builtins::kFunctionApply) ||
@@ -1688,7 +1689,7 @@ void Debug::HandleStepIn(Handle<JSFunction> function,
         // function.
         if (!holder.is_null() && holder->IsJSFunction()) {
           Handle<JSFunction> js_function = Handle<JSFunction>::cast(holder);
-          if (!js_function->IsNative()) {
+          if (!js_function->IsFromNativeScript()) {
             Debug::FloodWithOneShot(js_function);
           } else if (js_function->shared()->bound()) {
             // Handle Function.prototype.bind
@@ -2030,7 +2031,7 @@ void Debug::PrepareForBreakPoints() {
 
           if (!shared->allows_lazy_compilation()) continue;
           if (!shared->script()->IsScript()) continue;
-          if (function->IsNative()) continue;
+          if (function->IsFromNativeScript()) continue;
           if (shared->code()->gc_metadata() == active_code_marker) continue;
 
           if (shared->is_generator()) {

diff --git a/src/isolate.cc b/src/isolate.cc
index a93f4c0..5c5a2da 100644 (file)
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -325,13 +325,10 @@ void Isolate::PushStackTraceAndDie(unsigned int magic,
 // call to this function is encountered it is skipped.  The seen_caller
 // in/out parameter is used to remember if the caller has been seen
 // yet.
-static bool IsVisibleInStackTrace(StackFrame* raw_frame,
+static bool IsVisibleInStackTrace(JSFunction* fun,
                                   Object* caller,
+                                  Object* receiver,
                                   bool* seen_caller) {
-  // Only display JS frames.
-  if (!raw_frame->is_java_script()) return false;
-  JavaScriptFrame* frame = JavaScriptFrame::cast(raw_frame);
-  JSFunction* fun = frame->function();
   if ((fun == caller) && !(*seen_caller)) {
     *seen_caller = true;
     return false;
@@ -344,8 +341,10 @@ static bool IsVisibleInStackTrace(StackFrame* raw_frame,
   // 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 (frame->receiver()->IsJSBuiltinsObject() ||
-        (fun->IsBuiltin() && !fun->shared()->native())) {
+    if (receiver->IsJSBuiltinsObject()) return false;
+    if (fun->IsBuiltin()) {
+      return fun->shared()->native();
+    } else if (fun->IsFromNativeScript() || fun->IsFromExtensionScript()) {
       return false;
     }
   }
@@ -369,50 +368,50 @@ Handle<JSArray> Isolate::CaptureSimpleStackTrace(Handle<JSObject> error_object,
   int frames_seen = 0;
   int sloppy_frames = 0;
   bool encountered_strict_function = false;
-  for (StackFrameIterator iter(this);
+  for (JavaScriptFrameIterator iter(this);
        !iter.done() && frames_seen < limit;
        iter.Advance()) {
-    StackFrame* raw_frame = iter.frame();
-    if (IsVisibleInStackTrace(raw_frame, *caller, &seen_caller)) {
-      JavaScriptFrame* frame = JavaScriptFrame::cast(raw_frame);
-      // Set initial size to the maximum inlining level + 1 for the outermost
-      // function.
-      List<FrameSummary> frames(FLAG_max_inlining_levels + 1);
-      frame->Summarize(&frames);
-      for (int i = frames.length() - 1; i >= 0; i--) {
-        Handle<JSFunction> fun = frames[i].function();
-        // Filter out frames from other security contexts.
-        if (!this->context()->HasSameSecurityTokenAs(fun->context())) continue;
-        if (cursor + 4 > elements->length()) {
-          int new_capacity = JSObject::NewElementsCapacity(elements->length());
-          Handle<FixedArray> new_elements =
-              factory()->NewFixedArrayWithHoles(new_capacity);
-          for (int i = 0; i < cursor; i++) {
-            new_elements->set(i, elements->get(i));
-          }
-          elements = new_elements;
+    JavaScriptFrame* frame = iter.frame();
+    // Set initial size to the maximum inlining level + 1 for the outermost
+    // function.
+    List<FrameSummary> frames(FLAG_max_inlining_levels + 1);
+    frame->Summarize(&frames);
+    for (int i = frames.length() - 1; i >= 0; i--) {
+      Handle<JSFunction> fun = frames[i].function();
+      Handle<Object> recv = frames[i].receiver();
+      // Filter out internal frames that we do not want to show.
+      if (!IsVisibleInStackTrace(*fun, *caller, *recv, &seen_caller)) continue;
+      // Filter out frames from other security contexts.
+      if (!this->context()->HasSameSecurityTokenAs(fun->context())) continue;
+      if (cursor + 4 > elements->length()) {
+        int new_capacity = JSObject::NewElementsCapacity(elements->length());
+        Handle<FixedArray> new_elements =
+            factory()->NewFixedArrayWithHoles(new_capacity);
+        for (int i = 0; i < cursor; i++) {
+          new_elements->set(i, elements->get(i));
         }
-        ASSERT(cursor + 4 <= elements->length());
-
-        Handle<Object> recv = frames[i].receiver();
-        Handle<Code> code = frames[i].code();
-        Handle<Smi> offset(Smi::FromInt(frames[i].offset()), this);
-        // The stack trace API should not expose receivers and function
-        // objects on frames deeper than the top-most one with a strict
-        // mode function.  The number of sloppy frames is stored as
-        // first element in the result array.
-        if (!encountered_strict_function) {
-          if (fun->shared()->strict_mode() == STRICT) {
-            encountered_strict_function = true;
-          } else {
-            sloppy_frames++;
-          }
+        elements = new_elements;
+      }
+      ASSERT(cursor + 4 <= elements->length());
+
+
+      Handle<Code> code = frames[i].code();
+      Handle<Smi> offset(Smi::FromInt(frames[i].offset()), this);
+      // The stack trace API should not expose receivers and function
+      // objects on frames deeper than the top-most one with a strict
+      // mode function.  The number of sloppy frames is stored as
+      // first element in the result array.
+      if (!encountered_strict_function) {
+        if (fun->shared()->strict_mode() == STRICT) {
+          encountered_strict_function = true;
+        } else {
+          sloppy_frames++;
         }
-        elements->set(cursor++, *recv);
-        elements->set(cursor++, *fun);
-        elements->set(cursor++, *code);
-        elements->set(cursor++, *offset);
       }
+      elements->set(cursor++, *recv);
+      elements->set(cursor++, *fun);
+      elements->set(cursor++, *code);
+      elements->set(cursor++, *offset);
       frames_seen++;
     }
   }

diff --git a/src/objects-inl.h b/src/objects-inl.h
index 50c9f3c..95b2572 100644 (file)
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -5657,7 +5657,7 @@ bool JSFunction::IsBuiltin() {
 }
 
 
-bool JSFunction::IsNative() {
+bool JSFunction::IsFromNativeScript() {
   Object* script = shared()->script();
   bool native = script->IsScript() &&
                 Script::cast(script)->type()->value() == Script::TYPE_NATIVE;
@@ -5666,6 +5666,13 @@ bool JSFunction::IsNative() {
 }
 
 
+bool JSFunction::IsFromExtensionScript() {
+  Object* script = shared()->script();
+  return script->IsScript() &&
+         Script::cast(script)->type()->value() == Script::TYPE_EXTENSION;
+}
+
+
 bool JSFunction::NeedsArgumentsAdaption() {
   return shared()->formal_parameter_count() !=
       SharedFunctionInfo::kDontAdaptArgumentsSentinel;

diff --git a/src/objects.h b/src/objects.h
index 74aaedf..3e83eda 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -7634,7 +7634,10 @@ class JSFunction: public JSObject {
   inline bool IsBuiltin();
 
   // Tells whether this function is defined in a native script.
-  inline bool IsNative();
+  inline bool IsFromNativeScript();
+
+  // Tells whether this function is defined in an extension script.
+  inline bool IsFromExtensionScript();
 
   // Tells whether or not the function needs arguments adaption.
   inline bool NeedsArgumentsAdaption();

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 1417425..0408596 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -6694,6 +6694,33 @@ TEST(SimpleExtensions) {
 }
 
 
+static const char* kStackTraceFromExtensionSource =
+  "function foo() {"
+  "  throw new Error();"
+  "}"
+  "function bar() {"
+  "  foo();"
+  "}";
+
+
+TEST(StackTraceInExtension) {
+  v8::HandleScope handle_scope(CcTest::isolate());
+  v8::RegisterExtension(new Extension("stacktracetest",
+                        kStackTraceFromExtensionSource));
+  const char* extension_names[] = { "stacktracetest" };
+  v8::ExtensionConfiguration extensions(1, extension_names);
+  v8::Handle<Context> context =
+      Context::New(CcTest::isolate(), &extensions);
+  Context::Scope lock(context);
+  CompileRun("function user() { bar(); }"
+             "var error;"
+             "try{ user(); } catch (e) { error = e; }");
+  CHECK_EQ(-1, CompileRun("error.stack.indexOf('foo')")->Int32Value());
+  CHECK_EQ(-1, CompileRun("error.stack.indexOf('bar')")->Int32Value());
+  CHECK_NE(-1, CompileRun("error.stack.indexOf('user')")->Int32Value());
+}
+
+
 TEST(NullExtensions) {
   v8::HandleScope handle_scope(CcTest::isolate());
   v8::RegisterExtension(new Extension("nulltest", NULL));

diff --git a/test/mjsunit/stack-traces-overflow.js b/test/mjsunit/stack-traces-overflow.js
index 1c67680..54afe75 100644 (file)
--- a/test/mjsunit/stack-traces-overflow.js
+++ b/test/mjsunit/stack-traces-overflow.js
@@ -25,6 +25,8 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+// Flags: --stack-size=100
+
 function rec1(a) { rec1(a+1); }
 function rec2(a) { rec3(a+1); }
 function rec3(a) { rec2(a+1); }