Revert "[error] extend error stack w/ function parameters"

This reverts commit 97628eee.

Reason for revert: breaks compilation in Lite mode, which does not allow overriding of certain flags. See https://logs.chromium.org/logs/v8/buildbucket/cr-buildbucket.appspot.com/8926078411629093216/+/steps/build/0/steps/compile/0/stdout.

Original change's description:
> [error] extend error stack w/ function parameters
> 
> Extend FrameArray to hold weak references to parameters for functions in
> the call stack. The goal here is to provide more metadata for postmortem
> tools (such as llnode), especially in cases of rethrowing (this will be
> particularly useful when using postmortem with promises on Node.js).
> 
> Besides postmortem, these changes allow us to print a more detailed
> stack trace for errors with parameters types (or even values), which can
> be useful since JavaScript functions can receive any number of
> parameters of any type, and having a function behave differently
> according to the number of parameters received as well as their types is
> a common pattern on JS libraries and frameworks.
> 
> R=​bmeurer@google.com, yangguo@google.com
> 
> Change-Id: Idf0984d0dbac16041f11d738d4b1c095a8eecd61
> Reviewed-on: https://chromium-review.googlesource.com/c/1289489
> Commit-Queue: Yang Guo <yangguo@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#58468}

TBR=yangguo@chromium.org,bmeurer@google.com,bmeurer@chromium.org,mat@mmarchini.me

Change-Id: Ide0a434c1521ab2bbeca6821397ff63ba7d40fe5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/1390128Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58469}

src/flag-definitions.h

@@ -1072,9 +1072,6 @@ DEFINE_INT(fuzzer_random_seed, 0,
 DEFINE_BOOL(trace_rail, false, "trace RAIL mode")
 DEFINE_BOOL(print_all_exceptions, false,
             "print exception object and stack trace on each thrown exception")
-DEFINE_BOOL(
-    detailed_error_stack_trace, false,
-    "includes arguments for each function call in the error stack frames array")
 
 // runtime.cc
 DEFINE_BOOL(runtime_call_stats, false, "report runtime call counts and times")

src/frames.cc

@@ -1082,10 +1082,9 @@ void JavaScriptFrame::Summarize(std::vector<FrameSummary>* functions) const {
   Code code = LookupCode();
   int offset = static_cast<int>(pc() - code->InstructionStart());
   AbstractCode abstract_code = AbstractCode::cast(code);
-  Handle<FixedArray> params = GetParameters();
-  FrameSummary::JavaScriptFrameSummary summary(
-      isolate(), receiver(), function(), abstract_code, offset, IsConstructor(),
-      *params);
+  FrameSummary::JavaScriptFrameSummary summary(isolate(), receiver(),
+                                               function(), abstract_code,
+                                               offset, IsConstructor());
   functions->push_back(summary);
 }
 
@@ -1243,20 +1242,6 @@ int JavaScriptFrame::ComputeParametersCount() const {
   return GetNumberOfIncomingArguments();
 }
 
-Handle<FixedArray> JavaScriptFrame::GetParameters() const {
-  if (V8_LIKELY(!FLAG_detailed_error_stack_trace)) {
-    return isolate()->factory()->empty_fixed_array();
-  }
-  int param_count = ComputeParametersCount();
-  Handle<FixedArray> parameters =
-      isolate()->factory()->NewFixedArray(param_count);
-  for (int i = 0; i < param_count; i++) {
-    parameters->set(i, GetParameter(i));
-  }
-
-  return parameters;
-}
-
 int JavaScriptBuiltinContinuationFrame::ComputeParametersCount() const {
   // Assert that the first allocatable register is also the argument count
   // register.
@@ -1293,15 +1278,13 @@ void JavaScriptBuiltinContinuationWithCatchFrame::SetException(
 
 FrameSummary::JavaScriptFrameSummary::JavaScriptFrameSummary(
     Isolate* isolate, Object* receiver, JSFunction function,
-    AbstractCode abstract_code, int code_offset, bool is_constructor,
-    FixedArray parameters)
+    AbstractCode abstract_code, int code_offset, bool is_constructor)
     : FrameSummaryBase(isolate, FrameSummary::JAVA_SCRIPT),
       receiver_(receiver, isolate),
       function_(function, isolate),
       abstract_code_(abstract_code, isolate),
       code_offset_(code_offset),
-      is_constructor_(is_constructor),
-      parameters_(parameters, isolate) {
+      is_constructor_(is_constructor) {
   DCHECK(abstract_code->IsBytecodeArray() ||
          Code::cast(abstract_code)->kind() != Code::OPTIMIZED_FUNCTION);
 }
@@ -1547,10 +1530,9 @@ void OptimizedFrame::Summarize(std::vector<FrameSummary>* frames) const {
       }
 
       // Append full summary of the encountered JS frame.
-      Handle<FixedArray> params = GetParameters();
-      FrameSummary::JavaScriptFrameSummary summary(
-          isolate(), *receiver, *function, *abstract_code, code_offset,
-          is_constructor, *params);
+      FrameSummary::JavaScriptFrameSummary summary(isolate(), *receiver,
+                                                   *function, *abstract_code,
+                                                   code_offset, is_constructor);
       frames->push_back(summary);
       is_constructor = false;
     } else if (it->kind() == TranslatedFrame::kConstructStub) {
@@ -1761,10 +1743,9 @@ void InterpretedFrame::WriteInterpreterRegister(int register_index,
 void InterpretedFrame::Summarize(std::vector<FrameSummary>* functions) const {
   DCHECK(functions->empty());
   AbstractCode abstract_code = AbstractCode::cast(GetBytecodeArray());
-  Handle<FixedArray> params = GetParameters();
   FrameSummary::JavaScriptFrameSummary summary(
       isolate(), receiver(), function(), abstract_code, GetBytecodeOffset(),
-      IsConstructor(), *params);
+      IsConstructor());
   functions->push_back(summary);
 }
 

src/frames.h

@@ -442,7 +442,6 @@ class BuiltinExitFrame : public ExitFrame {
   inline Object* new_target_slot_object() const;
 
   friend class StackFrameIteratorBase;
-  friend class FrameArrayBuilder;
 };
 
 class StandardFrame;
@@ -477,15 +476,13 @@ class FrameSummary {
    public:
     JavaScriptFrameSummary(Isolate* isolate, Object* receiver,
                            JSFunction function, AbstractCode abstract_code,
-                           int code_offset, bool is_constructor,
-                           FixedArray parameters);
+                           int code_offset, bool is_constructor);
 
     Handle<Object> receiver() const { return receiver_; }
     Handle<JSFunction> function() const { return function_; }
     Handle<AbstractCode> abstract_code() const { return abstract_code_; }
     int code_offset() const { return code_offset_; }
     bool is_constructor() const { return is_constructor_; }
-    Handle<FixedArray> parameters() const { return parameters_; }
     bool is_subject_to_debugging() const;
     int SourcePosition() const;
     int SourceStatementPosition() const;
@@ -499,7 +496,6 @@ class FrameSummary {
     Handle<AbstractCode> abstract_code_;
     int code_offset_;
     bool is_constructor_;
-    Handle<FixedArray> parameters_;
   };
 
   class WasmFrameSummary : public FrameSummaryBase {
@@ -694,7 +690,6 @@ class JavaScriptFrame : public StandardFrame {
   inline Address GetParameterSlot(int index) const;
   Object* GetParameter(int index) const override;
   int ComputeParametersCount() const override;
-  Handle<FixedArray> GetParameters() const;
 
   // Debugger access.
   void SetParameterValue(int index, Object* value) const;

src/isolate.cc

@@ -470,6 +470,8 @@ StackTraceFailureMessage::StackTraceFailureMessage(Isolate* isolate, void* ptr1,
   }
 }
 
+namespace {
+
 class FrameArrayBuilder {
  public:
   FrameArrayBuilder(Isolate* isolate, FrameSkipMode mode, int limit,
@@ -505,19 +507,8 @@ class FrameArrayBuilder {
     // The stored bytecode offset is relative to a different base than what
     // is used in the source position table, hence the subtraction.
     offset -= BytecodeArray::kHeaderSize - kHeapObjectTag;
-
-    Handle<FixedArray> parameters = isolate_->factory()->empty_fixed_array();
-    if (V8_UNLIKELY(FLAG_detailed_error_stack_trace)) {
-      int param_count = function->shared()->internal_formal_parameter_count();
-      parameters = isolate_->factory()->NewFixedArray(param_count);
-      for (int i = 0; i < param_count; i++) {
-        parameters->set(i,
-                        generator_object->parameters_and_registers()->get(i));
-      }
-    }
-
     elements_ = FrameArray::AppendJSFrame(elements_, receiver, function, code,
-                                          offset, flags, parameters);
+                                          offset, flags);
   }
 
   void AppendPromiseAllFrame(Handle<Context> context, int offset) {
@@ -530,12 +521,8 @@ class FrameArrayBuilder {
 
     Handle<Object> receiver(native_context->promise_function(), isolate_);
     Handle<AbstractCode> code(AbstractCode::cast(function->code()), isolate_);
-
-    // TODO(mmarchini) save Promises list from Promise.all()
-    Handle<FixedArray> parameters = isolate_->factory()->empty_fixed_array();
-
     elements_ = FrameArray::AppendJSFrame(elements_, receiver, function, code,
-                                          offset, flags, parameters);
+                                          offset, flags);
   }
 
   void AppendJavaScriptFrame(
@@ -553,13 +540,9 @@ class FrameArrayBuilder {
     if (IsStrictFrame(function)) flags |= FrameArray::kIsStrict;
     if (is_constructor) flags |= FrameArray::kIsConstructor;
 
-    Handle<FixedArray> parameters = isolate_->factory()->empty_fixed_array();
-    if (V8_UNLIKELY(FLAG_detailed_error_stack_trace))
-      parameters = summary.parameters();
-
     elements_ = FrameArray::AppendJSFrame(
         elements_, TheHoleToUndefined(isolate_, summary.receiver()), function,
-        abstract_code, offset, flags, parameters);
+        abstract_code, offset, flags);
   }
 
   void AppendWasmCompiledFrame(
@@ -606,18 +589,9 @@ class FrameArrayBuilder {
     if (IsStrictFrame(function)) flags |= FrameArray::kIsStrict;
     if (exit_frame->IsConstructor()) flags |= FrameArray::kIsConstructor;
 
-    Handle<FixedArray> parameters = isolate_->factory()->empty_fixed_array();
-    if (V8_UNLIKELY(FLAG_detailed_error_stack_trace)) {
-      int param_count = exit_frame->ComputeParametersCount();
-      parameters = isolate_->factory()->NewFixedArray(param_count);
-      for (int i = 0; i < param_count; i++) {
-        parameters->set(i, exit_frame->GetParameter(i));
-      }
-    }
-
     elements_ = FrameArray::AppendJSFrame(elements_, receiver, function,
                                           Handle<AbstractCode>::cast(code),
-                                          offset, flags, parameters);
+                                          offset, flags);
   }
 
   bool full() { return elements_->FrameCount() >= limit_; }
@@ -816,6 +790,8 @@ void CaptureAsyncStackTrace(Isolate* isolate, Handle<JSPromise> promise,
   }
 }
 
+}  // namespace
+
 Handle<Object> Isolate::CaptureSimpleStackTrace(Handle<JSReceiver> error_object,
                                                 FrameSkipMode mode,
                                                 Handle<Object> caller) {

src/objects.cc

@@ -10733,8 +10733,7 @@ Handle<FrameArray> FrameArray::AppendJSFrame(Handle<FrameArray> in,
                                              Handle<Object> receiver,
                                              Handle<JSFunction> function,
                                              Handle<AbstractCode> code,
-                                             int offset, int flags,
-                                             Handle<FixedArray> parameters) {
+                                             int offset, int flags) {
   const int frame_count = in->FrameCount();
   const int new_length = LengthFor(frame_count + 1);
   Handle<FrameArray> array =
@@ -10744,7 +10743,6 @@ Handle<FrameArray> FrameArray::AppendJSFrame(Handle<FrameArray> in,
   array->SetCode(frame_count, *code);
   array->SetOffset(frame_count, Smi::FromInt(offset));
   array->SetFlags(frame_count, Smi::FromInt(flags));
-  array->SetParameters(frame_count, *parameters);
   array->set(kFrameCountIndex, Smi::FromInt(frame_count + 1));
   return array;
 }

src/objects/frame-array.h

@@ -25,8 +25,7 @@ class Handle;
   V(Function, JSFunction)             \
   V(Code, AbstractCode)               \
   V(Offset, Smi)                      \
-  V(Flags, Smi)                       \
-  V(Parameters, FixedArray)
+  V(Flags, Smi)
 
 // Container object for data collected during simple stack trace captures.
 class FrameArray : public FixedArray {
@@ -60,8 +59,7 @@ class FrameArray : public FixedArray {
                                           Handle<Object> receiver,
                                           Handle<JSFunction> function,
                                           Handle<AbstractCode> code, int offset,
-                                          int flags,
-                                          Handle<FixedArray> parameters);
+                                          int flags);
   static Handle<FrameArray> AppendWasmFrame(
       Handle<FrameArray> in, Handle<WasmInstanceObject> wasm_instance,
       int wasm_function_index, wasm::WasmCode* code, int offset, int flags);
@@ -88,9 +86,7 @@ class FrameArray : public FixedArray {
 
   static const int kFlagsOffset = 4;
 
-  static const int kParametersOffset = 5;
-
-  static const int kElementsPerFrame = 6;
+  static const int kElementsPerFrame = 5;
 
   // Array layout indices.
 

test/cctest/heap/test-heap.cc

@@ -46,7 +46,6 @@
 #include "src/ic/ic.h"
 #include "src/macro-assembler-inl.h"
 #include "src/objects-inl.h"
-#include "src/objects/frame-array-inl.h"
 #include "src/objects/heap-number-inl.h"
 #include "src/objects/js-array-inl.h"
 #include "src/objects/js-collection-inl.h"
@@ -2953,13 +2952,6 @@ TEST(Regress1465) {
   CHECK_EQ(1, transitions_after);
 }
 
-static i::Handle<JSObject> GetByName(const char* name) {
-  return i::Handle<JSObject>::cast(
-      v8::Utils::OpenHandle(*v8::Local<v8::Object>::Cast(
-          CcTest::global()
-              ->Get(CcTest::isolate()->GetCurrentContext(), v8_str(name))
-              .ToLocalChecked())));
-}
 
 #ifdef DEBUG
 static void AddTransitions(int transitions_count) {
@@ -2972,6 +2964,15 @@ static void AddTransitions(int transitions_count) {
 }
 
 
+static i::Handle<JSObject> GetByName(const char* name) {
+  return i::Handle<JSObject>::cast(
+      v8::Utils::OpenHandle(*v8::Local<v8::Object>::Cast(
+          CcTest::global()
+              ->Get(CcTest::isolate()->GetCurrentContext(), v8_str(name))
+              .ToLocalChecked())));
+}
+
+
 static void AddPropertyTo(
     int gc_count, Handle<JSObject> object, const char* property_name) {
   Isolate* isolate = CcTest::i_isolate();
@@ -3487,160 +3488,6 @@ UNINITIALIZED_TEST(ReleaseStackTraceData) {
   isolate->Dispose();
 }
 
-// TODO(mmarchini) also write tests for async/await and Promise.all
-void DetailedErrorStackTraceTest(const char* src,
-                                 std::function<void(Handle<FrameArray>)> test) {
-  FLAG_detailed_error_stack_trace = true;
-  CcTest::InitializeVM();
-  v8::HandleScope scope(CcTest::isolate());
-
-  v8::TryCatch try_catch(CcTest::isolate());
-  CompileRun(src);
-
-  CHECK(try_catch.HasCaught());
-  Handle<Object> exception = v8::Utils::OpenHandle(*try_catch.Exception());
-
-  Isolate* isolate = CcTest::i_isolate();
-  Handle<Name> key = isolate->factory()->stack_trace_symbol();
-
-  Handle<FrameArray> stack_trace(
-      FrameArray::cast(
-          Handle<JSArray>::cast(
-              Object::GetProperty(isolate, exception, key).ToHandleChecked())
-              ->elements()),
-      isolate);
-
-  test(stack_trace);
-}
-
-// * Test interpreted function error
-TEST(DetailedErrorStackTrace) {
-  FLAG_opt = false;
-  static const char* source =
-      "function func1(arg1) {       "
-      "  let err = new Error();     "
-      "  throw err;                 "
-      "}                            "
-      "function func2(arg1, arg2) { "
-      "  func1(42);                 "
-      "}                            "
-      "class Foo {};                "
-      "function main(arg1, arg2) {  "
-      "  func2(arg1, false);        "
-      "}                            "
-      "var foo = new Foo();         "
-      "main(foo);                   ";
-
-  DetailedErrorStackTraceTest(source, [](Handle<FrameArray> stack_trace) {
-    FixedArrayArgType foo_parameters = stack_trace->Parameters(0);
-    CHECK_EQ(foo_parameters->length(), 1);
-    CHECK(foo_parameters->get(0)->IsSmi());
-    CHECK_EQ(Smi::ToInt(foo_parameters->get(0)), 42);
-
-    FixedArrayArgType bar_parameters = stack_trace->Parameters(1);
-    CHECK_EQ(bar_parameters->length(), 2);
-    CHECK(bar_parameters->get(0)->IsJSObject());
-    CHECK(bar_parameters->get(1)->IsBoolean());
-    Handle<Object> foo = Handle<Object>::cast(GetByName("foo"));
-    CHECK_EQ(bar_parameters->get(0), *foo);
-    CHECK(!bar_parameters->get(1)->BooleanValue(CcTest::i_isolate()));
-
-    FixedArrayArgType main_parameters = stack_trace->Parameters(2);
-    CHECK_EQ(main_parameters->length(), 2);
-    CHECK(main_parameters->get(0)->IsJSObject());
-    CHECK(main_parameters->get(1)->IsUndefined());
-    CHECK_EQ(main_parameters->get(0), *foo);
-  });
-}
-
-// * Test optimized function error
-TEST(DetailedErrorStackTraceOpt) {
-  FLAG_always_opt = true;
-  static const char* source =
-      "function func1(arg1) {       "
-      "  let err = new Error();     "
-      "  throw err;                 "
-      "}                            "
-      "function func2(arg1, arg2) { "
-      "  func1(42);                 "
-      "}                            "
-      "class Foo {};                "
-      "function main(arg1, arg2) {  "
-      "  func2(arg1, false);        "
-      "}                            "
-      "var foo = new Foo();         "
-      "main(foo);                   ";
-
-  DetailedErrorStackTraceTest(source, [](Handle<FrameArray> stack_trace) {
-    FixedArrayArgType foo_parameters = stack_trace->Parameters(0);
-    CHECK_EQ(foo_parameters->length(), 1);
-    CHECK(foo_parameters->get(0)->IsSmi());
-    CHECK_EQ(Smi::ToInt(foo_parameters->get(0)), 42);
-
-    FixedArrayArgType bar_parameters = stack_trace->Parameters(1);
-    CHECK_EQ(bar_parameters->length(), 2);
-    CHECK(bar_parameters->get(0)->IsJSObject());
-    CHECK(bar_parameters->get(1)->IsBoolean());
-    Handle<Object> foo = Handle<Object>::cast(GetByName("foo"));
-    CHECK_EQ(bar_parameters->get(0), *foo);
-    CHECK(!bar_parameters->get(1)->BooleanValue(CcTest::i_isolate()));
-
-    FixedArrayArgType main_parameters = stack_trace->Parameters(2);
-    CHECK_EQ(main_parameters->length(), 2);
-    CHECK(main_parameters->get(0)->IsJSObject());
-    CHECK(main_parameters->get(1)->IsUndefined());
-    CHECK_EQ(main_parameters->get(0), *foo);
-  });
-}
-
-// * Test optimized function with inline frame error
-TEST(DetailedErrorStackTraceInline) {
-  FLAG_allow_natives_syntax = true;
-  static const char* source =
-      "function add(x) {                 "
-      " if (x == 42)                     "
-      "  throw new Error();              "
-      " return x + x;                    "
-      "}                                 "
-      "add(0);                           "
-      "add(1);                           "
-      "function foo(x) {                 "
-      " return add(x + 1)                "
-      "}                                 "
-      "foo(40);                          "
-      "%OptimizeFunctionOnNextCall(foo); "
-      "foo(41);                          ";
-
-  DetailedErrorStackTraceTest(source, [](Handle<FrameArray> stack_trace) {
-    FixedArrayArgType parameters_add = stack_trace->Parameters(0);
-    CHECK_EQ(parameters_add->length(), 1);
-    CHECK(parameters_add->get(0)->IsSmi());
-    CHECK_EQ(Smi::ToInt(parameters_add->get(0)), 42);
-
-    FixedArrayArgType parameters_foo = stack_trace->Parameters(1);
-    CHECK_EQ(parameters_foo->length(), 1);
-    CHECK(parameters_foo->get(0)->IsSmi());
-    CHECK_EQ(Smi::ToInt(parameters_foo->get(0)), 41);
-  });
-}
-
-// * Test builtin exit error
-TEST(DetailedErrorStackTraceBuiltinExit) {
-  static const char* source =
-      "function test(arg1) {           "
-      "  (new Number()).toFixed(arg1); "
-      "}                               "
-      "test(9999);                     ";
-
-  DetailedErrorStackTraceTest(source, [](Handle<FrameArray> stack_trace) {
-    FixedArrayArgType parameters = stack_trace->Parameters(0);
-
-    CHECK_EQ(parameters->length(), 2);
-    CHECK(parameters->get(0)->IsSmi());
-    CHECK_EQ(Smi::ToInt(parameters->get(0)), 9999);
-  });
-}
-
 TEST(Regress169928) {
   FLAG_allow_natives_syntax = true;
 #ifndef V8_LITE_MODE