[v8-dev] [v8] r9063 committed - Fixed a bug in the chaining of fixup position...

Tue, 30 Aug 2011 00:37:40 -0700 

Revision: 9063
Author:   [email protected]
Date:     Tue Aug 30 00:36:31 2011
Log:      Fixed a bug in the chaining of fixup position
The ARM and MIPS assemblers had a bug where they did not handle the last element in the list of code positions correctly during the fixup of offsets for forward jumps. This happened when the first instruction contained a forward jump to a 
label, and that label was used in a forward jump later, too.

Unified the code for Assembler::next on ARM and MIPS while we were there.

Added test cases, even for ia32/x64, which seem to be correct, even I don't
fully understand why... %-}

BUG=v8:1644
Review URL: http://codereview.chromium.org/7786001
http://code.google.com/p/v8/source/detail?r=9063

Modified:
 /branches/bleeding_edge/src/arm/assembler-arm.cc
 /branches/bleeding_edge/src/mips/assembler-mips.cc
 /branches/bleeding_edge/test/cctest/test-assembler-arm.cc
 /branches/bleeding_edge/test/cctest/test-assembler-ia32.cc
 /branches/bleeding_edge/test/cctest/test-assembler-mips.cc
 /branches/bleeding_edge/test/cctest/test-assembler-x64.cc

=======================================
--- /branches/bleeding_edge/src/arm/assembler-arm.cc Mon Jul 18 03:44:13 2011 +++ /branches/bleeding_edge/src/arm/assembler-arm.cc Tue Aug 30 00:36:31 2011 
@@ -692,11 +692,11 @@
 void Assembler::next(Label* L) {
   ASSERT(L->is_linked());
   int link = target_at(L->pos());
-  if (link > 0) {
-    L->link_to(link);
-  } else {
-    ASSERT(link == kEndOfChain);
+  if (link == kEndOfChain) {
     L->Unuse();
+  } else {
+    ASSERT(link >= 0);
+    L->link_to(link);
   }
 }

=======================================
--- /branches/bleeding_edge/src/mips/assembler-mips.cc Mon Jul 18 07:46:35 2011 +++ /branches/bleeding_edge/src/mips/assembler-mips.cc Tue Aug 30 00:36:31 2011 
@@ -780,10 +780,10 @@
 void Assembler::next(Label* L) {
   ASSERT(L->is_linked());
   int link = target_at(L->pos());
-  ASSERT(link > 0 || link == kEndOfChain);
   if (link == kEndOfChain) {
     L->Unuse();
-  } else if (link > 0) {
+  } else {
+    ASSERT(link >= 0);
     L->link_to(link);
   }
 }
=======================================
--- /branches/bleeding_edge/test/cctest/test-assembler-arm.cc Mon Apr 18 06:53:11 2011 +++ /branches/bleeding_edge/test/cctest/test-assembler-arm.cc Tue Aug 30 00:36:31 2011 
@@ -1009,5 +1009,19 @@
   CHECK_EQ(0x00000000, i.c);
   CHECK_EQ(0xffffffff, i.d);
 }
+
+
+TEST(12) {
+  // Test chaining of label usages within instructions (issue 1644).
+  InitializeVM();
+  v8::HandleScope scope;
+  Assembler assm(Isolate::Current(), NULL, 0);
+
+  Label target;
+  __ b(eq, &target);
+  __ b(ne, &target);
+  __ bind(&target);
+  __ nop();
+}

 #undef __
=======================================
--- /branches/bleeding_edge/test/cctest/test-assembler-ia32.cc Wed May 11 06:26:07 2011 +++ /branches/bleeding_edge/test/cctest/test-assembler-ia32.cc Tue Aug 30 00:36:31 2011 
@@ -393,5 +393,19 @@
   CHECK_EQ(kGreater, f(3.3, 2.2));
   CHECK_EQ(kNaN, f(OS::nan_value(), 1.1));
 }
+
+
+TEST(AssemblerIa3210) {
+  // Test chaining of label usages within instructions (issue 1644).
+  InitializeVM();
+  v8::HandleScope scope;
+  Assembler assm(Isolate::Current(), NULL, 0);
+
+  Label target;
+  __ j(equal, &target);
+  __ j(not_equal, &target);
+  __ bind(&target);
+  __ nop();
+}

 #undef __
=======================================
--- /branches/bleeding_edge/test/cctest/test-assembler-mips.cc Thu Aug 25 05:12:25 2011 +++ /branches/bleeding_edge/test/cctest/test-assembler-mips.cc Tue Aug 30 00:36:31 2011 
@@ -1258,5 +1258,19 @@
     CHECK_ROUND_RESULT(cvt);
   }
 }
+
+
+TEST(MIPS15) {
+  // Test chaining of label usages within instructions (issue 1644).
+  InitializeVM();
+  v8::HandleScope scope;
+  Assembler assm(Isolate::Current(), NULL, 0);
+
+  Label target;
+  __ beq(v0, v1, &target);
+  __ bne(v0, v1, &target);
+  __ bind(&target);
+  __ nop();
+}

 #undef __
=======================================
--- /branches/bleeding_edge/test/cctest/test-assembler-x64.cc Thu Mar 31 09:17:37 2011 +++ /branches/bleeding_edge/test/cctest/test-assembler-x64.cc Tue Aug 30 00:36:31 2011 
@@ -46,6 +46,7 @@
 using v8::internal::byte;
 using v8::internal::greater;
 using v8::internal::less_equal;
+using v8::internal::equal;
 using v8::internal::not_equal;
 using v8::internal::r13;
 using v8::internal::r15;
@@ -344,5 +345,18 @@
     CHECK(!Operand(rsp, rbp, times_1, offset).AddressUsesRegister(r13));
   }
 }
+
+
+TEST(AssemblerX64LabelChaining) {
+  // Test chaining of label usages within instructions (issue 1644).
+  v8::HandleScope scope;
+  Assembler assm(Isolate::Current(), NULL, 0);
+
+  Label target;
+  __ j(equal, &target);
+  __ j(not_equal, &target);
+  __ bind(&target);
+  __ nop();
+}

 #undef __

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to