Title: [226767] trunk
- Revision
- 226767
- Author
- sbar...@apple.com
- Date
- 2018-01-11 00:16:06 -0800 (Thu, 11 Jan 2018)
Log Message
Our for-in caching is wrong when we add indexed properties on things in the prototype chain
https://bugs.webkit.org/show_bug.cgi?id=181508
Reviewed by Yusuke Suzuki.
JSTests:
* stress/for-in-prototype-with-indexed-properties-should-prevent-caching.js: Added.
(assert):
(test1.foo):
(test1):
(test2.foo):
(test2):
Source/_javascript_Core:
Our for-in caching would cache structure chains that had prototypes with
indexed properties. Clearly this is wrong. This caching breaks when a prototype
adds new indexed properties. We would continue to enumerate the old cached
state of properties, and not include the new indexed properties.
The old code used to prevent caching only if the base structure had
indexed properties. This patch extends it to prevent caching if the
base, or any structure in the prototype chain, has indexed properties.
* runtime/Structure.cpp:
(JSC::Structure::canCachePropertyNameEnumerator const):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (226766 => 226767)
--- trunk/JSTests/ChangeLog 2018-01-11 07:26:55 UTC (rev 226766)
+++ trunk/JSTests/ChangeLog 2018-01-11 08:16:06 UTC (rev 226767)
@@ -1,3 +1,17 @@
+2018-01-11 Saam Barati <sbar...@apple.com>
+
+ Our for-in caching is wrong when we add indexed properties on things in the prototype chain
+ https://bugs.webkit.org/show_bug.cgi?id=181508
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/for-in-prototype-with-indexed-properties-should-prevent-caching.js: Added.
+ (assert):
+ (test1.foo):
+ (test1):
+ (test2.foo):
+ (test2):
+
2018-01-09 Mark Lam <mark....@apple.com>
ASSERTION FAILED: pair.second->m_type & PropertyNode::Getter
Added: trunk/JSTests/stress/for-in-prototype-with-indexed-properties-should-prevent-caching.js (0 => 226767)
--- trunk/JSTests/stress/for-in-prototype-with-indexed-properties-should-prevent-caching.js (rev 0)
+++ trunk/JSTests/stress/for-in-prototype-with-indexed-properties-should-prevent-caching.js 2018-01-11 08:16:06 UTC (rev 226767)
@@ -0,0 +1,77 @@
+"use strict";
+
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+
+
+function test1() {
+ function foo(o) {
+ let result = [];
+ for (let p in o)
+ result.push(p);
+ return result;
+ }
+ noInline(foo);
+
+ let p = {};
+ let x = {__proto__: p};
+ p[0] = 25;
+ for (let i = 0; i < 20; ++i) {
+ let result = foo(x);
+ assert(result.length === 1);
+ assert(result[0] === "0");
+ }
+
+ p[1] = 30;
+ for (let i = 0; i < 20; ++i) {
+ let result = foo(x);
+ assert(result.length === 2);
+ assert(result[0] === "0");
+ assert(result[1] === "1");
+ }
+
+ p[2] = {};
+ for (let i = 0; i < 20; ++i) {
+ let result = foo(x);
+ assert(result.length === 3);
+ assert(result[0] === "0");
+ assert(result[1] === "1");
+ assert(result[2] === "2");
+ }
+}
+test1();
+
+function test2() {
+ function foo(o) {
+ let result = [];
+ for (let p in o)
+ result.push(p);
+ return result;
+ }
+ noInline(foo);
+
+ let p = {};
+ let x = {__proto__: p};
+ for (let i = 0; i < 20; ++i) {
+ let result = foo(x);
+ assert(result.length === 0);
+ }
+
+ p[0] = 30;
+ for (let i = 0; i < 20; ++i) {
+ let result = foo(x);
+ assert(result.length === 1);
+ assert(result[0] === "0");
+ }
+
+ p[1] = {};
+ for (let i = 0; i < 20; ++i) {
+ let result = foo(x);
+ assert(result.length === 2);
+ assert(result[0] === "0");
+ assert(result[1] === "1");
+ }
+}
+test2();
Modified: trunk/Source/_javascript_Core/ChangeLog (226766 => 226767)
--- trunk/Source/_javascript_Core/ChangeLog 2018-01-11 07:26:55 UTC (rev 226766)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-01-11 08:16:06 UTC (rev 226767)
@@ -1,3 +1,22 @@
+2018-01-11 Saam Barati <sbar...@apple.com>
+
+ Our for-in caching is wrong when we add indexed properties on things in the prototype chain
+ https://bugs.webkit.org/show_bug.cgi?id=181508
+
+ Reviewed by Yusuke Suzuki.
+
+ Our for-in caching would cache structure chains that had prototypes with
+ indexed properties. Clearly this is wrong. This caching breaks when a prototype
+ adds new indexed properties. We would continue to enumerate the old cached
+ state of properties, and not include the new indexed properties.
+
+ The old code used to prevent caching only if the base structure had
+ indexed properties. This patch extends it to prevent caching if the
+ base, or any structure in the prototype chain, has indexed properties.
+
+ * runtime/Structure.cpp:
+ (JSC::Structure::canCachePropertyNameEnumerator const):
+
2018-01-10 JF Bastien <jfbast...@apple.com>
Poison small JSObject derivatives which only contain pointers
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (226766 => 226767)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2018-01-11 07:26:55 UTC (rev 226766)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2018-01-11 08:16:06 UTC (rev 226767)
@@ -1246,26 +1246,31 @@
bool Structure::canCachePropertyNameEnumerator() const
{
- if (isDictionary())
- return false;
+ auto canCache = [] (const Structure* structure) {
+ if (structure->isDictionary())
+ return false;
+ if (hasIndexedProperties(structure->indexingType()))
+ return false;
+ if (structure->typeInfo().overridesGetPropertyNames())
+ return false;
+ return true;
+ };
- if (hasIndexedProperties(indexingType()))
+ if (!canCache(this))
return false;
- if (typeInfo().overridesGetPropertyNames())
- return false;
-
StructureChain* structureChain = m_cachedPrototypeChain.get();
ASSERT(structureChain);
WriteBarrier<Structure>* structure = structureChain->head();
while (true) {
if (!structure->get())
- break;
- if (structure->get()->isDictionary() || structure->get()->typeInfo().overridesGetPropertyNames())
+ return true;
+ if (!canCache(structure->get()))
return false;
structure++;
}
-
+
+ ASSERT_NOT_REACHED();
return true;
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
