Title: [267032] trunk
- Revision
- 267032
- Author
- [email protected]
- Date
- 2020-09-14 13:06:24 -0700 (Mon, 14 Sep 2020)
Log Message
Remove bogus asserts in FTLLower that assume programs are compiled with sensible speculations
https://bugs.webkit.org/show_bug.cgi?id=216485
<rdar://problem/68562804>
Reviewed by Keith Miller.
JSTests:
* stress/ftl-should-not-assume-speculations-are-sensible.js: Added.
(foo):
Source/_javascript_Core:
We had an assert inside lowCell that if a value was not part of the JSValue
hashmap of values, then the type must not conform to being a cell. However,
consider a program like this:
```
x = ArithAdd(i32, i32) <-- x is an i32 here
if (b) {
Check(Cell:@x)
ArrayifyToStructure(@x, thingy)
}
<-- HERE
```
@x will live in FTLLower's i32 hashmap, but because of the AI rule for
ArrayifyToStructure, it will also have SpecCell in its type. This is totally
valid, and asserting that this isn't possible is wrong. (Obviously the above
speculation is stupid, as we will always exit at the Check, but it's valid IR.)
This patch removes this assertion from lowCell, and removes similar assertions
from other low* functions.
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::lowInt32):
(JSC::FTL::DFG::LowerDFGToB3::lowInt52):
(JSC::FTL::DFG::LowerDFGToB3::lowCell):
(JSC::FTL::DFG::LowerDFGToB3::lowBoolean):
(JSC::FTL::DFG::LowerDFGToB3::lowDouble):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (267031 => 267032)
--- trunk/JSTests/ChangeLog 2020-09-14 19:54:19 UTC (rev 267031)
+++ trunk/JSTests/ChangeLog 2020-09-14 20:06:24 UTC (rev 267032)
@@ -1,3 +1,14 @@
+2020-09-14 Saam Barati <[email protected]>
+
+ Remove bogus asserts in FTLLower that assume programs are compiled with sensible speculations
+ https://bugs.webkit.org/show_bug.cgi?id=216485
+ <rdar://problem/68562804>
+
+ Reviewed by Keith Miller.
+
+ * stress/ftl-should-not-assume-speculations-are-sensible.js: Added.
+ (foo):
+
2020-09-14 Alexey Shvayka <[email protected]>
Make a few built-in methods throw if called as top-level functions
Added: trunk/JSTests/stress/ftl-should-not-assume-speculations-are-sensible.js (0 => 267032)
--- trunk/JSTests/stress/ftl-should-not-assume-speculations-are-sensible.js (rev 0)
+++ trunk/JSTests/stress/ftl-should-not-assume-speculations-are-sensible.js 2020-09-14 20:06:24 UTC (rev 267032)
@@ -0,0 +1,18 @@
+let a3 = {z: 1000};
+let a1 = [];
+a1[0] = 0;
+
+function foo(x, y) {
+ 'use strict';
+ for (let i = 0; i < 20; ++i) {
+ for (let j = 0; j < x.z; ++j) {
+ foo(0, i);
+ y[0] = undefined;
+ }
+ }
+}
+
+foo(a3, []);
+for (let i = 0; i < 2; ++i) {
+ foo(a3, a1);
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (267031 => 267032)
--- trunk/Source/_javascript_Core/ChangeLog 2020-09-14 19:54:19 UTC (rev 267031)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-09-14 20:06:24 UTC (rev 267032)
@@ -1,3 +1,39 @@
+2020-09-14 Saam Barati <[email protected]>
+
+ Remove bogus asserts in FTLLower that assume programs are compiled with sensible speculations
+ https://bugs.webkit.org/show_bug.cgi?id=216485
+ <rdar://problem/68562804>
+
+ Reviewed by Keith Miller.
+
+ We had an assert inside lowCell that if a value was not part of the JSValue
+ hashmap of values, then the type must not conform to being a cell. However,
+ consider a program like this:
+
+ ```
+ x = ArithAdd(i32, i32) <-- x is an i32 here
+ if (b) {
+ Check(Cell:@x)
+ ArrayifyToStructure(@x, thingy)
+ }
+ <-- HERE
+ ```
+
+ @x will live in FTLLower's i32 hashmap, but because of the AI rule for
+ ArrayifyToStructure, it will also have SpecCell in its type. This is totally
+ valid, and asserting that this isn't possible is wrong. (Obviously the above
+ speculation is stupid, as we will always exit at the Check, but it's valid IR.)
+
+ This patch removes this assertion from lowCell, and removes similar assertions
+ from other low* functions.
+
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::lowInt32):
+ (JSC::FTL::DFG::LowerDFGToB3::lowInt52):
+ (JSC::FTL::DFG::LowerDFGToB3::lowCell):
+ (JSC::FTL::DFG::LowerDFGToB3::lowBoolean):
+ (JSC::FTL::DFG::LowerDFGToB3::lowDouble):
+
2020-09-14 Alexey Shvayka <[email protected]>
Make a few built-in methods throw if called as top-level functions
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (267031 => 267032)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-09-14 19:54:19 UTC (rev 267031)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-09-14 20:06:24 UTC (rev 267032)
@@ -17015,7 +17015,6 @@
return result;
}
- DFG_ASSERT(m_graph, m_node, !(provenType(edge) & SpecInt32Only), provenType(edge));
if (mayHaveTypeCheck(edge.useKind()))
terminate(Uncountable);
return m_out.int32Zero;
@@ -17050,7 +17049,6 @@
break;
}
- DFG_ASSERT(m_graph, m_node, !provenType(edge), provenType(edge));
if (mayHaveTypeCheck(edge.useKind()))
terminate(Uncountable);
return m_out.int64Zero;
@@ -17122,7 +17120,6 @@
return uncheckedValue;
}
- DFG_ASSERT(m_graph, m_node, !(provenType(edge) & SpecCellCheck), provenType(edge));
if (mayHaveTypeCheck(edge.useKind()))
terminate(Uncountable);
return m_out.intPtrZero;
@@ -17283,7 +17280,6 @@
return result;
}
- DFG_ASSERT(m_graph, m_node, !(provenType(edge) & SpecBoolean), provenType(edge));
if (mayHaveTypeCheck(edge.useKind()))
terminate(Uncountable);
return m_out.booleanFalse;
@@ -17296,7 +17292,6 @@
LoweredNodeValue value = m_doubleValues.get(edge.node());
if (isValid(value))
return value.value();
- DFG_ASSERT(m_graph, m_node, !provenType(edge), provenType(edge));
if (mayHaveTypeCheck(edge.useKind()))
terminate(Uncountable);
return m_out.doubleZero;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
