[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}

src/builtins/builtins-constructor-gen.cc

@@ -738,8 +738,7 @@ TF_BUILTIN(NumberConstructor, ConstructorBuiltinsAssembler) {
   }
 }
 
-TF_BUILTIN(GenericConstructorLazyDeoptContinuation,
-           ConstructorBuiltinsAssembler) {
+TF_BUILTIN(GenericLazyDeoptContinuation, 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)                                                           \
-  API(HandleApiCallAsFunction)                                                 \
-  API(HandleApiCallAsConstructor)                                              \
+  CPP(HandleApiCall)                                                           \
+  CPP(HandleApiCallAsFunction)                                                 \
+  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

@@ -2742,6 +2742,7 @@ Reduction JSCallReducer::ReduceCallApiFunction(
   }
 
   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)
@@ -2750,6 +2751,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(
@@ -2881,6 +2884,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));
@@ -2888,6 +2895,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);
@@ -5857,8 +5865,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,
-      target, context, parameters, num_parameters, frame_state,
+      jsgraph(), shared, Builtins::kGenericLazyDeoptContinuation, target,
+      context, parameters, num_parameters, frame_state,
       ContinuationFrameStateMode::LAZY);
 
   Node* result =
@@ -6853,9 +6861,8 @@ Reduction JSCallReducer::ReduceNumberConstructor(Node* node) {
   int stack_parameter_count = arraysize(stack_parameters);
   Node* continuation_frame_state =
       CreateJavaScriptBuiltinContinuationFrameState(
-          jsgraph(), shared_info,
-          Builtins::kGenericConstructorLazyDeoptContinuation, target, context,
-          stack_parameters, stack_parameter_count, frame_state,
+          jsgraph(), shared_info, Builtins::kGenericLazyDeoptContinuation,
+          target, context, stack_parameters, stack_parameter_count, frame_state,
           ContinuationFrameStateMode::LAZY);
 
   // Convert the {value} to a Number.

src/isolate.cc

@@ -821,7 +821,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,7 @@ 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;
   v8::Isolate::CreateParams create_params;
   create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
   isolate_1 = v8::Isolate::New(create_params);
@@ -27937,18 +27938,23 @@ UNINITIALIZED_TEST(NestedIsolates) {
         CompileRun("f2(); result //# sourceURL=isolate2c")
             ->ToString(context)
             .ToLocalChecked();
-    v8::Local<v8::String> expectation = v8_str(isolate_2,
-                                               "Error\n"
-                                               "    at f2 (isolate2a:1:104)\n"
-                                               "    at isolate2b:1:1\n"
-                                               "    at f2 (isolate2a:1:71)\n"
-                                               "    at isolate2b:1:1\n"
-                                               "    at f2 (isolate2a:1:71)\n"
-                                               "    at isolate2b:1:1\n"
-                                               "    at f2 (isolate2a:1:71)\n"
-                                               "    at isolate2b:1:1\n"
-                                               "    at f2 (isolate2a:1:71)\n"
-                                               "    at isolate2c:1:1");
+    v8::Local<v8::String> expectation =
+        v8_str(isolate_2,
+               "Error\n"
+               "    at f2 (isolate2a:1:104)\n"
+               "    at isolate2b:1:1\n"
+               "    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 isolate2b:1:1\n"
+               "    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.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

@@ -34,17 +34,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(3, Realm.shared.error_1.length);
+assertSame(6, Realm.shared.error_0.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

+// 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.
+
+// 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

+// 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
+
+// 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);