- 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);