[v8-dev] MIPS: Error found in test262 on ARM: BinaryOpStub could call out to a built-in and push parameters … (issue 14850023)

Tue, 14 May 2013 17:12:59 -0700 

Reviewers: Paul Lind, mvstanton, danno, kisg, kilvadyb,

Description:
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=


Please review this at https://codereview.chromium.org/14850023/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/mips/code-stubs-mips.h
  M src/mips/code-stubs-mips.cc
  M src/mips/simulator-mips.cc


Index: src/mips/code-stubs-mips.cc
diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc
index f1c2553d1c23e8b4475d2d315784d45df653cbdf..e180a6014059e9f423293d234affaa079c727929 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,
Index: src/mips/code-stubs-mips.h
diff --git a/src/mips/code-stubs-mips.h b/src/mips/code-stubs-mips.h
index 3a84644a18c332ee1049d1c6976de5086a675d73..ec7d147988f4a2f9c1189723b19db796574ff3f4 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_;
 };

Index: src/mips/simulator-mips.cc
diff --git a/src/mips/simulator-mips.cc b/src/mips/simulator-mips.cc
index 467345807a69651ba240bf2b9f1dec764d28cb98..ffc8679b8fb59f26e2c8285a669f6f667867115d 100644 
--- a/src/mips/simulator-mips.cc
+++ b/src/mips/simulator-mips.cc
@@ -526,7 +526,7 @@ void MipsDebugger::Debug() {
           HeapObject* obj = reinterpret_cast<HeapObject*>(*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);


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group. 
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to