Revision: 3215 Author: [email protected] Date: Wed Nov 4 06:45:50 2009 Log: 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 http://code.google.com/p/v8/source/detail?r=3215 Added: /branches/bleeding_edge/test/mjsunit/regress/regress-491.js Modified: /branches/bleeding_edge/src/arm/assembler-arm.cc /branches/bleeding_edge/src/arm/assembler-arm.h /branches/bleeding_edge/src/arm/codegen-arm.cc /branches/bleeding_edge/src/arm/codegen-arm.h /branches/bleeding_edge/src/arm/debug-arm.cc /branches/bleeding_edge/src/arm/fast-codegen-arm.cc /branches/bleeding_edge/src/debug.h ======================================= --- /dev/null +++ /branches/bleeding_edge/test/mjsunit/regress/regress-491.js Wed Nov 4 06:45:50 2009 @@ -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); +} ======================================= --- /branches/bleeding_edge/src/arm/assembler-arm.cc Wed Nov 4 02:04:22 2009 +++ /branches/bleeding_edge/src/arm/assembler-arm.cc Wed Nov 4 06:45:50 2009 @@ -1316,6 +1316,11 @@ uint32_t dummy2; return fits_shifter(imm32, &dummy1, &dummy2, NULL); } + + +void Assembler::BlockConstPoolFor(int instructions) { + BlockConstPoolBefore(pc_offset() + instructions * kInstrSize); +} // Debugging ======================================= --- /branches/bleeding_edge/src/arm/assembler-arm.h Wed Nov 4 02:04:22 2009 +++ /branches/bleeding_edge/src/arm/assembler-arm.h Wed Nov 4 06:45:50 2009 @@ -685,6 +685,10 @@ // 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. ======================================= --- /branches/bleeding_edge/src/arm/codegen-arm.cc Wed Nov 4 05:56:41 2009 +++ /branches/bleeding_edge/src/arm/codegen-arm.cc Wed Nov 4 06:45:50 2009 @@ -322,13 +322,22 @@ 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 @@ // 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. ======================================= --- /branches/bleeding_edge/src/arm/codegen-arm.h Wed Nov 4 05:56:41 2009 +++ /branches/bleeding_edge/src/arm/codegen-arm.h Wed Nov 4 06:45:50 2009 @@ -187,10 +187,6 @@ 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); ======================================= --- /branches/bleeding_edge/src/arm/debug-arm.cc Thu Oct 15 04:52:53 2009 +++ /branches/bleeding_edge/src/arm/debug-arm.cc Wed Nov 4 06:45:50 2009 @@ -61,7 +61,7 @@ // Restore the JS frame exit code. void BreakLocationIterator::ClearDebugBreakAtReturn() { rinfo()->PatchCode(original_rinfo()->pc(), - CodeGenerator::kJSReturnSequenceLength); + Debug::kARMJSReturnSequenceLength); } ======================================= --- /branches/bleeding_edge/src/arm/fast-codegen-arm.cc Wed Nov 4 06:33:37 2009 +++ /branches/bleeding_edge/src/arm/fast-codegen-arm.cc Wed Nov 4 06:45:50 2009 @@ -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 @@ __ 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 } } ======================================= --- /branches/bleeding_edge/src/debug.h Mon Sep 28 05:25:21 2009 +++ /branches/bleeding_edge/src/debug.h Wed Nov 4 06:45:50 2009 @@ -377,6 +377,8 @@ 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); --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
