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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9063 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc
index 89df079..0ec3692 100644 (file)
--- a/src/arm/assembler-arm.cc
+++ b/src/arm/assembler-arm.cc
@@ -692,11 +692,11 @@ void Assembler::bind(Label* L) {
 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);
   }
 }
 

diff --git a/src/mips/assembler-mips.cc b/src/mips/assembler-mips.cc
index 51642e0..28ac557 100644 (file)
--- a/src/mips/assembler-mips.cc
+++ b/src/mips/assembler-mips.cc
@@ -780,10 +780,10 @@ void Assembler::bind(Label* L) {
 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);
   }
 }

diff --git a/test/cctest/test-assembler-arm.cc b/test/cctest/test-assembler-arm.cc
index 1703203..ecbf956 100644 (file)
--- a/test/cctest/test-assembler-arm.cc
+++ b/test/cctest/test-assembler-arm.cc
@@ -1010,4 +1010,18 @@ TEST(11) {
   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 __

diff --git a/test/cctest/test-assembler-ia32.cc b/test/cctest/test-assembler-ia32.cc
index e9d799b..839b7f5 100644 (file)
--- a/test/cctest/test-assembler-ia32.cc
+++ b/test/cctest/test-assembler-ia32.cc
@@ -394,4 +394,18 @@ TEST(AssemblerIa329) {
   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 __

diff --git a/test/cctest/test-assembler-mips.cc b/test/cctest/test-assembler-mips.cc
index 065569d..ca11a2a 100644 (file)
--- a/test/cctest/test-assembler-mips.cc
+++ b/test/cctest/test-assembler-mips.cc
@@ -1259,4 +1259,18 @@ TEST(MIPS14) {
   }
 }
 
+
+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 __

diff --git a/test/cctest/test-assembler-x64.cc b/test/cctest/test-assembler-x64.cc
index ea70f54..28f7c9b 100644 (file)
--- a/test/cctest/test-assembler-x64.cc
+++ b/test/cctest/test-assembler-x64.cc
@@ -46,6 +46,7 @@ using v8::internal::Operand;
 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;
@@ -345,4 +346,17 @@ TEST(OperandRegisterDependency) {
   }
 }
 
+
+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 __