[JSC] ClassExpr should not store result in the middle of evaluation
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Jun 2019 07:16:34 +0000 (07:16 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Jun 2019 07:16:34 +0000 (07:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199106

Reviewed by Tadeu Zagallo.

JSTests:

* stress/class-expression-should-store-result-at-last.js: Added.
(shouldThrow):
(shouldThrow.let.a):

Source/JavaScriptCore:

Let's consider the case,

    let a = class A {
        static get[a=0x12345678]() {
        }
    };

When evaluating `class A` expression, we should not use the local register for `let a`
until we finally store it to that register. Otherwise, `a=0x12345678` will override it.
Out BytecodeGenerator does that this by using tempDestination and finalDestination, but
we did not do that in ClassExprNode.

This patch leverages tempDestination and finalDestination to store `class A` result finally,
while we attempt to reduce mov.

* bytecompiler/NodesCodegen.cpp:
(JSC::ClassExprNode::emitBytecode):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246708 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/class-expression-should-store-result-at-last.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

index 0abc105..51df831 100644 (file)
@@ -1,3 +1,14 @@
+2019-06-22  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] ClassExpr should not store result in the middle of evaluation
+        https://bugs.webkit.org/show_bug.cgi?id=199106
+
+        Reviewed by Tadeu Zagallo.
+
+        * stress/class-expression-should-store-result-at-last.js: Added.
+        (shouldThrow):
+        (shouldThrow.let.a):
+
 2019-06-20  Justin Michaud  <justin_michaud@apple.com>
 
         [WASM-References] Add extra tests for Wasm references + fix element parsing and subtyping bugs
diff --git a/JSTests/stress/class-expression-should-store-result-at-last.js b/JSTests/stress/class-expression-should-store-result-at-last.js
new file mode 100644 (file)
index 0000000..3787ae3
--- /dev/null
@@ -0,0 +1,22 @@
+function shouldThrow(func, errorMessage) {
+    var errorThrown = false;
+    var error = null;
+    try {
+        func();
+    } catch (e) {
+        errorThrown = true;
+        error = e;
+    }
+    if (!errorThrown)
+        throw new Error('not thrown');
+    if (String(error) !== errorMessage)
+        throw new Error(`bad error: ${String(error)}`);
+}
+
+shouldThrow(() => {
+    let a = class c {
+        static get[(a=0x12345678, b=0x42424242)]()
+        {
+        }
+    };
+}, `ReferenceError: Cannot access uninitialized variable.`);
index 4c61ff7..e496160 100644 (file)
@@ -1,3 +1,28 @@
+2019-06-22  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] ClassExpr should not store result in the middle of evaluation
+        https://bugs.webkit.org/show_bug.cgi?id=199106
+
+        Reviewed by Tadeu Zagallo.
+
+        Let's consider the case,
+
+            let a = class A {
+                static get[a=0x12345678]() {
+                }
+            };
+
+        When evaluating `class A` expression, we should not use the local register for `let a`
+        until we finally store it to that register. Otherwise, `a=0x12345678` will override it.
+        Out BytecodeGenerator does that this by using tempDestination and finalDestination, but
+        we did not do that in ClassExprNode.
+
+        This patch leverages tempDestination and finalDestination to store `class A` result finally,
+        while we attempt to reduce mov.
+
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::ClassExprNode::emitBytecode):
+
 2019-06-21  Sihui Liu  <sihui_liu@apple.com>
 
         openDatabase should return an empty object when WebSQL is disabled
index 629d416..addb342 100644 (file)
@@ -3979,7 +3979,7 @@ RegisterID* ClassExprNode::emitBytecode(BytecodeGenerator& generator, RegisterID
         generator.emitNode(superclass.get(), m_classHeritage);
     }
 
-    RefPtr<RegisterID> constructor;
+    RefPtr<RegisterID> constructor = generator.tempDestination(dst);
     bool needsHomeObject = false;
 
     if (m_constructorExpression) {
@@ -3987,10 +3987,10 @@ RegisterID* ClassExprNode::emitBytecode(BytecodeGenerator& generator, RegisterID
         FunctionMetadataNode* metadata = static_cast<FuncExprNode*>(m_constructorExpression)->metadata();
         metadata->setEcmaName(ecmaName());
         metadata->setClassSource(m_classSource);
-        constructor = generator.emitNode(dst, m_constructorExpression);
+        constructor = generator.emitNode(constructor.get(), m_constructorExpression);
         needsHomeObject = m_classHeritage || metadata->superBinding() == SuperBinding::Needed;
     } else
-        constructor = generator.emitNewDefaultConstructor(generator.finalDestination(dst), m_classHeritage ? ConstructorKind::Extends : ConstructorKind::Base, m_name, ecmaName(), m_classSource);
+        constructor = generator.emitNewDefaultConstructor(constructor.get(), m_classHeritage ? ConstructorKind::Extends : ConstructorKind::Base, m_name, ecmaName(), m_classSource);
 
     const auto& propertyNames = generator.propertyNames();
     RefPtr<RegisterID> prototype = generator.emitNewObject(generator.newTemporary());
@@ -4047,7 +4047,7 @@ RegisterID* ClassExprNode::emitBytecode(BytecodeGenerator& generator, RegisterID
         generator.popLexicalScope(this);
     }
 
-    return generator.move(dst, constructor.get());
+    return generator.move(generator.finalDestination(dst, constructor.get()), constructor.get());
 }
 
 // ------------------------------ ImportDeclarationNode -----------------------