Log Message
Cherry-pick r266813. rdar://problem/69582551
OutOfBoundsSaneChain operations should use their own heap locations
https://bugs.webkit.org/show_bug.cgi?id=216328
<rdar://problem/68568039>
Reviewed by Keith Miller.
JSTests:
* stress/out-of-bounds-sane-chain-need-their-own-heap-location.js: Added.
(foo):
Source/_javascript_Core:
There is code in local CSE that does some basic bounds check elimination
for PutByVal. It does this analysis by seeing if a particular heap location
is already defined, and if so, it eliminates the bounds check for the
PutByVal. This doesn't work for OutOfBoundsSaneChain for the obvious reason
that these GetByVals are not proven to be in bounds. So GetByVal's in the
OutOfBoundsSaneChain mode reusing non OutOfBoundsSaneChain heap locations
can lead to a bug where we mistakenly remove a bounds check. The fix is to
have all OutOfBoundsSaneChain operations use distinct heaps, and for CSE to
not query those heaps.
* dfg/DFGArrayMode.h:
(JSC::DFG::ArrayMode::isAnySaneChain const): Deleted.
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGHeapLocation.cpp:
(WTF::printInternal):
* dfg/DFGHeapLocation.h:
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266813 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
- branches/safari-610-branch/JSTests/ChangeLog
- branches/safari-610-branch/Source/_javascript_Core/ChangeLog
- branches/safari-610-branch/Source/_javascript_Core/dfg/DFGArrayMode.h
- branches/safari-610-branch/Source/_javascript_Core/dfg/DFGClobberize.h
- branches/safari-610-branch/Source/_javascript_Core/dfg/DFGHeapLocation.cpp
- branches/safari-610-branch/Source/_javascript_Core/dfg/DFGHeapLocation.h
Added Paths
Diff
Modified: branches/safari-610-branch/JSTests/ChangeLog (267584 => 267585)
--- branches/safari-610-branch/JSTests/ChangeLog 2020-09-25 18:21:00 UTC (rev 267584)
+++ branches/safari-610-branch/JSTests/ChangeLog 2020-09-25 18:21:03 UTC (rev 267585)
@@ -1,3 +1,52 @@
+2020-09-25 Alan Coon <[email protected]>
+
+ Cherry-pick r266813. rdar://problem/69582551
+
+ OutOfBoundsSaneChain operations should use their own heap locations
+ https://bugs.webkit.org/show_bug.cgi?id=216328
+ <rdar://problem/68568039>
+
+ Reviewed by Keith Miller.
+
+ JSTests:
+
+ * stress/out-of-bounds-sane-chain-need-their-own-heap-location.js: Added.
+ (foo):
+
+ Source/_javascript_Core:
+
+ There is code in local CSE that does some basic bounds check elimination
+ for PutByVal. It does this analysis by seeing if a particular heap location
+ is already defined, and if so, it eliminates the bounds check for the
+ PutByVal. This doesn't work for OutOfBoundsSaneChain for the obvious reason
+ that these GetByVals are not proven to be in bounds. So GetByVal's in the
+ OutOfBoundsSaneChain mode reusing non OutOfBoundsSaneChain heap locations
+ can lead to a bug where we mistakenly remove a bounds check. The fix is to
+ have all OutOfBoundsSaneChain operations use distinct heaps, and for CSE to
+ not query those heaps.
+
+ * dfg/DFGArrayMode.h:
+ (JSC::DFG::ArrayMode::isAnySaneChain const): Deleted.
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGHeapLocation.cpp:
+ (WTF::printInternal):
+ * dfg/DFGHeapLocation.h:
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266813 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-09-09 Saam Barati <[email protected]>
+
+ OutOfBoundsSaneChain operations should use their own heap locations
+ https://bugs.webkit.org/show_bug.cgi?id=216328
+ <rdar://problem/68568039>
+
+ Reviewed by Keith Miller.
+
+ * stress/out-of-bounds-sane-chain-need-their-own-heap-location.js: Added.
+ (foo):
+
2020-09-18 Alan Coon <[email protected]>
Cherry-pick r266907. rdar://problem/69178126
Added: branches/safari-610-branch/JSTests/stress/out-of-bounds-sane-chain-need-their-own-heap-location.js (0 => 267585)
--- branches/safari-610-branch/JSTests/stress/out-of-bounds-sane-chain-need-their-own-heap-location.js (rev 0)
+++ branches/safari-610-branch/JSTests/stress/out-of-bounds-sane-chain-need-their-own-heap-location.js 2020-09-25 18:21:03 UTC (rev 267585)
@@ -0,0 +1,13 @@
+const a0 = [0];
+function foo() {
+ for (let i=1; i<100; i++) {
+ a0[i];
+ a0[i] = undefined;
+ let x = [];
+ for (let j = 0; j < 20; j++) {}
+ }
+}
+
+for (let i=0; i<10000; i++) {
+ foo();
+}
Modified: branches/safari-610-branch/Source/_javascript_Core/ChangeLog (267584 => 267585)
--- branches/safari-610-branch/Source/_javascript_Core/ChangeLog 2020-09-25 18:21:00 UTC (rev 267584)
+++ branches/safari-610-branch/Source/_javascript_Core/ChangeLog 2020-09-25 18:21:03 UTC (rev 267585)
@@ -1,3 +1,67 @@
+2020-09-25 Alan Coon <[email protected]>
+
+ Cherry-pick r266813. rdar://problem/69582551
+
+ OutOfBoundsSaneChain operations should use their own heap locations
+ https://bugs.webkit.org/show_bug.cgi?id=216328
+ <rdar://problem/68568039>
+
+ Reviewed by Keith Miller.
+
+ JSTests:
+
+ * stress/out-of-bounds-sane-chain-need-their-own-heap-location.js: Added.
+ (foo):
+
+ Source/_javascript_Core:
+
+ There is code in local CSE that does some basic bounds check elimination
+ for PutByVal. It does this analysis by seeing if a particular heap location
+ is already defined, and if so, it eliminates the bounds check for the
+ PutByVal. This doesn't work for OutOfBoundsSaneChain for the obvious reason
+ that these GetByVals are not proven to be in bounds. So GetByVal's in the
+ OutOfBoundsSaneChain mode reusing non OutOfBoundsSaneChain heap locations
+ can lead to a bug where we mistakenly remove a bounds check. The fix is to
+ have all OutOfBoundsSaneChain operations use distinct heaps, and for CSE to
+ not query those heaps.
+
+ * dfg/DFGArrayMode.h:
+ (JSC::DFG::ArrayMode::isAnySaneChain const): Deleted.
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGHeapLocation.cpp:
+ (WTF::printInternal):
+ * dfg/DFGHeapLocation.h:
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266813 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-09-09 Saam Barati <[email protected]>
+
+ OutOfBoundsSaneChain operations should use their own heap locations
+ https://bugs.webkit.org/show_bug.cgi?id=216328
+ <rdar://problem/68568039>
+
+ Reviewed by Keith Miller.
+
+ There is code in local CSE that does some basic bounds check elimination
+ for PutByVal. It does this analysis by seeing if a particular heap location
+ is already defined, and if so, it eliminates the bounds check for the
+ PutByVal. This doesn't work for OutOfBoundsSaneChain for the obvious reason
+ that these GetByVals are not proven to be in bounds. So GetByVal's in the
+ OutOfBoundsSaneChain mode reusing non OutOfBoundsSaneChain heap locations
+ can lead to a bug where we mistakenly remove a bounds check. The fix is to
+ have all OutOfBoundsSaneChain operations use distinct heaps, and for CSE to
+ not query those heaps.
+
+ * dfg/DFGArrayMode.h:
+ (JSC::DFG::ArrayMode::isAnySaneChain const): Deleted.
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGHeapLocation.cpp:
+ (WTF::printInternal):
+ * dfg/DFGHeapLocation.h:
+
2020-09-18 Alan Coon <[email protected]>
Cherry-pick r266907. rdar://problem/69178126
Modified: branches/safari-610-branch/Source/_javascript_Core/dfg/DFGArrayMode.h (267584 => 267585)
--- branches/safari-610-branch/Source/_javascript_Core/dfg/DFGArrayMode.h 2020-09-25 18:21:00 UTC (rev 267584)
+++ branches/safari-610-branch/Source/_javascript_Core/dfg/DFGArrayMode.h 2020-09-25 18:21:03 UTC (rev 267585)
@@ -287,11 +287,6 @@
return speculation() == Array::OutOfBoundsSaneChain;
}
- bool isAnySaneChain() const
- {
- return isInBoundsSaneChain() || isOutOfBoundsSaneChain();
- }
-
bool isOutOfBounds() const
{
return speculation() == Array::OutOfBounds || speculation() == Array::OutOfBoundsSaneChain;
Modified: branches/safari-610-branch/Source/_javascript_Core/dfg/DFGClobberize.h (267584 => 267585)
--- branches/safari-610-branch/Source/_javascript_Core/dfg/DFGClobberize.h 2020-09-25 18:21:00 UTC (rev 267584)
+++ branches/safari-610-branch/Source/_javascript_Core/dfg/DFGClobberize.h 2020-09-25 18:21:03 UTC (rev 267585)
@@ -938,7 +938,7 @@
if (mode.isInBounds() || mode.isOutOfBoundsSaneChain()) {
read(Butterfly_publicLength);
read(IndexedInt32Properties);
- LocationKind kind = mode.isOutOfBoundsSaneChain() ? IndexedPropertyInt32OrOtherLoc : indexedPropertyLoc;
+ LocationKind kind = mode.isOutOfBoundsSaneChain() ? IndexedPropertyInt32OutOfBoundsSaneChainLoc : indexedPropertyLoc;
def(HeapLocation(kind, IndexedInt32Properties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
return;
}
@@ -950,11 +950,16 @@
read(Butterfly_publicLength);
read(IndexedDoubleProperties);
LocationKind kind;
- if (node->hasDoubleResult())
- kind = mode.isAnySaneChain() ? IndexedPropertyDoubleSaneChainLoc : IndexedPropertyDoubleLoc;
- else {
+ if (node->hasDoubleResult()) {
+ if (mode.isInBoundsSaneChain())
+ kind = IndexedPropertyDoubleSaneChainLoc;
+ else if (mode.isOutOfBoundsSaneChain())
+ kind = IndexedPropertyDoubleOutOfBoundsSaneChainLoc;
+ else
+ kind = IndexedPropertyDoubleLoc;
+ } else {
ASSERT(mode.isOutOfBoundsSaneChain());
- kind = IndexedPropertyDoubleOrOtherSaneChainLoc;
+ kind = IndexedPropertyDoubleOrOtherOutOfBoundsSaneChainLoc;
}
def(HeapLocation(kind, IndexedDoubleProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
return;
@@ -966,7 +971,7 @@
if (mode.isInBounds() || mode.isOutOfBoundsSaneChain()) {
read(Butterfly_publicLength);
read(IndexedContiguousProperties);
- def(HeapLocation(indexedPropertyLoc, IndexedContiguousProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
+ def(HeapLocation(mode.isOutOfBoundsSaneChain() ? IndexedPropertyJSOutOfBoundsSaneChainLoc : indexedPropertyLoc, IndexedContiguousProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
return;
}
clobberTop();
@@ -1055,7 +1060,7 @@
if (node->arrayMode().mayStoreToHole())
write(Butterfly_publicLength);
def(HeapLocation(indexedPropertyLoc, IndexedInt32Properties, base, index), LazyNode(value));
- def(HeapLocation(IndexedPropertyInt32OrOtherLoc, IndexedInt32Properties, base, index), LazyNode(value));
+ def(HeapLocation(IndexedPropertyInt32OutOfBoundsSaneChainLoc, IndexedInt32Properties, base, index), LazyNode(value));
return;
case Array::Double:
@@ -1071,6 +1076,7 @@
write(Butterfly_publicLength);
def(HeapLocation(IndexedPropertyDoubleLoc, IndexedDoubleProperties, base, index), LazyNode(value));
def(HeapLocation(IndexedPropertyDoubleSaneChainLoc, IndexedDoubleProperties, base, index), LazyNode(value));
+ def(HeapLocation(IndexedPropertyDoubleOutOfBoundsSaneChainLoc, IndexedDoubleProperties, base, index), LazyNode(value));
return;
case Array::Contiguous:
@@ -1085,6 +1091,7 @@
if (node->arrayMode().mayStoreToHole())
write(Butterfly_publicLength);
def(HeapLocation(indexedPropertyLoc, IndexedContiguousProperties, base, index), LazyNode(value));
+ def(HeapLocation(IndexedPropertyJSOutOfBoundsSaneChainLoc, IndexedContiguousProperties, base, index), LazyNode(value));
return;
case Array::ArrayStorage:
Modified: branches/safari-610-branch/Source/_javascript_Core/dfg/DFGHeapLocation.cpp (267584 => 267585)
--- branches/safari-610-branch/Source/_javascript_Core/dfg/DFGHeapLocation.cpp 2020-09-25 18:21:00 UTC (rev 267584)
+++ branches/safari-610-branch/Source/_javascript_Core/dfg/DFGHeapLocation.cpp 2020-09-25 18:21:03 UTC (rev 267585)
@@ -142,16 +142,20 @@
out.print("IndexedPropertyDoubleSaneChainLoc");
return;
- case IndexedPropertyDoubleOrOtherSaneChainLoc:
- out.print("IndexedPropertyDoubleOrOtherSaneChainLoc");
+ case IndexedPropertyDoubleOutOfBoundsSaneChainLoc:
+ out.print("IndexedPropertyDoubleOutOfBoundsSaneChainLoc");
return;
+ case IndexedPropertyDoubleOrOtherOutOfBoundsSaneChainLoc:
+ out.print("IndexedPropertyDoubleOrOtherOutOfBoundsSaneChainLoc");
+ return;
+
case IndexedPropertyInt32Loc:
out.print("IndexedPropertyInt32Loc");
return;
- case IndexedPropertyInt32OrOtherLoc:
- out.print("IndexedPropertyInt32OrOtherLoc");
+ case IndexedPropertyInt32OutOfBoundsSaneChainLoc:
+ out.print("IndexedPropertyInt32OutOfBoundsSaneChainLoc");
return;
case IndexedPropertyInt52Loc:
@@ -162,6 +166,10 @@
out.print("IndexedPropertyJSLoc");
return;
+ case IndexedPropertyJSOutOfBoundsSaneChainLoc:
+ out.print("IndexedPropertyJSOutOfBoundsSaneChainLoc");
+ return;
+
case IndexedPropertyStorageLoc:
out.print("IndexedPropertyStorageLoc");
return;
Modified: branches/safari-610-branch/Source/_javascript_Core/dfg/DFGHeapLocation.h (267584 => 267585)
--- branches/safari-610-branch/Source/_javascript_Core/dfg/DFGHeapLocation.h 2020-09-25 18:21:00 UTC (rev 267584)
+++ branches/safari-610-branch/Source/_javascript_Core/dfg/DFGHeapLocation.h 2020-09-25 18:21:03 UTC (rev 267585)
@@ -49,10 +49,12 @@
HasIndexedPropertyLoc,
IndexedPropertyDoubleLoc,
IndexedPropertyDoubleSaneChainLoc,
- IndexedPropertyDoubleOrOtherSaneChainLoc,
+ IndexedPropertyDoubleOutOfBoundsSaneChainLoc,
+ IndexedPropertyDoubleOrOtherOutOfBoundsSaneChainLoc,
IndexedPropertyInt32Loc,
- IndexedPropertyInt32OrOtherLoc,
+ IndexedPropertyInt32OutOfBoundsSaneChainLoc,
IndexedPropertyInt52Loc,
+ IndexedPropertyJSOutOfBoundsSaneChainLoc,
IndexedPropertyJSLoc,
IndexedPropertyStorageLoc,
InvalidationPointLoc,
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
