Fix issue 491: constantpool dump violates ARM debugger assertion for return point

The generation of the return sequence is now protected from having the constant pool emitted inside of it in both compilers.

BUG=http://code.google.com/p/v8/issues/detail?id=491
TEST=test/mjsunit/regress/regress-491.js
Review URL: http://codereview.chromium.org/362003

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

Author: sgjesse@chromium.org

src/arm/assembler-arm.cc

@@ -1318,6 +1318,11 @@ bool Assembler::ImmediateFitsAddrMode1Instruction(int32_t imm32) {
 }
 
 
+void Assembler::BlockConstPoolFor(int instructions) {
+  BlockConstPoolBefore(pc_offset() + instructions * kInstrSize);
+}
+
+
 // Debugging
 void Assembler::RecordJSReturn() {
   WriteRecordedPositions();

src/arm/assembler-arm.h

@@ -685,6 +685,10 @@ class Assembler : public Malloced {
   // Check whether an immediate fits an addressing mode 1 instruction.
   bool ImmediateFitsAddrMode1Instruction(int32_t imm32);
 
+  // Postpone the generation of the constant pool for the specified number of
+  // instructions.
+  void BlockConstPoolFor(int instructions);
+
   // Debugging
 
   // Mark address of the ExitJSFrame code.

src/arm/codegen-arm.cc

@@ -322,13 +322,22 @@ void CodeGenerator::GenCode(FunctionLiteral* fun) {
     Label check_exit_codesize;
     masm_->bind(&check_exit_codesize);
 
+    // Calculate the exact length of the return sequence and make sure that
+    // the constant pool is not emitted inside of the return sequence.
+    int32_t sp_delta = (scope_->num_parameters() + 1) * kPointerSize;
+    int return_sequence_length = Debug::kARMJSReturnSequenceLength;
+    if (!masm_->ImmediateFitsAddrMode1Instruction(sp_delta)) {
+      // Additional mov instruction generated.
+      return_sequence_length++;
+    }
+    masm_->BlockConstPoolFor(return_sequence_length);
+
     // Tear down the frame which will restore the caller's frame pointer and
     // the link register.
     frame_->Exit();
 
     // Here we use masm_-> instead of the __ macro to avoid the code coverage
     // tool from instrumenting as we rely on the code size here.
-    int32_t sp_delta = (scope_->num_parameters() + 1) * kPointerSize;
     masm_->add(sp, sp, Operand(sp_delta));
     masm_->Jump(lr);
 
@@ -338,15 +347,8 @@ void CodeGenerator::GenCode(FunctionLiteral* fun) {
     // can be encoded in the instruction and which immediate values requires
     // use of an additional instruction for moving the immediate to a temporary
     // register.
-#ifdef DEBUG
-    int expected_return_sequence_length = kJSReturnSequenceLength;
-    if (!masm_->ImmediateFitsAddrMode1Instruction(sp_delta)) {
-      // Additional mov instruction generated.
-      expected_return_sequence_length++;
-    }
-    ASSERT_EQ(expected_return_sequence_length,
+    ASSERT_EQ(return_sequence_length,
               masm_->InstructionsGeneratedSince(&check_exit_codesize));
-#endif
   }
 
   // Code generation state must be reset.

src/arm/codegen-arm.h

@@ -187,10 +187,6 @@ class CodeGenerator: public AstVisitor {
 
   static const int kUnknownIntValue = -1;
 
-  // Number of instructions used for the JS return sequence. The constant is
-  // used by the debugger to patch the JS return sequence.
-  static const int kJSReturnSequenceLength = 4;
-
  private:
   // Construction/Destruction
   CodeGenerator(int buffer_size, Handle<Script> script, bool is_eval);

src/arm/debug-arm.cc

@@ -61,7 +61,7 @@ void BreakLocationIterator::SetDebugBreakAtReturn() {
 // Restore the JS frame exit code.
 void BreakLocationIterator::ClearDebugBreakAtReturn() {
   rinfo()->PatchCode(original_rinfo()->pc(),
-                     CodeGenerator::kJSReturnSequenceLength);
+                     Debug::kARMJSReturnSequenceLength);
 }
 
 

src/arm/fast-codegen-arm.cc

@@ -28,6 +28,7 @@
 #include "v8.h"
 
 #include "codegen-inl.h"
+#include "debug.h"
 #include "fast-codegen.h"
 #include "parser.h"
 
@@ -118,34 +119,37 @@ void FastCodeGenerator::EmitReturnSequence(int position) {
       __ push(r0);
       __ CallRuntime(Runtime::kTraceExit, 1);
     }
-#ifdef DEBUG
+
     // Add a label for checking the size of the code used for returning.
     Label check_exit_codesize;
     masm_->bind(&check_exit_codesize);
-#endif
+
+    // Calculate the exact length of the return sequence and make sure that
+    // the constant pool is not emitted inside of the return sequence.
+    int num_parameters = function_->scope()->num_parameters();
+    int32_t sp_delta = (num_parameters + 1) * kPointerSize;
+    int return_sequence_length = Debug::kARMJSReturnSequenceLength;
+    if (!masm_->ImmediateFitsAddrMode1Instruction(sp_delta)) {
+      // Additional mov instruction generated.
+      return_sequence_length++;
+    }
+    masm_->BlockConstPoolFor(return_sequence_length);
+
     CodeGenerator::RecordPositions(masm_, position);
     __ RecordJSReturn();
     __ mov(sp, fp);
     __ ldm(ia_w, sp, fp.bit() | lr.bit());
-    int num_parameters = function_->scope()->num_parameters();
-    __ add(sp, sp, Operand((num_parameters + 1) * kPointerSize));
+    __ add(sp, sp, Operand(sp_delta));
     __ Jump(lr);
-#ifdef DEBUG
+
   // Check that the size of the code used for returning matches what is
   // expected by the debugger. The add instruction above is an addressing
   // mode 1 instruction where there are restrictions on which immediate values
   // can be encoded in the instruction and which immediate values requires
   // use of an additional instruction for moving the immediate to a temporary
   // register.
-  int expected_return_sequence_length = CodeGenerator::kJSReturnSequenceLength;
-  if (!masm_->ImmediateFitsAddrMode1Instruction((num_parameters + 1) *
-                                                kPointerSize)) {
-    // Additional mov instruction generated.
-    expected_return_sequence_length++;
-  }
   ASSERT_EQ(expected_return_sequence_length,
             masm_->InstructionsGeneratedSince(&check_exit_codesize));
-#endif
   }
 }
 

src/debug.h

@@ -377,6 +377,8 @@ class Debug {
   static const int kX64CallInstructionLength = 13;
   static const int kX64JSReturnSequenceLength = 13;
 
+  static const int kARMJSReturnSequenceLength = 4;
+
   // Code generator routines.
   static void GenerateLoadICDebugBreak(MacroAssembler* masm);
   static void GenerateStoreICDebugBreak(MacroAssembler* masm);

test/mjsunit/regress/regress-491.js

@@ -0,0 +1,47 @@
+// Copyright 2009 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.
+
+// See: http://code.google.com/p/v8/issues/detail?id=491
+// This should not hit any asserts in debug mode on ARM.
+
+function function_with_n_strings(n) {
+  var source = '(function f(){';
+  for (var i = 0; i < n; i++) {
+    if (i != 0) source += ';';
+    source += '"x"';
+  }
+  source += '})()';
+  eval(source);
+}
+
+var i;
+for (i = 500; i < 600; i++) {
+  function_with_n_strings(i);
+}
+for (i = 1100; i < 1200; i++) {
+  function_with_n_strings(i);
+}