Reviewers: Søren Gjesse, Kevin Millikin,
Description:
Add more generic version of reloc info padding to ensure enough space for
reloc
patching during deoptimization (fixes issue 1174).
The old version only added extra space when we did indirect calls, but
the problem remains the same with normal calls that can be represented
as a single byte. When doing patching each call will always be at
least 2 bytes long because we use RUNTIME_ENTY as the reloc mode.
Please review this at http://codereview.chromium.org/6541053/
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/assembler.h
M src/assembler.cc
M src/ia32/lithium-codegen-ia32.h
M src/ia32/lithium-codegen-ia32.cc
A test/mjsunit/regress/regress-1174.js
Index: src/assembler.cc
===================================================================
--- src/assembler.cc (revision 6848)
+++ src/assembler.cc (working copy)
@@ -228,6 +228,7 @@
WriteTaggedPC(pc_delta, kEmbeddedObjectTag);
} else if (rmode == RelocInfo::CODE_TARGET) {
WriteTaggedPC(pc_delta, kCodeTargetTag);
+ ASSERT(begin_pos - pos_ <= RelocInfo::kMaxCallSize);
} else if (RelocInfo::IsPosition(rmode)) {
// Use signed delta-encoding for data.
intptr_t data_delta = rinfo->data() - last_data_;
@@ -251,6 +252,7 @@
WriteExtraTaggedPC(pc_delta, kPCJumpTag);
WriteExtraTaggedData(rinfo->data() - last_data_, kCommentTag);
last_data_ = rinfo->data();
+ ASSERT(begin_pos - pos_ == RelocInfo::kRelocCommentSize);
} else {
// For all other modes we simply use the mode as the extra tag.
// None of these modes need a data component.
Index: src/assembler.h
===================================================================
--- src/assembler.h (revision 6848)
+++ src/assembler.h (working copy)
@@ -184,6 +184,14 @@
// we do not normally record relocation info.
static const char* kFillerCommentString;
+ // The size of a comment is equal to tree bytes for the extra tagged pc +
+ // the tag for the data, and kPointerSize for the actual pointer to the
+ // comment.
+ static const int kRelocCommentSize = 3 + kPointerSize;
+
+ // The maximum size for a call instruction including pc-jump.
+ static const int kMaxCallSize = 6;
+
enum Mode {
// Please note the order is important (see IsCodeTarget,
IsGCRelocMode).
CONSTRUCT_CALL, // code target that is a call to a JavaScript
constructor.
Index: src/ia32/lithium-codegen-ia32.cc
===================================================================
--- src/ia32/lithium-codegen-ia32.cc (revision 6848)
+++ src/ia32/lithium-codegen-ia32.cc (working copy)
@@ -55,7 +55,9 @@
// 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);
+ // We can use up to RelocInfo::kMaxCallSize bytes for storing call,
+ // if this includes a long variable length pc-jump.
+ codegen_->AddRelocPadding(RelocInfo::kMaxCallSize);
}
codegen_->RecordSafepoint(pointers_, deoptimization_index_);
}
@@ -78,6 +80,7 @@
return GeneratePrologue() &&
GenerateBody() &&
GenerateDeferredCode() &&
+ GenerateRelocPadding() &&
GenerateSafepointTable();
}
@@ -122,6 +125,15 @@
}
+bool LCodeGen::GenerateRelocPadding() {
+ while(reloc_padding_count_ > 0) {
+ __ RecordComment(RelocInfo::kFillerCommentString, true);
+ reloc_padding_count_ -= RelocInfo::kRelocCommentSize;
+ }
+ return !is_aborted();
+}
+
+
bool LCodeGen::GeneratePrologue() {
ASSERT(is_generating());
@@ -382,6 +394,13 @@
ASSERT(instr != NULL);
LPointerMap* pointers = instr->pointer_map();
RecordPosition(pointers->position());
+ // A call will only take up 1 byte in the reloc_info, but patching in the
+ // deoptimizer will take up two since it uses RUNTIME_ENTY as relocation
+ // mode to avoid issues with GC. Any pc-jumps will be the same for both
+ // relocation info objects since we patch at the same spot as the
original
+ // call.
+ AddRelocPadding(1);
+
if (!adjusted) {
__ mov(esi, Operand(ebp, StandardFrameConstants::kContextOffset));
}
@@ -2302,9 +2321,9 @@
__ 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);
+ // We can use up to RelocInfo::kMaxCallSize bytes for storing call, if
+ // this includes a long variable length pc-jump.
+ AddRelocPadding(RelocInfo::kMaxCallSize);
__ call(FieldOperand(edi, JSFunction::kCodeEntryOffset));
}
Index: src/ia32/lithium-codegen-ia32.h
===================================================================
--- src/ia32/lithium-codegen-ia32.h (revision 6848)
+++ src/ia32/lithium-codegen-ia32.h (working copy)
@@ -60,6 +60,7 @@
status_(UNUSED),
deferred_(8),
osr_pc_offset_(-1),
+ reloc_padding_count_(0),
resolver_(this) {
PopulateDeoptimizationLiteralsWithInlinedFunctions();
}
@@ -102,6 +103,8 @@
// Emit frame translation commands for an environment.
void WriteTranslation(LEnvironment* environment, Translation*
translation);
+ void AddRelocPadding(int count) { reloc_padding_count_ += count; }
+
// Declare methods that deal with the individual node types.
#define DECLARE_DO(type) void Do##type(L##type* node);
LITHIUM_CONCRETE_INSTRUCTION_LIST(DECLARE_DO)
@@ -151,6 +154,9 @@
bool GeneratePrologue();
bool GenerateBody();
bool GenerateDeferredCode();
+ // Pad the reloc info to ensure that we have enough space to patch during
+ // deoptimization.
+ bool GenerateRelocPadding();
bool GenerateSafepointTable();
void CallCode(Handle<Code> code, RelocInfo::Mode mode, LInstruction*
instr,
@@ -250,6 +256,7 @@
TranslationBuffer translations_;
ZoneList<LDeferredCode*> deferred_;
int osr_pc_offset_;
+ int reloc_padding_count_;
// Builder that keeps track of safepoints in the code. The table
// itself is emitted at the end of the generated code.
Index: test/mjsunit/regress/regress-1174.js
===================================================================
--- test/mjsunit/regress/regress-1174.js (revision 0)
+++ test/mjsunit/regress/regress-1174.js (revision 0)
@@ -0,0 +1,43 @@
+// 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
+
+// Test that we do not crash when doing deoptimization of a function that
has
+// reloc info that only take up 1 byte per call (like KeyedStoreIC).
+
+function Regular() {
+ this[0] >>= 0;
+ this[1] ^= 1;
+}
+
+function foo() {
+ var regular = new Regular();
+ %DeoptimizeFunction(Regular);
+}
+
+foo();
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
