summaryrefslogtreecommitdiff
path: root/deps
diff options
context:
space:
mode:
authorMichaël Zasso <targos@protonmail.com>2018-07-16 22:03:01 +0200
committerMichaël Zasso <targos@protonmail.com>2018-07-26 08:34:33 +0200
commit68aa129ac1989d9dbb62c408ea4d478d3a7fc20e (patch)
tree4010b76a5260f4841537320902aef4083bc91106 /deps
parent728078074439996d9f2d0f53e5fce12152540926 (diff)
downloadandroid-node-v8-68aa129ac1989d9dbb62c408ea4d478d3a7fc20e.tar.gz
android-node-v8-68aa129ac1989d9dbb62c408ea4d478d3a7fc20e.tar.bz2
android-node-v8-68aa129ac1989d9dbb62c408ea4d478d3a7fc20e.zip
deps: cherry-pick 907d7bc from upstream V8
Original commit message: [promise] Implement Swallowed Rejection Hook. This extends the current Promise Rejection Hook with two new events kPromiseRejectAfterResolved kPromiseResolveAfterResolved which are used to detect (and signal) misuse of the Promise constructor. Specifically the common bug like new Promise((res, rej) => { res(1); throw new Error("something") }); where the error is silently swallowed by the Promise constructor without the user ever noticing can be caught via this hook. Doc: https://goo.gl/2stLUY Bug: v8:7919 Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng Change-Id: I890a7e766cdd1be88db94844fb744f72823dba33 Reviewed-on: https://chromium-review.googlesource.com/1126099 Reviewed-by: Maya Lekova <mslekova@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#54309} Refs: https://github.com/v8/v8/commit/907d7bcd18c13a04a14eea6699e54167494bf9f9 PR-URL: https://github.com/nodejs/node/pull/21838 Refs: https://github.com/nodejs/promises-debugging/issues/8 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yang Guo <yangguo@chromium.org> Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Diffstat (limited to 'deps')
-rw-r--r--deps/v8/include/v8.h4
-rw-r--r--deps/v8/src/builtins/builtins-promise-gen.cc34
-rw-r--r--deps/v8/src/builtins/builtins-promise-gen.h7
-rw-r--r--deps/v8/src/compiler/js-call-reducer.cc4
-rw-r--r--deps/v8/src/isolate.cc3
-rw-r--r--deps/v8/src/runtime/runtime-promise.cc20
-rw-r--r--deps/v8/src/runtime/runtime.h4
-rw-r--r--deps/v8/test/cctest/test-api.cc116
8 files changed, 148 insertions, 44 deletions
diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h
index 6c56f918e3..2bacd1a480 100644
--- a/deps/v8/include/v8.h
+++ b/deps/v8/include/v8.h
@@ -6520,7 +6520,9 @@ typedef void (*PromiseHook)(PromiseHookType type, Local<Promise> promise,
// --- Promise Reject Callback ---
enum PromiseRejectEvent {
kPromiseRejectWithNoHandler = 0,
- kPromiseHandlerAddedAfterReject = 1
+ kPromiseHandlerAddedAfterReject = 1,
+ kPromiseRejectAfterResolved = 2,
+ kPromiseResolveAfterResolved = 3,
};
class PromiseRejectMessage {
diff --git a/deps/v8/src/builtins/builtins-promise-gen.cc b/deps/v8/src/builtins/builtins-promise-gen.cc
index 6dcf9f5372..aef9e40ac8 100644
--- a/deps/v8/src/builtins/builtins-promise-gen.cc
+++ b/deps/v8/src/builtins/builtins-promise-gen.cc
@@ -246,6 +246,8 @@ Node* PromiseBuiltinsAssembler::CreatePromiseResolvingFunctionsContext(
Node* const context =
CreatePromiseContext(native_context, kPromiseContextLength);
StoreContextElementNoWriteBarrier(context, kPromiseSlot, promise);
+ StoreContextElementNoWriteBarrier(context, kAlreadyResolvedSlot,
+ FalseConstant());
StoreContextElementNoWriteBarrier(context, kDebugEventSlot, debug_event);
return context;
}
@@ -736,17 +738,27 @@ TF_BUILTIN(PromiseCapabilityDefaultReject, PromiseBuiltinsAssembler) {
Node* const promise = LoadContextElement(context, kPromiseSlot);
// 3. Let alreadyResolved be F.[[AlreadyResolved]].
+ Label if_already_resolved(this, Label::kDeferred);
+ Node* const already_resolved =
+ LoadContextElement(context, kAlreadyResolvedSlot);
+
// 4. If alreadyResolved.[[Value]] is true, return undefined.
- // We use undefined as a marker for the [[AlreadyResolved]] state.
- ReturnIf(IsUndefined(promise), UndefinedConstant());
+ GotoIf(IsTrue(already_resolved), &if_already_resolved);
// 5. Set alreadyResolved.[[Value]] to true.
- StoreContextElementNoWriteBarrier(context, kPromiseSlot, UndefinedConstant());
+ StoreContextElementNoWriteBarrier(context, kAlreadyResolvedSlot,
+ TrueConstant());
// 6. Return RejectPromise(promise, reason).
Node* const debug_event = LoadContextElement(context, kDebugEventSlot);
Return(CallBuiltin(Builtins::kRejectPromise, context, promise, reason,
debug_event));
+
+ BIND(&if_already_resolved);
+ {
+ Return(CallRuntime(Runtime::kPromiseRejectAfterResolved, context, promise,
+ reason));
+ }
}
// ES #sec-promise-resolve-functions
@@ -758,16 +770,26 @@ TF_BUILTIN(PromiseCapabilityDefaultResolve, PromiseBuiltinsAssembler) {
Node* const promise = LoadContextElement(context, kPromiseSlot);
// 3. Let alreadyResolved be F.[[AlreadyResolved]].
+ Label if_already_resolved(this, Label::kDeferred);
+ Node* const already_resolved =
+ LoadContextElement(context, kAlreadyResolvedSlot);
+
// 4. If alreadyResolved.[[Value]] is true, return undefined.
- // We use undefined as a marker for the [[AlreadyResolved]] state.
- ReturnIf(IsUndefined(promise), UndefinedConstant());
+ GotoIf(IsTrue(already_resolved), &if_already_resolved);
// 5. Set alreadyResolved.[[Value]] to true.
- StoreContextElementNoWriteBarrier(context, kPromiseSlot, UndefinedConstant());
+ StoreContextElementNoWriteBarrier(context, kAlreadyResolvedSlot,
+ TrueConstant());
// The rest of the logic (and the catch prediction) is
// encapsulated in the dedicated ResolvePromise builtin.
Return(CallBuiltin(Builtins::kResolvePromise, context, promise, resolution));
+
+ BIND(&if_already_resolved);
+ {
+ Return(CallRuntime(Runtime::kPromiseResolveAfterResolved, context, promise,
+ resolution));
+ }
}
TF_BUILTIN(PromiseConstructorLazyDeoptContinuation, PromiseBuiltinsAssembler) {
diff --git a/deps/v8/src/builtins/builtins-promise-gen.h b/deps/v8/src/builtins/builtins-promise-gen.h
index 46fb9338b4..4954b383fe 100644
--- a/deps/v8/src/builtins/builtins-promise-gen.h
+++ b/deps/v8/src/builtins/builtins-promise-gen.h
@@ -17,11 +17,12 @@ typedef compiler::CodeAssemblerState CodeAssemblerState;
class PromiseBuiltinsAssembler : public CodeStubAssembler {
public:
enum PromiseResolvingFunctionContextSlot {
- // The promise which resolve/reject callbacks fulfill. If this is
- // undefined, then we've already visited this callback and it
- // should be a no-op.
+ // The promise which resolve/reject callbacks fulfill.
kPromiseSlot = Context::MIN_CONTEXT_SLOTS,
+ // Whether the callback was already invoked.
+ kAlreadyResolvedSlot,
+
// Whether to trigger a debug event or not. Used in catch
// prediction.
kDebugEventSlot,
diff --git a/deps/v8/src/compiler/js-call-reducer.cc b/deps/v8/src/compiler/js-call-reducer.cc
index 45664593c5..1bf59adb4a 100644
--- a/deps/v8/src/compiler/js-call-reducer.cc
+++ b/deps/v8/src/compiler/js-call-reducer.cc
@@ -5408,6 +5408,10 @@ Reduction JSCallReducer::ReducePromiseConstructor(Node* node) {
promise_context, promise, effect, control);
effect = graph()->NewNode(
simplified()->StoreField(AccessBuilder::ForContextSlot(
+ PromiseBuiltinsAssembler::kAlreadyResolvedSlot)),
+ promise_context, jsgraph()->FalseConstant(), effect, control);
+ effect = graph()->NewNode(
+ simplified()->StoreField(AccessBuilder::ForContextSlot(
PromiseBuiltinsAssembler::kDebugEventSlot)),
promise_context, jsgraph()->TrueConstant(), effect, control);
diff --git a/deps/v8/src/isolate.cc b/deps/v8/src/isolate.cc
index dd66479f73..0e457df4d1 100644
--- a/deps/v8/src/isolate.cc
+++ b/deps/v8/src/isolate.cc
@@ -3872,10 +3872,9 @@ void Isolate::SetPromiseRejectCallback(PromiseRejectCallback callback) {
void Isolate::ReportPromiseReject(Handle<JSPromise> promise,
Handle<Object> value,
v8::PromiseRejectEvent event) {
- DCHECK_EQ(v8::Promise::kRejected, promise->status());
if (promise_reject_callback_ == nullptr) return;
Handle<FixedArray> stack_trace;
- if (event == v8::kPromiseRejectWithNoHandler && value->IsJSObject()) {
+ if (event != v8::kPromiseHandlerAddedAfterReject && value->IsJSObject()) {
stack_trace = GetDetailedStackTrace(Handle<JSObject>::cast(value));
}
promise_reject_callback_(v8::PromiseRejectMessage(
diff --git a/deps/v8/src/runtime/runtime-promise.cc b/deps/v8/src/runtime/runtime-promise.cc
index f5b9db3c02..6c4f7d69eb 100644
--- a/deps/v8/src/runtime/runtime-promise.cc
+++ b/deps/v8/src/runtime/runtime-promise.cc
@@ -38,6 +38,26 @@ RUNTIME_FUNCTION(Runtime_PromiseRejectEventFromStack) {
return isolate->heap()->undefined_value();
}
+RUNTIME_FUNCTION(Runtime_PromiseRejectAfterResolved) {
+ DCHECK_EQ(2, args.length());
+ HandleScope scope(isolate);
+ CONVERT_ARG_HANDLE_CHECKED(JSPromise, promise, 0);
+ CONVERT_ARG_HANDLE_CHECKED(Object, reason, 1);
+ isolate->ReportPromiseReject(promise, reason,
+ v8::kPromiseRejectAfterResolved);
+ return isolate->heap()->undefined_value();
+}
+
+RUNTIME_FUNCTION(Runtime_PromiseResolveAfterResolved) {
+ DCHECK_EQ(2, args.length());
+ HandleScope scope(isolate);
+ CONVERT_ARG_HANDLE_CHECKED(JSPromise, promise, 0);
+ CONVERT_ARG_HANDLE_CHECKED(Object, resolution, 1);
+ isolate->ReportPromiseReject(promise, resolution,
+ v8::kPromiseResolveAfterResolved);
+ return isolate->heap()->undefined_value();
+}
+
RUNTIME_FUNCTION(Runtime_PromiseRevokeReject) {
DCHECK_EQ(1, args.length());
HandleScope scope(isolate);
diff --git a/deps/v8/src/runtime/runtime.h b/deps/v8/src/runtime/runtime.h
index f8997c50ad..382bac7d2a 100644
--- a/deps/v8/src/runtime/runtime.h
+++ b/deps/v8/src/runtime/runtime.h
@@ -432,7 +432,9 @@ namespace internal {
F(PromiseRevokeReject, 1, 1) \
F(PromiseStatus, 1, 1) \
F(RejectPromise, 3, 1) \
- F(ResolvePromise, 2, 1)
+ F(ResolvePromise, 2, 1) \
+ F(PromiseRejectAfterResolved, 2, 1) \
+ F(PromiseResolveAfterResolved, 2, 1)
#define FOR_EACH_INTRINSIC_PROXY(F) \
F(CheckProxyGetSetTrapResult, 2, 1) \
diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc
index 953648fe42..366b940d61 100644
--- a/deps/v8/test/cctest/test-api.cc
+++ b/deps/v8/test/cctest/test-api.cc
@@ -17700,6 +17700,8 @@ TEST(RethrowBogusErrorStackTrace) {
v8::PromiseRejectEvent reject_event = v8::kPromiseRejectWithNoHandler;
int promise_reject_counter = 0;
int promise_revoke_counter = 0;
+int promise_reject_after_resolved_counter = 0;
+int promise_resolve_after_resolved_counter = 0;
int promise_reject_msg_line_number = -1;
int promise_reject_msg_column_number = -1;
int promise_reject_line_number = -1;
@@ -17709,40 +17711,56 @@ int promise_reject_frame_count = -1;
void PromiseRejectCallback(v8::PromiseRejectMessage reject_message) {
v8::Local<v8::Object> global = CcTest::global();
v8::Local<v8::Context> context = CcTest::isolate()->GetCurrentContext();
- CHECK_EQ(v8::Promise::PromiseState::kRejected,
+ CHECK_NE(v8::Promise::PromiseState::kPending,
reject_message.GetPromise()->State());
- if (reject_message.GetEvent() == v8::kPromiseRejectWithNoHandler) {
- promise_reject_counter++;
- global->Set(context, v8_str("rejected"), reject_message.GetPromise())
- .FromJust();
- global->Set(context, v8_str("value"), reject_message.GetValue()).FromJust();
- v8::Local<v8::Message> message = v8::Exception::CreateMessage(
- CcTest::isolate(), reject_message.GetValue());
- v8::Local<v8::StackTrace> stack_trace = message->GetStackTrace();
-
- promise_reject_msg_line_number = message->GetLineNumber(context).FromJust();
- promise_reject_msg_column_number =
- message->GetStartColumn(context).FromJust() + 1;
-
- if (!stack_trace.IsEmpty()) {
- promise_reject_frame_count = stack_trace->GetFrameCount();
- if (promise_reject_frame_count > 0) {
- CHECK(stack_trace->GetFrame(0)
- ->GetScriptName()
- ->Equals(context, v8_str("pro"))
- .FromJust());
- promise_reject_line_number = stack_trace->GetFrame(0)->GetLineNumber();
- promise_reject_column_number = stack_trace->GetFrame(0)->GetColumn();
- } else {
- promise_reject_line_number = -1;
- promise_reject_column_number = -1;
+ switch (reject_message.GetEvent()) {
+ case v8::kPromiseRejectWithNoHandler: {
+ promise_reject_counter++;
+ global->Set(context, v8_str("rejected"), reject_message.GetPromise())
+ .FromJust();
+ global->Set(context, v8_str("value"), reject_message.GetValue())
+ .FromJust();
+ v8::Local<v8::Message> message = v8::Exception::CreateMessage(
+ CcTest::isolate(), reject_message.GetValue());
+ v8::Local<v8::StackTrace> stack_trace = message->GetStackTrace();
+
+ promise_reject_msg_line_number =
+ message->GetLineNumber(context).FromJust();
+ promise_reject_msg_column_number =
+ message->GetStartColumn(context).FromJust() + 1;
+
+ if (!stack_trace.IsEmpty()) {
+ promise_reject_frame_count = stack_trace->GetFrameCount();
+ if (promise_reject_frame_count > 0) {
+ CHECK(stack_trace->GetFrame(0)
+ ->GetScriptName()
+ ->Equals(context, v8_str("pro"))
+ .FromJust());
+ promise_reject_line_number =
+ stack_trace->GetFrame(0)->GetLineNumber();
+ promise_reject_column_number = stack_trace->GetFrame(0)->GetColumn();
+ } else {
+ promise_reject_line_number = -1;
+ promise_reject_column_number = -1;
+ }
}
+ break;
+ }
+ case v8::kPromiseHandlerAddedAfterReject: {
+ promise_revoke_counter++;
+ global->Set(context, v8_str("revoked"), reject_message.GetPromise())
+ .FromJust();
+ CHECK(reject_message.GetValue().IsEmpty());
+ break;
+ }
+ case v8::kPromiseRejectAfterResolved: {
+ promise_reject_after_resolved_counter++;
+ break;
+ }
+ case v8::kPromiseResolveAfterResolved: {
+ promise_resolve_after_resolved_counter++;
+ break;
}
- } else {
- promise_revoke_counter++;
- global->Set(context, v8_str("revoked"), reject_message.GetPromise())
- .FromJust();
- CHECK(reject_message.GetValue().IsEmpty());
}
}
@@ -17765,6 +17783,8 @@ v8::Local<v8::Value> RejectValue() {
void ResetPromiseStates() {
promise_reject_counter = 0;
promise_revoke_counter = 0;
+ promise_reject_after_resolved_counter = 0;
+ promise_resolve_after_resolved_counter = 0;
promise_reject_msg_line_number = -1;
promise_reject_msg_column_number = -1;
promise_reject_line_number = -1;
@@ -17990,6 +18010,40 @@ TEST(PromiseRejectCallback) {
CHECK_EQ(0, promise_revoke_counter);
CHECK(RejectValue()->Equals(env.local(), v8_str("sss")).FromJust());
+ ResetPromiseStates();
+
+ // Swallowed exceptions in the Promise constructor.
+ CompileRun(
+ "var v0 = new Promise(\n"
+ " function(res, rej) {\n"
+ " res(1);\n"
+ " throw new Error();\n"
+ " }\n"
+ ");\n");
+ CHECK(!GetPromise("v0")->HasHandler());
+ CHECK_EQ(0, promise_reject_counter);
+ CHECK_EQ(0, promise_revoke_counter);
+ CHECK_EQ(1, promise_reject_after_resolved_counter);
+ CHECK_EQ(0, promise_resolve_after_resolved_counter);
+
+ ResetPromiseStates();
+
+ // Duplication resolve.
+ CompileRun(
+ "var r;\n"
+ "var y0 = new Promise(\n"
+ " function(res, rej) {\n"
+ " r = res;\n"
+ " throw new Error();\n"
+ " }\n"
+ ");\n"
+ "r(1);\n");
+ CHECK(!GetPromise("y0")->HasHandler());
+ CHECK_EQ(1, promise_reject_counter);
+ CHECK_EQ(0, promise_revoke_counter);
+ CHECK_EQ(0, promise_reject_after_resolved_counter);
+ CHECK_EQ(1, promise_resolve_after_resolved_counter);
+
// Test stack frames.
env->GetIsolate()->SetCaptureStackTraceForUncaughtExceptions(true);