Skip to content

Commit d22346d

Browse files
targostargos
committed
deps: fix async await desugaring in V8
This is a backport of https://codereview.chromium.org/2672313003/. The patch did not land in V8 because it was superseded by another one but it is much easier to backport to V8 5.5, was reviewed and passed tests. Original commit message: [async await] Fix async function desugaring Previously we rewrote the return statement in an async function from `return expr;` to `return %ResolvePromise(.promise, expr), .promise`. This can lead to incorrect behavior in the presence of try-finally. This patch stores the `expr` of the return statement in a temporary variable, resolves and returns the promise at the end of the finally block. BUG=v8:5896 PR-URL: #12004 Fixes: #11960 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent bc664cb commit d22346d

File tree

5 files changed

+187
-47
lines changed

5 files changed

+187
-47
lines changed

‎deps/v8/include/v8-version.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
#define V8_MAJOR_VERSION 5
1212
#define V8_MINOR_VERSION 5
1313
#define V8_BUILD_NUMBER 372
14-
#define V8_PATCH_LEVEL 42
14+
#define V8_PATCH_LEVEL 43
1515

1616
// Use 1 for candidates and 0 otherwise.
1717
// (Boolean macro values are not supported by all preprocessors.)

‎deps/v8/src/parsing/parser-base.h‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,14 +411,23 @@ class ParserBase {
411411
}
412412

413413
void set_promise_variable(typename Types::Variable* variable) {
414-
DCHECK(variable != NULL);
414+
DCHECK_NOT_NULL(variable);
415415
DCHECK(IsAsyncFunction(kind()));
416416
promise_variable_ = variable;
417417
}
418418
typename Types::Variable* promise_variable() const {
419419
return promise_variable_;
420420
}
421421

422+
void set_async_return_variable(typename Types::Variable* variable) {
423+
DCHECK_NOT_NULL(variable);
424+
DCHECK(IsAsyncFunction(kind()));
425+
async_return_variable_ = variable;
426+
}
427+
typename Types::Variable* async_return_variable() const {
428+
return async_return_variable_;
429+
}
430+
422431
const ZoneList<DestructuringAssignment>&
423432
destructuring_assignments_to_rewrite() const {
424433
return destructuring_assignments_to_rewrite_;
@@ -488,6 +497,8 @@ class ParserBase {
488497
// For async functions, this variable holds a temporary for the Promise
489498
// being created as output of the async function.
490499
Variable* promise_variable_;
500+
Variable* async_return_variable_;
501+
Variable* is_rejection_variable_;
491502

492503
FunctionState** function_state_stack_;
493504
FunctionState* outer_function_state_;
@@ -1468,6 +1479,8 @@ ParserBase<Impl>::FunctionState::FunctionState(
14681479
expected_property_count_(0),
14691480
generator_object_variable_(nullptr),
14701481
promise_variable_(nullptr),
1482+
async_return_variable_(nullptr),
1483+
is_rejection_variable_(nullptr),
14711484
function_state_stack_(function_state_stack),
14721485
outer_function_state_(*function_state_stack),
14731486
destructuring_assignments_to_rewrite_(16, scope->zone()),

‎deps/v8/src/parsing/parser.cc‎

Lines changed: 153 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1701,10 +1701,17 @@ Expression* Parser::RewriteReturn(Expression* return_value, int pos) {
17011701
return_value = factory()->NewConditional(is_undefined, ThisExpression(pos),
17021702
is_object_conditional, pos);
17031703
}
1704+
17041705
if (is_generator()) {
17051706
return_value = BuildIteratorResult(return_value, true);
17061707
} else if (is_async_function()) {
1707-
return_value = BuildResolvePromise(return_value, return_value->position());
1708+
// In an async function,
1709+
// return expr;
1710+
// is rewritten as
1711+
// return .async_return_value = expr;
1712+
return_value = factory()->NewAssignment(
1713+
Token::ASSIGN, factory()->NewVariableProxy(AsyncReturnVariable()),
1714+
return_value, kNoSourcePosition);
17081715
}
17091716
return return_value;
17101717
}
@@ -2991,72 +2998,168 @@ Block* Parser::BuildParameterInitializationBlock(
29912998
return init_block;
29922999
}
29933000

3001+
Block* Parser::BuildRejectPromiseOnExceptionForParameters(Block* inner_block) {
3002+
// .promise = %AsyncFunctionPromiseCreate();
3003+
// try {
3004+
// <inner_block>
3005+
// } catch (.catch) {
3006+
// return %RejectPromise(.promise, .catch), .promise;
3007+
// }
3008+
Block* result = factory()->NewBlock(nullptr, 2, true, kNoSourcePosition);
3009+
{
3010+
// .promise = %AsyncFunctionPromiseCreate();
3011+
Expression* create_promise = factory()->NewCallRuntime(
3012+
Context::ASYNC_FUNCTION_PROMISE_CREATE_INDEX,
3013+
new (zone()) ZoneList<Expression*>(0, zone()), kNoSourcePosition);
3014+
Assignment* assign_promise = factory()->NewAssignment(
3015+
Token::INIT, factory()->NewVariableProxy(PromiseVariable()),
3016+
create_promise, kNoSourcePosition);
3017+
Statement* set_promise =
3018+
factory()->NewExpressionStatement(assign_promise, kNoSourcePosition);
3019+
result->statements()->Add(set_promise, zone());
3020+
}
3021+
// catch (.catch) {
3022+
// return %RejectPromise(.promise, .catch), .promise;
3023+
// }
3024+
Scope* catch_scope = NewScope(CATCH_SCOPE);
3025+
catch_scope->set_is_hidden();
3026+
Variable* catch_variable =
3027+
catch_scope->DeclareLocal(ast_value_factory()->dot_catch_string(), VAR,
3028+
kCreatedInitialized, NORMAL_VARIABLE);
3029+
Block* catch_block = factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);
3030+
{
3031+
// return %RejectPromise(.promise, .catch), .promise;
3032+
Expression* reject_return_promise = factory()->NewBinaryOperation(
3033+
Token::COMMA, BuildRejectPromise(catch_variable),
3034+
factory()->NewVariableProxy(PromiseVariable(), kNoSourcePosition),
3035+
kNoSourcePosition);
3036+
catch_block->statements()->Add(
3037+
factory()->NewReturnStatement(reject_return_promise, kNoSourcePosition),
3038+
zone());
3039+
}
3040+
TryStatement* try_catch_statement =
3041+
factory()->NewTryCatchStatementForAsyncAwait(inner_block, catch_scope,
3042+
catch_variable, catch_block,
3043+
kNoSourcePosition);
3044+
result->statements()->Add(try_catch_statement, zone());
3045+
return result;
3046+
}
3047+
29943048
Block* Parser::BuildRejectPromiseOnException(Block* inner_block, bool* ok) {
3049+
// .is_rejection = false;
29953050
// .promise = %AsyncFunctionPromiseCreate();
29963051
// try {
29973052
// <inner_block>
29983053
// } catch (.catch) {
2999-
// %RejectPromise(.promise, .catch);
3000-
// return .promise;
3054+
// .is_rejection = true;
3055+
// .async_return_value = .catch;
30013056
// } finally {
3057+
// .is_rejection
3058+
// ? %RejectPromise(.promise, .async_return_value)
3059+
// : %ResolvePromise(.promise, .async_return_value);
30023060
// %AsyncFunctionPromiseRelease(.promise);
3061+
// return .promise;
30033062
// }
3004-
Block* result = factory()->NewBlock(nullptr, 2, true, kNoSourcePosition);
3063+
Block* result = factory()->NewBlock(nullptr, 3, true, kNoSourcePosition);
30053064

3006-
// .promise = %AsyncFunctionPromiseCreate();
3007-
Statement* set_promise;
3065+
Variable* is_rejection_var =
3066+
scope()->NewTemporary(ast_value_factory()->empty_string());
30083067
{
3068+
// .is_rejection = false;
3069+
Assignment* set_is_rejection = factory()->NewAssignment(
3070+
Token::INIT, factory()->NewVariableProxy(is_rejection_var),
3071+
factory()->NewBooleanLiteral(false, kNoSourcePosition),
3072+
kNoSourcePosition);
3073+
result->statements()->Add(
3074+
factory()->NewExpressionStatement(set_is_rejection, kNoSourcePosition),
3075+
zone());
3076+
// .promise = %AsyncFunctionPromiseCreate();
30093077
Expression* create_promise = factory()->NewCallRuntime(
30103078
Context::ASYNC_FUNCTION_PROMISE_CREATE_INDEX,
30113079
new (zone()) ZoneList<Expression*>(0, zone()), kNoSourcePosition);
30123080
Assignment* assign_promise = factory()->NewAssignment(
30133081
Token::INIT, factory()->NewVariableProxy(PromiseVariable()),
30143082
create_promise, kNoSourcePosition);
3015-
set_promise =
3083+
Statement* set_promise =
30163084
factory()->NewExpressionStatement(assign_promise, kNoSourcePosition);
3085+
result->statements()->Add(set_promise, zone());
30173086
}
3018-
result->statements()->Add(set_promise, zone());
30193087

3020-
// catch (.catch) { return %RejectPromise(.promise, .catch), .promise }
3088+
// catch (.catch) {
3089+
// .is_rejection = true;
3090+
// .async_return_value = .catch;
3091+
// }
30213092
Scope* catch_scope = NewScope(CATCH_SCOPE);
30223093
catch_scope->set_is_hidden();
30233094
Variable* catch_variable =
30243095
catch_scope->DeclareLocal(ast_value_factory()->dot_catch_string(), VAR,
30253096
kCreatedInitialized, NORMAL_VARIABLE);
30263097
Block* catch_block = factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);
3027-
3028-
Expression* promise_reject = BuildRejectPromise(
3029-
factory()->NewVariableProxy(catch_variable), kNoSourcePosition);
3030-
ReturnStatement* return_promise_reject =
3031-
factory()->NewReturnStatement(promise_reject, kNoSourcePosition);
3032-
catch_block->statements()->Add(return_promise_reject, zone());
3098+
{
3099+
// .is_rejection = true;
3100+
DCHECK_NOT_NULL(is_rejection_var);
3101+
Assignment* set_is_rejection = factory()->NewAssignment(
3102+
Token::ASSIGN, factory()->NewVariableProxy(is_rejection_var),
3103+
factory()->NewBooleanLiteral(true, kNoSourcePosition),
3104+
kNoSourcePosition);
3105+
catch_block->statements()->Add(
3106+
factory()->NewExpressionStatement(set_is_rejection, kNoSourcePosition),
3107+
zone());
3108+
// .async_return_value = .catch;
3109+
Assignment* set_async_return_var = factory()->NewAssignment(
3110+
Token::ASSIGN, factory()->NewVariableProxy(AsyncReturnVariable()),
3111+
factory()->NewVariableProxy(catch_variable), kNoSourcePosition);
3112+
catch_block->statements()->Add(factory()->NewExpressionStatement(
3113+
set_async_return_var, kNoSourcePosition),
3114+
zone());
3115+
}
30333116

30343117
TryStatement* try_catch_statement =
30353118
factory()->NewTryCatchStatementForAsyncAwait(inner_block, catch_scope,
30363119
catch_variable, catch_block,
30373120
kNoSourcePosition);
3038-
3039-
// There is no TryCatchFinally node, so wrap it in an outer try/finally
30403121
Block* outer_try_block =
30413122
factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);
30423123
outer_try_block->statements()->Add(try_catch_statement, zone());
30433124

3044-
// finally { %AsyncFunctionPromiseRelease(.promise) }
3125+
// finally {
3126+
// .is_rejection
3127+
// ? %RejectPromise(.promise, .async_return_value)
3128+
// : %ResolvePromise(.promise, .async_return_value);
3129+
// %AsyncFunctionPromiseRelease(.promise);
3130+
// return .promise;
3131+
// }
30453132
Block* finally_block =
30463133
factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);
30473134
{
3135+
// .is_rejection
3136+
// ? %RejectPromise(.promise, .async_return_value)
3137+
// : %ResolvePromise(.promise, .async_return_value);
3138+
Expression* resolve_or_reject_promise =
3139+
factory()->NewConditional(factory()->NewVariableProxy(is_rejection_var),
3140+
BuildRejectPromise(AsyncReturnVariable()),
3141+
BuildResolvePromise(), kNoSourcePosition);
3142+
finally_block->statements()->Add(
3143+
factory()->NewExpressionStatement(resolve_or_reject_promise,
3144+
kNoSourcePosition),
3145+
zone());
3146+
// %AsyncFunctionPromiseRelease(.promise);
30483147
ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(1, zone());
30493148
args->Add(factory()->NewVariableProxy(PromiseVariable()), zone());
30503149
Expression* call_promise_release = factory()->NewCallRuntime(
30513150
Context::ASYNC_FUNCTION_PROMISE_RELEASE_INDEX, args, kNoSourcePosition);
30523151
Statement* promise_release = factory()->NewExpressionStatement(
30533152
call_promise_release, kNoSourcePosition);
30543153
finally_block->statements()->Add(promise_release, zone());
3154+
3155+
// return .promise;
3156+
Statement* return_promise = factory()->NewReturnStatement(
3157+
factory()->NewVariableProxy(PromiseVariable()), kNoSourcePosition);
3158+
finally_block->statements()->Add(return_promise, zone());
30553159
}
30563160

30573161
Statement* try_finally_statement = factory()->NewTryFinallyStatement(
30583162
outer_try_block, finally_block, kNoSourcePosition);
3059-
30603163
result->statements()->Add(try_finally_statement, zone());
30613164
return result;
30623165
}
@@ -3072,31 +3175,25 @@ Expression* Parser::BuildCreateJSGeneratorObject(int pos, FunctionKind kind) {
30723175
pos);
30733176
}
30743177

3075-
Expression* Parser::BuildResolvePromise(Expression* value, int pos) {
3076-
// %ResolvePromise(.promise, value), .promise
3178+
Expression* Parser::BuildResolvePromise() {
3179+
// %ResolvePromise(.promise, .async_return_variable), .promise
30773180
ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(2, zone());
30783181
args->Add(factory()->NewVariableProxy(PromiseVariable()), zone());
3079-
args->Add(value, zone());
3080-
Expression* call_runtime =
3081-
factory()->NewCallRuntime(Context::PROMISE_RESOLVE_INDEX, args, pos);
3082-
return factory()->NewBinaryOperation(
3083-
Token::COMMA, call_runtime,
3084-
factory()->NewVariableProxy(PromiseVariable()), pos);
3182+
args->Add(factory()->NewVariableProxy(AsyncReturnVariable()), zone());
3183+
return factory()->NewCallRuntime(Context::PROMISE_RESOLVE_INDEX, args,
3184+
kNoSourcePosition);
30853185
}
30863186

3087-
Expression* Parser::BuildRejectPromise(Expression* value, int pos) {
3088-
// %RejectPromiseNoDebugEvent(.promise, value, true), .promise
3187+
Expression* Parser::BuildRejectPromise(Variable* value) {
3188+
// %RejectPromiseNoDebugEvent(.promise, .value, true)
30893189
// The NoDebugEvent variant disables the additional debug event for the
30903190
// rejection since a debug event already happened for the exception that got
30913191
// us here.
30923192
ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(2, zone());
30933193
args->Add(factory()->NewVariableProxy(PromiseVariable()), zone());
3094-
args->Add(value, zone());
3095-
Expression* call_runtime = factory()->NewCallRuntime(
3096-
Context::REJECT_PROMISE_NO_DEBUG_EVENT_INDEX, args, pos);
3097-
return factory()->NewBinaryOperation(
3098-
Token::COMMA, call_runtime,
3099-
factory()->NewVariableProxy(PromiseVariable()), pos);
3194+
args->Add(factory()->NewVariableProxy(value), zone());
3195+
return factory()->NewCallRuntime(
3196+
Context::REJECT_PROMISE_NO_DEBUG_EVENT_INDEX, args, kNoSourcePosition);
31003197
}
31013198

31023199
Variable* Parser::PromiseVariable() {
@@ -3111,6 +3208,19 @@ Variable* Parser::PromiseVariable() {
31113208
return promise;
31123209
}
31133210

3211+
Variable* Parser::AsyncReturnVariable() {
3212+
// Based on the various compilation paths, there are many different
3213+
// code paths which may be the first to access the return value
3214+
// temporary. Whichever comes first should create it and stash it in
3215+
// the FunctionState.
3216+
Variable* async_return = function_state_->async_return_variable();
3217+
if (async_return == nullptr) {
3218+
async_return = scope()->NewTemporary(ast_value_factory()->empty_string());
3219+
function_state_->set_async_return_variable(async_return);
3220+
}
3221+
return async_return;
3222+
}
3223+
31143224
Expression* Parser::BuildInitialYield(int pos, FunctionKind kind) {
31153225
Expression* allocation = BuildCreateJSGeneratorObject(pos, kind);
31163226
VariableProxy* init_proxy =
@@ -3235,7 +3345,7 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
32353345

32363346
// TODO(littledan): Merge the two rejection blocks into one
32373347
if (IsAsyncFunction(kind)) {
3238-
init_block = BuildRejectPromiseOnException(init_block, CHECK_OK);
3348+
init_block = BuildRejectPromiseOnExceptionForParameters(init_block);
32393349
}
32403350

32413351
DCHECK_NOT_NULL(init_block);
@@ -4176,14 +4286,16 @@ void Parser::RewriteAsyncFunctionBody(ZoneList<Statement*>* body, Block* block,
41764286
// .generator_object = %CreateGeneratorObject();
41774287
// BuildRejectPromiseOnException({
41784288
// ... block ...
4179-
// return %ResolvePromise(.promise, expr), .promise;
4289+
// .async_return_var = expr;
41804290
// })
41814291
// }
41824292

4183-
return_value = BuildResolvePromise(return_value, return_value->position());
4184-
block->statements()->Add(
4185-
factory()->NewReturnStatement(return_value, return_value->position()),
4186-
zone());
4293+
Assignment* set_async_return_var = factory()->NewAssignment(
4294+
Token::ASSIGN, factory()->NewVariableProxy(AsyncReturnVariable()),
4295+
return_value, kNoSourcePosition);
4296+
block->statements()->Add(factory()->NewExpressionStatement(
4297+
set_async_return_var, kNoSourcePosition),
4298+
zone());
41874299
block = BuildRejectPromiseOnException(block, CHECK_OK_VOID);
41884300
body->Add(block, zone());
41894301
}

‎deps/v8/src/parsing/parser.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@ class Parser : public ParserBase<Parser> {
491491
Block* BuildParameterInitializationBlock(
492492
const ParserFormalParameters& parameters, bool* ok);
493493
Block* BuildRejectPromiseOnException(Block* block, bool* ok);
494+
Block* BuildRejectPromiseOnExceptionForParameters(Block* block);
494495

495496
// Consumes the ending }.
496497
ZoneList<Statement*>* ParseEagerFunctionBody(
@@ -576,9 +577,10 @@ class Parser : public ParserBase<Parser> {
576577

577578
Expression* BuildInitialYield(int pos, FunctionKind kind);
578579
Expression* BuildCreateJSGeneratorObject(int pos, FunctionKind kind);
579-
Expression* BuildResolvePromise(Expression* value, int pos);
580-
Expression* BuildRejectPromise(Expression* value, int pos);
580+
Expression* BuildResolvePromise();
581+
Expression* BuildRejectPromise(Variable* value);
581582
Variable* PromiseVariable();
583+
Variable* AsyncReturnVariable();
582584

583585
// Generic AST generator for throwing errors from compiled code.
584586
Expression* NewThrowError(Runtime::FunctionId function_id,
@@ -983,8 +985,7 @@ class Parser : public ParserBase<Parser> {
983985
auto* init_block = BuildParameterInitializationBlock(parameters, ok);
984986
if (!*ok) return;
985987
if (is_async) {
986-
init_block = BuildRejectPromiseOnException(init_block, ok);
987-
if (!*ok) return;
988+
init_block = BuildRejectPromiseOnExceptionForParameters(init_block);
988989
}
989990
if (init_block != nullptr) body->Add(init_block, zone());
990991
}

‎deps/v8/test/mjsunit/regress/regress-5896.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2017 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
// Flags: --allow-natives-syntax
5+
async function foo() {
6+
try {
7+
return 1;
8+
} finally {
9+
return Promise.resolve().then(() => { return 2; });
10+
}
11+
}
12+
foo()
13+
.then(value => { found = value; assertEquals(2, value) })
14+
.catch((e) => { %AbortJS(''+e); });

0 commit comments

Comments
 (0)