Changeset 289166 in webkit for trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

Timestamp:

Feb 6, 2022, 4:34:45 AM (3 years ago)

Author:

[email protected]

Message:

Object literal doesn't properly resolve name clash between an accessor and a constant property
​https://bugs.webkit.org/show_bug.cgi?id=220574

Patch by Alexey Shvayka <​[email protected]> on 2022-02-06
Reviewed by Yusuke Suzuki.

JSTests:

• stress/class-static-accessor-name-clash-with-field.js: Added.
• stress/object-literal-accessor-name-clash-with-constant.js: Added.

Source/JavaScriptCore:

The spec [1] calls DefineOwnProperty for every property node, whether it's a
getter, a setter, or a value. JSC attempts to reduce emitted bytecodes by setting
up a getter and a setter at once.

However, there is a slower path that exactly matches the spec, which was called only
if a spread syntax or a computed property was encountered. With this patch, the slower
path is also taken in case of a constant property (including a shorthand) with the
same name as an accessor.

That causes an incomplete accessor descriptor to correctly overwrite the existing
data one, which aligns JSC with V8 and SpiderMonkey.

This bug doesn't exist for static class fields and accessors because initialization
of class fields is deferred [2] and they always overwrite eponymous static methods /
accessors, no matter the order in source code. No reproduction for private elements either.

[1]: ​https://tc39.es/ecma262/#sec-runtime-semantics-methoddefinitionevaluation (step 11 of "get", step 10 of "set")
[2]: ​https://tc39.es/ecma262/#sec-runtime-semantics-classdefinitionevaluation (step 31.a)

• bytecompiler/NodesCodegen.cpp:

(JSC::PropertyListNode::emitBytecode):

LayoutTests:

Adjusted test now passes on V8 and SpiderMonkey as well.

• js/class-syntax-method-names-expected.txt:
• js/script-tests/class-syntax-method-names.js:

File:

• trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

@@ -662 +662 @@
     if (p) {
         // Build a list of getter/setter pairs to try to put them at the same time. If we encounter
-        // a computed property or a spread, just emit everything as that may override previous values.
+        // a constant property by the same name as accessor or a computed property or a spread,
+        // just emit everything as that may override previous values.
         bool canOverrideProperties = false;
 
@@ -676 +677 @@
             }
 
-            if (node->m_type & PropertyNode::Constant)
+            GetterSetterMap& map = node->isStaticClassProperty() ? staticMap : instanceMap;
+            if (node->m_type & PropertyNode::Constant) {
+                if (map.contains(node->name()->impl())) {
+                    canOverrideProperties = true;
+                    break;
+                }
                 continue;
+            }
 
             // Duplicates are possible.
             GetterSetterPair pair(node, static_cast<PropertyNode*>(nullptr));
-            GetterSetterMap& map = node->isStaticClassProperty() ? staticMap : instanceMap;
             GetterSetterMap::AddResult result = map.add(node->name()->impl(), pair);
             auto& resultPair = result.iterator->value;