REGRESSION(r203368): broke some test262 tests
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 3 Aug 2016 18:50:57 +0000 (18:50 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 3 Aug 2016 18:50:57 +0000 (18:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160479

Reviewed by Mark Lam.

JSTests:

Added a stress test for this case, since we don't always run test262.

* stress/freeze-setter.js: Added.
(let.o.set foo):

Source/JavaScriptCore:

The optimization in r203368 overlooked a subtle detail: freezing should not set ReadOnly on
Accessor properties.

* runtime/Structure.cpp:
(JSC::Structure::nonPropertyTransition):
* runtime/StructureTransitionTable.h:
(JSC::setsDontDeleteOnAllProperties):
(JSC::setsReadOnlyOnNonAccessorProperties):
(JSC::setsReadOnlyOnAllProperties): Deleted.

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

JSTests/ChangeLog
JSTests/stress/freeze-setter.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/Structure.cpp
Source/JavaScriptCore/runtime/StructureTransitionTable.h

index bca6006..f24fb0b 100644 (file)
@@ -1,3 +1,15 @@
+2016-08-03  Filip Pizlo  <fpizlo@apple.com>
+
+        REGRESSION(r203368): broke some test262 tests
+        https://bugs.webkit.org/show_bug.cgi?id=160479
+
+        Reviewed by Mark Lam.
+        
+        Added a stress test for this case, since we don't always run test262.
+
+        * stress/freeze-setter.js: Added.
+        (let.o.set foo):
+
 2016-08-03  Saam Barati  <sbarati@apple.com>
 
         Implement nested rest destructuring w.r.t the ES7 spec
diff --git a/JSTests/stress/freeze-setter.js b/JSTests/stress/freeze-setter.js
new file mode 100644 (file)
index 0000000..e43caf1
--- /dev/null
@@ -0,0 +1,20 @@
+//@ runDefault
+
+"use strict";
+
+let x;
+
+let o = {
+    set foo(value)
+    {
+        x = value;
+    }
+};
+
+Object.freeze(o);
+
+o.foo = 42;
+
+if (x != 42)
+    throw "Error: bad result: " + x;
+
index 4394a9e..4886bea 100644 (file)
@@ -1,3 +1,20 @@
+2016-08-03  Filip Pizlo  <fpizlo@apple.com>
+
+        REGRESSION(r203368): broke some test262 tests
+        https://bugs.webkit.org/show_bug.cgi?id=160479
+
+        Reviewed by Mark Lam.
+        
+        The optimization in r203368 overlooked a subtle detail: freezing should not set ReadOnly on
+        Accessor properties.
+
+        * runtime/Structure.cpp:
+        (JSC::Structure::nonPropertyTransition):
+        * runtime/StructureTransitionTable.h:
+        (JSC::setsDontDeleteOnAllProperties):
+        (JSC::setsReadOnlyOnNonAccessorProperties):
+        (JSC::setsReadOnlyOnAllProperties): Deleted.
+
 2016-08-03  Csaba Osztrogon√°c  <ossy@webkit.org>
 
         Lacking support on a arm-traditional disassembler.
index 1e4594a..3a6413e 100644 (file)
@@ -652,12 +652,8 @@ Structure* Structure::nonPropertyTransition(VM& vm, Structure* structure, NonPro
     if (preventsExtensions(transitionKind))
         transition->setDidPreventExtensions(true);
     
-    unsigned additionalPropertyAttributes = 0;
-    if (setsDontDeleteOnAllProperties(transitionKind))
-        additionalPropertyAttributes |= DontDelete;
-    if (setsReadOnlyOnAllProperties(transitionKind))
-        additionalPropertyAttributes |= ReadOnly;
-    if (additionalPropertyAttributes) {
+    if (setsDontDeleteOnAllProperties(transitionKind)
+        || setsReadOnlyOnNonAccessorProperties(transitionKind)) {
         // We pin the property table on transitions that do wholesale editing of the property
         // table, since our logic for walking the property transition chain to rematerialize the
         // table doesn't know how to take into account such wholesale edits.
@@ -668,8 +664,12 @@ Structure* Structure::nonPropertyTransition(VM& vm, Structure* structure, NonPro
         transition->pinForCaching();
         
         if (transition->propertyTable()) {
-            for (auto& entry : *transition->propertyTable().get())
-                entry.attributes |= additionalPropertyAttributes;
+            for (auto& entry : *transition->propertyTable().get()) {
+                if (setsDontDeleteOnAllProperties(transitionKind))
+                    entry.attributes |= DontDelete;
+                if (setsReadOnlyOnNonAccessorProperties(transitionKind) && !(entry.attributes & Accessor))
+                    entry.attributes |= ReadOnly;
+            }
         }
     } else {
         transition->propertyTable().set(vm, transition, structure->takePropertyTableOrCloneIfPinned(vm));
@@ -677,7 +677,7 @@ Structure* Structure::nonPropertyTransition(VM& vm, Structure* structure, NonPro
         checkOffset(transition->m_offset, transition->inlineCapacity());
     }
     
-    if (setsReadOnlyOnAllProperties(transitionKind)
+    if (setsReadOnlyOnNonAccessorProperties(transitionKind)
         && transition->propertyTable()
         && !transition->propertyTable()->isEmpty())
         transition->setHasReadOnlyOrGetterSetterPropertiesExcludingProto(true);
index f0de6d3..92901a2 100644 (file)
@@ -130,7 +130,7 @@ inline bool setsDontDeleteOnAllProperties(NonPropertyTransition transition)
     }
 }
 
-inline bool setsReadOnlyOnAllProperties(NonPropertyTransition transition)
+inline bool setsReadOnlyOnNonAccessorProperties(NonPropertyTransition transition)
 {
     switch (transition) {
     case NonPropertyTransition::Freeze: