Make sure we always have room for patching the reloc info during lazy...

Make sure we always have room for patching the reloc info during lazy deoptimization (fixes issue 1156).

Before we could have calls to builtins that would not be in the
relocation info since this used a register as target. Whenever we have
this case (from lithium codegen) we now emit a comment in the reloc
info.

Review URL: http://codereview.chromium.org/6499015

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

src/assembler.cc

@@ -68,7 +68,7 @@ const double DoubleConstant::min_int = kMinInt;
 const double DoubleConstant::one_half = 0.5;
 const double DoubleConstant::minus_zero = -0.0;
 const double DoubleConstant::negative_infinity = -V8_INFINITY;
-
+const char* RelocInfo::kFillerCommentString = "DEOPTIMIZATION PADDING";
 
 // -----------------------------------------------------------------------------
 // Implementation of Label

src/assembler.h

@@ -178,6 +178,12 @@ class RelocInfo BASE_EMBEDDED {
   // invalid/uninitialized position value.
   static const int kNoPosition = -1;
 
+  // This string is used to add padding comments to the reloc info in cases
+  // where we are not sure to have enough space for patching in during
+  // lazy deoptimization. This is the case if we have indirect calls for which
+  // we do not normally record relocation info.
+  static const char* kFillerCommentString;
+
   enum Mode {
     // Please note the order is important (see IsCodeTarget, IsGCRelocMode).
     CONSTRUCT_CALL,  // code target that is a call to a JavaScript constructor.

src/ia32/assembler-ia32.cc

@@ -2607,8 +2607,8 @@ void Assembler::RecordDebugBreakSlot() {
 }
 
 
-void Assembler::RecordComment(const char* msg) {
-  if (FLAG_code_comments) {
+void Assembler::RecordComment(const char* msg, bool force) {
+  if (FLAG_code_comments || force) {
     EnsureSpace ensure_space(this);
     RecordRelocInfo(RelocInfo::COMMENT, reinterpret_cast<intptr_t>(msg));
   }

src/ia32/assembler-ia32.h

@@ -957,8 +957,9 @@ class Assembler : public Malloced {
   void RecordDebugBreakSlot();
 
   // Record a comment relocation entry that can be used by a disassembler.
-  // Use --code-comments to enable.
-  void RecordComment(const char* msg);
+  // Use --code-comments to enable, or provide "force = true" flag to always
+  // write a comment.
+  void RecordComment(const char* msg, bool force = false);
 
   // Writes a single byte or word of data in the code stream.  Used for
   // inline tables, e.g., jump-tables.

src/ia32/deoptimizer-ia32.cc

@@ -80,6 +80,7 @@ void Deoptimizer::DeoptimizeFunction(JSFunction* function) {
   Address prev_address = code_start_address;
   for (unsigned i = 0; i < table.length(); ++i) {
     Address curr_address = code_start_address + table.GetPcOffset(i);
+    ASSERT_GE(curr_address - prev_address, patch_size());
     ZapCodeRange(prev_address, curr_address);
 
     SafepointEntry safepoint_entry = table.GetEntry(i);
@@ -97,7 +98,8 @@ void Deoptimizer::DeoptimizeFunction(JSFunction* function) {
                       RelocInfo::RUNTIME_ENTRY,
                       reinterpret_cast<intptr_t>(deopt_entry));
       reloc_info_writer.Write(&rinfo);
-
+      ASSERT_GE(reloc_info_writer.pos(),
+                reloc_info->address() + ByteArray::kHeaderSize);
       curr_address += patch_size();
     }
     prev_address = curr_address;

src/ia32/lithium-codegen-ia32.cc

@@ -43,13 +43,20 @@ class SafepointGenerator : public PostCallGenerator {
  public:
   SafepointGenerator(LCodeGen* codegen,
                      LPointerMap* pointers,
-                     int deoptimization_index)
+                     int deoptimization_index,
+                     bool ensure_reloc_space = false)
       : codegen_(codegen),
         pointers_(pointers),
-        deoptimization_index_(deoptimization_index) { }
+        deoptimization_index_(deoptimization_index),
+        ensure_reloc_space_(ensure_reloc_space) { }
   virtual ~SafepointGenerator() { }
 
   virtual void Generate() {
+    // Ensure that we have enough space in the reloc info to patch
+    // this with calls when doing deoptimization.
+    if (ensure_reloc_space_) {
+      codegen_->masm()->RecordComment(RelocInfo::kFillerCommentString, true);
+    }
     codegen_->RecordSafepoint(pointers_, deoptimization_index_);
   }
 
@@ -57,6 +64,7 @@ class SafepointGenerator : public PostCallGenerator {
   LCodeGen* codegen_;
   LPointerMap* pointers_;
   int deoptimization_index_;
+  bool ensure_reloc_space_;
 };
 
 
@@ -2221,7 +2229,8 @@ void LCodeGen::DoApplyArguments(LApplyArguments* instr) {
   RegisterEnvironmentForDeoptimization(env);
   SafepointGenerator safepoint_generator(this,
                                          pointers,
-                                         env->deoptimization_index());
+                                         env->deoptimization_index(),
+                                         true);
   v8::internal::ParameterCount actual(eax);
   __ InvokeFunction(function, actual, CALL_FUNCTION, &safepoint_generator);
 }
@@ -2292,6 +2301,10 @@ void LCodeGen::CallKnownFunction(Handle<JSFunction> function,
   if (*function == *graph()->info()->closure()) {
     __ CallSelf();
   } else {
+    // This is an indirect call and will not be recorded in the reloc info.
+    // Add a comment to the reloc info in case we need to patch this during
+    // deoptimization.
+    __ RecordComment(RelocInfo::kFillerCommentString, true);
     __ call(FieldOperand(edi, JSFunction::kCodeEntryOffset));
   }
 
@@ -3731,9 +3744,13 @@ void LCodeGen::DoDeleteProperty(LDeleteProperty* instr) {
   LEnvironment* env = instr->deoptimization_environment();
   RecordPosition(pointers->position());
   RegisterEnvironmentForDeoptimization(env);
+  // Create safepoint generator that will also ensure enough space in the
+  // reloc info for patching in deoptimization (since this is invoking a
+  // builtin)
   SafepointGenerator safepoint_generator(this,
                                          pointers,
-                                         env->deoptimization_index());
+                                         env->deoptimization_index(),
+                                         true);
   __ mov(esi, Operand(ebp, StandardFrameConstants::kContextOffset));
   __ push(Immediate(Smi::FromInt(strict_mode_flag())));
   __ InvokeBuiltin(Builtins::DELETE, CALL_FUNCTION, &safepoint_generator);

test/mjsunit/regress/regress-1156.js

+// Copyright 2011 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 --nouse-inlining
+
+// Test that we do not crash we invoke builtins from optimized code that
+// is then deoptimized.
+
+function foo(a) {
+  delete a[1];
+  delete a[2];
+  delete a[3];
+  delete a[4];
+  delete a[5];
+  return void 0;
+}
+
+function call_and_deopt() {
+  var b = [1,2,3];
+  foo(b);
+  foo(b);
+  %DeoptimizeFunction(foo);
+}
+
+call_and_deopt();