- Revision
- 272243
- Author
- [email protected]
- Date
- 2021-02-02 15:24:48 -0800 (Tue, 02 Feb 2021)
Log Message
Completion value of a finally block should not be ignored if completion is abrupt
https://bugs.webkit.org/show_bug.cgi?id=221238
Reviewed by Yusuke Suzuki.
JSTests:
* stress/completion-value.js:
Correct tests to match spec.
* test262/expectations.yaml:
Mark two test cases as passing.
Source/_javascript_Core:
Per https://tc39.es/ecma262/#sec-try-statement-runtime-semantics-evaluation,
the completion value of a finally block is ignored *just* when it is a normal completion,
but we've been ignoring it in all cases.
This patch ensures that when a finally block is exited with a break or continue statement,
its completion value is used as the completion value for the entire TryStatement.
(Note: This behavior will be important for the upcoming "do expressions" proposal,
as it is effectively a reification of completion values.)
* bytecompiler/NodesCodegen.cpp:
(JSC::TryNode::emitBytecode):
1. Don't throw away the result of evaluating the finally block.
2. Only use the try or catch block result if we make it all the way to the end of the finally block.
Modified Paths
Diff
Modified: trunk/JSTests/ChangeLog (272242 => 272243)
--- trunk/JSTests/ChangeLog 2021-02-02 23:23:33 UTC (rev 272242)
+++ trunk/JSTests/ChangeLog 2021-02-02 23:24:48 UTC (rev 272243)
@@ -1,3 +1,16 @@
+2021-02-02 Ross Kirsling <[email protected]>
+
+ Completion value of a finally block should not be ignored if completion is abrupt
+ https://bugs.webkit.org/show_bug.cgi?id=221238
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/completion-value.js:
+ Correct tests to match spec.
+
+ * test262/expectations.yaml:
+ Mark two test cases as passing.
+
2021-02-01 Yusuke Suzuki <[email protected]>
[JSC] TypedArray#fill should be implemented in C++
Modified: trunk/JSTests/stress/completion-value.js (272242 => 272243)
--- trunk/JSTests/stress/completion-value.js 2021-02-02 23:23:33 UTC (rev 272242)
+++ trunk/JSTests/stress/completion-value.js 2021-02-02 23:24:48 UTC (rev 272243)
@@ -206,8 +206,8 @@
shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; break; }; } while (false);`), 42);
shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { break; } finally {-2}; } while (false);`), undefined);
shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; break; } finally {-2}; } while (false);`), 42);
-shouldBe(eval(`99; do { -99; try { 42 } catch (e) { -1 } finally { -2; break; -3 }; } while (false);`), 42);
-shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; } finally { -2; break; -3 }; } while (false);`), 42);
+shouldBe(eval(`99; do { -99; try { 42 } catch (e) { -1 } finally { -2; break; -3 }; } while (false);`), -2);
+shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; } finally { -2; break; -3 }; } while (false);`), -2);
// Break Statement where it is not normally available with other surrounding statements.
shouldBe(eval(`99; do { -99; if (true) { break; }; -77 } while (false);`), undefined);
@@ -224,8 +224,8 @@
shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; break; }; -77 } while (false);`), 42);
shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { break; } finally {-2}; -77 } while (false);`), undefined);
shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; break; } finally {-2}; -77 } while (false);`), 42);
-shouldBe(eval(`99; do { -99; try { 42 } catch (e) { -1 } finally { -2; break; -3 }; -77 } while (false);`), 42);
-shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; } finally { -2; break; -3 }; -77 } while (false);`), 42);
+shouldBe(eval(`99; do { -99; try { 42 } catch (e) { -1 } finally { -2; break; -3 }; -77 } while (false);`), -2);
+shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; } finally { -2; break; -3 }; -77 } while (false);`), -2);
// Continue Statement where it is not normally available.
shouldBe(eval(`99; do { -99; if (true) { continue; }; } while (false);`), undefined);
@@ -242,8 +242,8 @@
shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; continue; }; } while (false);`), 42);
shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { continue; } finally {-2}; } while (false);`), undefined);
shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; continue; } finally {-2}; } while (false);`), 42);
-shouldBe(eval(`99; do { -99; try { 42 } catch (e) { -1 } finally { -2; continue; -3 }; } while (false);`), 42);
-shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; } finally { -2; continue; -3 }; } while (false);`), 42);
+shouldBe(eval(`99; do { -99; try { 42 } catch (e) { -1 } finally { -2; continue; -3 }; } while (false);`), -2);
+shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; } finally { -2; continue; -3 }; } while (false);`), -2);
// Continue Statement where it is not normally available with other surrounding statements.
shouldBe(eval(`99; do { -99; if (true) { continue; }; -77 } while (false);`), undefined);
@@ -260,8 +260,8 @@
shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; continue; }; -77 } while (false);`), 42);
shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { continue; } finally {-2}; -77 } while (false);`), undefined);
shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; continue; } finally {-2}; -77 } while (false);`), 42);
-shouldBe(eval(`99; do { -99; try { 42 } catch (e) { -1 } finally { -2; continue; -3 }; -77 } while (false);`), 42);
-shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; } finally { -2; continue; -3 }; -77 } while (false);`), 42);
+shouldBe(eval(`99; do { -99; try { 42 } catch (e) { -1 } finally { -2; continue; -3 }; -77 } while (false);`), -2);
+shouldBe(eval(`99; do { -99; try { [].x.x } catch (e) { 42; } finally { -2; continue; -3 }; -77 } while (false);`), -2);
// Early break to a label.
shouldBe(eval(`99; label: do { 1; if (true) { break label; 2; }; } while (false);`), undefined);
Modified: trunk/JSTests/test262/expectations.yaml (272242 => 272243)
--- trunk/JSTests/test262/expectations.yaml 2021-02-02 23:23:33 UTC (rev 272242)
+++ trunk/JSTests/test262/expectations.yaml 2021-02-02 23:24:48 UTC (rev 272243)
@@ -1985,9 +1985,6 @@
test/language/statements/switch/syntax/redeclaration/var-name-redeclaration-attempt-with-generator.js:
default: 'Test262: This statement should not be evaluated.'
strict mode: 'Test262: This statement should not be evaluated.'
-test/language/statements/try/completion-values.js:
- default: 'Test262Error: Expected SameValue(«39», «42») to be true'
- strict mode: 'Test262Error: Expected SameValue(«39», «42») to be true'
test/language/statements/variable/dstr/ary-init-iter-get-err-array-prototype.js:
default: 'TypeError: Spread syntax requires ...iterable[Symbol.iterator] to be a function'
strict mode: 'TypeError: Spread syntax requires ...iterable[Symbol.iterator] to be a function'
Modified: trunk/Source/_javascript_Core/ChangeLog (272242 => 272243)
--- trunk/Source/_javascript_Core/ChangeLog 2021-02-02 23:23:33 UTC (rev 272242)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-02-02 23:24:48 UTC (rev 272243)
@@ -1,3 +1,25 @@
+2021-02-02 Ross Kirsling <[email protected]>
+
+ Completion value of a finally block should not be ignored if completion is abrupt
+ https://bugs.webkit.org/show_bug.cgi?id=221238
+
+ Reviewed by Yusuke Suzuki.
+
+ Per https://tc39.es/ecma262/#sec-try-statement-runtime-semantics-evaluation,
+ the completion value of a finally block is ignored *just* when it is a normal completion,
+ but we've been ignoring it in all cases.
+
+ This patch ensures that when a finally block is exited with a break or continue statement,
+ its completion value is used as the completion value for the entire TryStatement.
+
+ (Note: This behavior will be important for the upcoming "do expressions" proposal,
+ as it is effectively a reification of completion values.)
+
+ * bytecompiler/NodesCodegen.cpp:
+ (JSC::TryNode::emitBytecode):
+ 1. Don't throw away the result of evaluating the finally block.
+ 2. Only use the try or catch block result if we make it all the way to the end of the finally block.
+
2021-02-02 Don Olmstead <[email protected]>
REGRESSION(r269309): [Cocoa] RemoteInspectorCocoa files are being compiled twice
Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (272242 => 272243)
--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2021-02-02 23:23:33 UTC (rev 272242)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2021-02-02 23:24:48 UTC (rev 272243)
@@ -4449,11 +4449,16 @@
{
// NOTE: The catch and finally blocks must be labeled explicitly, so the
// optimizer knows they may be jumped to from anywhere.
+ ASSERT(m_catchBlock || m_finallyBlock);
- if (generator.shouldBeConcernedWithCompletionValue() && m_tryBlock->hasEarlyBreakOrContinue())
- generator.emitLoad(dst, jsUndefined());
+ RefPtr<RegisterID> tryCatchDst = dst;
+ if (generator.shouldBeConcernedWithCompletionValue()) {
+ if (m_finallyBlock)
+ tryCatchDst = generator.newTemporary();
- ASSERT(m_catchBlock || m_finallyBlock);
+ if (m_finallyBlock || m_tryBlock->hasEarlyBreakOrContinue())
+ generator.emitLoad(tryCatchDst.get(), jsUndefined());
+ }
RefPtr<Label> catchLabel;
RefPtr<Label> catchEndLabel;
@@ -4481,7 +4486,7 @@
if (!m_catchBlock && m_finallyBlock)
finallyTryData = tryData;
- generator.emitNode(dst, m_tryBlock);
+ generator.emitNode(tryCatchDst.get(), m_tryBlock);
if (m_finallyBlock)
generator.emitJump(*finallyLabel);
@@ -4512,9 +4517,9 @@
generator.emitProfileControlFlow(m_tryBlock->endOffset() + 1);
if (m_finallyBlock)
- generator.emitNode(dst, m_catchBlock);
+ generator.emitNode(tryCatchDst.get(), m_catchBlock);
else
- generator.emitNodeInTailPosition(dst, m_catchBlock);
+ generator.emitNodeInTailPosition(tryCatchDst.get(), m_catchBlock);
generator.emitLoad(thrownValueRegister.get(), jsUndefined());
if (m_catchPattern)
@@ -4541,9 +4546,22 @@
generator.restoreScopeRegister();
int finallyStartOffset = m_catchBlock ? m_catchBlock->endOffset() + 1 : m_tryBlock->endOffset() + 1;
- generator.emitProfileControlFlow(finallyStartOffset);
- generator.emitNodeInTailPosition(m_finallyBlock);
+
+ // The completion value of a finally block is ignored *just* when it is a normal completion.
+ if (generator.shouldBeConcernedWithCompletionValue()) {
+ ASSERT(dst != tryCatchDst.get());
+ if (m_finallyBlock->hasEarlyBreakOrContinue())
+ generator.emitLoad(dst, jsUndefined());
+ generator.emitProfileControlFlow(finallyStartOffset);
+ generator.emitNodeInTailPosition(dst, m_finallyBlock);
+
+ generator.move(dst, tryCatchDst.get());
+ } else {
+ generator.emitProfileControlFlow(finallyStartOffset);
+ generator.emitNodeInTailPosition(m_finallyBlock);
+ }
+
generator.emitFinallyCompletion(finallyContext.value(), *finallyEndLabel);
generator.emitLabel(*finallyEndLabel);
generator.emitProfileControlFlow(m_finallyBlock->endOffset() + 1);
