[v8-extras] Harden resolvePromise() and rejectPromise().

The V8 Extras API provides `resolvePromise()` and `rejectPromise()`
functions that bypass the safety net of the resolve/reject closures
that you get from using the Promise constructor. So it's the
responsibility of the user to make sure that the promises are still
pending. This adds release mode checking and hard aborts to make
sure we catch misuse of these APIs early.

This also turns the DCHECK's in the C++ implementation into actual
CHECK's to make sure we crash hard if the invariants are violated.

Bug: chromium:931949, chromium:931640
Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Change-Id: I98a6f424d2a3cfbb608fed21036caff6e2510ec3
Reviewed-on: https://chromium-review.googlesource.com/c/1472291
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59610}
This commit is contained in:
Benedikt Meurer 2019-02-14 09:26:51 +01:00 committed by Commit Bot
parent ee0e4c33bb
commit aa00ee22f8
3 changed files with 24 additions and 3 deletions

View File

@ -50,6 +50,7 @@ namespace internal {
V(kOperandIsNotAFunction, "Operand is not a function") \
V(kOperandIsNotAGeneratorObject, "Operand is not a generator object") \
V(kOperandIsNotASmi, "Operand is not a smi") \
V(kPromiseAlreadySettled, "Promise already settled") \
V(kReceivedInvalidReturnAddress, "Received invalid return address") \
V(kRegisterDidNotMatchExpectedRoot, "Register did not match expected root") \
V(kRegisterWasClobbered, "Register was clobbered") \

View File

@ -964,9 +964,19 @@ TF_BUILTIN(PromiseInternalReject, PromiseBuiltinsAssembler) {
Node* const promise = Parameter(Descriptor::kPromise);
Node* const reason = Parameter(Descriptor::kReason);
Node* const context = Parameter(Descriptor::kContext);
// Main V8 Extras invariant that {promise} is still "pending" at
// this point, aka that {promise} is not resolved multiple times.
Label if_promise_is_settled(this, Label::kDeferred);
GotoIfNot(IsPromiseStatus(PromiseStatus(promise), v8::Promise::kPending),
&if_promise_is_settled);
// We pass true to trigger the debugger's on exception handler.
Return(CallBuiltin(Builtins::kRejectPromise, context, promise, reason,
TrueConstant()));
BIND(&if_promise_is_settled);
Abort(AbortReason::kPromiseAlreadySettled);
}
// V8 Extras: v8.resolvePromise(promise, resolution)
@ -974,7 +984,17 @@ TF_BUILTIN(PromiseInternalResolve, PromiseBuiltinsAssembler) {
Node* const promise = Parameter(Descriptor::kPromise);
Node* const resolution = Parameter(Descriptor::kResolution);
Node* const context = Parameter(Descriptor::kContext);
// Main V8 Extras invariant that {promise} is still "pending" at
// this point, aka that {promise} is not resolved multiple times.
Label if_promise_is_settled(this, Label::kDeferred);
GotoIfNot(IsPromiseStatus(PromiseStatus(promise), v8::Promise::kPending),
&if_promise_is_settled);
Return(CallBuiltin(Builtins::kResolvePromise, context, promise, resolution));
BIND(&if_promise_is_settled);
Abort(AbortReason::kPromiseAlreadySettled);
}
// ES#sec-promise.prototype.then

View File

@ -5871,7 +5871,7 @@ Handle<Object> JSPromise::Fulfill(Handle<JSPromise> promise,
Isolate* const isolate = promise->GetIsolate();
// 1. Assert: The value of promise.[[PromiseState]] is "pending".
DCHECK_EQ(Promise::kPending, promise->status());
CHECK_EQ(Promise::kPending, promise->status());
// 2. Let reactions be promise.[[PromiseFulfillReactions]].
Handle<Object> reactions(promise->reactions(), isolate);
@ -5899,7 +5899,7 @@ Handle<Object> JSPromise::Reject(Handle<JSPromise> promise,
isolate->factory()->undefined_value());
// 1. Assert: The value of promise.[[PromiseState]] is "pending".
DCHECK_EQ(Promise::kPending, promise->status());
CHECK_EQ(Promise::kPending, promise->status());
// 2. Let reactions be promise.[[PromiseRejectReactions]].
Handle<Object> reactions(promise->reactions(), isolate);
@ -6001,7 +6001,7 @@ Handle<Object> JSPromise::TriggerPromiseReactions(Isolate* isolate,
Handle<Object> reactions,
Handle<Object> argument,
PromiseReaction::Type type) {
DCHECK(reactions->IsSmi() || reactions->IsPromiseReaction());
CHECK(reactions->IsSmi() || reactions->IsPromiseReaction());
// We need to reverse the {reactions} here, since we record them
// on the JSPromise in the reverse order.