Title: [208056] branches/safari-602-branch/Source/_javascript_Core
Revision
208056
Author
mark....@apple.com
Date
2016-10-28 10:52:40 -0700 (Fri, 28 Oct 2016)

Log Message

Merge r208018. rdar://problem/28962887, rdar://problem/28962912

    2016-10-27  Mark Lam  <mark....@apple.com>

    JSFunction::put() should not allow caching of lazily reified properties.
    https://bugs.webkit.org/show_bug.cgi?id=164081

    Reviewed by Geoffrey Garen.

    It is incorrect for JSFunction::put() to return PutPropertySlots that indicates
    that its lazily reified properties (e.g. .caller, and .arguments) are cacheable.
    The reason for this is:

    1. Currently, a cacheable put may only consist of the following types of put
       operations:
       a. putting a new property at an offset in the object storage.
       b. changing the value of an existing property at an offset in the object storage.
       c. invoking the setter for a property at an offset in the object storage.

       Returning a PutPropertySlot that indicates the property is cacheable means that
       the property put must be one of the above operations.

       For lazily reified properties, JSFunction::put() implements complex conditional
       behavior that is different than the set of cacheable put operations above.
       Hence, it should not claim that the property put is cacheable.
    
    2. Cacheable puts are cached on the original structure of the object before the
       put operation.

       Reifying a lazy property will trigger a structure transition.  Even though
       subsequent puts to such a property may be cacheable after the structure
       transition, it is incorrect to indicate that the property put is cacheable
       because the caching is on the original structure, not the new transitioned
       structure.

    Also fixed some missing exception checks.

    * jit/JITOperations.cpp:
    * runtime/JSFunction.cpp:
    (JSC::JSFunction::put):
    (JSC::JSFunction::reifyLazyPropertyIfNeeded):
    (JSC::JSFunction::reifyBoundNameIfNeeded):
    * runtime/JSFunction.h:

Modified Paths

Diff

Modified: branches/safari-602-branch/Source/_javascript_Core/ChangeLog (208055 => 208056)


--- branches/safari-602-branch/Source/_javascript_Core/ChangeLog	2016-10-28 17:42:47 UTC (rev 208055)
+++ branches/safari-602-branch/Source/_javascript_Core/ChangeLog	2016-10-28 17:52:40 UTC (rev 208056)
@@ -1,3 +1,49 @@
+2016-10-28  Mark Lam  <mark....@apple.com>
+
+        Merge r208018. rdar://problem/28962887, rdar://problem/28962912
+
+    2016-10-27  Mark Lam  <mark....@apple.com>
+
+            JSFunction::put() should not allow caching of lazily reified properties.
+            https://bugs.webkit.org/show_bug.cgi?id=164081
+
+            Reviewed by Geoffrey Garen.
+
+            It is incorrect for JSFunction::put() to return PutPropertySlots that indicates
+            that its lazily reified properties (e.g. .caller, and .arguments) are cacheable.
+            The reason for this is:
+
+            1. Currently, a cacheable put may only consist of the following types of put
+               operations:
+               a. putting a new property at an offset in the object storage.
+               b. changing the value of an existing property at an offset in the object storage.
+               c. invoking the setter for a property at an offset in the object storage.
+
+               Returning a PutPropertySlot that indicates the property is cacheable means that
+               the property put must be one of the above operations.
+
+               For lazily reified properties, JSFunction::put() implements complex conditional
+               behavior that is different than the set of cacheable put operations above.
+               Hence, it should not claim that the property put is cacheable.
+    
+            2. Cacheable puts are cached on the original structure of the object before the
+               put operation.
+
+               Reifying a lazy property will trigger a structure transition.  Even though
+               subsequent puts to such a property may be cacheable after the structure
+               transition, it is incorrect to indicate that the property put is cacheable
+               because the caching is on the original structure, not the new transitioned
+               structure.
+
+            Also fixed some missing exception checks.
+
+            * jit/JITOperations.cpp:
+            * runtime/JSFunction.cpp:
+            (JSC::JSFunction::put):
+            (JSC::JSFunction::reifyLazyPropertyIfNeeded):
+            (JSC::JSFunction::reifyBoundNameIfNeeded):
+            * runtime/JSFunction.h:
+
 2016-10-27  Mark Lam  <mark....@apple.com>
 
         Merge r207518. rdar://problem/28216050, rdar://problem/28216232

Modified: branches/safari-602-branch/Source/_javascript_Core/jit/JITOperations.cpp (208055 => 208056)


--- branches/safari-602-branch/Source/_javascript_Core/jit/JITOperations.cpp	2016-10-28 17:42:47 UTC (rev 208055)
+++ branches/safari-602-branch/Source/_javascript_Core/jit/JITOperations.cpp	2016-10-28 17:52:40 UTC (rev 208056)
@@ -193,6 +193,9 @@
     PropertySlot slot(baseValue, PropertySlot::InternalMethodType::VMInquiry);
 
     baseValue.getPropertySlot(exec, ident, slot);
+    if (UNLIKELY(vm->exception()))
+        return encodedJSValue();
+
     if (stubInfo->considerCaching(baseValue.structureOrNull()) && !slot.isTaintedByProxy() && (slot.isCacheableValue() || slot.isCacheableGetter() || slot.isUnset()))
         repatchGetByID(exec, baseValue, ident, slot, *stubInfo, GetByIDKind::Pure);
 
@@ -388,7 +391,9 @@
 
     Structure* structure = baseValue.isCell() ? baseValue.asCell()->structure(*vm) : nullptr;
     baseValue.putInline(exec, ident, value, slot);
-    
+    if (UNLIKELY(vm->exception()))
+        return;
+
     if (accessType != static_cast<AccessType>(stubInfo->accessType))
         return;
     
@@ -413,7 +418,9 @@
 
     Structure* structure = baseValue.isCell() ? baseValue.asCell()->structure(*vm) : nullptr;    
     baseValue.putInline(exec, ident, value, slot);
-    
+    if (UNLIKELY(vm->exception()))
+        return;
+
     if (accessType != static_cast<AccessType>(stubInfo->accessType))
         return;
     

Modified: branches/safari-602-branch/Source/_javascript_Core/runtime/JSFunction.cpp (208055 => 208056)


--- branches/safari-602-branch/Source/_javascript_Core/runtime/JSFunction.cpp	2016-10-28 17:42:47 UTC (rev 208055)
+++ branches/safari-602-branch/Source/_javascript_Core/runtime/JSFunction.cpp	2016-10-28 17:52:40 UTC (rev 208056)
@@ -424,17 +424,17 @@
         return Base::put(thisObject, exec, propertyName, value, slot);
 
     if (propertyName == exec->propertyNames().prototype) {
+        slot.disableCaching();
         // Make sure prototype has been reified, such that it can only be overwritten
         // following the rules set out in ECMA-262 8.12.9.
-        PropertySlot slot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
-        thisObject->methodTable(exec->vm())->getOwnPropertySlot(thisObject, exec, propertyName, slot);
+        PropertySlot getSlot(thisObject, PropertySlot::InternalMethodType::VMInquiry);
+        thisObject->methodTable(exec->vm())->getOwnPropertySlot(thisObject, exec, propertyName, getSlot);
         if (thisObject->m_rareData)
             thisObject->m_rareData->clear("Store to prototype property of a function");
-        // Don't allow this to be cached, since a [[Put]] must clear m_rareData.
-        PutPropertySlot dontCache(thisObject);
-        return Base::put(thisObject, exec, propertyName, value, dontCache);
+        return Base::put(thisObject, exec, propertyName, value, slot);
     }
     if (thisObject->jsExecutable()->isStrictMode() && (propertyName == exec->propertyNames().arguments || propertyName == exec->propertyNames().caller)) {
+        slot.disableCaching();
         // This will trigger the property to be reified, if this is not already the case!
         bool okay = thisObject->hasProperty(exec, propertyName);
         ASSERT_UNUSED(okay, okay);
@@ -441,11 +441,14 @@
         return Base::put(thisObject, exec, propertyName, value, slot);
     }
     if (propertyName == exec->propertyNames().arguments || propertyName == exec->propertyNames().caller) {
+        slot.disableCaching();
         if (slot.isStrictMode())
             throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
         return false;
     }
-    thisObject->reifyLazyPropertyIfNeeded(exec, propertyName);
+    LazyPropertyType propType = thisObject->reifyLazyPropertyIfNeeded(exec, propertyName);
+    if (propType == LazyPropertyType::IsLazyProperty)
+        slot.disableCaching();
     return Base::put(thisObject, exec, propertyName, value, slot);
 }
 
@@ -636,15 +639,18 @@
     rareData->setHasReifiedName();
 }
 
-void JSFunction::reifyLazyPropertyIfNeeded(ExecState* exec, PropertyName propertyName)
+JSFunction::LazyPropertyType JSFunction::reifyLazyPropertyIfNeeded(ExecState* exec, PropertyName propertyName)
 {
     if (propertyName == exec->propertyNames().length) {
         if (!hasReifiedLength())
             reifyLength(exec);
+        return LazyPropertyType::IsLazyProperty;
     } else if (propertyName == exec->propertyNames().name) {
         if (!hasReifiedName())
             reifyName(exec);
+        return LazyPropertyType::IsLazyProperty;
     }
+    return LazyPropertyType::NotLazyProperty;
 }
 
 } // namespace JSC

Modified: branches/safari-602-branch/Source/_javascript_Core/runtime/JSFunction.h (208055 => 208056)


--- branches/safari-602-branch/Source/_javascript_Core/runtime/JSFunction.h	2016-10-28 17:42:47 UTC (rev 208055)
+++ branches/safari-602-branch/Source/_javascript_Core/runtime/JSFunction.h	2016-10-28 17:52:40 UTC (rev 208056)
@@ -192,8 +192,10 @@
     void reifyLength(ExecState*);
     void reifyName(ExecState*);
     void reifyName(ExecState*, String name);
-    void reifyLazyPropertyIfNeeded(ExecState*, PropertyName propertyName);
 
+    enum class LazyPropertyType { NotLazyProperty, IsLazyProperty };
+    LazyPropertyType reifyLazyPropertyIfNeeded(ExecState*, PropertyName);
+
     friend class LLIntOffsetsExtractor;
 
     static EncodedJSValue argumentsGetter(ExecState*, EncodedJSValue, PropertyName);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to