Title: [222590] trunk
- Revision
- 222590
- Author
- [email protected]
- Date
- 2017-09-27 17:44:28 -0700 (Wed, 27 Sep 2017)
Log Message
Propagate hasBeenFlattenedBefore in Structure's transition constructor and fix our for-in caching to fail when the prototype chain has an object with a dictionary structure
https://bugs.webkit.org/show_bug.cgi?id=177523
Reviewed by Mark Lam.
JSTests:
* stress/prototype-chain-has-dictionary-structure-for-in-caching.js: Added.
(assert):
(Test):
(addMethods.Test.prototype.string_appeared_here.i.methodNumber):
(addMethods):
(i.Test.prototype.propName):
Source/_javascript_Core:
There was a bug in Structure's transition constructor where it didn't
propagate forward the hasBeenFlattenedBefore bit. In practice, this meant
that every time we asked a dictionary structure if it has been flattened
before, it would return false. This patch fixes this bug. It also fixes
a bug that this uncovers in our for-in implementation. Our implementation
would cache the property name enumerator even when the prototype chain
included a structure that is as dictionary. This is wrong because that
prototype object may add properties without transitioning, and the for-in
loop would vend a stale set of prototype properties.
* jit/JITOperations.cpp:
* runtime/JSPropertyNameEnumerator.h:
(JSC::propertyNameEnumerator):
* runtime/Structure.cpp:
(JSC::Structure::Structure):
(JSC::Structure::canCachePropertyNameEnumerator const):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (222589 => 222590)
--- trunk/JSTests/ChangeLog 2017-09-28 00:27:09 UTC (rev 222589)
+++ trunk/JSTests/ChangeLog 2017-09-28 00:44:28 UTC (rev 222590)
@@ -1,3 +1,17 @@
+2017-09-27 Saam Barati <[email protected]>
+
+ Propagate hasBeenFlattenedBefore in Structure's transition constructor and fix our for-in caching to fail when the prototype chain has an object with a dictionary structure
+ https://bugs.webkit.org/show_bug.cgi?id=177523
+
+ Reviewed by Mark Lam.
+
+ * stress/prototype-chain-has-dictionary-structure-for-in-caching.js: Added.
+ (assert):
+ (Test):
+ (addMethods.Test.prototype.string_appeared_here.i.methodNumber):
+ (addMethods):
+ (i.Test.prototype.propName):
+
2017-09-27 Mark Lam <[email protected]>
Yarr::Parser::tryConsumeGroupName() should check for the end of the pattern.
Added: trunk/JSTests/stress/prototype-chain-has-dictionary-structure-for-in-caching.js (0 => 222590)
--- trunk/JSTests/stress/prototype-chain-has-dictionary-structure-for-in-caching.js (rev 0)
+++ trunk/JSTests/stress/prototype-chain-has-dictionary-structure-for-in-caching.js 2017-09-28 00:44:28 UTC (rev 222590)
@@ -0,0 +1,36 @@
+
+function assert(b) {
+ if (!b)
+ throw new Error("Bad")
+}
+
+var Test = function(){};
+
+let methodNumber = 0;
+function addMethods() {
+ const methodCount = 65;
+ for (var i = 0; i < methodCount; i++){
+ Test.prototype['myMethod' + i + methodNumber] = function(){};
+ ++methodNumber;
+ }
+}
+
+addMethods();
+
+var test1 = new Test();
+
+for (var k in test1) { }
+
+let test2 = new Test();
+
+for (let i = 0; i < 100; ++i ) {
+ let propName = 'myAdditionalMethod' + i;
+ Test.prototype[propName] = function(){};
+ let foundNewPrototypeProperty = false;
+ for (let k in test2) {
+ if (propName === k)
+ foundNewPrototypeProperty = true;
+ }
+ assert(foundNewPrototypeProperty);
+ addMethods();
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (222589 => 222590)
--- trunk/Source/_javascript_Core/ChangeLog 2017-09-28 00:27:09 UTC (rev 222589)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-09-28 00:44:28 UTC (rev 222590)
@@ -1,3 +1,27 @@
+2017-09-27 Saam Barati <[email protected]>
+
+ Propagate hasBeenFlattenedBefore in Structure's transition constructor and fix our for-in caching to fail when the prototype chain has an object with a dictionary structure
+ https://bugs.webkit.org/show_bug.cgi?id=177523
+
+ Reviewed by Mark Lam.
+
+ There was a bug in Structure's transition constructor where it didn't
+ propagate forward the hasBeenFlattenedBefore bit. In practice, this meant
+ that every time we asked a dictionary structure if it has been flattened
+ before, it would return false. This patch fixes this bug. It also fixes
+ a bug that this uncovers in our for-in implementation. Our implementation
+ would cache the property name enumerator even when the prototype chain
+ included a structure that is as dictionary. This is wrong because that
+ prototype object may add properties without transitioning, and the for-in
+ loop would vend a stale set of prototype properties.
+
+ * jit/JITOperations.cpp:
+ * runtime/JSPropertyNameEnumerator.h:
+ (JSC::propertyNameEnumerator):
+ * runtime/Structure.cpp:
+ (JSC::Structure::Structure):
+ (JSC::Structure::canCachePropertyNameEnumerator const):
+
2017-09-27 Mark Lam <[email protected]>
Yarr::Parser::tryConsumeGroupName() should check for the end of the pattern.
Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (222589 => 222590)
--- trunk/Source/_javascript_Core/jit/JITOperations.cpp 2017-09-28 00:27:09 UTC (rev 222589)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp 2017-09-28 00:44:28 UTC (rev 222590)
@@ -2407,9 +2407,12 @@
{
VM& vm = exec->vm();
NativeCallFrameTracer tracer(&vm, exec);
+ auto scope = DECLARE_THROW_SCOPE(vm);
JSObject* base = cell->toObject(exec, exec->lexicalGlobalObject());
+ RETURN_IF_EXCEPTION(scope, { });
+ scope.release();
return propertyNameEnumerator(exec, base);
}
Modified: trunk/Source/_javascript_Core/runtime/JSPropertyNameEnumerator.h (222589 => 222590)
--- trunk/Source/_javascript_Core/runtime/JSPropertyNameEnumerator.h 2017-09-28 00:27:09 UTC (rev 222589)
+++ trunk/Source/_javascript_Core/runtime/JSPropertyNameEnumerator.h 2017-09-28 00:44:28 UTC (rev 222590)
@@ -133,11 +133,11 @@
ASSERT(propertyNames.size() < UINT32_MAX);
- normalizePrototypeChain(exec, structure);
+ bool successfullyNormalizedChain = normalizePrototypeChain(exec, structure) != InvalidPrototypeChain;
enumerator = JSPropertyNameEnumerator::create(vm, structure, indexedLength, numberStructureProperties, WTFMove(propertyNames));
enumerator->setCachedPrototypeChain(vm, structure->prototypeChain(exec));
- if (!indexedLength && structure->canCachePropertyNameEnumerator())
+ if (!indexedLength && successfullyNormalizedChain && structure->canCachePropertyNameEnumerator())
structure->setCachedPropertyNameEnumerator(vm, enumerator);
return enumerator;
}
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (222589 => 222590)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2017-09-28 00:27:09 UTC (rev 222589)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2017-09-28 00:44:28 UTC (rev 222590)
@@ -253,7 +253,8 @@
, m_offset(invalidOffset)
{
setDictionaryKind(previous->dictionaryKind());
- setIsPinnedPropertyTable(previous->hasBeenFlattenedBefore());
+ setIsPinnedPropertyTable(false);
+ setHasBeenFlattenedBefore(previous->hasBeenFlattenedBefore());
setHasGetterSetterProperties(previous->hasGetterSetterProperties());
setHasCustomGetterSetterProperties(previous->hasCustomGetterSetterProperties());
setHasReadOnlyOrGetterSetterPropertiesExcludingProto(previous->hasReadOnlyOrGetterSetterPropertiesExcludingProto());
@@ -1287,6 +1288,8 @@
while (true) {
if (!structure->get())
break;
+ if (structure->get()->isDictionary())
+ return false;
if (structure->get()->typeInfo().overridesGetPropertyNames())
return false;
structure++;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
