Title: [202936] trunk/Source/_javascript_Core
- Revision
- 202936
- Author
- sbar...@apple.com
- Date
- 2016-07-07 15:15:04 -0700 (Thu, 07 Jul 2016)
Log Message
ToThis constant folding in DFG is incorrect when the structure indicates that toThis is overridden
https://bugs.webkit.org/show_bug.cgi?id=159501
<rdar://problem/27109354>
Reviewed by Mark Lam.
We *cannot* constant fold ToThis when the structure of an object
indicates that toThis() is overridden. isToThisAnIdentity() inside
AbstractInterpreterInlines accidentally wrote the opposite rule.
The rule was written as we can constant fold ToThis only when
toThis() is overridden. To fix the bug, we must write the rule
as isToThisAnIdentity() can only be true as long as the structure
set indicates that no structures override toThis().
We could probably get more clever in the future and notice
when we're dealing with a constant |this| values. For example,
a ToThis might occur on a constant JSLexicalEnvironment. We could
implement the rules of JSLexicalEnvironment's toThis() implementation
inside AI/constant folding.
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::isToThisAnIdentity):
* tests/stress/to-this-on-constant-lexical-environment.js: Added.
(foo.bar):
(foo.inner):
(foo):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (202935 => 202936)
--- trunk/Source/_javascript_Core/ChangeLog 2016-07-07 21:49:41 UTC (rev 202935)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-07-07 22:15:04 UTC (rev 202936)
@@ -1,3 +1,32 @@
+2016-07-07 Saam Barati <sbar...@apple.com>
+
+ ToThis constant folding in DFG is incorrect when the structure indicates that toThis is overridden
+ https://bugs.webkit.org/show_bug.cgi?id=159501
+ <rdar://problem/27109354>
+
+ Reviewed by Mark Lam.
+
+ We *cannot* constant fold ToThis when the structure of an object
+ indicates that toThis() is overridden. isToThisAnIdentity() inside
+ AbstractInterpreterInlines accidentally wrote the opposite rule.
+ The rule was written as we can constant fold ToThis only when
+ toThis() is overridden. To fix the bug, we must write the rule
+ as isToThisAnIdentity() can only be true as long as the structure
+ set indicates that no structures override toThis().
+
+ We could probably get more clever in the future and notice
+ when we're dealing with a constant |this| values. For example,
+ a ToThis might occur on a constant JSLexicalEnvironment. We could
+ implement the rules of JSLexicalEnvironment's toThis() implementation
+ inside AI/constant folding.
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::isToThisAnIdentity):
+ * tests/stress/to-this-on-constant-lexical-environment.js: Added.
+ (foo.bar):
+ (foo.inner):
+ (foo):
+
2016-07-07 Benjamin Poulain <benja...@webkit.org>
[JSC] Array.prototype.includes uses ToInt32 instead of ToInteger on the index argument
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (202935 => 202936)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2016-07-07 21:49:41 UTC (rev 202935)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2016-07-07 22:15:04 UTC (rev 202936)
@@ -167,7 +167,7 @@
if (type.isObject() && type.overridesToThis())
overridesToThis = true;
});
- return overridesToThis;
+ return !overridesToThis;
}
return false;
Added: trunk/Source/_javascript_Core/tests/stress/to-this-on-constant-lexical-environment.js (0 => 202936)
--- trunk/Source/_javascript_Core/tests/stress/to-this-on-constant-lexical-environment.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/to-this-on-constant-lexical-environment.js 2016-07-07 22:15:04 UTC (rev 202936)
@@ -0,0 +1,19 @@
+"use strict";
+
+function foo() {
+ function bar(i) {
+ return this;
+ }
+ function inner() {
+ let result;
+ for (let i = 0; i < 1000000; i++)
+ result = bar(i);
+ return result;
+ }
+ noInline(inner);
+ return inner();
+}
+
+let result = foo();
+if (result !== undefined)
+ throw new Error("Bad result");
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
