- Revision
- 230740
- Author
- jfbast...@apple.com
- Date
- 2018-04-17 16:48:00 -0700 (Tue, 17 Apr 2018)
Log Message
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.
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/_javascript_Core:
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):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (230739 => 230740)
--- trunk/JSTests/ChangeLog 2018-04-17 23:41:09 UTC (rev 230739)
+++ trunk/JSTests/ChangeLog 2018-04-17 23:48:00 UTC (rev 230740)
@@ -1,3 +1,24 @@
+2018-04-17 JF Bastien <jfbast...@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 <fpi...@apple.com>
PutStackSinkingPhase should know that KillStack means ConflictingFlush
Added: trunk/JSTests/stress/put-by-id-direct-strict-transition.js (0 => 230740)
--- trunk/JSTests/stress/put-by-id-direct-strict-transition.js (rev 0)
+++ trunk/JSTests/stress/put-by-id-direct-strict-transition.js 2018-04-17 23:48:00 UTC (rev 230740)
@@ -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');
+}
Added: trunk/JSTests/stress/put-by-id-direct-transition.js (0 => 230740)
--- trunk/JSTests/stress/put-by-id-direct-transition.js (rev 0)
+++ trunk/JSTests/stress/put-by-id-direct-transition.js 2018-04-17 23:48:00 UTC (rev 230740)
@@ -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');
+}
Added: trunk/JSTests/stress/put-getter-setter-by-id-strict-transition.js (0 => 230740)
--- trunk/JSTests/stress/put-getter-setter-by-id-strict-transition.js (rev 0)
+++ trunk/JSTests/stress/put-getter-setter-by-id-strict-transition.js 2018-04-17 23:48:00 UTC (rev 230740)
@@ -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');
+}
Added: trunk/JSTests/stress/put-getter-setter-by-id-transition.js (0 => 230740)
--- trunk/JSTests/stress/put-getter-setter-by-id-transition.js (rev 0)
+++ trunk/JSTests/stress/put-getter-setter-by-id-transition.js 2018-04-17 23:48:00 UTC (rev 230740)
@@ -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');
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (230739 => 230740)
--- trunk/Source/_javascript_Core/ChangeLog 2018-04-17 23:41:09 UTC (rev 230739)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-04-17 23:48:00 UTC (rev 230740)
@@ -1,3 +1,17 @@
+2018-04-17 JF Bastien <jfbast...@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 <fpi...@apple.com>
JSGenericTypedArrayView<>::visitChildren has a race condition reading m_mode and m_vector
Modified: trunk/Source/_javascript_Core/runtime/JSObjectInlines.h (230739 => 230740)
--- trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2018-04-17 23:41:09 UTC (rev 230739)
+++ trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2018-04-17 23:48:00 UTC (rev 230740)
@@ -287,12 +287,13 @@
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 @@
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;
}
