From 7201bad99d16c0e4e9f33ebc14ddb80446b06a51 Mon Sep 17 00:00:00 2001
From: bmeurer <bmeurer@chromium.org>
Date: Sun, 30 Oct 2016 23:43:04 -0700
Subject: [PATCH] [turbofan] Properly deal with out-of-bounds fields in
 EscapeAnalysis.

Conflicting type feedback on Load/StoreICs can lead to out-of-bounds
field access, which is essentially dead code, but EscapeAnalysis was
confused about those. For now, mark the objects as escaping in these
cases, middle-term we want to deal better with this kind of compile-
time known dead code.

BUG=chromium:658185,v8:4586
R=jarin@chromium.org

Review-Url: https://codereview.chromium.org/2459273002
Cr-Commit-Position: refs/heads/master@{#40662}
---
 src/compiler/escape-analysis.cc              | 30 ++++++++++++++++++--
 test/mjsunit/regress/regress-crbug-658185.js | 20 +++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)
 create mode 100644 test/mjsunit/regress/regress-crbug-658185.js

diff --git a/src/compiler/escape-analysis.cc b/src/compiler/escape-analysis.cc
index a190a5adc0..f175bd5e81 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
diff --git a/test/mjsunit/regress/regress-crbug-658185.js b/test/mjsunit/regress/regress-crbug-658185.js
new file mode 100644
index 0000000000..60de8d6458
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-658185.js
@@ -0,0 +1,20 @@
+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --turbo-escape
+
+var t = 0;
+function foo() {
+  var o = {x:1};
+  var p = {y:2.5, x:0};
+  o = [];
+  for (var i = 0; i < 2; ++i) {
+    t = o.x;
+    o = p;
+  }
+}
+foo();
+foo();
+%OptimizeFunctionOnNextCall(foo);
+foo();