Reland "[stack-trace] Include API functions in Error.stack stack trace"

This is a reland of 3dd5661204

The reland introduces a new flag "--experimental-stack-trace-frames".
The flag is disabled by default, but enabled for relevant tests.
The flag stays disabled by default until API frames are eagerly
symbolized to prevent leaks in blink web tests.

Original change's description:
> [stack-trace] Include API functions in Error.stack stack trace
>
> This CL extends Error.stack to include frames of functions declared
> with the C++ FunctionTemplate API. For example, "print" in d8.
>
> Two changes are necessary:
>   - HandleApiCall and friends need to go through an BUILTIN_EXIT frame
>     instead of an EXIT frame. The existing stack-trace machinery will
>     then pick up FunctionTemplate frames without additional changes.
>   - Turbofan doesn't go through HandleApiCall, but instead uses an
>     ASM builtin to enter FunctionTemplate functions. A "marker"
>     frame state is needed to include these frames in the stack trace.
>
> Note: This CL only includes these frames in Error.stack,
> but not (yet) in the stack-trace API (v8.h).
>
> Bug: v8:8742,v8:6802
> Change-Id: Ic0631af883cf56e0d0122a2e0c54e36fed324d91
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1609835
> Commit-Queue: Simon Zünd <szuend@chromium.org>
> Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#61602}

Bug: v8:8742, v8:6802
Change-Id: I1d3b79cdf0b2edcbaeff1ec15e10deeca725f017
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1621925
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61683}

src/builtins/builtins-constructor-gen.cc

@@ -738,8 +738,7 @@ TF_BUILTIN(NumberConstructor, ConstructorBuiltinsAssembler) {
   }
 }
 
-TF_BUILTIN(GenericConstructorLazyDeoptContinuation,
+TF_BUILTIN(GenericLazyDeoptContinuation, ConstructorBuiltinsAssembler) {
-           ConstructorBuiltinsAssembler) {
   Node* result = Parameter(Descriptor::kResult);
   Return(result);
 }

src/builtins/builtins-definitions.h

@@ -167,9 +167,9 @@ namespace internal {
   /* API callback handling */                                                  \
   ASM(CallApiCallback, ApiCallback)                                            \
   ASM(CallApiGetter, ApiGetter)                                                \
-  API(HandleApiCall)                                                           \
+  CPP(HandleApiCall)                                                           \
-  API(HandleApiCallAsFunction)                                                 \
+  CPP(HandleApiCallAsFunction)                                                 \
-  API(HandleApiCallAsConstructor)                                              \
+  CPP(HandleApiCallAsConstructor)                                              \
                                                                                \
   /* Adapters for Turbofan into runtime */                                     \
   TFC(AllocateInYoungGeneration, Allocate)                                     \
@@ -1003,7 +1003,7 @@ namespace internal {
   /* TypedArray */                                                             \
   /* ES #sec-typedarray-constructors */                                        \
   TFJ(TypedArrayBaseConstructor, 0, kReceiver)                                 \
-  TFJ(GenericConstructorLazyDeoptContinuation, 1, kReceiver, kResult)          \
+  TFJ(GenericLazyDeoptContinuation, 1, kReceiver, kResult)                     \
   TFJ(TypedArrayConstructor, SharedFunctionInfo::kDontAdaptArgumentsSentinel)  \
   CPP(TypedArrayPrototypeBuffer)                                               \
   /* ES6 #sec-get-%typedarray%.prototype.bytelength */                         \

src/compiler/frame-states.cc

@@ -206,6 +206,17 @@ Node* CreateJavaScriptBuiltinContinuationFrameState(
       shared.object());
 }
 
+Node* CreateGenericLazyDeoptContinuationFrameState(
+    JSGraph* graph, const SharedFunctionInfoRef& shared, Node* target,
+    Node* context, Node* receiver, Node* outer_frame_state) {
+  Node* stack_parameters[]{receiver};
+  const int stack_parameter_count = arraysize(stack_parameters);
+  return CreateJavaScriptBuiltinContinuationFrameState(
+      graph, shared, Builtins::kGenericLazyDeoptContinuation, target, context,
+      stack_parameters, stack_parameter_count, outer_frame_state,
+      ContinuationFrameStateMode::LAZY);
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/frame-states.h

@@ -161,6 +161,10 @@ Node* CreateJavaScriptBuiltinContinuationFrameState(
     int stack_parameter_count, Node* outer_frame_state,
     ContinuationFrameStateMode mode);
 
+Node* CreateGenericLazyDeoptContinuationFrameState(
+    JSGraph* graph, const SharedFunctionInfoRef& shared, Node* target,
+    Node* context, Node* receiver, Node* outer_frame_state);
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/js-call-reducer.cc

@@ -2738,6 +2738,7 @@ Reduction JSCallReducer::ReduceCallApiFunction(
   DCHECK_EQ(IrOpcode::kJSCall, node->opcode());
   CallParameters const& p = CallParametersOf(node->op());
   int const argc = static_cast<int>(p.arity()) - 2;
+  Node* target = NodeProperties::GetValueInput(node, 0);
   Node* global_proxy =
       jsgraph()->Constant(native_context().global_proxy_object());
   Node* receiver = (p.convert_mode() == ConvertReceiverMode::kNullOrUndefined)
@@ -2746,6 +2747,8 @@ Reduction JSCallReducer::ReduceCallApiFunction(
   Node* holder;
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
+  Node* context = NodeProperties::GetContextInput(node);
+  Node* frame_state = NodeProperties::GetFrameStateInput(node);
 
   // See if we can optimize this API call to {shared}.
   Handle<FunctionTemplateInfo> function_template_info(
@@ -2884,6 +2887,10 @@ Reduction JSCallReducer::ReduceCallApiFunction(
   ApiFunction api_function(v8::ToCData<Address>(call_handler_info->callback()));
   ExternalReference function_reference = ExternalReference::Create(
       &api_function, ExternalReference::DIRECT_API_CALL);
+
+  Node* continuation_frame_state = CreateGenericLazyDeoptContinuationFrameState(
+      jsgraph(), shared, target, context, receiver, frame_state);
+
   node->InsertInput(graph()->zone(), 0,
                     jsgraph()->HeapConstant(call_api_callback.code()));
   node->ReplaceInput(1, jsgraph()->ExternalConstant(function_reference));
@@ -2891,6 +2898,7 @@ Reduction JSCallReducer::ReduceCallApiFunction(
   node->InsertInput(graph()->zone(), 3, jsgraph()->Constant(data));
   node->InsertInput(graph()->zone(), 4, holder);
   node->ReplaceInput(5, receiver);       // Update receiver input.
+  node->ReplaceInput(7 + argc, continuation_frame_state);
   node->ReplaceInput(8 + argc, effect);  // Update effect input.
   NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
   return Changed(node);
@@ -5940,8 +5948,8 @@ Reduction JSCallReducer::ReduceTypedArrayConstructor(
   Node* const parameters[] = {jsgraph()->TheHoleConstant()};
   int const num_parameters = static_cast<int>(arraysize(parameters));
   frame_state = CreateJavaScriptBuiltinContinuationFrameState(
-      jsgraph(), shared, Builtins::kGenericConstructorLazyDeoptContinuation,
+      jsgraph(), shared, Builtins::kGenericLazyDeoptContinuation, target,
-      target, context, parameters, num_parameters, frame_state,
+      context, parameters, num_parameters, frame_state,
       ContinuationFrameStateMode::LAZY);
 
   Node* result =
@@ -6936,9 +6944,8 @@ Reduction JSCallReducer::ReduceNumberConstructor(Node* node) {
   int stack_parameter_count = arraysize(stack_parameters);
   Node* continuation_frame_state =
       CreateJavaScriptBuiltinContinuationFrameState(
-          jsgraph(), shared_info,
+          jsgraph(), shared_info, Builtins::kGenericLazyDeoptContinuation,
-          Builtins::kGenericConstructorLazyDeoptContinuation, target, context,
+          target, context, stack_parameters, stack_parameter_count, frame_state,
-          stack_parameters, stack_parameter_count, frame_state,
           ContinuationFrameStateMode::LAZY);
 
   // Convert the {value} to a Number.

src/flag-definitions.h

@@ -963,6 +963,8 @@ DEFINE_BOOL(expose_trigger_failure, false, "expose trigger-failure extension")
 DEFINE_INT(stack_trace_limit, 10, "number of stack frames to capture")
 DEFINE_BOOL(builtins_in_stack_traces, false,
             "show built-in functions in stack traces")
+DEFINE_BOOL(experimental_stack_trace_frames, false,
+            "enable experimental frames (API/Builtins) and stack trace layout")
 DEFINE_BOOL(disallow_code_generation_from_strings, false,
             "disallow eval and friends")
 DEFINE_BOOL(expose_async_hooks, false, "expose async_hooks object")

src/isolate.cc

@@ -708,6 +708,13 @@ class FrameArrayBuilder {
     // Filter out internal frames that we do not want to show.
     if (!IsVisibleInStackTrace(function)) return;
 
+    // TODO(szuend): Remove this check once the flag is enabled
+    //               by default.
+    if (!FLAG_experimental_stack_trace_frames &&
+        function->shared()->IsApiFunction()) {
+      return;
+    }
+
     Handle<Object> receiver(exit_frame->receiver(), isolate_);
     Handle<Code> code(exit_frame->LookupCode(), isolate_);
     const int offset =
@@ -821,7 +828,8 @@ class FrameArrayBuilder {
     // internal call sites in the stack trace for debugging purposes.
     if (!FLAG_builtins_in_stack_traces &&
         !function->shared()->IsUserJavaScript()) {
-      return function->shared()->native();
+      return function->shared()->native() ||
+             function->shared()->IsApiFunction();
     }
     return true;
   }

test/cctest/test-api.cc

@@ -27881,6 +27881,8 @@ UNINITIALIZED_TEST(NestedIsolates) {
   // call into the other isolate. Recurse a few times, trigger GC along the way,
   // and finally capture a stack trace. Check that the stack trace only includes
   // frames from its own isolate.
+  i::FLAG_stack_trace_limit = 20;
+  i::FLAG_experimental_stack_trace_frames = true;
   v8::Isolate::CreateParams create_params;
   create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
   isolate_1 = v8::Isolate::New(create_params);
@@ -27937,18 +27939,23 @@ UNINITIALIZED_TEST(NestedIsolates) {
         CompileRun("f2(); result //# sourceURL=isolate2c")
             ->ToString(context)
             .ToLocalChecked();
-    v8::Local<v8::String> expectation = v8_str(isolate_2,
+    v8::Local<v8::String> expectation =
-                                               "Error\n"
+        v8_str(isolate_2,
-                                               "    at f2 (isolate2a:1:104)\n"
+               "Error\n"
-                                               "    at isolate2b:1:1\n"
+               "    at f2 (isolate2a:1:104)\n"
-                                               "    at f2 (isolate2a:1:71)\n"
+               "    at isolate2b:1:1\n"
-                                               "    at isolate2b:1:1\n"
+               "    at call_isolate_1 (<anonymous>)\n"
-                                               "    at f2 (isolate2a:1:71)\n"
+               "    at f2 (isolate2a:1:71)\n"
-                                               "    at isolate2b:1:1\n"
+               "    at isolate2b:1:1\n"
-                                               "    at f2 (isolate2a:1:71)\n"
+               "    at call_isolate_1 (<anonymous>)\n"
-                                               "    at isolate2b:1:1\n"
+               "    at f2 (isolate2a:1:71)\n"
-                                               "    at f2 (isolate2a:1:71)\n"
+               "    at isolate2b:1:1\n"
-                                               "    at isolate2c:1:1");
+               "    at call_isolate_1 (<anonymous>)\n"
+               "    at f2 (isolate2a:1:71)\n"
+               "    at isolate2b:1:1\n"
+               "    at call_isolate_1 (<anonymous>)\n"
+               "    at f2 (isolate2a:1:71)\n"
+               "    at isolate2c:1:1");
     CHECK(result->StrictEquals(expectation));
   }
 

test/message/mjsunit/fail/assert-promise-result-wasm-compile-fail.js

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Flags: --experimental-stack-trace-frames
+
 // Test wasm compilation explicitly, since this creates a promise which is only
 // resolved later, i.e. the message queue gets empty in-between.
 // The important part here is that d8 exits with a non-zero exit code.

test/message/mjsunit/fail/assert-promise-result-wasm-compile-fail.out

@@ -6,5 +6,6 @@ Error
     at *%(basename)s:{NUMBER}:1
 
 CompileError: WebAssembly.compile(): BufferSource argument is empty
+    at WebAssembly.compile (<anonymous>)
     at test (*%(basename)s:{NUMBER}:23)
 

test/mjsunit/cross-realm-filtering.js

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Flags: --experimental-stack-trace-frames
+
 var realms = [Realm.current(), Realm.create()];
 
 // Check stack trace filtering across security contexts.
@@ -34,17 +36,17 @@ function assertNotIn(thrower, error) {
 
 Realm.eval(realms[1], script);
 assertSame(2, Realm.shared.error_0.length);
-assertSame(3, Realm.shared.error_1.length);
+assertSame(4, Realm.shared.error_1.length);
 
-assertTrue(Realm.shared.thrower_1 === Realm.shared.error_1[1].getFunction());
+assertTrue(Realm.shared.thrower_1 === Realm.shared.error_1[2].getFunction());
 assertNotIn(Realm.shared.thrower_0, Realm.shared.error_0);
 assertNotIn(Realm.shared.thrower_0, Realm.shared.error_1);
 
 Realm.eval(realms[0], script);
-assertSame(4, Realm.shared.error_0.length);
+assertSame(6, Realm.shared.error_0.length);
-assertSame(3, Realm.shared.error_1.length);
+assertSame(4, Realm.shared.error_1.length);
 
-assertTrue(Realm.shared.thrower_0 === Realm.shared.error_0[1].getFunction());
+assertTrue(Realm.shared.thrower_0 === Realm.shared.error_0[2].getFunction());
 assertNotIn(Realm.shared.thrower_1, Realm.shared.error_0);
 assertNotIn(Realm.shared.thrower_1, Realm.shared.error_1);
 

test/mjsunit/stack-trace-cpp-function-template-1.js

@@ -0,0 +1,37 @@
+// Copyright 2019 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.
+
+// Flags: --experimental-stack-trace-frames
+
+// Verifies that "print" shows up in Error.stack:
+// Error
+//     at foo (...)
+//     at Object.toString (...)
+//     at print (<anonymous>)
+//     at bar (...)
+//     at (...)
+let prepareStackTraceCalled = false;
+Error.prepareStackTrace = (e, frames) => {
+  prepareStackTraceCalled = true;
+  assertEquals(5, frames.length);
+
+  assertEquals(foo, frames[0].getFunction());
+  assertEquals(object.toString, frames[1].getFunction());
+  assertEquals("print", frames[2].getFunctionName());
+  assertEquals(bar, frames[3].getFunction());
+  return frames;
+};
+
+function foo() { throw new Error(); }
+const object = { toString: () => { return foo(); } };
+
+function bar() {
+  print(object);
+}
+
+try { bar(); } catch(e) {
+  // Trigger prepareStackTrace.
+  e.stack;
+}
+assertTrue(prepareStackTraceCalled);

test/mjsunit/stack-trace-cpp-function-template-2.js

@@ -0,0 +1,45 @@
+// Copyright 2019 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.
+
+// Flags: --allow-natives-syntax --opt --experimental-stack-trace-frames
+
+// Verifies that "print" shows up in Error.stack when "bar" is optimized
+// by Turbofan:
+// Error
+//     at foo (...)
+//     at Object.toString (...)
+//     at print (<anonymous>)
+//     at bar (...)
+//     at (...)
+let prepareStackTraceCalled = false;
+Error.prepareStackTrace = (e, frames) => {
+  prepareStackTraceCalled = true;
+  assertEquals(5, frames.length);
+
+  assertEquals(foo, frames[0].getFunction());
+  assertEquals(object.toString, frames[1].getFunction());
+  assertEquals("print", frames[2].getFunctionName());
+  assertEquals(bar, frames[3].getFunction());
+  return frames;
+};
+
+function foo() { throw new Error(); }
+const object = { toString: () => { return foo(); } };
+
+function bar() {
+  print(object);
+}
+
+%PrepareFunctionForOptimization(bar);
+try { bar(); } catch (e) {}
+try { bar(); } catch (e) {}
+%OptimizeFunctionOnNextCall(bar);
+
+try { bar(); } catch(e) {
+  // Trigger prepareStackTrace.
+  e.stack;
+}
+
+assertOptimized(bar);
+assertTrue(prepareStackTraceCalled);