Commit c75a02c5 authored by Shu-yu Guo's avatar Shu-yu Guo Committed by Commit Bot

Fix completion value for finally blocks

Finally blocks that unconditionally result in an abrupt completion
immediately are currently incorrectly returning the existing completion
value instead of undefined.

Bug: v8:10978
Change-Id: Ida2e27d9cc9711236a1fb30368bfc7213d0f7140
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2473382Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70619}
parent 81ba8d1e
......@@ -246,23 +246,40 @@ void Processor::VisitTryFinallyStatement(TryFinallyStatement* node) {
is_set_ = true;
Visit(node->finally_block());
node->set_finally_block(replacement_->AsBlock());
// Save .result value at the beginning of the finally block and restore it
// at the end again: ".backup = .result; ...; .result = .backup"
// This is necessary because the finally block does not normally contribute
// to the completion value.
CHECK_NOT_NULL(closure_scope());
Variable* backup = closure_scope()->NewTemporary(
factory()->ast_value_factory()->dot_result_string());
Expression* backup_proxy = factory()->NewVariableProxy(backup);
Expression* result_proxy = factory()->NewVariableProxy(result_);
Expression* save = factory()->NewAssignment(
Token::ASSIGN, backup_proxy, result_proxy, kNoSourcePosition);
Expression* restore = factory()->NewAssignment(
Token::ASSIGN, result_proxy, backup_proxy, kNoSourcePosition);
node->finally_block()->statements()->InsertAt(
0, factory()->NewExpressionStatement(save, kNoSourcePosition), zone());
node->finally_block()->statements()->Add(
factory()->NewExpressionStatement(restore, kNoSourcePosition), zone());
if (is_set_) {
// Save .result value at the beginning of the finally block and restore it
// at the end again: ".backup = .result; ...; .result = .backup" This is
// necessary because the finally block does not normally contribute to the
// completion value.
Variable* backup = closure_scope()->NewTemporary(
factory()->ast_value_factory()->dot_result_string());
Expression* backup_proxy = factory()->NewVariableProxy(backup);
Expression* result_proxy = factory()->NewVariableProxy(result_);
Expression* save = factory()->NewAssignment(
Token::ASSIGN, backup_proxy, result_proxy, kNoSourcePosition);
Expression* restore = factory()->NewAssignment(
Token::ASSIGN, result_proxy, backup_proxy, kNoSourcePosition);
node->finally_block()->statements()->InsertAt(
0, factory()->NewExpressionStatement(save, kNoSourcePosition),
zone());
node->finally_block()->statements()->Add(
factory()->NewExpressionStatement(restore, kNoSourcePosition),
zone());
} else {
// If is_set_ is false, it means the finally block has a 'break' or a
// 'continue' and was not preceded by a statement that assigned to
// .result. Try-finally statements return the abrupt completions from the
// finally block, meaning this case should get an undefined.
//
// Since the finally block will definitely result in an abrupt completion,
// there's no need to save and restore the .result.
Expression* undef = factory()->NewUndefinedLiteral(kNoSourcePosition);
Expression* assignment = SetResult(undef);
node->finally_block()->statements()->InsertAt(
0, factory()->NewExpressionStatement(assignment, kNoSourcePosition),
zone());
}
// We can't tell whether the finally-block is guaranteed to set .result, so
// reset is_set_ before visiting the try-block.
is_set_ = false;
......
......@@ -683,9 +683,6 @@
'language/statements/class/elements/arrow-body-indirect-eval-err-contains-arguments': [FAIL],
'language/statements/class/elements/nested-indirect-eval-err-contains-arguments': [FAIL],
# http://crbug/v8/10978
'language/statements/try/completion-values': [FAIL],
######################## NEEDS INVESTIGATION ###########################
# https://bugs.chromium.org/p/v8/issues/detail?id=7833
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment