[async] Add Promise.all() support to --async-stack-traces.

This adds support for Promise.all() to --async-stack-traces (also at
zero cost, since we can derive the relevant information from the resolve
element closure and context). In case of `Promise.all(a)` the stack
trace even tells you which element of `a` is responsible, for example

```js
async function fine() {}
async function thrower() { await fine(); throw new Error(); }
async function test() { await Promise.all([fine(), thrower()]); }
```

will generate the following stack trace

```
Error
    at thrower (something.js:1:9)
    at async Promise.all (index 1)
    at async test (something.js:3:3)
```

so it not only shows the async Promise.all() frames, but even tells the
user exactly that the second element of `[fine(), thrower()]` is the
relevant one.

Bug: v8:7522
Change-Id: I279a845888e06053cf0e3c9338ab71caabaabf45
Reviewed-on: https://chromium-review.googlesource.com/c/1299248
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57023}

src/bootstrapper.cc

@@ -2386,8 +2386,10 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
 
     InstallSpeciesGetter(isolate_, promise_fun);
 
-    SimpleInstallFunction(isolate_, promise_fun, "all", Builtins::kPromiseAll,
-                          1, true, BuiltinFunctionId::kPromiseAll);
+    Handle<JSFunction> promise_all = SimpleInstallFunction(
+        isolate_, promise_fun, "all", Builtins::kPromiseAll, 1, true,
+        BuiltinFunctionId::kPromiseAll);
+    native_context()->set_promise_all(*promise_all);
 
     SimpleInstallFunction(isolate_, promise_fun, "race", Builtins::kPromiseRace,
                           1, true, BuiltinFunctionId::kPromiseRace);
@@ -4360,6 +4362,7 @@ void Bootstrapper::ExportFromRuntime(Isolate* isolate,
           {"isConstructor", Builtins::kCallSitePrototypeIsConstructor},
           {"isEval", Builtins::kCallSitePrototypeIsEval},
           {"isNative", Builtins::kCallSitePrototypeIsNative},
+          {"isPromiseAll", Builtins::kCallSitePrototypeIsPromiseAll},
           {"isToplevel", Builtins::kCallSitePrototypeIsToplevel},
           {"toString", Builtins::kCallSitePrototypeToString}};
 

src/builtins/builtins-callsite.cc

@@ -169,6 +169,14 @@ BUILTIN(CallSitePrototypeIsNative) {
   return isolate->heap()->ToBoolean(it.Frame()->IsNative());
 }
 
+BUILTIN(CallSitePrototypeIsPromiseAll) {
+  HandleScope scope(isolate);
+  CHECK_CALLSITE(recv, "isPromiseAll");
+  FrameArrayIterator it(isolate, GetFrameArray(isolate, recv),
+                        GetFrameIndex(isolate, recv));
+  return isolate->heap()->ToBoolean(it.Frame()->IsPromiseAll());
+}
+
 BUILTIN(CallSitePrototypeIsToplevel) {
   HandleScope scope(isolate);
   CHECK_CALLSITE(recv, "isToplevel");

src/builtins/builtins-definitions.h

@@ -466,6 +466,7 @@ namespace internal {
   CPP(CallSitePrototypeIsConstructor)                                          \
   CPP(CallSitePrototypeIsEval)                                                 \
   CPP(CallSitePrototypeIsNative)                                               \
+  CPP(CallSitePrototypeIsPromiseAll)                                           \
   CPP(CallSitePrototypeIsToplevel)                                             \
   CPP(CallSitePrototypeToString)                                               \
                                                                                \

src/builtins/builtins-promise-gen.h

@@ -29,7 +29,8 @@ class PromiseBuiltinsAssembler : public CodeStubAssembler {
     kPromiseContextLength,
   };
 
- protected:
+  // TODO(bmeurer): Move this to a proper context map in contexts.h?
+  // Similar to the AwaitContext that we introduced for await closures.
   enum PromiseAllResolveElementContextSlots {
     // Remaining elements count
     kPromiseAllResolveElementRemainingSlot = Context::MIN_CONTEXT_SLOTS,
@@ -43,7 +44,6 @@ class PromiseBuiltinsAssembler : public CodeStubAssembler {
     kPromiseAllResolveElementLength
   };
 
- public:
   enum FunctionContextSlot {
     kCapabilitySlot = Context::MIN_CONTEXT_SLOTS,
 

src/builtins/builtins.cc

@@ -297,6 +297,7 @@ bool Builtins::IsLazy(int index) {
     case kInterpreterEnterBytecodeAdvance:
     case kInterpreterEnterBytecodeDispatch:
     case kInterpreterEntryTrampoline:
+    case kPromiseAllResolveElementClosure:  // https://crbug.com/v8/7522
     case kPromiseConstructorLazyDeoptContinuation:
     case kRecordWrite:  // https://crbug.com/chromium/765301.
     case kThrowWasmTrapDivByZero:             // Required by wasm.

src/contexts.h

@@ -84,6 +84,7 @@ enum ContextLookupFlags {
   V(FUNCTION_HAS_INSTANCE_INDEX, JSFunction, function_has_instance)       \
   V(OBJECT_VALUE_OF, JSFunction, object_value_of)                         \
   V(OBJECT_TO_STRING, JSFunction, object_to_string)                       \
+  V(PROMISE_ALL_INDEX, JSFunction, promise_all)                           \
   V(PROMISE_CATCH_INDEX, JSFunction, promise_catch)                       \
   V(PROMISE_FUNCTION_INDEX, JSFunction, promise_function)                 \
   V(RANGE_ERROR_FUNCTION_INDEX, JSFunction, range_error_function)         \

src/isolate.cc

@@ -22,6 +22,7 @@
 #include "src/base/sys-info.h"
 #include "src/base/utils/random-number-generator.h"
 #include "src/bootstrapper.h"
+#include "src/builtins/builtins-promise-gen.h"
 #include "src/builtins/constants-table-builder.h"
 #include "src/cancelable-task.h"
 #include "src/code-stubs.h"
@@ -437,6 +438,20 @@ class FrameArrayBuilder {
                                           offset, flags);
   }
 
+  void AppendPromiseAllFrame(Handle<Context> context, int offset) {
+    if (full()) return;
+    int flags = FrameArray::kIsAsync | FrameArray::kIsPromiseAll;
+
+    Handle<Context> native_context(context->native_context(), isolate_);
+    Handle<JSFunction> function(native_context->promise_all(), isolate_);
+    if (!IsVisibleInStackTrace(function)) return;
+
+    Handle<Object> receiver(native_context->promise_function(), isolate_);
+    Handle<AbstractCode> code(AbstractCode::cast(function->code()), isolate_);
+    elements_ = FrameArray::AppendJSFrame(elements_, receiver, function, code,
+                                          offset, flags);
+  }
+
   void AppendJavaScriptFrame(
       FrameSummary::JavaScriptFrameSummary const& summary) {
     // Filter out internal frames that we do not want to show.
@@ -666,6 +681,26 @@ void CaptureAsyncStackTrace(Isolate* isolate, Handle<JSPromise> promise,
         promise = handle(JSPromise::cast(async_generator_request->promise()),
                          isolate);
       }
+    } else if (IsBuiltinFunction(isolate, reaction->fulfill_handler(),
+                                 Builtins::kPromiseAllResolveElementClosure)) {
+      Handle<JSFunction> function(JSFunction::cast(reaction->fulfill_handler()),
+                                  isolate);
+      Handle<Context> context(function->context(), isolate);
+
+      // We store the offset of the promise into the {function}'s
+      // hash field for promise resolve element callbacks.
+      int const offset = Smi::ToInt(Smi::cast(function->GetIdentityHash())) - 1;
+      builder->AppendPromiseAllFrame(context, offset);
+
+      // Now peak into the Promise.all() resolve element context to
+      // find the promise capability that's being resolved when all
+      // the concurrent promises resolve.
+      int const index =
+          PromiseBuiltinsAssembler::kPromiseAllResolveElementCapabilitySlot;
+      Handle<PromiseCapability> capability(
+          PromiseCapability::cast(context->get(index)), isolate);
+      if (!capability->promise()->IsJSPromise()) return;
+      promise = handle(JSPromise::cast(capability->promise()), isolate);
     } else {
       // We have some generic promise chain here, so try to
       // continue with the chained promise on the reaction
@@ -682,6 +717,7 @@ void CaptureAsyncStackTrace(Isolate* isolate, Handle<JSPromise> promise,
       } else {
         // Otherwise the {promise_or_capability} must be undefined here.
         CHECK(promise_or_capability->IsUndefined(isolate));
+        return;
       }
     }
   }

src/messages.cc

@@ -307,6 +307,7 @@ void JSStackFrame::FromFrameArray(Isolate* isolate, Handle<FrameArray> array,
   is_constructor_ = (flags & FrameArray::kIsConstructor) != 0;
   is_strict_ = (flags & FrameArray::kIsStrict) != 0;
   is_async_ = (flags & FrameArray::kIsAsync) != 0;
+  is_promise_all_ = (flags & FrameArray::kIsPromiseAll) != 0;
 }
 
 JSStackFrame::JSStackFrame(Isolate* isolate, Handle<Object> receiver,
@@ -608,12 +609,23 @@ MaybeHandle<String> JSStackFrame::ToString() {
 
   const bool is_toplevel = IsToplevel();
   const bool is_async = IsAsync();
+  const bool is_promise_all = IsPromiseAll();
   const bool is_constructor = IsConstructor();
   const bool is_method_call = !(is_toplevel || is_constructor);
 
   if (is_async) {
     builder.AppendCString("async ");
   }
+  if (is_promise_all) {
+    // For `Promise.all(iterable)` frames we interpret the {offset_}
+    // as the element index into `iterable` where the error occurred.
+    builder.AppendCString("Promise.all (index ");
+    Handle<String> index_string = isolate_->factory()->NumberToString(
+        handle(Smi::FromInt(offset_), isolate_), isolate_);
+    builder.AppendString(index_string);
+    builder.AppendCString(")");
+    return builder.Finish();
+  }
   if (is_method_call) {
     AppendMethodCall(isolate_, this, &builder);
   } else if (is_constructor) {

src/messages.h

@@ -73,6 +73,7 @@ class StackFrameBase {
   virtual bool IsToplevel() = 0;
   virtual bool IsEval();
   virtual bool IsAsync() const = 0;
+  virtual bool IsPromiseAll() const = 0;
   virtual bool IsConstructor() = 0;
   virtual bool IsStrict() const = 0;
 
@@ -111,6 +112,7 @@ class JSStackFrame : public StackFrameBase {
   bool IsNative() override;
   bool IsToplevel() override;
   bool IsAsync() const override { return is_async_; }
+  bool IsPromiseAll() const override { return is_promise_all_; }
   bool IsConstructor() override { return is_constructor_; }
   bool IsStrict() const override { return is_strict_; }
 
@@ -130,6 +132,7 @@ class JSStackFrame : public StackFrameBase {
 
   bool is_async_ : 1;
   bool is_constructor_ : 1;
+  bool is_promise_all_ : 1;
   bool is_strict_ : 1;
 
   friend class FrameArrayIterator;
@@ -155,6 +158,7 @@ class WasmStackFrame : public StackFrameBase {
   bool IsNative() override { return false; }
   bool IsToplevel() override { return false; }
   bool IsAsync() const override { return false; }
+  bool IsPromiseAll() const override { return false; }
   bool IsConstructor() override { return false; }
   bool IsStrict() const override { return false; }
   bool IsInterpreted() const { return code_ == nullptr; }

src/objects/frame-array.h

@@ -51,7 +51,8 @@ class FrameArray : public FixedArray {
     kIsStrict = 1 << 3,
     kIsConstructor = 1 << 4,
     kAsmJsAtNumberConversion = 1 << 5,
-    kIsAsync = 1 << 6
+    kIsAsync = 1 << 6,
+    kIsPromiseAll = 1 << 7
   };
 
   static Handle<FrameArray> AppendJSFrame(Handle<FrameArray> in,

test/mjsunit/async-stack-traces-prepare-stacktrace-4.js

@@ -0,0 +1,35 @@
+// Copyright 2018 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: --async-stack-traces
+
+// Check that Error.prepareStackTrace properly exposes async
+// stack frames and special Promise.all() stack frames.
+Error.prepareStackTrace = (e, frames) => {
+  assertEquals(two, frames[0].getFunction());
+  assertEquals(two.name, frames[0].getFunctionName());
+  assertFalse(frames[0].isAsync());
+  assertEquals(Promise.all, frames[1].getFunction());
+  assertTrue(frames[1].isAsync());
+  assertTrue(frames[1].isPromiseAll());
+  assertEquals(one, frames[2].getFunction());
+  assertEquals(one.name, frames[2].getFunctionName());
+  assertTrue(frames[2].isAsync());
+  return frames;
+};
+
+async function one(x) {
+  return await Promise.all([two(x)]);
+}
+
+async function two(x) {
+  try {
+    x = await x;
+    throw new Error();
+  } catch (e) {
+    return e.stack;
+  }
+}
+
+one(1).catch(e => setTimeout(_ => {throw e}, 0));

test/mjsunit/async-stack-traces-promise-all.js

@@ -0,0 +1,38 @@
+// Copyright 2018 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 --async-stack-traces
+
+// Basic test with Promise.all().
+(function() {
+  async function fine() { }
+
+  async function thrower() {
+    await fine();
+    throw new Error();
+  }
+
+  async function driver() {
+    await Promise.all([fine(), fine(), thrower(), thrower()]);
+  }
+
+  async function test(f) {
+    try {
+      await f();
+      assertUnreachable();
+    } catch (e) {
+      assertInstanceof(e, Error);
+      assertMatches(/Error.+at thrower.+at async Promise.all \(index 2\).+at async driver.+at async test/ms, e.stack);
+    }
+  }
+
+  assertPromiseResult((async () => {
+    await test(driver);
+    await test(driver);
+    %OptimizeFunctionOnNextCall(thrower);
+    await test(driver);
+    %OptimizeFunctionOnNextCall(driver);
+    await test(driver);
+  })());
+})();