A put is not an ExistingProperty put when we transition a structure because of an...
authorjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Apr 2018 23:48:00 +0000 (23:48 +0000)
committerjfbastien@apple.com <jfbastien@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Apr 2018 23:48:00 +0000 (23:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184706
<rdar://problem/38871451>

Reviewed by Saam Barati.

JSTests:

* stress/put-by-id-direct-strict-transition.js: Added.
(const.foo):
(j.const.obj.set hello):
* stress/put-by-id-direct-transition.js: Added.
(const.foo):
(j.const.obj.set hello):
* stress/put-getter-setter-by-id-strict-transition.js: Added.
(const.foo):
(j.const.obj.set hello):
* stress/put-getter-setter-by-id-transition.js: Added.
(const.foo):
(j.const.obj.set hello):

Source/JavaScriptCore:

When putting a property on a structure and the slot is a different
type, the slot can't be said to have already been existing.

* runtime/JSObjectInlines.h:
(JSC::JSObject::putDirectInternal):

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

JSTests/ChangeLog
JSTests/stress/put-by-id-direct-strict-transition.js [new file with mode: 0644]
JSTests/stress/put-by-id-direct-transition.js [new file with mode: 0644]
JSTests/stress/put-getter-setter-by-id-strict-transition.js [new file with mode: 0644]
JSTests/stress/put-getter-setter-by-id-transition.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSObjectInlines.h

index f832bbd..8270b82 100644 (file)
@@ -1,3 +1,24 @@
+2018-04-17  JF Bastien  <jfbastien@apple.com>
+
+        A put is not an ExistingProperty put when we transition a structure because of an attributes change
+        https://bugs.webkit.org/show_bug.cgi?id=184706
+        <rdar://problem/38871451>
+
+        Reviewed by Saam Barati.
+
+        * stress/put-by-id-direct-strict-transition.js: Added.
+        (const.foo):
+        (j.const.obj.set hello):
+        * stress/put-by-id-direct-transition.js: Added.
+        (const.foo):
+        (j.const.obj.set hello):
+        * stress/put-getter-setter-by-id-strict-transition.js: Added.
+        (const.foo):
+        (j.const.obj.set hello):
+        * stress/put-getter-setter-by-id-transition.js: Added.
+        (const.foo):
+        (j.const.obj.set hello):
+
 2018-04-16  Filip Pizlo  <fpizlo@apple.com>
 
         PutStackSinkingPhase should know that KillStack means ConflictingFlush
diff --git a/JSTests/stress/put-by-id-direct-strict-transition.js b/JSTests/stress/put-by-id-direct-strict-transition.js
new file mode 100644 (file)
index 0000000..6951617
--- /dev/null
@@ -0,0 +1,13 @@
+"use strict"
+
+let theglobal = 0;
+for (theglobal = 0; theglobal < 100000; ++theglobal)
+    ;
+const foo = (ignored, arg1) => { theglobal = arg1; };
+for (let j = 0; j < 10000; ++j) {
+    const obj = {
+        set hello(ignored) {},
+        [theglobal]: 0
+    };
+    foo(obj, 'hello');
+}
diff --git a/JSTests/stress/put-by-id-direct-transition.js b/JSTests/stress/put-by-id-direct-transition.js
new file mode 100644 (file)
index 0000000..611f8ed
--- /dev/null
@@ -0,0 +1,11 @@
+let theglobal = 0;
+for (theglobal = 0; theglobal < 100000; ++theglobal)
+    ;
+const foo = (ignored, arg1) => { theglobal = arg1; };
+for (let j = 0; j < 10000; ++j) {
+    const obj = {
+        set hello(ignored) {},
+        [theglobal]: 0
+    };
+    foo(obj, 'hello');
+}
diff --git a/JSTests/stress/put-getter-setter-by-id-strict-transition.js b/JSTests/stress/put-getter-setter-by-id-strict-transition.js
new file mode 100644 (file)
index 0000000..9abd44e
--- /dev/null
@@ -0,0 +1,13 @@
+"use strict"
+
+let theglobal = 0;
+for (theglobal = 0; theglobal < 100000; ++theglobal)
+    ;
+const foo = (ignored, arg1) => { theglobal = arg1; };
+for (let j = 0; j < 10000; ++j) {
+    const obj = {
+        [theglobal]: 0,
+        set hello(ignored) {}
+    };
+    foo(obj, 'hello');
+}
diff --git a/JSTests/stress/put-getter-setter-by-id-transition.js b/JSTests/stress/put-getter-setter-by-id-transition.js
new file mode 100644 (file)
index 0000000..7683ccd
--- /dev/null
@@ -0,0 +1,11 @@
+let theglobal = 0;
+for (theglobal = 0; theglobal < 100000; ++theglobal)
+    ;
+const foo = (ignored, arg1) => { theglobal = arg1; };
+for (let j = 0; j < 10000; ++j) {
+    const obj = {
+        [theglobal]: 0,
+        set hello(ignored) {}
+    };
+    foo(obj, 'hello');
+}
index 5ab4684..67eff05 100644 (file)
@@ -1,3 +1,17 @@
+2018-04-17  JF Bastien  <jfbastien@apple.com>
+
+        A put is not an ExistingProperty put when we transition a structure because of an attributes change
+        https://bugs.webkit.org/show_bug.cgi?id=184706
+        <rdar://problem/38871451>
+
+        Reviewed by Saam Barati.
+
+        When putting a property on a structure and the slot is a different
+        type, the slot can't be said to have already been existing.
+
+        * runtime/JSObjectInlines.h:
+        (JSC::JSObject::putDirectInternal):
+
 2018-04-17  Filip Pizlo  <fpizlo@apple.com>
 
         JSGenericTypedArrayView<>::visitChildren has a race condition reading m_mode and m_vector
index 794ee20..5e2701f 100644 (file)
@@ -287,12 +287,13 @@ ALWAYS_INLINE bool JSObject::putDirectInternal(VM& vm, PropertyName propertyName
 
             putDirect(vm, offset, value);
             structure->didReplaceProperty(offset);
-            slot.setExistingProperty(this, offset);
 
             if ((attributes & PropertyAttribute::Accessor) != (currentAttributes & PropertyAttribute::Accessor) || (attributes & PropertyAttribute::CustomAccessor) != (currentAttributes & PropertyAttribute::CustomAccessor)) {
                 ASSERT(!(attributes & PropertyAttribute::ReadOnly));
                 setStructure(vm, Structure::attributeChangeTransition(vm, structure, propertyName, attributes));
-            }
+            } else
+                slot.setExistingProperty(this, offset);
+
             return true;
         }
 
@@ -344,13 +345,14 @@ ALWAYS_INLINE bool JSObject::putDirectInternal(VM& vm, PropertyName propertyName
                 vm, propertyName, value, slot.context() == PutPropertySlot::PutById);
         }
 
-        slot.setExistingProperty(this, offset);
         putDirect(vm, offset, value);
 
         if ((attributes & PropertyAttribute::Accessor) != (currentAttributes & PropertyAttribute::Accessor) || (attributes & PropertyAttribute::CustomAccessor) != (currentAttributes & PropertyAttribute::CustomAccessor)) {
             ASSERT(!(attributes & PropertyAttribute::ReadOnly));
             setStructure(vm, Structure::attributeChangeTransition(vm, structure, propertyName, attributes));
-        }
+        } else
+            slot.setExistingProperty(this, offset);
+
         return true;
     }