Revert "Fix stack trace accessor behavior."

This reverts r22089.

TBR=verwaest@chromium.org

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

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

include/v8.h

@@ -5586,7 +5586,7 @@ class Internals {
   static const int kNullValueRootIndex = 7;
   static const int kTrueValueRootIndex = 8;
   static const int kFalseValueRootIndex = 9;
-  static const int kEmptyStringRootIndex = 162;
+  static const int kEmptyStringRootIndex = 160;
 
   // The external allocation limit should be below 256 MB on all architectures
   // to avoid that resource-constrained embedders run low on memory.

src/bootstrapper.cc

@@ -2211,17 +2211,6 @@ bool Genesis::InstallSpecialObjects(Handle<Context> native_context) {
         false);
   }
 
-  // Expose the stack trace symbol to native JS.
-  RETURN_ON_EXCEPTION_VALUE(
-      isolate,
-      JSObject::SetOwnPropertyIgnoreAttributes(
-          handle(native_context->builtins(), isolate),
-          factory->InternalizeOneByteString(
-              STATIC_ASCII_VECTOR("stack_trace_symbol")),
-          factory->stack_trace_symbol(),
-          NONE),
-      false);
-
   // Expose the debug global object in global if a name for it is specified.
   if (FLAG_expose_debug_as != NULL && strlen(FLAG_expose_debug_as) != 0) {
     // If loading fails we just bail out without installing the

src/heap.cc

@@ -2917,8 +2917,6 @@ void Heap::CreateInitialObjects() {
   set_uninitialized_symbol(*factory->NewPrivateSymbol());
   set_megamorphic_symbol(*factory->NewPrivateSymbol());
   set_observed_symbol(*factory->NewPrivateSymbol());
-  set_stack_trace_symbol(*factory->NewPrivateSymbol());
-  set_detailed_stack_trace_symbol(*factory->NewPrivateSymbol());
 
   Handle<SeededNumberDictionary> slow_element_dictionary =
       SeededNumberDictionary::New(isolate(), 0, TENURED);

src/heap.h

@@ -193,8 +193,6 @@ namespace internal {
   V(Symbol, observed_symbol, ObservedSymbol)                                   \
   V(Symbol, uninitialized_symbol, UninitializedSymbol)                         \
   V(Symbol, megamorphic_symbol, MegamorphicSymbol)                             \
-  V(Symbol, stack_trace_symbol, StackTraceSymbol)                              \
-  V(Symbol, detailed_stack_trace_symbol, DetailedStackTraceSymbol)             \
   V(FixedArray, materialized_objects, MaterializedObjects)                     \
   V(FixedArray, allocation_sites_scratchpad, AllocationSitesScratchpad)        \
   V(FixedArray, microtask_queue, MicrotaskQueue)
@@ -342,6 +340,7 @@ namespace internal {
   V(strict_compare_ic_string, "===")                                     \
   V(infinity_string, "Infinity")                                         \
   V(minus_infinity_string, "-Infinity")                                  \
+  V(hidden_stack_trace_string, "v8::hidden_stack_trace")                 \
   V(query_colon_string, "(?:)")                                          \
   V(Generator_string, "Generator")                                       \
   V(throw_string, "throw")                                               \

src/isolate.cc

@@ -353,23 +353,10 @@ static bool IsVisibleInStackTrace(StackFrame* raw_frame,
 }
 
 
-Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSObject> error_object,
-                                                Handle<Object> caller) {
-  // Get stack trace limit.
-  Handle<Object> error = Object::GetProperty(
-      this, js_builtins_object(), "$Error").ToHandleChecked();
-  if (!error->IsJSObject()) return factory()->undefined_value();
-
-  Handle<String> stackTraceLimit =
-      factory()->InternalizeUtf8String("stackTraceLimit");
-  ASSERT(!stackTraceLimit.is_null());
-  Handle<Object> stack_trace_limit =
-      JSObject::GetDataProperty(Handle<JSObject>::cast(error),
-                                stackTraceLimit);
-  if (!stack_trace_limit->IsNumber()) return factory()->undefined_value();
-  int limit = FastD2IChecked(stack_trace_limit->Number());
+Handle<JSArray> Isolate::CaptureSimpleStackTrace(Handle<JSObject> error_object,
+                                                 Handle<Object> caller,
+                                                 int limit) {
   limit = Max(limit, 0);  // Ensure that limit is not negative.
-
   int initial_size = Min(limit, 10);
   Handle<FixedArray> elements =
       factory()->NewFixedArrayWithHoles(initial_size * 4 + 1);
@@ -439,25 +426,15 @@ Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSObject> error_object,
 void Isolate::CaptureAndSetDetailedStackTrace(Handle<JSObject> error_object) {
   if (capture_stack_trace_for_uncaught_exceptions_) {
     // Capture stack trace for a detailed exception message.
-    Handle<Name> key = factory()->detailed_stack_trace_symbol();
+    Handle<String> key = factory()->hidden_stack_trace_string();
     Handle<JSArray> stack_trace = CaptureCurrentStackTrace(
         stack_trace_for_uncaught_exceptions_frame_limit_,
         stack_trace_for_uncaught_exceptions_options_);
-    JSObject::SetProperty(
-        error_object, key, stack_trace, NONE, STRICT).Assert();
+    JSObject::SetHiddenProperty(error_object, key, stack_trace);
   }
 }
 
 
-void Isolate::CaptureAndSetSimpleStackTrace(Handle<JSObject> error_object,
-                                            Handle<Object> caller) {
-  // Capture stack trace for simple stack trace string formatting.
-  Handle<Name> key = factory()->stack_trace_symbol();
-  Handle<Object> stack_trace = CaptureSimpleStackTrace(error_object, caller);
-  JSObject::SetProperty(error_object, key, stack_trace, NONE, STRICT).Assert();
-}
-
-
 Handle<JSArray> Isolate::CaptureCurrentStackTrace(
     int frame_limit, StackTrace::StackTraceOptions options) {
   // Ensure no negative values.
@@ -803,7 +780,25 @@ Object* Isolate::StackOverflow() {
   Handle<JSObject> exception = factory()->CopyJSObject(boilerplate);
   DoThrow(*exception, NULL);
 
-  CaptureAndSetSimpleStackTrace(exception, factory()->undefined_value());
+  // Get stack trace limit.
+  Handle<Object> error = Object::GetProperty(
+      this, js_builtins_object(), "$Error").ToHandleChecked();
+  if (!error->IsJSObject()) return heap()->exception();
+
+  Handle<String> stackTraceLimit =
+      factory()->InternalizeUtf8String("stackTraceLimit");
+  ASSERT(!stackTraceLimit.is_null());
+  Handle<Object> stack_trace_limit =
+      JSObject::GetDataProperty(Handle<JSObject>::cast(error),
+                                stackTraceLimit);
+  if (!stack_trace_limit->IsNumber()) return heap()->exception();
+  int limit = FastD2IChecked(stack_trace_limit->Number());
+  if (limit < 0) limit = 0;
+  Handle<JSArray> stack_trace = CaptureSimpleStackTrace(
+      exception, factory()->undefined_value(), limit);
+  JSObject::SetHiddenProperty(exception,
+                              factory()->hidden_stack_trace_string(),
+                              stack_trace);
   return heap()->exception();
 }
 
@@ -1059,16 +1054,13 @@ void Isolate::DoThrow(Object* exception, MessageLocation* location) {
       if (capture_stack_trace_for_uncaught_exceptions_) {
         if (IsErrorObject(exception_handle)) {
           // We fetch the stack trace that corresponds to this error object.
-          Handle<Name> key = factory()->detailed_stack_trace_symbol();
-          // Look up as own property.  If the lookup fails, the exception is
-          // probably not a valid Error object.  In that case, we fall through
-          // and capture the stack trace at this throw site.
-          LookupIterator lookup(
-              exception_handle, key, LookupIterator::CHECK_OWN_REAL);
-          Handle<Object> stack_trace_property;
-          if (Object::GetProperty(&lookup).ToHandle(&stack_trace_property) &&
-              stack_trace_property->IsJSArray()) {
-            stack_trace_object = Handle<JSArray>::cast(stack_trace_property);
+          Handle<String> key = factory()->hidden_stack_trace_string();
+          Object* stack_property =
+              JSObject::cast(*exception_handle)->GetHiddenProperty(key);
+          // Property lookup may have failed.  In this case it's probably not
+          // a valid Error object.
+          if (stack_property->IsJSArray()) {
+            stack_trace_object = Handle<JSArray>(JSArray::cast(stack_property));
           }
         }
         if (stack_trace_object.is_null()) {

src/isolate.h

@@ -697,11 +697,11 @@ class Isolate {
   Handle<JSArray> CaptureCurrentStackTrace(
       int frame_limit,
       StackTrace::StackTraceOptions options);
-  Handle<Object> CaptureSimpleStackTrace(Handle<JSObject> error_object,
-                                         Handle<Object> caller);
+
+  Handle<JSArray> CaptureSimpleStackTrace(Handle<JSObject> error_object,
+                                          Handle<Object> caller,
+                                          int limit);
   void CaptureAndSetDetailedStackTrace(Handle<JSObject> error_object);
-  void CaptureAndSetSimpleStackTrace(Handle<JSObject> error_object,
-                                     Handle<Object> caller);
 
   // Returns if the top context may access the given global object. If
   // the result is false, the pending exception is guaranteed to be

src/messages.js

@@ -1080,8 +1080,7 @@ function GetStackFrames(raw_stack) {
 var formatting_custom_stack_trace = false;
 
 
-function FormatStackTrace(obj, raw_stack) {
-  var frames = GetStackFrames(raw_stack);
+function FormatStackTrace(obj, error_string, frames) {
   if (IS_FUNCTION($Error.prepareStackTrace) && !formatting_custom_stack_trace) {
     var array = [];
     %MoveArrayContents(frames, array);
@@ -1098,7 +1097,7 @@ function FormatStackTrace(obj, raw_stack) {
   }
 
   var lines = new InternalArray();
-  lines.push(FormatErrorString(obj));
+  lines.push(error_string);
   for (var i = 0; i < frames.length; i++) {
     var frame = frames[i];
     var line;
@@ -1133,45 +1132,45 @@ function GetTypeName(receiver, requireConstructor) {
 }
 
 
-var stack_trace_symbol;  // Set during bootstrapping.
-var formatted_stack_trace_symbol = NEW_PRIVATE("formatted stack trace");
-
-
-// Format the stack trace if not yet done, and return it.
-// Cache the formatted stack trace on the holder.
-function StackTraceGetter() {
-  var formatted_stack_trace = GET_PRIVATE(this, formatted_stack_trace_symbol);
-  if (IS_UNDEFINED(formatted_stack_trace)) {
-    var holder = this;
-    while (!HAS_PRIVATE(holder, stack_trace_symbol)) {
-      holder = %GetPrototype(holder);
-      if (!holder) return UNDEFINED;
-    }
-    var stack_trace = GET_PRIVATE(holder, stack_trace_symbol);
-    if (IS_UNDEFINED(stack_trace)) return UNDEFINED;
-    formatted_stack_trace = FormatStackTrace(holder, stack_trace);
-    SET_PRIVATE(holder, stack_trace_symbol, UNDEFINED);
-    SET_PRIVATE(holder, formatted_stack_trace_symbol, formatted_stack_trace);
-  }
-  return formatted_stack_trace;
-};
-
-
-// If the receiver equals the holder, set the formatted stack trace that the
-// getter returns.
-function StackTraceSetter(v) {
-  if (HAS_PRIVATE(this, stack_trace_symbol)) {
-    SET_PRIVATE(this, stack_trace_symbol, UNDEFINED);
-    SET_PRIVATE(this, formatted_stack_trace_symbol, v);
+function captureStackTrace(obj, cons_opt) {
+  var stackTraceLimit = $Error.stackTraceLimit;
+  if (!stackTraceLimit || !IS_NUMBER(stackTraceLimit)) return;
+  if (stackTraceLimit < 0 || stackTraceLimit > 10000) {
+    stackTraceLimit = 10000;
   }
-};
+  var stack = %CollectStackTrace(obj,
+                                 cons_opt ? cons_opt : captureStackTrace,
+                                 stackTraceLimit);
+
+  var error_string = FormatErrorString(obj);
+
+  // Set the 'stack' property on the receiver.  If the receiver is the same as
+  // holder of this setter, the accessor pair is turned into a data property.
+  var setter = function(v) {
+    // Set data property on the receiver (not necessarily holder).
+    %DefineDataPropertyUnchecked(this, 'stack', v, NONE);
+    if (this === obj) {
+      // Release context values if holder is the same as the receiver.
+      stack = error_string = UNDEFINED;
+    }
+  };
 
+  // The holder of this getter ('obj') may not be the receiver ('this').
+  // When this getter is called the first time, we use the context values to
+  // format a stack trace string and turn this accessor pair into a data
+  // property (on the holder).
+  var getter = function() {
+    // Stack is still a raw array awaiting to be formatted.
+    var result = FormatStackTrace(obj, error_string, GetStackFrames(stack));
+    // Replace this accessor to return result directly.
+    %DefineAccessorPropertyUnchecked(
+        obj, 'stack', function() { return result }, setter, DONT_ENUM);
+    // Release context values.
+    stack = error_string = UNDEFINED;
+    return result;
+  };
 
-function captureStackTrace(obj, cons_opt) {
-  // Define accessors first, as this may fail and throw.
-  ObjectDefineProperty(obj, 'stack', { get: StackTraceGetter,
-                                       set: StackTraceSetter});
-  %CollectStackTrace(obj, cons_opt ? cons_opt : captureStackTrace);
+  %DefineAccessorPropertyUnchecked(obj, 'stack', getter, setter, DONT_ENUM);
 }
 
 
@@ -1307,8 +1306,40 @@ InstallFunctions($Error.prototype, DONT_ENUM, ['toString', ErrorToString]);
 function SetUpStackOverflowBoilerplate() {
   var boilerplate = MakeRangeError('stack_overflow', []);
 
+  var error_string = boilerplate.name + ": " + boilerplate.message;
+
+  // Set the 'stack' property on the receiver.  If the receiver is the same as
+  // holder of this setter, the accessor pair is turned into a data property.
+  var setter = function(v) {
+    %DefineDataPropertyUnchecked(this, 'stack', v, NONE);
+    // Tentatively clear the hidden property. If the receiver is the same as
+    // holder, we release the raw stack trace this way.
+    %GetAndClearOverflowedStackTrace(this);
+  };
+
+  // The raw stack trace is stored as a hidden property on the holder of this
+  // getter, which may not be the same as the receiver.  Find the holder to
+  // retrieve the raw stack trace and then turn this accessor pair into a
+  // data property.
+  var getter = function() {
+    var holder = this;
+    while (!IS_ERROR(holder)) {
+      holder = %GetPrototype(holder);
+      if (IS_NULL(holder)) return MakeSyntaxError('illegal_access', []);
+    }
+    var stack = %GetAndClearOverflowedStackTrace(holder);
+    // We may not have captured any stack trace.
+    if (IS_UNDEFINED(stack)) return stack;
+
+    var result = FormatStackTrace(holder, error_string, GetStackFrames(stack));
+    // Replace this accessor to return result directly.
+    %DefineAccessorPropertyUnchecked(
+        holder, 'stack', function() { return result }, setter, DONT_ENUM);
+    return result;
+  };
+
   %DefineAccessorPropertyUnchecked(
-      boilerplate, 'stack', StackTraceGetter, StackTraceSetter, DONT_ENUM);
+      boilerplate, 'stack', getter, setter, DONT_ENUM);
 
   return boilerplate;
 }

src/runtime.cc

@@ -14428,15 +14428,30 @@ RUNTIME_FUNCTION(Runtime_GetScript) {
 // native code offset.
 RUNTIME_FUNCTION(Runtime_CollectStackTrace) {
   HandleScope scope(isolate);
-  ASSERT(args.length() == 2);
+  ASSERT(args.length() == 3);
   CONVERT_ARG_HANDLE_CHECKED(JSObject, error_object, 0);
   CONVERT_ARG_HANDLE_CHECKED(Object, caller, 1);
+  CONVERT_NUMBER_CHECKED(int32_t, limit, Int32, args[2]);
 
   // Optionally capture a more detailed stack trace for the message.
   isolate->CaptureAndSetDetailedStackTrace(error_object);
   // Capture a simple stack trace for the stack property.
-  isolate->CaptureAndSetSimpleStackTrace(error_object, caller);
-  return isolate->heap()->undefined_value();
+  return *isolate->CaptureSimpleStackTrace(error_object, caller, limit);
+}
+
+
+// Retrieve the stack trace.  This is the raw stack trace that yet has to
+// be formatted.  Since we only need this once, clear it afterwards.
+RUNTIME_FUNCTION(Runtime_GetAndClearOverflowedStackTrace) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 1);
+  CONVERT_ARG_HANDLE_CHECKED(JSObject, error_object, 0);
+  Handle<String> key = isolate->factory()->hidden_stack_trace_string();
+  Handle<Object> result(error_object->GetHiddenProperty(key), isolate);
+  if (result->IsTheHole()) return isolate->heap()->undefined_value();
+  RUNTIME_ASSERT(result->IsJSArray() || result->IsUndefined());
+  JSObject::DeleteHiddenProperty(error_object, key);
+  return *result;
 }
 
 

src/runtime.h

@@ -196,7 +196,8 @@ namespace internal {
   F(FunctionIsAPIFunction, 1, 1) \
   F(FunctionIsBuiltin, 1, 1) \
   F(GetScript, 1, 1) \
-  F(CollectStackTrace, 2, 1) \
+  F(CollectStackTrace, 3, 1) \
+  F(GetAndClearOverflowedStackTrace, 1, 1) \
   F(GetV8Version, 0, 1) \
   \
   F(SetCode, 2, 1) \

test/mjsunit/regress/regress-3404.js

-// Copyright 2014 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-function testError(error) {
-  // Reconfigure e.stack to be non-configurable
-  var desc1 = Object.getOwnPropertyDescriptor(error, "stack");
-  Object.defineProperty(error, "stack",
-                        {get: desc1.get, set: desc1.set, configurable: false});
-
-  var desc2 = Object.getOwnPropertyDescriptor(error, "stack");
-  assertFalse(desc2.configurable);
-  assertEquals(desc1.get, desc2.get);
-  assertEquals(desc2.get, desc2.get);
-}
-
-function stackOverflow() {
-  function f() { f(); }
-  try { f() } catch (e) { return e; }
-}
-
-function referenceError() {
-  try { g() } catch (e) { return e; }
-}
-
-testError(referenceError());
-testError(stackOverflow());

test/mjsunit/runtime-gen/collectstacktrace.js

@@ -3,4 +3,5 @@
 // Flags: --allow-natives-syntax --harmony
 var _error_object = new Object();
 var _caller = new Object();
-%CollectStackTrace(_error_object, _caller);
+var _limit = 32;
+%CollectStackTrace(_error_object, _caller, _limit);

test/mjsunit/runtime-gen/getandclearoverflowedstacktrace.js

+// Copyright 2014 the V8 project authors. All rights reserved.
+// AUTO-GENERATED BY tools/generate-runtime-tests.py, DO NOT MODIFY
+// Flags: --allow-natives-syntax --harmony
+var _error_object = new Object();
+%GetAndClearOverflowedStackTrace(_error_object);

test/mjsunit/stack-traces-overflow.js

@@ -61,8 +61,8 @@ try {
 function testErrorPrototype(prototype) {
   var object = {};
   object.__proto__ = prototype;
-  object.stack = "123";  // Overwriting stack property fails.
-  assertEquals(prototype.stack, object.stack);
+  object.stack = "123";
+  assertEquals("123", object.stack);
   assertTrue("123" != prototype.stack);
 }
 
@@ -126,8 +126,6 @@ try {
   rec1(0);
 } catch (e) {
   assertEquals(undefined, e.stack);
-  e.stack = "abc";
-  assertEquals("abc", e.stack);
 }
 
 Error.stackTraceLimit = 3;

test/mjsunit/stack-traces.js

@@ -331,23 +331,3 @@ Error.prepareStackTrace = function() { Error.prepareStackTrace = "custom"; };
 new Error().stack;
 
 assertEquals("custom", Error.prepareStackTrace);
-
-// Check that the formatted stack trace can be set to undefined.
-error = new Error();
-error.stack = undefined;
-assertEquals(undefined, error.stack);
-
-// Check that the stack trace accessors are not forcibly set.
-var my_error = {};
-Object.freeze(my_error);
-assertThrows(function() { Error.captureStackTrace(my_error); });
-
-my_error = {};
-Object.preventExtensions(my_error);
-assertThrows(function() { Error.captureStackTrace(my_error); });
-
-var fake_error = {};
-my_error = new Error();
-var stolen_getter = Object.getOwnPropertyDescriptor(my_error, 'stack').get;
-Object.defineProperty(fake_error, 'stack', { get: stolen_getter });
-assertEquals(undefined, fake_error.stack);

tools/generate-runtime-tests.py

@@ -47,11 +47,11 @@ EXPAND_MACROS = [
 # that the parser doesn't bit-rot. Change the values as needed when you add,
 # remove or change runtime functions, but make sure we don't lose our ability
 # to parse them!
-EXPECTED_FUNCTION_COUNT = 415
-EXPECTED_FUZZABLE_COUNT = 330
+EXPECTED_FUNCTION_COUNT = 416
+EXPECTED_FUZZABLE_COUNT = 331
 EXPECTED_CCTEST_COUNT = 6
 EXPECTED_UNKNOWN_COUNT = 4
-EXPECTED_BUILTINS_COUNT = 811
+EXPECTED_BUILTINS_COUNT = 809
 
 
 # Don't call these at all.