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

diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc
index b129a6d485ff97c058afa151fea7f7332961d477..39bb3ee4b661116e81075bbecc09fdfb90dd19ad 100644 (file)
--- a/src/arm/assembler-arm.cc
+++ b/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();

diff --git a/src/arm/assembler-arm.h b/src/arm/assembler-arm.h
index df2b4cda2e2ed140f6f3b2256f2ec1cdcafc81d6..0c791b3493aee90f181a1e93d7a364faa037296d 100644 (file)
--- a/src/arm/assembler-arm.h
+++ b/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.

diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc
index 4198ae79a1067e2c1902507eeb97b783037eb481..566da4fa65999246c6980fde3deed44554bdc528 100644 (file)
--- a/src/arm/codegen-arm.cc
+++ b/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.

diff --git a/src/arm/codegen-arm.h b/src/arm/codegen-arm.h
index 8c2a283873b919acb17559980012b377bcd82360..1871e302596bee878029f784b1a1cc2a7e385432 100644 (file)
--- a/src/arm/codegen-arm.h
+++ b/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);

diff --git a/src/arm/debug-arm.cc b/src/arm/debug-arm.cc
index ef3365395069666cd57d7e84c5b0ce652e997986..102952da7b8464664318a6d549fcdc056f801792 100644 (file)
--- a/src/arm/debug-arm.cc
+++ b/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);
 }
 
 

diff --git a/src/arm/fast-codegen-arm.cc b/src/arm/fast-codegen-arm.cc
index d4018caa062f165a6ea055083b18a57a91d9d79e..6de81949348c34fbbd093eaf7f3f7d4bdb896ac5 100644 (file)
--- a/src/arm/fast-codegen-arm.cc
+++ b/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
   }
 }
 

diff --git a/src/debug.h b/src/debug.h
index 29c2bc203692e674851a0de001c22d0a4ab8eb4c..c8b79a4359f0ff986447ab88df507cb6b530a77e 100644 (file)
--- a/src/debug.h
+++ b/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);

diff --git a/test/mjsunit/regress/regress-491.js b/test/mjsunit/regress/regress-491.js
new file mode 100644 (file)
index 0000000..2cf5e20
--- /dev/null
+++ b/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);
+}