Caching of properties on objects that have named property getters is sometimes incorrect
https://bugs.webkit.org/show_bug.cgi?id=151453
<rdar://problem/23049343>
Reviewed by Gavin Barraclough.
Source/JavaScriptCore:
Add new GetOwnPropertySlotIsImpureForPropertyAbsence TypeInfo flag to be
used by objects that have a non-'OverrideBuiltins' named property getter.
This flag prevents caching of properties that are missing as a named
property with this name may later become available.
Objects with an 'OverrideBuiltins' named property getter will keep using
the GetOwnPropertySlotIsImpure TypeInfo flag, which prevents all property
caching since named properties can override own properties or properties
on the prototype.
* bytecode/ComplexGetStatus.cpp:
(JSC::ComplexGetStatus::computeFor):
* bytecode/PropertyCondition.cpp:
(JSC::PropertyCondition::isStillValid):
* jit/Repatch.cpp:
(JSC::tryCacheGetByID):
(JSC::tryRepatchIn):
* jsc.cpp:
* runtime/JSTypeInfo.h:
(JSC::TypeInfo::getOwnPropertySlotIsImpure):
(JSC::TypeInfo::getOwnPropertySlotIsImpureForPropertyAbsence):
(JSC::TypeInfo::prohibitsPropertyCaching): Deleted.
* runtime/Structure.h:
Source/WebCore:
In r188590, we dropped the JSC::GetOwnPropertySlotIsImpure TypeInfo flag for
interfaces that have a non-'OverrideBuiltins' named property getter in order
to allow caching of properties returns by GetOwnPropertySlot(). We assumed
this was safe as it was no longer possible for named properties to override
own properties (or properties on the prototype).
However, there is an issue when we cache the non-existence of a property.
Even though at one point the property did not exist, a named property with
this name may later become available. In such case, caching would cause us
to wrongly report a property as missing.
To address the problem, this patch introduces a new
GetOwnPropertySlotIsImpureForPropertyAbsence TypeInfo flag and uses it for
interfaces that have a non-'OverrideBuiltins' named property getter. This
will cause us to not cache the fact that a property is missing on such
objects, while maintaining the performance win from r188590 in the common
case.
Test: fast/dom/NamedNodeMap-named-getter-caching.html
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateHeader):
* bindings/scripts/test/JS/JSTestCustomNamedGetter.h:
* bindings/scripts/test/JS/JSTestEventTarget.h:
* bindings/scripts/test/JS/JSTestOverrideBuiltins.h:
LayoutTests:
Add layout test to make sure caching does not cause NamedNodeMap's
named property getter to sometimes wrongly report a property as
missing.
* fast/dom/NamedNodeMap-named-getter-caching-expected.txt: Added.
* fast/dom/NamedNodeMap-named-getter-caching.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@192693 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/jit/Repatch.cpp b/Source/JavaScriptCore/jit/Repatch.cpp
index 23ca680..c11cac3 100644
--- a/Source/JavaScriptCore/jit/Repatch.cpp
+++ b/Source/JavaScriptCore/jit/Repatch.cpp
@@ -283,6 +283,9 @@
if (structure->typeInfo().prohibitsPropertyCaching() || structure->isDictionary())
return GiveUpOnCache;
+ if (slot.isUnset() && structure->typeInfo().getOwnPropertySlotIsImpureForPropertyAbsence())
+ return GiveUpOnCache;
+
if (slot.isUnset()) {
conditionSet = generateConditionsForPropertyMiss(
vm, codeBlock, exec, structure, propertyName.impl());
@@ -485,7 +488,7 @@
if (forceICFailure(exec))
return GiveUpOnCache;
- if (!base->structure()->propertyAccessesAreCacheable())
+ if (!base->structure()->propertyAccessesAreCacheable() || (!wasFound && !base->structure()->propertyAccessesAreCacheableForAbsence()))
return GiveUpOnCache;
if (wasFound) {
