- Revision
- 254906
- Author
- [email protected]
- Date
- 2020-01-22 02:26:33 -0800 (Wed, 22 Jan 2020)
Log Message
Merge r250585 - ObjectAllocationSinkingPhase shouldn't insert hints for allocations which are no longer valid
https://bugs.webkit.org/show_bug.cgi?id=199361
<rdar://problem/52454940>
Reviewed by Yusuke Suzuki.
JSTests:
* stress/allocation-sinking-hints-are-valid-ssa-2.js: Added.
(main.fn):
(main.executor):
(main):
* stress/allocation-sinking-hints-are-valid-ssa.js: Added.
(main.fn):
(main.executor):
(main):
Source/_javascript_Core:
In a prior fix to the object allocation sinking phase, I added code where we
made sure to insert PutHints over Phis for fields of an object at control flow
merge points. However, that code didn't consider that the base of the PutHint
may no longer be a valid heap location. This could cause us to emit invalid
SSA code by referring to a node which does not dominate the PutHint location.
This patch fixes the bug to only emit the PutHints when valid.
This patch also makes it so that DFGValidate actually validates that the graph
is in valid SSA form. E.g, any use of a node N must be dominated by N.
* dfg/DFGObjectAllocationSinkingPhase.cpp:
* dfg/DFGValidate.cpp:
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.26/JSTests/ChangeLog (254905 => 254906)
--- releases/WebKitGTK/webkit-2.26/JSTests/ChangeLog 2020-01-22 10:19:44 UTC (rev 254905)
+++ releases/WebKitGTK/webkit-2.26/JSTests/ChangeLog 2020-01-22 10:26:33 UTC (rev 254906)
@@ -1,3 +1,20 @@
+2019-10-01 Saam Barati <[email protected]>
+
+ ObjectAllocationSinkingPhase shouldn't insert hints for allocations which are no longer valid
+ https://bugs.webkit.org/show_bug.cgi?id=199361
+ <rdar://problem/52454940>
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/allocation-sinking-hints-are-valid-ssa-2.js: Added.
+ (main.fn):
+ (main.executor):
+ (main):
+ * stress/allocation-sinking-hints-are-valid-ssa.js: Added.
+ (main.fn):
+ (main.executor):
+ (main):
+
2019-09-16 Saam Barati <[email protected]>
JSObject::putInlineSlow should not ignore "__proto__" for Proxy
Added: releases/WebKitGTK/webkit-2.26/JSTests/stress/allocation-sinking-hints-are-valid-ssa-2.js (0 => 254906)
--- releases/WebKitGTK/webkit-2.26/JSTests/stress/allocation-sinking-hints-are-valid-ssa-2.js (rev 0)
+++ releases/WebKitGTK/webkit-2.26/JSTests/stress/allocation-sinking-hints-are-valid-ssa-2.js 2020-01-22 10:26:33 UTC (rev 254906)
@@ -0,0 +1,31 @@
+function main() {
+ const arr = [0];
+ function executor(resolve, ...reject) {
+ arr;
+ if (resolve > arr) {
+ const fn = () => {
+ return fn;
+ };
+ for (const _ of arr) {
+ function fn() {}
+ arr.toString(arr, arr, arr, arr, arr, arr);
+ throw new Error();
+ }
+ } else {
+ for (const _ of [arr]) {
+ arr.toString();
+ }
+ const fn = () => {};
+ }
+ new Promise(executor, arr);
+ let some = {};
+ with(arr) {}
+ return reject;
+ }
+ executor();
+
+ for (let i = 0; i < 100; i++) {
+ executor();
+ }
+}
+main();
Added: releases/WebKitGTK/webkit-2.26/JSTests/stress/allocation-sinking-hints-are-valid-ssa.js (0 => 254906)
--- releases/WebKitGTK/webkit-2.26/JSTests/stress/allocation-sinking-hints-are-valid-ssa.js (rev 0)
+++ releases/WebKitGTK/webkit-2.26/JSTests/stress/allocation-sinking-hints-are-valid-ssa.js 2020-01-22 10:26:33 UTC (rev 254906)
@@ -0,0 +1,31 @@
+function main() {
+ const arr = [0];
+ function executor(resolve, ...reject) {
+ arr;
+ if (resolve > arr) {
+ const fn = () => {
+ return fn;
+ };
+ for (const _ of arr) {
+ function fn() {}
+ arr.toString(arr, arr, arr, arr, arr, arr);
+ throw new Error();
+ }
+ } else {
+ for (const _ of [arr]) {
+ arr.toString();
+ }
+ const fn = () => {};
+ }
+ new Promise(executor, arr);
+ let some = {};
+ with(arr) {}
+ return reject;
+ }
+ executor();
+
+ for (let i = 0; i < 100; i++) {
+ executor();
+ }
+}
+main();
Modified: releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/ChangeLog (254905 => 254906)
--- releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/ChangeLog 2020-01-22 10:19:44 UTC (rev 254905)
+++ releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/ChangeLog 2020-01-22 10:26:33 UTC (rev 254906)
@@ -1,3 +1,24 @@
+2019-10-01 Saam Barati <[email protected]>
+
+ ObjectAllocationSinkingPhase shouldn't insert hints for allocations which are no longer valid
+ https://bugs.webkit.org/show_bug.cgi?id=199361
+ <rdar://problem/52454940>
+
+ Reviewed by Yusuke Suzuki.
+
+ In a prior fix to the object allocation sinking phase, I added code where we
+ made sure to insert PutHints over Phis for fields of an object at control flow
+ merge points. However, that code didn't consider that the base of the PutHint
+ may no longer be a valid heap location. This could cause us to emit invalid
+ SSA code by referring to a node which does not dominate the PutHint location.
+ This patch fixes the bug to only emit the PutHints when valid.
+
+ This patch also makes it so that DFGValidate actually validates that the graph
+ is in valid SSA form. E.g, any use of a node N must be dominated by N.
+
+ * dfg/DFGObjectAllocationSinkingPhase.cpp:
+ * dfg/DFGValidate.cpp:
+
2019-09-16 Saam Barati <[email protected]>
JSObject::putInlineSlow should not ignore "__proto__" for Proxy
Modified: releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp (254905 => 254906)
--- releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp 2020-01-22 10:19:44 UTC (rev 254905)
+++ releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/dfg/DFGObjectAllocationSinkingPhase.cpp 2020-01-22 10:26:33 UTC (rev 254906)
@@ -1815,9 +1815,11 @@
availabilityCalculator.m_availability, identifier, phiDef->value());
for (PromotedHeapLocation location : hintsForPhi[variable->index()]) {
- m_insertionSet.insert(0,
- location.createHint(m_graph, block->at(0)->origin.withInvalidExit(), phiDef->value()));
- m_localMapping.set(location, phiDef->value());
+ if (m_heap.onlyLocalAllocation(location.base())) {
+ m_insertionSet.insert(0,
+ location.createHint(m_graph, block->at(0)->origin.withInvalidExit(), phiDef->value()));
+ m_localMapping.set(location, phiDef->value());
+ }
}
}
Modified: releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/dfg/DFGValidate.cpp (254905 => 254906)
--- releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/dfg/DFGValidate.cpp 2020-01-22 10:19:44 UTC (rev 254905)
+++ releases/WebKitGTK/webkit-2.26/Source/_javascript_Core/dfg/DFGValidate.cpp 2020-01-22 10:26:33 UTC (rev 254906)
@@ -31,6 +31,7 @@
#include "CodeBlockWithJITType.h"
#include "DFGClobberize.h"
#include "DFGClobbersExitState.h"
+#include "DFGDominators.h"
#include "DFGMayExit.h"
#include "JSCInlines.h"
#include <wtf/Assertions.h>
@@ -775,6 +776,10 @@
VALIDATE((), !m_graph.m_argumentFormats.isEmpty()); // We always have at least one entrypoint.
VALIDATE((), m_graph.m_rootToArguments.isEmpty()); // This is only used in CPS.
+ m_graph.initializeNodeOwners();
+
+ auto& dominators = m_graph.ensureSSADominators();
+
for (unsigned entrypointIndex : m_graph.m_entrypointIndexToCatchBytecodeOffset.keys())
VALIDATE((), entrypointIndex > 0); // By convention, 0 is the entrypoint index for the op_enter entrypoint, which can not be in a catch.
@@ -788,6 +793,8 @@
bool didSeeExitOK = false;
bool isOSRExited = false;
+ HashSet<Node*> nodesInThisBlock;
+
for (auto* node : *block) {
didSeeExitOK |= node->origin.exitOK;
switch (node->op()) {
@@ -906,7 +913,13 @@
});
break;
}
+
isOSRExited |= node->isPseudoTerminal();
+
+ m_graph.doToChildren(node, [&] (Edge child) {
+ VALIDATE((node), dominators.strictlyDominates(child->owner, block) || nodesInThisBlock.contains(child.node()));
+ });
+ nodesInThisBlock.add(node);
}
}
}
