Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(58)

Unified Diff: src/compiler/escape-analysis.cc

Issue 2459273002: [turbofan] Properly deal with out-of-bounds fields in EscapeAnalysis. (Closed)
Patch Set: Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | test/mjsunit/regress/regress-crbug-658185.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/compiler/escape-analysis.cc
diff --git a/src/compiler/escape-analysis.cc b/src/compiler/escape-analysis.cc
index a190a5adc0ecb14cf0c0996765be9a16ee050990..f175bd5e81af93ae01cace9451f2c91a96c3943c 100644
--- a/src/compiler/escape-analysis.cc
+++ b/src/compiler/escape-analysis.cc
@@ -1399,7 +1399,20 @@ void EscapeAnalysis::ProcessLoadField(Node* node) {
if (VirtualObject* object = GetVirtualObject(state, from)) {
if (!object->IsTracked()) return;
int offset = OffsetForFieldAccess(node);
- if (static_cast<size_t>(offset) >= object->field_count()) return;
+ if (static_cast<size_t>(offset) >= object->field_count()) {
+ // We have a load from a field that is not inside the {object}. This
+ // can only happen with conflicting type feedback and for dead {node}s.
+ // For now, we just mark the {object} as escaping.
+ // TODO(turbofan): Consider introducing an Undefined or None operator
+ // that we can replace this load with, since we know it's dead code.
+ if (status_analysis_->SetEscaped(from)) {
+ TRACE(
+ "Setting #%d (%s) to escaped because load field #%d from "
+ "offset %d outside of object\n",
+ from->id(), from->op()->mnemonic(), node->id(), offset);
+ }
+ return;
+ }
Node* value = object->GetField(offset);
if (value) {
value = ResolveReplacement(value);
@@ -1464,7 +1477,20 @@ void EscapeAnalysis::ProcessStoreField(Node* node) {
if (VirtualObject* object = GetVirtualObject(state, to)) {
if (!object->IsTracked()) return;
int offset = OffsetForFieldAccess(node);
- if (static_cast<size_t>(offset) >= object->field_count()) return;
+ if (static_cast<size_t>(offset) >= object->field_count()) {
+ // We have a store to a field that is not inside the {object}. This
+ // can only happen with conflicting type feedback and for dead {node}s.
+ // For now, we just mark the {object} as escaping.
+ // TODO(turbofan): Consider just eliminating the store in the reducer
+ // pass, as it's dead code anyways.
+ if (status_analysis_->SetEscaped(to)) {
+ TRACE(
+ "Setting #%d (%s) to escaped because store field #%d to "
+ "offset %d outside of object\n",
+ to->id(), to->op()->mnemonic(), node->id(), offset);
+ }
+ return;
+ }
Node* val = ResolveReplacement(NodeProperties::GetValueInput(node, 1));
// TODO(mstarzinger): The following is a workaround to not track some well
// known raw fields. We only ever store default initial values into these
« no previous file with comments | « no previous file | test/mjsunit/regress/regress-crbug-658185.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698