From 0c74af3d0faa3058e876df52f279c13b16b3f482 Mon Sep 17 00:00:00 2001 From: "lrn@chromium.org" Date: Tue, 8 Mar 2011 11:21:38 +0000 Subject: [PATCH] Fix Issue 1234. Ensure that there is always enough bytes between consequtive calls in unoptimized code to write a call instruction at the return points without overlapping. This handles the case where two return points were only four bytes apart (because the latter call was to a register). BUG=v8:1234 Review URL: http://codereview.chromium.org/6624091 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7089 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/ia32/macro-assembler-ia32.cc | 6 ++-- src/ia32/macro-assembler-ia32.h | 2 +- src/safepoint-table.h | 8 +++++ src/x64/assembler-x64.h | 7 +++++ src/x64/lithium-codegen-x64.cc | 25 ++++++++------- src/x64/lithium-codegen-x64.h | 3 ++ src/x64/macro-assembler-x64.cc | 49 +++++++++++++++++++++-------- src/x64/macro-assembler-x64.h | 53 +++++++++++++++++++++++--------- 8 files changed, 109 insertions(+), 44 deletions(-) diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc index 91b6651fe0..2ba95c4a64 100644 --- a/src/ia32/macro-assembler-ia32.cc +++ b/src/ia32/macro-assembler-ia32.cc @@ -1424,7 +1424,7 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected, const ParameterCount& actual, Handle code_constant, const Operand& code_operand, - Label* done, + NearLabel* done, InvokeFlag flag, PostCallGenerator* post_call_generator) { bool definitely_matches = false; @@ -1492,7 +1492,7 @@ void MacroAssembler::InvokeCode(const Operand& code, const ParameterCount& actual, InvokeFlag flag, PostCallGenerator* post_call_generator) { - Label done; + NearLabel done; InvokePrologue(expected, actual, Handle::null(), code, &done, flag, post_call_generator); if (flag == CALL_FUNCTION) { @@ -1512,7 +1512,7 @@ void MacroAssembler::InvokeCode(Handle code, RelocInfo::Mode rmode, InvokeFlag flag, PostCallGenerator* post_call_generator) { - Label done; + NearLabel done; Operand dummy(eax); InvokePrologue(expected, actual, code, dummy, &done, flag, post_call_generator); diff --git a/src/ia32/macro-assembler-ia32.h b/src/ia32/macro-assembler-ia32.h index 62bb0f3634..d7cfeb56c7 100644 --- a/src/ia32/macro-assembler-ia32.h +++ b/src/ia32/macro-assembler-ia32.h @@ -646,7 +646,7 @@ class MacroAssembler: public Assembler { const ParameterCount& actual, Handle code_constant, const Operand& code_operand, - Label* done, + NearLabel* done, InvokeFlag flag, PostCallGenerator* post_call_generator = NULL); diff --git a/src/safepoint-table.h b/src/safepoint-table.h index 8803d06f5b..2cb19942c4 100644 --- a/src/safepoint-table.h +++ b/src/safepoint-table.h @@ -228,6 +228,14 @@ class SafepointTableBuilder BASE_EMBEDDED { deoptimization_info_[index].pc_after_gap = pc; } + // Get the end pc offset of the last safepoint, including the code generated + // until the end of the gap following it. + unsigned GetPcAfterGap() { + int index = deoptimization_info_.length(); + if (index == 0) return 0; + return deoptimization_info_[index - 1].pc_after_gap; + } + // Emit the safepoint table after the body. The number of bits per // entry must be enough to hold all the pointer indexes. void Emit(Assembler* assembler, int bits_per_entry); diff --git a/src/x64/assembler-x64.h b/src/x64/assembler-x64.h index f6cd570936..ab26d745bf 100644 --- a/src/x64/assembler-x64.h +++ b/src/x64/assembler-x64.h @@ -395,6 +395,13 @@ class Operand BASE_EMBEDDED { // Does not check the "reg" part of the Operand. bool AddressUsesRegister(Register reg) const; + // Queries related to the size of the generated instruction. + // Whether the generated instruction will have a REX prefix. + bool requires_rex() const { return rex_ != 0; } + // Size of the ModR/M, SIB and displacement parts of the generated + // instruction. + int operand_size() const { return len_; } + private: byte rex_; byte buf_[6]; diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 29564ce1a6..c5124a0a7e 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -39,7 +39,7 @@ namespace internal { // When invoking builtins, we need to record the safepoint in the middle of // the invoke instruction sequence generated by the macro assembler. -class SafepointGenerator : public PostCallGenerator { +class SafepointGenerator : public CallWrapper { public: SafepointGenerator(LCodeGen* codegen, LPointerMap* pointers, @@ -48,29 +48,29 @@ class SafepointGenerator : public PostCallGenerator { : codegen_(codegen), pointers_(pointers), deoptimization_index_(deoptimization_index), - ensure_reloc_space_(ensure_reloc_space), - previous_safepoint_position_(-kMinSafepointSize) { } + ensure_reloc_space_(ensure_reloc_space) { } virtual ~SafepointGenerator() { } - virtual void Generate() { + virtual void BeforeCall(int call_size) { + ASSERT(call_size >= 0); // Ensure that we have enough space after the previous safepoint position - // for the generated code there. - int position = codegen_->masm()->pc_offset(); - ASSERT(position > previous_safepoint_position_); - if (position < previous_safepoint_position_ + kMinSafepointSize) { - int padding_size = - previous_safepoint_position_ + kMinSafepointSize - position; + // for the jump generated there. + int call_end = codegen_->masm()->pc_offset() + call_size; + int prev_jump_end = codegen_->LastSafepointEnd() + kMinSafepointSize; + if (call_end < prev_jump_end) { + int padding_size = prev_jump_end - call_end; STATIC_ASSERT(kMinSafepointSize <= 9); // One multibyte nop is enough. codegen_->masm()->nop(padding_size); - position += padding_size; } + } + + virtual void AfterCall() { // 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); } codegen_->RecordSafepoint(pointers_, deoptimization_index_); - previous_safepoint_position_ = position; } private: @@ -80,7 +80,6 @@ class SafepointGenerator : public PostCallGenerator { LPointerMap* pointers_; int deoptimization_index_; bool ensure_reloc_space_; - int previous_safepoint_position_; }; diff --git a/src/x64/lithium-codegen-x64.h b/src/x64/lithium-codegen-x64.h index ab0dffb1e4..8b49759aba 100644 --- a/src/x64/lithium-codegen-x64.h +++ b/src/x64/lithium-codegen-x64.h @@ -210,6 +210,9 @@ class LCodeGen BASE_EMBEDDED { int arguments, int deoptimization_index); void RecordPosition(int position); + int LastSafepointEnd() { + return static_cast(safepoints_.GetPcAfterGap()); + } static Condition TokenToCondition(Token::Value op, bool is_unsigned); void EmitGoto(int block, LDeferredCode* deferred_stack_check = NULL); diff --git a/src/x64/macro-assembler-x64.cc b/src/x64/macro-assembler-x64.cc index b468e82f95..c6829c5bfb 100644 --- a/src/x64/macro-assembler-x64.cc +++ b/src/x64/macro-assembler-x64.cc @@ -638,7 +638,7 @@ MaybeObject* MacroAssembler::TryJumpToExternalReference( void MacroAssembler::InvokeBuiltin(Builtins::JavaScript id, InvokeFlag flag, - PostCallGenerator* post_call_generator) { + CallWrapper* call_wrapper) { // Calls are not allowed in some stubs. ASSERT(flag == JUMP_FUNCTION || allow_stub_calls()); @@ -647,7 +647,7 @@ void MacroAssembler::InvokeBuiltin(Builtins::JavaScript id, // parameter count to avoid emitting code to do the check. ParameterCount expected(0); GetBuiltinEntry(rdx, id); - InvokeCode(rdx, expected, expected, flag, post_call_generator); + InvokeCode(rdx, expected, expected, flag, call_wrapper); } @@ -1424,20 +1424,41 @@ void MacroAssembler::Jump(Handle code_object, RelocInfo::Mode rmode) { void MacroAssembler::Call(ExternalReference ext) { +#ifdef DEBUG + int pre_position = pc_offset(); +#endif movq(kScratchRegister, ext); call(kScratchRegister); +#ifdef DEBUG + int post_position = pc_offset(); + CHECK_EQ(pre_position + CallSize(ext), post_position); +#endif } void MacroAssembler::Call(Address destination, RelocInfo::Mode rmode) { +#ifdef DEBUG + int pre_position = pc_offset(); +#endif movq(kScratchRegister, destination, rmode); call(kScratchRegister); +#ifdef DEBUG + int post_position = pc_offset(); + CHECK_EQ(pre_position + CallSize(destination, rmode), post_position); +#endif } void MacroAssembler::Call(Handle code_object, RelocInfo::Mode rmode) { +#ifdef DEBUG + int pre_position = pc_offset(); +#endif ASSERT(RelocInfo::IsCodeTarget(rmode)); call(code_object, rmode); +#ifdef DEBUG + int post_position = pc_offset(); + CHECK_EQ(pre_position + CallSize(code_object), post_position); +#endif } @@ -1868,7 +1889,7 @@ void MacroAssembler::InvokeCode(Register code, const ParameterCount& expected, const ParameterCount& actual, InvokeFlag flag, - PostCallGenerator* post_call_generator) { + CallWrapper* call_wrapper) { NearLabel done; InvokePrologue(expected, actual, @@ -1876,10 +1897,11 @@ void MacroAssembler::InvokeCode(Register code, code, &done, flag, - post_call_generator); + call_wrapper); if (flag == CALL_FUNCTION) { + if (call_wrapper != NULL) call_wrapper->BeforeCall(CallSize(code)); call(code); - if (post_call_generator != NULL) post_call_generator->Generate(); + if (call_wrapper != NULL) call_wrapper->AfterCall(); } else { ASSERT(flag == JUMP_FUNCTION); jmp(code); @@ -1893,7 +1915,7 @@ void MacroAssembler::InvokeCode(Handle code, const ParameterCount& actual, RelocInfo::Mode rmode, InvokeFlag flag, - PostCallGenerator* post_call_generator) { + CallWrapper* call_wrapper) { NearLabel done; Register dummy = rax; InvokePrologue(expected, @@ -1902,10 +1924,11 @@ void MacroAssembler::InvokeCode(Handle code, dummy, &done, flag, - post_call_generator); + call_wrapper); if (flag == CALL_FUNCTION) { + if (call_wrapper != NULL) call_wrapper->BeforeCall(CallSize(code)); Call(code, rmode); - if (post_call_generator != NULL) post_call_generator->Generate(); + if (call_wrapper != NULL) call_wrapper->AfterCall(); } else { ASSERT(flag == JUMP_FUNCTION); Jump(code, rmode); @@ -1917,7 +1940,7 @@ void MacroAssembler::InvokeCode(Handle code, void MacroAssembler::InvokeFunction(Register function, const ParameterCount& actual, InvokeFlag flag, - PostCallGenerator* post_call_generator) { + CallWrapper* call_wrapper) { ASSERT(function.is(rdi)); movq(rdx, FieldOperand(function, JSFunction::kSharedFunctionInfoOffset)); movq(rsi, FieldOperand(function, JSFunction::kContextOffset)); @@ -1928,14 +1951,14 @@ void MacroAssembler::InvokeFunction(Register function, movq(rdx, FieldOperand(rdi, JSFunction::kCodeEntryOffset)); ParameterCount expected(rbx); - InvokeCode(rdx, expected, actual, flag, post_call_generator); + InvokeCode(rdx, expected, actual, flag, call_wrapper); } void MacroAssembler::InvokeFunction(JSFunction* function, const ParameterCount& actual, InvokeFlag flag, - PostCallGenerator* post_call_generator) { + CallWrapper* call_wrapper) { ASSERT(function->is_compiled()); // Get the function and setup the context. Move(rdi, Handle(function)); @@ -1946,7 +1969,7 @@ void MacroAssembler::InvokeFunction(JSFunction* function, // the Code object every time we call the function. movq(rdx, FieldOperand(rdi, JSFunction::kCodeEntryOffset)); ParameterCount expected(function->shared()->formal_parameter_count()); - InvokeCode(rdx, expected, actual, flag, post_call_generator); + InvokeCode(rdx, expected, actual, flag, call_wrapper); } else { // Invoke the cached code. Handle code(function->code()); @@ -1956,7 +1979,7 @@ void MacroAssembler::InvokeFunction(JSFunction* function, actual, RelocInfo::CODE_TARGET, flag, - post_call_generator); + call_wrapper); } } diff --git a/src/x64/macro-assembler-x64.h b/src/x64/macro-assembler-x64.h index 7a7f1a278a..5908cefb1b 100644 --- a/src/x64/macro-assembler-x64.h +++ b/src/x64/macro-assembler-x64.h @@ -61,7 +61,7 @@ typedef Operand MemOperand; // Forward declaration. class JumpTarget; -class PostCallGenerator; +class CallWrapper; struct SmiIndex { SmiIndex(Register index_register, ScaleFactor scale) @@ -199,32 +199,32 @@ class MacroAssembler: public Assembler { const ParameterCount& expected, const ParameterCount& actual, InvokeFlag flag, - PostCallGenerator* post_call_generator = NULL); + CallWrapper* call_wrapper = NULL); void InvokeCode(Handle code, const ParameterCount& expected, const ParameterCount& actual, RelocInfo::Mode rmode, InvokeFlag flag, - PostCallGenerator* post_call_generator = NULL); + CallWrapper* call_wrapper = NULL); // Invoke the JavaScript function in the given register. Changes the // current context to the context in the function before invoking. void InvokeFunction(Register function, const ParameterCount& actual, InvokeFlag flag, - PostCallGenerator* post_call_generator = NULL); + CallWrapper* call_wrapper = NULL); void InvokeFunction(JSFunction* function, const ParameterCount& actual, InvokeFlag flag, - PostCallGenerator* post_call_generator = NULL); + CallWrapper* call_wrapper = NULL); // Invoke specified builtin JavaScript function. Adds an entry to // the unresolved list if the name does not resolve. void InvokeBuiltin(Builtins::JavaScript id, InvokeFlag flag, - PostCallGenerator* post_call_generator = NULL); + CallWrapper* call_wrapper = NULL); // Store the function for the given builtin in the target register. void GetBuiltinFunction(Register target, Builtins::JavaScript id); @@ -626,6 +626,26 @@ class MacroAssembler: public Assembler { void Call(ExternalReference ext); void Call(Handle code_object, RelocInfo::Mode rmode); + // The size of the code generated for different call instructions. + int CallSize(Address destination, RelocInfo::Mode rmode) { + return kCallInstructionLength; + } + int CallSize(ExternalReference ext) { + return kCallInstructionLength; + } + int CallSize(Handle code_object) { + // Code calls use 32-bit relative addressing. + return kShortCallInstructionLength; + } + int CallSize(Register target) { + // Opcode: REX_opt FF /2 m64 + return (target.high_bit() != 0) ? 3 : 2; + } + int CallSize(const Operand& target) { + // Opcode: REX_opt FF /2 m64 + return (target.requires_rex() ? 2 : 1) + target.operand_size(); + } + // Emit call to the code we are currently generating. void CallSelf() { Handle self(reinterpret_cast(CodeObject().location())); @@ -1018,7 +1038,7 @@ class MacroAssembler: public Assembler { Register code_register, LabelType* done, InvokeFlag flag, - PostCallGenerator* post_call_generator); + CallWrapper* call_wrapper); // Activation support. void EnterFrame(StackFrame::Type type); @@ -1084,13 +1104,17 @@ class CodePatcher { // Helper class for generating code or data associated with the code -// right after a call instruction. As an example this can be used to +// right before or after a call instruction. As an example this can be used to // generate safepoint data after calls for crankshaft. -class PostCallGenerator { +class CallWrapper { public: - PostCallGenerator() { } - virtual ~PostCallGenerator() { } - virtual void Generate() = 0; + CallWrapper() { } + virtual ~CallWrapper() { } + // Called just before emitting a call. Argument is the size of the generated + // call code. + virtual void BeforeCall(int call_size) = 0; + // Called just after emitting a call, i.e., at the return site for the call. + virtual void AfterCall() = 0; }; @@ -1801,7 +1825,7 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected, Register code_register, LabelType* done, InvokeFlag flag, - PostCallGenerator* post_call_generator) { + CallWrapper* call_wrapper) { bool definitely_matches = false; NearLabel invoke; if (expected.is_immediate()) { @@ -1851,8 +1875,9 @@ void MacroAssembler::InvokePrologue(const ParameterCount& expected, } if (flag == CALL_FUNCTION) { + if (call_wrapper != NULL) call_wrapper->BeforeCall(CallSize(adaptor)); Call(adaptor, RelocInfo::CODE_TARGET); - if (post_call_generator != NULL) post_call_generator->Generate(); + if (call_wrapper != NULL) call_wrapper->AfterCall(); jmp(done); } else { Jump(adaptor, RelocInfo::CODE_TARGET);