Handle arguments objects in frame when materializing arguments

R=mstarzinger@chromium.org
BUG=347262

Review URL: https://codereview.chromium.org/177293009

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19584 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/deoptimizer.cc

@@ -2998,8 +2998,7 @@ SlotRef SlotRefValueBuilder::ComputeSlotForNextArgument(
     }
 
     case Translation::ARGUMENTS_OBJECT:
-      // This can be only emitted for local slots not for argument slots.
-      break;
+      return SlotRef::NewArgumentsObject(iterator->Next());
 
     case Translation::CAPTURED_OBJECT: {
       return SlotRef::NewDeferredObject(iterator->Next());
@@ -3049,7 +3048,7 @@ SlotRef SlotRefValueBuilder::ComputeSlotForNextArgument(
       break;
   }
 
-  UNREACHABLE();
+  FATAL("We should never get here - unexpected deopt info.");
   return SlotRef();
 }
 
@@ -3129,9 +3128,8 @@ SlotRefValueBuilder::SlotRefValueBuilder(JavaScriptFrame* frame,
         // the nested slots of captured objects
         number_of_slots--;
         SlotRef& slot = slot_refs_.last();
-        if (slot.Representation() == SlotRef::DEFERRED_OBJECT) {
-          number_of_slots += slot.DeferredObjectLength();
-        }
+        ASSERT(slot.Representation() != SlotRef::ARGUMENTS_OBJECT);
+        number_of_slots += slot.GetChildrenCount();
         if (slot.Representation() == SlotRef::DEFERRED_OBJECT ||
             slot.Representation() == SlotRef::DUPLICATE_OBJECT) {
           should_deopt = true;
@@ -3185,7 +3183,7 @@ Handle<Object> SlotRef::GetValue(Isolate* isolate) {
       return literal_;
 
     default:
-      UNREACHABLE();
+      FATAL("We should never get here - unexpected deopt info.");
       return Handle<Object>::null();
   }
 }
@@ -3215,19 +3213,18 @@ Handle<Object> SlotRefValueBuilder::GetPreviouslyMaterialized(
       previously_materialized_objects_->get(object_index), isolate);
   materialized_objects_.Add(return_value);
 
-  // Now need to skip all nested objects (and possibly read them from
-  // the materialization store, too)
+  // Now need to skip all the nested objects (and possibly read them from
+  // the materialization store, too).
   for (int i = 0; i < length; i++) {
     SlotRef& slot = slot_refs_[current_slot_];
     current_slot_++;
 
-    // For nested deferred objects, we need to read its properties
-    if (slot.Representation() == SlotRef::DEFERRED_OBJECT) {
-      length += slot.DeferredObjectLength();
-    }
+    // We need to read all the nested objects - add them to the
+    // number of objects we need to process.
+    length += slot.GetChildrenCount();
 
-    // For nested deferred and duplicate objects, we need to put them into
-    // our materialization array
+    // Put the nested deferred/duplicate objects into our materialization
+    // array.
     if (slot.Representation() == SlotRef::DEFERRED_OBJECT ||
         slot.Representation() == SlotRef::DUPLICATE_OBJECT) {
       int nested_object_index = materialized_objects_.length();
@@ -3253,8 +3250,20 @@ Handle<Object> SlotRefValueBuilder::GetNext(Isolate* isolate, int lvl) {
     case SlotRef::LITERAL: {
       return slot.GetValue(isolate);
     }
+    case SlotRef::ARGUMENTS_OBJECT: {
+      // We should never need to materialize an arguments object,
+      // but we still need to put something into the array
+      // so that the indexing is consistent.
+      materialized_objects_.Add(isolate->factory()->undefined_value());
+      int length = slot.GetChildrenCount();
+      for (int i = 0; i < length; ++i) {
+        // We don't need the argument, just ignore it
+        GetNext(isolate, lvl + 1);
+      }
+      return isolate->factory()->undefined_value();
+    }
     case SlotRef::DEFERRED_OBJECT: {
-      int length = slot.DeferredObjectLength();
+      int length = slot.GetChildrenCount();
       ASSERT(slot_refs_[current_slot_].Representation() == SlotRef::LITERAL ||
              slot_refs_[current_slot_].Representation() == SlotRef::TAGGED);
 
@@ -3323,7 +3332,7 @@ Handle<Object> SlotRefValueBuilder::GetNext(Isolate* isolate, int lvl) {
       break;
   }
 
-  UNREACHABLE();
+  FATAL("We should never get here - unexpected deopt slot kind.");
   return Handle<Object>::null();
 }
 

src/deoptimizer.h

@@ -794,7 +794,9 @@ class SlotRef BASE_EMBEDDED {
                        // with the DeferredObjectLength() method
                        // (the SlotRefs of the nested objects follow
                        // this SlotRef in the depth-first order.)
-    DUPLICATE_OBJECT   // Duplicated object of a deferred object.
+    DUPLICATE_OBJECT,  // Duplicated object of a deferred object.
+    ARGUMENTS_OBJECT   // Arguments object - only used to keep indexing
+                       // in sync, it should not be materialized.
   };
 
   SlotRef()
@@ -806,6 +808,13 @@ class SlotRef BASE_EMBEDDED {
   SlotRef(Isolate* isolate, Object* literal)
       : literal_(literal, isolate), representation_(LITERAL) { }
 
+  static SlotRef NewArgumentsObject(int length) {
+    SlotRef slot;
+    slot.representation_ = ARGUMENTS_OBJECT;
+    slot.deferred_object_length_ = length;
+    return slot;
+  }
+
   static SlotRef NewDeferredObject(int length) {
     SlotRef slot;
     slot.representation_ = DEFERRED_OBJECT;
@@ -822,7 +831,14 @@ class SlotRef BASE_EMBEDDED {
     return slot;
   }
 
-  int DeferredObjectLength() { return deferred_object_length_; }
+  int GetChildrenCount() {
+    if (representation_ == DEFERRED_OBJECT ||
+        representation_ == ARGUMENTS_OBJECT) {
+      return deferred_object_length_;
+    } else {
+      return 0;
+    }
+  }
 
   int DuplicateObjectId() { return duplicate_object_id_; }
 

test/mjsunit/regress/regress-347262.js

+// Copyright 2014 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+(function ArgumentsObjectWithOtherArgumentsInFrame() {
+  function g() {
+    return g.arguments;
+  }
+
+  function f(x) {
+    g();
+    return arguments[0];
+  }
+  f();
+  f();
+  %OptimizeFunctionOnNextCall(f);
+  f();
+})();
+
+
+(function ArgumentsObjectWithOtherArgumentsDeopt() {
+  function g(y) {
+    y.o2 = 2;
+    return g.arguments;
+  }
+
+  function f(x) {
+    var o1 = { o2 : 1 };
+    var a = g(o1);
+    o1.o2 = 3;
+    return arguments[0] + a[0].o2;
+  }
+  f(0);
+  f(0);
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals(3, f(0));
+})();