Fix materialization of arguments objects with unknown values.

This fixes the deoptimizer to materialize arguments objects of correct
length even in cases where the actual argument values are unknown and
were optimized away by Crankshaft. This can happen if only the length
property or the identity of an arguments object is used.

R=svenpanne@chromium.org
BUG=chromium:163530
TEST=mjsunit/regress/regress-crbug-163530

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

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

src/arm/lithium-codegen-arm.cc

@@ -587,8 +587,8 @@ MemOperand LCodeGen::ToHighMemOperand(LOperand* op) const {
 
 void LCodeGen::WriteTranslation(LEnvironment* environment,
                                 Translation* translation,
-                                int* arguments_index,
-                                int* arguments_count) {
+                                int* pushed_arguments_index,
+                                int* pushed_arguments_count) {
   if (environment == NULL) return;
 
   // The translation includes one command per value in the environment.
@@ -600,13 +600,13 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
   // arguments index points to the first element of a sequence of tagged
   // values on the stack that represent the arguments. This needs to be
   // kept in sync with the LArgumentsElements implementation.
-  *arguments_index = -environment->parameter_count();
-  *arguments_count = environment->parameter_count();
+  *pushed_arguments_index = -environment->parameter_count();
+  *pushed_arguments_count = environment->parameter_count();
 
   WriteTranslation(environment->outer(),
                    translation,
-                   arguments_index,
-                   arguments_count);
+                   pushed_arguments_index,
+                   pushed_arguments_count);
   bool has_closure_id = !info()->closure().is_null() &&
       *info()->closure() != *environment->closure();
   int closure_id = has_closure_id
@@ -639,13 +639,20 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
   }
 
   // Inlined frames which push their arguments cause the index to be
-  // bumped and a new stack area to be used for materialization.
-  if (environment->entry() != NULL &&
-      environment->entry()->arguments_pushed()) {
-    *arguments_index = *arguments_index < 0
-        ? GetStackSlotCount()
-        : *arguments_index + *arguments_count;
-    *arguments_count = environment->entry()->arguments_count() + 1;
+  // bumped and another stack area to be used for materialization,
+  // otherwise actual argument values are unknown for inlined frames.
+  bool arguments_known = true;
+  int arguments_index = *pushed_arguments_index;
+  int arguments_count = *pushed_arguments_count;
+  if (environment->entry() != NULL) {
+    arguments_known = environment->entry()->arguments_pushed();
+    arguments_index = arguments_index < 0
+        ? GetStackSlotCount() : arguments_index + arguments_count;
+    arguments_count = environment->entry()->arguments_count() + 1;
+    if (environment->entry()->arguments_pushed()) {
+      *pushed_arguments_index = arguments_index;
+      *pushed_arguments_count = arguments_count;
+    }
   }
 
   for (int i = 0; i < translation_size; ++i) {
@@ -660,8 +667,9 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
                          environment->spilled_registers()[value->index()],
                          environment->HasTaggedValueAt(i),
                          environment->HasUint32ValueAt(i),
-                         *arguments_index,
-                         *arguments_count);
+                         arguments_known,
+                         arguments_index,
+                         arguments_count);
       } else if (
           value->IsDoubleRegister() &&
           environment->spilled_double_registers()[value->index()] != NULL) {
@@ -671,8 +679,9 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
             environment->spilled_double_registers()[value->index()],
             false,
             false,
-            *arguments_index,
-            *arguments_count);
+            arguments_known,
+            arguments_index,
+            arguments_count);
       }
     }
 
@@ -680,8 +689,9 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
                      value,
                      environment->HasTaggedValueAt(i),
                      environment->HasUint32ValueAt(i),
-                     *arguments_index,
-                     *arguments_count);
+                     arguments_known,
+                     arguments_index,
+                     arguments_count);
   }
 }
 
@@ -690,13 +700,15 @@ void LCodeGen::AddToTranslation(Translation* translation,
                                 LOperand* op,
                                 bool is_tagged,
                                 bool is_uint32,
+                                bool arguments_known,
                                 int arguments_index,
                                 int arguments_count) {
   if (op == NULL) {
     // TODO(twuerthinger): Introduce marker operands to indicate that this value
     // is not present and must be reconstructed from the deoptimizer. Currently
     // this is only used for the arguments object.
-    translation->StoreArgumentsObject(arguments_index, arguments_count);
+    translation->StoreArgumentsObject(
+        arguments_known, arguments_index, arguments_count);
   } else if (op->IsStackSlot()) {
     if (is_tagged) {
       translation->StoreStackSlot(op->index());

src/arm/lithium-codegen-arm.h

@@ -279,6 +279,7 @@ class LCodeGen BASE_EMBEDDED {
                         LOperand* op,
                         bool is_tagged,
                         bool is_uint32,
+                        bool arguments_known,
                         int arguments_index,
                         int arguments_count);
   void RegisterDependentCodeForEmbeddedMaps(Handle<Code> code);

src/deoptimizer.cc

@@ -885,8 +885,9 @@ void Deoptimizer::MaterializeHeapObjects(JavaScriptFrameIterator* it) {
         frame->SetExpression(i, *arguments);
         ASSERT_EQ(Memory::Object_at(descriptor.slot_address()), *arguments);
         if (trace_) {
-          PrintF("Materializing %sarguments object for %p: ",
+          PrintF("Materializing %sarguments object of length %d for %p: ",
                  frame->has_adapted_arguments() ? "(adapted) " : "",
+                 arguments->elements()->length(),
                  reinterpret_cast<void*>(descriptor.slot_address()));
           arguments->ShortPrint();
           PrintF("\n");
@@ -1180,6 +1181,7 @@ void Deoptimizer::DoTranslateCommand(TranslationIterator* iterator,
     }
 
     case Translation::ARGUMENTS_OBJECT: {
+      bool args_known = iterator->Next();
       int args_index = iterator->Next() + 1;  // Skip receiver.
       int args_length = iterator->Next() - 1;  // Skip receiver.
       if (trace_) {
@@ -1187,7 +1189,7 @@ void Deoptimizer::DoTranslateCommand(TranslationIterator* iterator,
                output_[frame_index]->GetTop() + output_offset,
                output_offset);
         isolate_->heap()->arguments_marker()->ShortPrint();
-        PrintF(" ; arguments object\n");
+        PrintF(" ; %sarguments object\n", args_known ? "" : "dummy ");
       }
       // Use the arguments marker value as a sentinel and fill in the arguments
       // object after the deoptimized frame is built.
@@ -1200,7 +1202,9 @@ void Deoptimizer::DoTranslateCommand(TranslationIterator* iterator,
       // actual arguments object after the deoptimized frame is built.
       for (int i = 0; i < args_length; i++) {
         unsigned input_offset = input_->GetOffsetFromSlotIndex(args_index + i);
-        intptr_t input_value = input_->GetFrameSlot(input_offset);
+        intptr_t input_value = args_known
+            ? input_->GetFrameSlot(input_offset)
+            : reinterpret_cast<intptr_t>(isolate_->heap()->the_hole_value());
         AddArgumentsObjectValue(input_value);
       }
       return;
@@ -1841,8 +1845,11 @@ void Translation::StoreLiteral(int literal_id) {
 }
 
 
-void Translation::StoreArgumentsObject(int args_index, int args_length) {
+void Translation::StoreArgumentsObject(bool args_known,
+                                       int args_index,
+                                       int args_length) {
   buffer_->Add(ARGUMENTS_OBJECT, zone());
+  buffer_->Add(args_known, zone());
   buffer_->Add(args_index, zone());
   buffer_->Add(args_length, zone());
 }
@@ -1873,9 +1880,9 @@ int Translation::NumberOfOperandsFor(Opcode opcode) {
     case BEGIN:
     case ARGUMENTS_ADAPTOR_FRAME:
     case CONSTRUCT_STUB_FRAME:
-    case ARGUMENTS_OBJECT:
       return 2;
     case JS_FRAME:
+    case ARGUMENTS_OBJECT:
       return 3;
   }
   UNREACHABLE();

src/deoptimizer.h

@@ -675,7 +675,7 @@ class Translation BASE_EMBEDDED {
   void StoreUint32StackSlot(int index);
   void StoreDoubleStackSlot(int index);
   void StoreLiteral(int literal_id);
-  void StoreArgumentsObject(int args_index, int args_length);
+  void StoreArgumentsObject(bool args_known, int args_index, int args_length);
   void MarkDuplicate();
 
   Zone* zone() const { return zone_; }

src/ia32/lithium-codegen-ia32.cc

@@ -568,8 +568,8 @@ Operand LCodeGen::HighOperand(LOperand* op) {
 
 void LCodeGen::WriteTranslation(LEnvironment* environment,
                                 Translation* translation,
-                                int* arguments_index,
-                                int* arguments_count) {
+                                int* pushed_arguments_index,
+                                int* pushed_arguments_count) {
   if (environment == NULL) return;
 
   // The translation includes one command per value in the environment.
@@ -581,13 +581,13 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
   // arguments index points to the first element of a sequence of tagged
   // values on the stack that represent the arguments. This needs to be
   // kept in sync with the LArgumentsElements implementation.
-  *arguments_index = -environment->parameter_count();
-  *arguments_count = environment->parameter_count();
+  *pushed_arguments_index = -environment->parameter_count();
+  *pushed_arguments_count = environment->parameter_count();
 
   WriteTranslation(environment->outer(),
                    translation,
-                   arguments_index,
-                   arguments_count);
+                   pushed_arguments_index,
+                   pushed_arguments_count);
   bool has_closure_id = !info()->closure().is_null() &&
       *info()->closure() != *environment->closure();
   int closure_id = has_closure_id
@@ -621,13 +621,20 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
   }
 
   // Inlined frames which push their arguments cause the index to be
-  // bumped and another stack area to be used for materialization.
-  if (environment->entry() != NULL &&
-      environment->entry()->arguments_pushed()) {
-    *arguments_index = *arguments_index < 0
-        ? GetStackSlotCount()
-        : *arguments_index + *arguments_count;
-    *arguments_count = environment->entry()->arguments_count() + 1;
+  // bumped and another stack area to be used for materialization,
+  // otherwise actual argument values are unknown for inlined frames.
+  bool arguments_known = true;
+  int arguments_index = *pushed_arguments_index;
+  int arguments_count = *pushed_arguments_count;
+  if (environment->entry() != NULL) {
+    arguments_known = environment->entry()->arguments_pushed();
+    arguments_index = arguments_index < 0
+        ? GetStackSlotCount() : arguments_index + arguments_count;
+    arguments_count = environment->entry()->arguments_count() + 1;
+    if (environment->entry()->arguments_pushed()) {
+      *pushed_arguments_index = arguments_index;
+      *pushed_arguments_count = arguments_count;
+    }
   }
 
   for (int i = 0; i < translation_size; ++i) {
@@ -642,8 +649,9 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
                          environment->spilled_registers()[value->index()],
                          environment->HasTaggedValueAt(i),
                          environment->HasUint32ValueAt(i),
-                         *arguments_index,
-                         *arguments_count);
+                         arguments_known,
+                         arguments_index,
+                         arguments_count);
       } else if (
           value->IsDoubleRegister() &&
           environment->spilled_double_registers()[value->index()] != NULL) {
@@ -653,8 +661,9 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
             environment->spilled_double_registers()[value->index()],
             false,
             false,
-            *arguments_index,
-            *arguments_count);
+            arguments_known,
+            arguments_index,
+            arguments_count);
       }
     }
 
@@ -662,8 +671,9 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
                      value,
                      environment->HasTaggedValueAt(i),
                      environment->HasUint32ValueAt(i),
-                     *arguments_index,
-                     *arguments_count);
+                     arguments_known,
+                     arguments_index,
+                     arguments_count);
   }
 }
 
@@ -672,13 +682,15 @@ void LCodeGen::AddToTranslation(Translation* translation,
                                 LOperand* op,
                                 bool is_tagged,
                                 bool is_uint32,
+                                bool arguments_known,
                                 int arguments_index,
                                 int arguments_count) {
   if (op == NULL) {
     // TODO(twuerthinger): Introduce marker operands to indicate that this value
     // is not present and must be reconstructed from the deoptimizer. Currently
     // this is only used for the arguments object.
-    translation->StoreArgumentsObject(arguments_index, arguments_count);
+    translation->StoreArgumentsObject(
+        arguments_known, arguments_index, arguments_count);
   } else if (op->IsStackSlot()) {
     if (is_tagged) {
       translation->StoreStackSlot(op->index());

src/ia32/lithium-codegen-ia32.h

@@ -261,6 +261,7 @@ class LCodeGen BASE_EMBEDDED {
                         LOperand* op,
                         bool is_tagged,
                         bool is_uint32,
+                        bool arguments_known,
                         int arguments_index,
                         int arguments_count);
   void RegisterDependentCodeForEmbeddedMaps(Handle<Code> code);

src/objects.cc

@@ -8980,8 +8980,7 @@ void DeoptimizationInputData::DeoptimizationInputDataPrint(FILE* out) {
 
         case Translation::UINT32_REGISTER: {
           int reg_code = iterator.Next();
-          PrintF(out,
-                 "{input=%s (unsigned)}",
+          PrintF(out, "{input=%s (unsigned)}",
                  converter.NameOfCPURegister(reg_code));
           break;
         }
@@ -9024,9 +9023,11 @@ void DeoptimizationInputData::DeoptimizationInputDataPrint(FILE* out) {
         }
 
         case Translation::ARGUMENTS_OBJECT: {
+          bool args_known = iterator.Next();
           int args_index = iterator.Next();
           int args_length = iterator.Next();
-          PrintF(out, "{index=%d, length=%d}", args_index, args_length);
+          PrintF(out, "{index=%d, length=%d, known=%d}",
+                 args_index, args_length, args_known);
           break;
         }
       }

src/x64/lithium-codegen-x64.cc

@@ -477,8 +477,8 @@ Operand LCodeGen::ToOperand(LOperand* op) const {
 
 void LCodeGen::WriteTranslation(LEnvironment* environment,
                                 Translation* translation,
-                                int* arguments_index,
-                                int* arguments_count) {
+                                int* pushed_arguments_index,
+                                int* pushed_arguments_count) {
   if (environment == NULL) return;
 
   // The translation includes one command per value in the environment.
@@ -490,13 +490,13 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
   // arguments index points to the first element of a sequence of tagged
   // values on the stack that represent the arguments. This needs to be
   // kept in sync with the LArgumentsElements implementation.
-  *arguments_index = -environment->parameter_count();
-  *arguments_count = environment->parameter_count();
+  *pushed_arguments_index = -environment->parameter_count();
+  *pushed_arguments_count = environment->parameter_count();
 
   WriteTranslation(environment->outer(),
                    translation,
-                   arguments_index,
-                   arguments_count);
+                   pushed_arguments_index,
+                   pushed_arguments_count);
   bool has_closure_id = !info()->closure().is_null() &&
       *info()->closure() != *environment->closure();
   int closure_id = has_closure_id
@@ -529,13 +529,20 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
   }
 
   // Inlined frames which push their arguments cause the index to be
-  // bumped and a new stack area to be used for materialization.
-  if (environment->entry() != NULL &&
-      environment->entry()->arguments_pushed()) {
-    *arguments_index = *arguments_index < 0
-        ? GetStackSlotCount()
-        : *arguments_index + *arguments_count;
-    *arguments_count = environment->entry()->arguments_count() + 1;
+  // bumped and another stack area to be used for materialization,
+  // otherwise actual argument values are unknown for inlined frames.
+  bool arguments_known = true;
+  int arguments_index = *pushed_arguments_index;
+  int arguments_count = *pushed_arguments_count;
+  if (environment->entry() != NULL) {
+    arguments_known = environment->entry()->arguments_pushed();
+    arguments_index = arguments_index < 0
+        ? GetStackSlotCount() : arguments_index + arguments_count;
+    arguments_count = environment->entry()->arguments_count() + 1;
+    if (environment->entry()->arguments_pushed()) {
+      *pushed_arguments_index = arguments_index;
+      *pushed_arguments_count = arguments_count;
+    }
   }
 
   for (int i = 0; i < translation_size; ++i) {
@@ -550,8 +557,9 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
                          environment->spilled_registers()[value->index()],
                          environment->HasTaggedValueAt(i),
                          environment->HasUint32ValueAt(i),
-                         *arguments_index,
-                         *arguments_count);
+                         arguments_known,
+                         arguments_index,
+                         arguments_count);
       } else if (
           value->IsDoubleRegister() &&
           environment->spilled_double_registers()[value->index()] != NULL) {
@@ -561,8 +569,9 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
             environment->spilled_double_registers()[value->index()],
             false,
             false,
-            *arguments_index,
-            *arguments_count);
+            arguments_known,
+            arguments_index,
+            arguments_count);
       }
     }
 
@@ -570,8 +579,9 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
                      value,
                      environment->HasTaggedValueAt(i),
                      environment->HasUint32ValueAt(i),
-                     *arguments_index,
-                     *arguments_count);
+                     arguments_known,
+                     arguments_index,
+                     arguments_count);
   }
 }
 
@@ -580,13 +590,15 @@ void LCodeGen::AddToTranslation(Translation* translation,
                                 LOperand* op,
                                 bool is_tagged,
                                 bool is_uint32,
+                                bool arguments_known,
                                 int arguments_index,
                                 int arguments_count) {
   if (op == NULL) {
     // TODO(twuerthinger): Introduce marker operands to indicate that this value
     // is not present and must be reconstructed from the deoptimizer. Currently
     // this is only used for the arguments object.
-    translation->StoreArgumentsObject(arguments_index, arguments_count);
+    translation->StoreArgumentsObject(
+        arguments_known, arguments_index, arguments_count);
   } else if (op->IsStackSlot()) {
     if (is_tagged) {
       translation->StoreStackSlot(op->index());

src/x64/lithium-codegen-x64.h

@@ -241,6 +241,7 @@ class LCodeGen BASE_EMBEDDED {
                         LOperand* op,
                         bool is_tagged,
                         bool is_uint32,
+                        bool arguments_known,
                         int arguments_index,
                         int arguments_count);
   void RegisterDependentCodeForEmbeddedMaps(Handle<Code> code);

test/mjsunit/regress/regress-crbug-163530.js

+// Copyright 2013 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
+
+// Test materialization of an arguments object with unknown argument
+// values in non-strict mode (length has to be zero).
+(function() {
+  var deoptimize = { deopt:true };
+  var object = {};
+
+  object.a = function A(x, y, z) {
+    assertSame(0, arguments.length);
+    return this.b();
+  };
+
+  object.b = function B() {
+    assertSame(0, arguments.length);
+    deoptimize.deopt;
+    return arguments.length;
+  };
+
+  assertSame(0, object.a());
+  assertSame(0, object.a());
+  %OptimizeFunctionOnNextCall(object.a);
+  assertSame(0, object.a());
+  delete deoptimize.deopt;
+  assertSame(0, object.a());
+})();
+
+
+// Test materialization of an arguments object with unknown argument
+// values in strict mode (length is allowed to exceed stack size).
+(function() {
+  'use strict';
+  var deoptimize = { deopt:true };
+  var object = {};
+
+  object.a = function A(x, y, z) {
+    assertSame(0, arguments.length);
+    return this.b(1, 2, 3, 4, 5, 6, 7, 8);
+  };
+
+  object.b = function B(a, b, c, d, e, f, g, h) {
+    assertSame(8, arguments.length);
+    deoptimize.deopt;
+    return arguments.length;
+  };
+
+  assertSame(8, object.a());
+  assertSame(8, object.a());
+  %OptimizeFunctionOnNextCall(object.a);
+  assertSame(8, object.a());
+  delete deoptimize.deopt;
+  assertSame(8, object.a());
+})();