From 77f19618a0398b0225f64cfa433815d30fe5fe6a Mon Sep 17 00:00:00 2001 From: "palfia@homejinni.com" Date: Wed, 15 May 2013 00:57:19 +0000 Subject: [PATCH] MIPS: Error found in test262 on ARM: BinaryOpStub could call out to a built-in and push parameters without an enclosing frame. Port r14665 (15349aa) This corrupted stackwalking. BUG= Review URL: https://codereview.chromium.org/14850023 Patch from Balazs Kilvady . git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14673 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/mips/code-stubs-mips.cc | 81 +++++++++++++++++++++++++++++++++++++-------- src/mips/code-stubs-mips.h | 11 ++++-- src/mips/simulator-mips.cc | 2 +- 3 files changed, 76 insertions(+), 18 deletions(-) diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc index f1c2553..48e7e2b 100644 --- a/src/mips/code-stubs-mips.cc +++ b/src/mips/code-stubs-mips.cc @@ -2400,8 +2400,12 @@ void BinaryOpStub::GenerateSmiStub(MacroAssembler* masm) { GenerateTypeTransition(masm); __ bind(&call_runtime); - GenerateRegisterArgsPush(masm); - GenerateCallRuntime(masm); + { + FrameScope scope(masm, StackFrame::INTERNAL); + GenerateRegisterArgsPush(masm); + GenerateCallRuntime(masm); + } + __ Ret(); } @@ -2426,7 +2430,8 @@ void BinaryOpStub::GenerateBothStringStub(MacroAssembler* masm) { __ GetObjectType(right, a2, a2); __ Branch(&call_runtime, ge, a2, Operand(FIRST_NONSTRING_TYPE)); - StringAddStub string_add_stub(NO_STRING_CHECK_IN_STUB); + StringAddStub string_add_stub((StringAddFlags) + (ERECT_FRAME | NO_STRING_CHECK_IN_STUB)); GenerateRegisterArgsPush(masm); __ TailCallStub(&string_add_stub); @@ -2746,8 +2751,12 @@ void BinaryOpStub::GenerateInt32Stub(MacroAssembler* masm) { } __ bind(&call_runtime); - GenerateRegisterArgsPush(masm); - GenerateCallRuntime(masm); + { + FrameScope scope(masm, StackFrame::INTERNAL); + GenerateRegisterArgsPush(masm); + GenerateCallRuntime(masm); + } + __ Ret(); } @@ -2794,8 +2803,12 @@ void BinaryOpStub::GenerateNumberStub(MacroAssembler* masm) { GenerateTypeTransition(masm); __ bind(&call_runtime); - GenerateRegisterArgsPush(masm); - GenerateCallRuntime(masm); + { + FrameScope scope(masm, StackFrame::INTERNAL); + GenerateRegisterArgsPush(masm); + GenerateCallRuntime(masm); + } + __ Ret(); } @@ -2818,8 +2831,12 @@ void BinaryOpStub::GenerateGeneric(MacroAssembler* masm) { } __ bind(&call_runtime); - GenerateRegisterArgsPush(masm); - GenerateCallRuntime(masm); + { + FrameScope scope(masm, StackFrame::INTERNAL); + GenerateRegisterArgsPush(masm); + GenerateCallRuntime(masm); + } + __ Ret(); } @@ -2835,7 +2852,8 @@ void BinaryOpStub::GenerateAddStrings(MacroAssembler* masm) { __ GetObjectType(left, a2, a2); __ Branch(&left_not_string, ge, a2, Operand(FIRST_NONSTRING_TYPE)); - StringAddStub string_add_left_stub(NO_STRING_CHECK_LEFT_IN_STUB); + StringAddStub string_add_left_stub((StringAddFlags) + (ERECT_FRAME | NO_STRING_CHECK_LEFT_IN_STUB)); GenerateRegisterArgsPush(masm); __ TailCallStub(&string_add_left_stub); @@ -2845,7 +2863,8 @@ void BinaryOpStub::GenerateAddStrings(MacroAssembler* masm) { __ GetObjectType(right, a2, a2); __ Branch(&call_runtime, ge, a2, Operand(FIRST_NONSTRING_TYPE)); - StringAddStub string_add_right_stub(NO_STRING_CHECK_RIGHT_IN_STUB); + StringAddStub string_add_right_stub((StringAddFlags) + (ERECT_FRAME | NO_STRING_CHECK_RIGHT_IN_STUB)); GenerateRegisterArgsPush(masm); __ TailCallStub(&string_add_right_stub); @@ -6181,7 +6200,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { __ lw(a1, MemOperand(sp, 0 * kPointerSize)); // Second argument. // Make sure that both arguments are strings if not known in advance. - if (flags_ == NO_STRING_ADD_FLAGS) { + if ((flags_ & NO_STRING_ADD_FLAGS) != 0) { __ JumpIfEitherSmi(a0, a1, &call_runtime); // Load instance types. __ lw(t0, FieldMemOperand(a0, HeapObject::kMapOffset)); @@ -6470,15 +6489,49 @@ void StringAddStub::Generate(MacroAssembler* masm) { // Just jump to runtime to add the two strings. __ bind(&call_runtime); - __ TailCallRuntime(Runtime::kStringAdd, 2, 1); + if ((flags_ & ERECT_FRAME) != 0) { + GenerateRegisterArgsPop(masm); + // Build a frame. + { + FrameScope scope(masm, StackFrame::INTERNAL); + GenerateRegisterArgsPush(masm); + __ CallRuntime(Runtime::kStringAdd, 2); + } + __ Ret(); + } else { + __ TailCallRuntime(Runtime::kStringAdd, 2, 1); + } if (call_builtin.is_linked()) { __ bind(&call_builtin); - __ InvokeBuiltin(builtin_id, JUMP_FUNCTION); + if ((flags_ & ERECT_FRAME) != 0) { + GenerateRegisterArgsPop(masm); + // Build a frame. + { + FrameScope scope(masm, StackFrame::INTERNAL); + GenerateRegisterArgsPush(masm); + __ InvokeBuiltin(builtin_id, CALL_FUNCTION); + } + __ Ret(); + } else { + __ InvokeBuiltin(builtin_id, JUMP_FUNCTION); + } } } +void StringAddStub::GenerateRegisterArgsPush(MacroAssembler* masm) { + __ push(a0); + __ push(a1); +} + + +void StringAddStub::GenerateRegisterArgsPop(MacroAssembler* masm) { + __ pop(a1); + __ pop(a0); +} + + void StringAddStub::GenerateConvertArgument(MacroAssembler* masm, int stack_offset, Register arg, diff --git a/src/mips/code-stubs-mips.h b/src/mips/code-stubs-mips.h index 3a84644..ec7d147 100644 --- a/src/mips/code-stubs-mips.h +++ b/src/mips/code-stubs-mips.h @@ -212,11 +212,13 @@ class StringHelper : public AllStatic { // Flag that indicates how to generate code for the stub StringAddStub. enum StringAddFlags { - NO_STRING_ADD_FLAGS = 0, + NO_STRING_ADD_FLAGS = 1 << 0, // Omit left string check in stub (left is definitely a string). - NO_STRING_CHECK_LEFT_IN_STUB = 1 << 0, + NO_STRING_CHECK_LEFT_IN_STUB = 1 << 1, // Omit right string check in stub (right is definitely a string). - NO_STRING_CHECK_RIGHT_IN_STUB = 1 << 1, + NO_STRING_CHECK_RIGHT_IN_STUB = 1 << 2, + // Stub needs a frame before calling the runtime + ERECT_FRAME = 1 << 3, // Omit both string checks in stub. NO_STRING_CHECK_IN_STUB = NO_STRING_CHECK_LEFT_IN_STUB | NO_STRING_CHECK_RIGHT_IN_STUB @@ -242,6 +244,9 @@ class StringAddStub: public PlatformCodeStub { Register scratch4, Label* slow); + void GenerateRegisterArgsPush(MacroAssembler* masm); + void GenerateRegisterArgsPop(MacroAssembler* masm); + const StringAddFlags flags_; }; diff --git a/src/mips/simulator-mips.cc b/src/mips/simulator-mips.cc index 4673458..ffc8679 100644 --- a/src/mips/simulator-mips.cc +++ b/src/mips/simulator-mips.cc @@ -526,7 +526,7 @@ void MipsDebugger::Debug() { HeapObject* obj = reinterpret_cast(*cur); int value = *cur; Heap* current_heap = v8::internal::Isolate::Current()->heap(); - if (current_heap->Contains(obj) || ((value & 1) == 0)) { + if (((value & 1) == 0) || current_heap->Contains(obj)) { PrintF(" ("); if ((value & 1) == 0) { PrintF("smi %d", value / 2); -- 2.7.4