From 77a71c90c7333d9418b8a0b10cf49feb737fafd8 Mon Sep 17 00:00:00 2001
From: "sgjesse@chromium.org"
 <sgjesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Wed, 4 Nov 2009 14:45:50 +0000
Subject: [PATCH] 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
---
 src/arm/assembler-arm.cc            |  5 +++
 src/arm/assembler-arm.h             |  4 +++
 src/arm/codegen-arm.cc              | 20 ++++++------
 src/arm/codegen-arm.h               |  4 ---
 src/arm/debug-arm.cc                |  2 +-
 src/arm/fast-codegen-arm.cc         | 28 +++++++++--------
 src/debug.h                         |  2 ++
 test/mjsunit/regress/regress-491.js | 47 +++++++++++++++++++++++++++++
 8 files changed, 86 insertions(+), 26 deletions(-)
 create mode 100644 test/mjsunit/regress/regress-491.js

diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc
index b129a6d48..39bb3ee4b 100644
--- 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 df2b4cda2..0c791b349 100644
--- 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 4198ae79a..566da4fa6 100644
--- 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 8c2a28387..1871e3025 100644
--- 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 ef3365395..102952da7 100644
--- 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 d4018caa0..6de819493 100644
--- 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 29c2bc203..c8b79a435 100644
--- 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
index 000000000..2cf5e20ed
--- /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);
+}
-- 
2.34.1