PutProperytSlot should inform the IC about the property before effects.
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Nov 2017 19:57:01 +0000 (19:57 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Nov 2017 19:57:01 +0000 (19:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179262

Reviewed by Mark Lam.

This patch fixes an issue where we choose to cache setters based on
incorrect information. If we did so we might end up OSR exiting
more than we would otherwise need to. The new model is that the
PutPropertySlot should inform the IC of what the property looked
like before any potential side effects might have occurred.

* runtime/JSObject.cpp:
(JSC::JSObject::putInlineSlow):
* runtime/Lookup.h:
(JSC::putEntry):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/Lookup.h

index 731be1a..03fa7bc 100644 (file)
@@ -1,3 +1,21 @@
+2017-11-03  Keith Miller  <keith_miller@apple.com>
+
+        PutProperytSlot should inform the IC about the property before effects.
+        https://bugs.webkit.org/show_bug.cgi?id=179262
+
+        Reviewed by Mark Lam.
+
+        This patch fixes an issue where we choose to cache setters based on
+        incorrect information. If we did so we might end up OSR exiting
+        more than we would otherwise need to. The new model is that the
+        PutPropertySlot should inform the IC of what the property looked
+        like before any potential side effects might have occurred.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::putInlineSlow):
+        * runtime/Lookup.h:
+        (JSC::putEntry):
+
 2017-11-03  Mark Lam  <mark.lam@apple.com>
 
         CachedCall (and its clients) needs overflow checks.
index 702b1e7..1037643 100644 (file)
@@ -776,19 +776,23 @@ bool JSObject::putInlineSlow(ExecState* exec, PropertyName propertyName, JSValue
 
             JSValue gs = obj->getDirect(offset);
             if (gs.isGetterSetter()) {
-                bool result = callSetter(exec, slot.thisValue(), gs, value, slot.isStrictMode() ? StrictMode : NotStrictMode);
-                RETURN_IF_EXCEPTION(scope, false);
+                // We need to make sure that we decide to cache this property before we potentially execute aribitrary JS.
                 if (!structure()->isDictionary())
                     slot.setCacheableSetter(obj, offset);
+
+                bool result = callSetter(exec, slot.thisValue(), gs, value, slot.isStrictMode() ? StrictMode : NotStrictMode);
+                RETURN_IF_EXCEPTION(scope, false);
                 return result;
             }
             if (gs.isCustomGetterSetter()) {
-                bool result = callCustomSetter(exec, gs, attributes & PropertyAttribute::CustomAccessor, obj, slot.thisValue(), value);
-                RETURN_IF_EXCEPTION(scope, false);
+                // We need to make sure that we decide to cache this property before we potentially execute aribitrary JS.
                 if (attributes & PropertyAttribute::CustomAccessor)
                     slot.setCustomAccessor(obj, jsCast<CustomGetterSetter*>(gs.asCell())->setter());
                 else
                     slot.setCustomValue(obj, jsCast<CustomGetterSetter*>(gs.asCell())->setter());
+
+                bool result = callCustomSetter(exec, gs, attributes & PropertyAttribute::CustomAccessor, obj, slot.thisValue(), value);
+                RETURN_IF_EXCEPTION(scope, false);
                 return result;
             }
             ASSERT(!(attributes & PropertyAttribute::Accessor));
index 6005d1c..08ddc70 100644 (file)
@@ -289,12 +289,14 @@ inline bool putEntry(ExecState* exec, const ClassInfo*, const HashTableValue* en
         ASSERT_WITH_MESSAGE(!(entry->attributes() & PropertyAttribute::DOMJITAttribute), "DOMJITAttribute supports readonly attributes currently.");
         bool isAccessor = entry->attributes() & PropertyAttribute::CustomAccessor;
         JSValue updateThisValue = entry->attributes() & PropertyAttribute::CustomAccessor ? slot.thisValue() : JSValue(base);
-        bool result = callCustomSetter(exec, entry->propertyPutter(), isAccessor, updateThisValue, value);
-        RETURN_IF_EXCEPTION(scope, false);
+        // We need to make sure that we decide to cache this property before we potentially execute aribitrary JS.
         if (isAccessor)
             slot.setCustomAccessor(base, entry->propertyPutter());
         else
             slot.setCustomValue(base, entry->propertyPutter());
+
+        bool result = callCustomSetter(exec, entry->propertyPutter(), isAccessor, updateThisValue, value);
+        RETURN_IF_EXCEPTION(scope, false);
         return result;
     }