Title: [203793] trunk/Source/_javascript_Core
- Revision
- 203793
- Author
- [email protected]
- Date
- 2016-07-27 14:11:09 -0700 (Wed, 27 Jul 2016)
Log Message
We don't optimize for-in properly in baseline JIT (maybe other JITs too) with an object with symbols
https://bugs.webkit.org/show_bug.cgi?id=160211
<rdar://problem/27572612>
Reviewed by Geoffrey Garen.
The fast for-in iteration mode assumes all inline/out-of-line properties
can be iterated in linear order. This is not true if we have Symbols
because Symbols should not be iterated by for-in.
* runtime/Structure.cpp:
(JSC::Structure::add):
* tests/stress/symbol-should-not-break-for-in.js: Added.
(assert):
(foo):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (203792 => 203793)
--- trunk/Source/_javascript_Core/ChangeLog 2016-07-27 21:10:23 UTC (rev 203792)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-07-27 21:11:09 UTC (rev 203793)
@@ -1,3 +1,21 @@
+2016-07-27 Saam Barati <[email protected]>
+
+ We don't optimize for-in properly in baseline JIT (maybe other JITs too) with an object with symbols
+ https://bugs.webkit.org/show_bug.cgi?id=160211
+ <rdar://problem/27572612>
+
+ Reviewed by Geoffrey Garen.
+
+ The fast for-in iteration mode assumes all inline/out-of-line properties
+ can be iterated in linear order. This is not true if we have Symbols
+ because Symbols should not be iterated by for-in.
+
+ * runtime/Structure.cpp:
+ (JSC::Structure::add):
+ * tests/stress/symbol-should-not-break-for-in.js: Added.
+ (assert):
+ (foo):
+
2016-07-27 Mark Lam <[email protected]>
The second argument for Function.prototype.apply should be array-like or null/undefined.
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (203792 => 203793)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2016-07-27 21:10:23 UTC (rev 203792)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2016-07-27 21:11:09 UTC (rev 203793)
@@ -995,7 +995,7 @@
ASSERT(!JSC::isValidOffset(get(vm, propertyName)));
checkConsistency();
- if (attributes & DontEnum)
+ if (attributes & DontEnum || propertyName.isSymbol())
setIsQuickPropertyAccessAllowedForEnumeration(false);
auto rep = propertyName.uid();
@@ -1055,6 +1055,7 @@
PropertyTable::iterator end = propertyTable()->end();
for (PropertyTable::iterator iter = propertyTable()->begin(); iter != end; ++iter) {
ASSERT(!isQuickPropertyAccessAllowedForEnumeration() || !(iter->attributes & DontEnum));
+ ASSERT(!isQuickPropertyAccessAllowedForEnumeration() || !iter->key->isSymbol());
if (!(iter->attributes & DontEnum) || mode.includeDontEnumProperties()) {
if (iter->key->isSymbol() && !propertyNames.includeSymbolProperties())
continue;
@@ -1346,6 +1347,7 @@
PropertyTable::iterator end = propertyTable()->end();
for (PropertyTable::iterator iter = propertyTable()->begin(); iter != end; ++iter) {
ASSERT(!(iter->attributes & DontEnum));
+ ASSERT(!iter->key->isSymbol());
}
}
Added: trunk/Source/_javascript_Core/tests/stress/symbol-should-not-break-for-in.js (0 => 203793)
--- trunk/Source/_javascript_Core/tests/stress/symbol-should-not-break-for-in.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/symbol-should-not-break-for-in.js 2016-07-27 21:11:09 UTC (rev 203793)
@@ -0,0 +1,29 @@
+function assert(b) {
+ if (!b)
+ throw new Error("bad assertion.");
+}
+
+function foo(o) {
+ let r = [];
+ for (let p in o)
+ r.push(o[p]);
+ return r;
+}
+noInline(foo);
+
+let o = {};
+o[Symbol()] = "symbol";
+o.prop = "prop";
+for (let i = 0; i < 1000; i++) {
+ let arr = foo(o);
+ assert(arr.length === 1);
+ assert(arr[0] === "prop");
+}
+
+o.prop2 = "prop2";
+for (let i = 0; i < 1000; i++) {
+ let arr = foo(o);
+ assert(arr.length === 2);
+ assert(arr[0] === "prop");
+ assert(arr[1] === "prop2");
+}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
