Diff
Modified: trunk/JSTests/ChangeLog (266714 => 266715)
--- trunk/JSTests/ChangeLog 2020-09-08 04:30:14 UTC (rev 266714)
+++ trunk/JSTests/ChangeLog 2020-09-08 07:21:07 UTC (rev 266715)
@@ -1,3 +1,20 @@
+2020-09-08 Yusuke Suzuki <[email protected]>
+
+ [JSC] Special property caching should check Structure's cacheability
+ https://bugs.webkit.org/show_bug.cgi?id=216222
+
+ Reviewed by Saam Barati.
+
+ * stress/module-namespace-object-caching.js: Added.
+ (shouldBe):
+ (async fn):
+ * stress/not-cache-over-uncacheable-dictionary.js: Added.
+ (shouldBe):
+ * stress/resources/to-string-module.js: Added.
+ (export.toString):
+ * stress/resources/value-of-module.js: Added.
+ (export.valueOf):
+
2020-09-04 Yusuke Suzuki <[email protected]>
[JSC] Align legacy Intl constructor behavior to spec
Added: trunk/JSTests/stress/module-namespace-object-caching.js (0 => 266715)
--- trunk/JSTests/stress/module-namespace-object-caching.js (rev 0)
+++ trunk/JSTests/stress/module-namespace-object-caching.js 2020-09-08 07:21:07 UTC (rev 266715)
@@ -0,0 +1,18 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+async function fn() {
+ // Both should have same structure.
+ const valueOfModule = await import('./resources/value-of-module.js');
+ const toStringModule = await import('./resources/to-string-module.js');
+
+ // These valueOf / toString should not be cached.
+ shouldBe(String(toStringModule), `2020`);
+ shouldBe(Number(toStringModule), 2020); // valueOf should not be cached since namespace object is impure for absent.
+
+ shouldBe(Number(valueOfModule), 42); // If the above access accidentally cached the value, this will not return 42.
+ shouldBe(String(valueOfModule), `42`);
+}
+fn().catch($vm.abort);
Added: trunk/JSTests/stress/not-cache-over-uncacheable-dictionary.js (0 => 266715)
--- trunk/JSTests/stress/not-cache-over-uncacheable-dictionary.js (rev 0)
+++ trunk/JSTests/stress/not-cache-over-uncacheable-dictionary.js 2020-09-08 07:21:07 UTC (rev 266715)
@@ -0,0 +1,14 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var object = {};
+$vm.toUncacheableDictionary(object);
+shouldBe(String(object), `[object Object]`);
+shouldBe(String(object), `[object Object]`);
+shouldBe(String(object), `[object Object]`);
+object[Symbol.toStringTag] = "OK";
+shouldBe(String(object), `[object OK]`);
+shouldBe(String(object), `[object OK]`);
+shouldBe(String(object), `[object OK]`);
Added: trunk/JSTests/stress/resources/to-string-module.js (0 => 266715)
--- trunk/JSTests/stress/resources/to-string-module.js (rev 0)
+++ trunk/JSTests/stress/resources/to-string-module.js 2020-09-08 07:21:07 UTC (rev 266715)
@@ -0,0 +1,4 @@
+export function toString()
+{
+ return "2020";
+}
Added: trunk/JSTests/stress/resources/value-of-module.js (0 => 266715)
--- trunk/JSTests/stress/resources/value-of-module.js (rev 0)
+++ trunk/JSTests/stress/resources/value-of-module.js 2020-09-08 07:21:07 UTC (rev 266715)
@@ -0,0 +1,4 @@
+export function valueOf()
+{
+ return 42;
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (266714 => 266715)
--- trunk/Source/_javascript_Core/ChangeLog 2020-09-08 04:30:14 UTC (rev 266714)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-09-08 07:21:07 UTC (rev 266715)
@@ -1,3 +1,24 @@
+2020-09-08 Yusuke Suzuki <[email protected]>
+
+ [JSC] Special property caching should check Structure's cacheability
+ https://bugs.webkit.org/show_bug.cgi?id=216222
+
+ Reviewed by Saam Barati.
+
+ While StructureRareData::cacheSpecialPropertySlow caches properties, the way it takes is incomplete.
+ It is not checking Structure's cacheability. We were caching miss condition even if structure is !propertyAccessesAreCacheableForAbsence.
+ We should perform the same check done in IC case. Strictly speaking, we can cache value for uncacheable-dictionary because we are setting
+ property change watchpoint (which will fire). But it sounds not so profitable if this structure is uncacheable.
+
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::convertToUncacheableDictionary):
+ * runtime/JSObject.h:
+ * runtime/StructureRareData.cpp:
+ (JSC::StructureRareData::cacheSpecialPropertySlow):
+ * tools/JSDollarVM.cpp:
+ (JSC::functionToUncacheableDictionary):
+ (JSC::JSDollarVM::finishCreation):
+
2020-09-07 Joonghun Park <[email protected]>
Unreviewed. Remove the build warning below since r266567.
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (266714 => 266715)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2020-09-08 04:30:14 UTC (rev 266714)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2020-09-08 07:21:07 UTC (rev 266715)
@@ -3738,6 +3738,16 @@
vm, Structure::toCacheableDictionaryTransition(vm, structure(vm), &deferredWatchpointFire));
}
+void JSObject::convertToUncacheableDictionary(VM& vm)
+{
+ if (structure(vm)->isUncacheableDictionary())
+ return;
+ DeferredStructureTransitionWatchpointFire deferredWatchpointFire(vm, structure(vm));
+ setStructure(
+ vm, Structure::toUncacheableDictionaryTransition(vm, structure(vm), &deferredWatchpointFire));
+}
+
+
void JSObject::shiftButterflyAfterFlattening(const GCSafeConcurrentJSLocker&, VM& vm, Structure* structure, size_t outOfLineCapacityAfter)
{
// This could interleave visitChildren because some old structure could have been a non
Modified: trunk/Source/_javascript_Core/runtime/JSObject.h (266714 => 266715)
--- trunk/Source/_javascript_Core/runtime/JSObject.h 2020-09-08 04:30:14 UTC (rev 266714)
+++ trunk/Source/_javascript_Core/runtime/JSObject.h 2020-09-08 07:21:07 UTC (rev 266715)
@@ -851,6 +851,7 @@
void setStructure(VM&, Structure*);
JS_EXPORT_PRIVATE void convertToDictionary(VM&);
+ JS_EXPORT_PRIVATE void convertToUncacheableDictionary(VM&);
void flattenDictionaryObject(VM& vm)
{
Modified: trunk/Source/_javascript_Core/runtime/StructureRareData.cpp (266714 => 266715)
--- trunk/Source/_javascript_Core/runtime/StructureRareData.cpp 2020-09-08 04:30:14 UTC (rev 266714)
+++ trunk/Source/_javascript_Core/runtime/StructureRareData.cpp 2020-09-08 07:21:07 UTC (rev 266715)
@@ -134,6 +134,12 @@
uid = vm.propertyNames->toPrimitiveSymbol.impl();
break;
}
+
+ if (!ownStructure->propertyAccessesAreCacheable() || ownStructure->isProxy()) {
+ giveUpOnSpecialPropertyCache(key);
+ return;
+ }
+
ObjectPropertyConditionSet conditionSet;
if (slot.isValue()) {
// We don't handle the own property case of special properties (toString, valueOf, @@toPrimitive, @@toStringTag) because we would never know if a new
@@ -144,11 +150,24 @@
// This will not create a condition for the current structure but that is good because we know that property
// is not on the ownStructure so we will transisition if one is added and this cache will no longer be used.
- prepareChainForCaching(globalObject, ownStructure, slot.slotBase());
+ auto cacheStatus = prepareChainForCaching(globalObject, ownStructure, slot.slotBase());
+ if (!cacheStatus) {
+ giveUpOnSpecialPropertyCache(key);
+ return;
+ }
conditionSet = generateConditionsForPrototypePropertyHit(vm, this, globalObject, ownStructure, slot.slotBase(), uid);
ASSERT(!conditionSet.isValid() || conditionSet.hasOneSlotBaseCondition());
} else if (slot.isUnset()) {
- prepareChainForCaching(globalObject, ownStructure, nullptr);
+ if (!ownStructure->propertyAccessesAreCacheableForAbsence()) {
+ giveUpOnSpecialPropertyCache(key);
+ return;
+ }
+
+ auto cacheStatus = prepareChainForCaching(globalObject, ownStructure, nullptr);
+ if (!cacheStatus) {
+ giveUpOnSpecialPropertyCache(key);
+ return;
+ }
conditionSet = generateConditionsForPropertyMiss(vm, this, globalObject, ownStructure, uid);
} else
return;
Modified: trunk/Source/_javascript_Core/tools/JSDollarVM.cpp (266714 => 266715)
--- trunk/Source/_javascript_Core/tools/JSDollarVM.cpp 2020-09-08 04:30:14 UTC (rev 266714)
+++ trunk/Source/_javascript_Core/tools/JSDollarVM.cpp 2020-09-08 07:21:07 UTC (rev 266715)
@@ -3153,6 +3153,19 @@
return JSValue::encode(jsBoolean(Gigacage::isEnabled()));
}
+static EncodedJSValue JSC_HOST_CALL functionToUncacheableDictionary(JSGlobalObject* globalObject, CallFrame* callFrame)
+{
+ DollarVMAssertScope assertScope;
+ VM& vm = globalObject->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+
+ JSObject* object = jsDynamicCast<JSObject*>(vm, callFrame->argument(0));
+ if (!object)
+ return throwVMTypeError(globalObject, scope, "Expected first argument to be an object"_s);
+ object->convertToUncacheableDictionary(vm);
+ return JSValue::encode(object);
+}
+
constexpr unsigned jsDollarVMPropertyAttributes = PropertyAttribute::ReadOnly | PropertyAttribute::DontEnum | PropertyAttribute::DontDelete;
void JSDollarVM::finishCreation(VM& vm)
@@ -3300,6 +3313,8 @@
addFunction(vm, "useJIT", functionUseJIT, 0);
addFunction(vm, "isGigacageEnabled", functionIsGigacageEnabled, 0);
+ addFunction(vm, "toUncacheableDictionary", functionToUncacheableDictionary, 1);
+
m_objectDoingSideEffectPutWithoutCorrectSlotStatusStructure.set(vm, this, ObjectDoingSideEffectPutWithoutCorrectSlotStatus::createStructure(vm, globalObject, jsNull()));
}
