Fix a crash when generating forward jumps to labels at very high assembly offsets

The first jump to a specific label was marked as jump to absolute
position -4. This value was stored in the assembly as a branch to a
offset (-4 - (instruction offset + 8)). The offset is only 24 bit
long on ARM. Thus instruction offsets higher than 2^23 - 12 would overflow
the offset.

Fix by denoting the first jump to a label by storing the jump
instruction location as the target. This will result in offset of -8,
which of course always fits in the branch instruction.

BUG=2736
TEST=cctest/test-assembler-arm/17
R=bmeurer@chromium.org, svenpanne@chromium.org

Review URL: https://codereview.chromium.org/17116006

Patch from Kimmo Kinnunen <kkinnunen@nvidia.com>.

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

diff --git a/AUTHORS b/AUTHORS
index 1a927c4..46e3a14 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -10,6 +10,7 @@ Hewlett-Packard Development Company, LP
 Igalia, S.L.
 Joyent, Inc.
 Bloomberg Finance L.P.
+NVIDIA Corporation
 
 Akinori MUSHA <knu@FreeBSD.org>
 Alexander Botero-Lowry <alexbl@FreeBSD.org>

diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc
index ba0dc4b..a9db5a5 100644 (file)
--- a/src/arm/assembler-arm.cc
+++ b/src/arm/assembler-arm.cc
@@ -764,10 +764,13 @@ int Assembler::GetCmpImmediateRawImmediate(Instr instr) {
 // Linked labels refer to unknown positions in the code
 // to be generated; pos() is the position of the last
 // instruction using the label.
-
-
-// The link chain is terminated by a negative code position (must be aligned)
-const int kEndOfChain = -4;
+//
+// The linked labels form a link chain by making the branch offset
+// in the instruction steam to point to the previous branch
+// instruction using the same label.
+//
+// The link chain is terminated by a branch offset pointing to the
+// same position.
 
 
 int Assembler::target_at(int pos)  {
@@ -790,7 +793,7 @@ int Assembler::target_at(int pos)  {
 void Assembler::target_at_put(int pos, int target_pos) {
   Instr instr = instr_at(pos);
   if ((instr & ~kImm24Mask) == 0) {
-    ASSERT(target_pos == kEndOfChain || target_pos >= 0);
+    ASSERT(target_pos == pos || target_pos >= 0);
     // Emitted label constant, not part of a branch.
     // Make label relative to Code* of generated Code object.
     instr_at_put(pos, target_pos + (Code::kHeaderSize - kHeapObjectTag));
@@ -886,27 +889,6 @@ void Assembler::bind_to(Label* L, int pos) {
 }
 
 
-void Assembler::link_to(Label* L, Label* appendix) {
-  if (appendix->is_linked()) {
-    if (L->is_linked()) {
-      // Append appendix to L's list.
-      int fixup_pos;
-      int link = L->pos();
-      do {
-        fixup_pos = link;
-        link = target_at(fixup_pos);
-      } while (link > 0);
-      ASSERT(link == kEndOfChain);
-      target_at_put(fixup_pos, appendix->pos());
-    } else {
-      // L is empty, simply use appendix.
-      *L = *appendix;
-    }
-  }
-  appendix->Unuse();  // appendix should not be used anymore
-}
-
-
 void Assembler::bind(Label* L) {
   ASSERT(!L->is_bound());  // label can only be bound once
   bind_to(L, pc_offset());
@@ -916,7 +898,9 @@ void Assembler::bind(Label* L) {
 void Assembler::next(Label* L) {
   ASSERT(L->is_linked());
   int link = target_at(L->pos());
-  if (link == kEndOfChain) {
+  if (link == L->pos()) {
+    // Branch target points to the same instuction. This is the end of the link
+    // chain.
     L->Unuse();
   } else {
     ASSERT(link >= 0);
@@ -1229,9 +1213,11 @@ int Assembler::branch_offset(Label* L, bool jump_elimination_allowed) {
     target_pos = L->pos();
   } else {
     if (L->is_linked()) {
-      target_pos = L->pos();  // L's link
+      // Point to previous instruction that uses the link.
+      target_pos = L->pos();
     } else {
-      target_pos = kEndOfChain;
+      // First entry of the link chain points to itself.
+      target_pos = pc_offset();
     }
     L->link_to(pc_offset());
   }
@@ -1245,17 +1231,16 @@ int Assembler::branch_offset(Label* L, bool jump_elimination_allowed) {
 
 void Assembler::label_at_put(Label* L, int at_offset) {
   int target_pos;
-  if (L->is_bound()) {
+  ASSERT(!L->is_bound());
+  if (L->is_linked()) {
+    // Point to previous instruction that uses the link.
     target_pos = L->pos();
   } else {
-    if (L->is_linked()) {
-      target_pos = L->pos();  // L's link
-    } else {
-      target_pos = kEndOfChain;
-    }
-    L->link_to(at_offset);
-    instr_at_put(at_offset, target_pos + (Code::kHeaderSize - kHeapObjectTag));
+    // First entry of the link chain points to itself.
+    target_pos = at_offset;
   }
+  L->link_to(at_offset);
+  instr_at_put(at_offset, target_pos + (Code::kHeaderSize - kHeapObjectTag));
 }
 
 

diff --git a/src/arm/assembler-arm.h b/src/arm/assembler-arm.h
index 496eb3e..f647848 100644 (file)
--- a/src/arm/assembler-arm.h
+++ b/src/arm/assembler-arm.h
@@ -1548,7 +1548,6 @@ class Assembler : public AssemblerBase {
   // Labels
   void print(Label* L);
   void bind_to(Label* L, int pos);
-  void link_to(Label* L, Label* appendix);
   void next(Label* L);
 
   enum UseConstantPoolMode {

diff --git a/test/cctest/test-assembler-arm.cc b/test/cctest/test-assembler-arm.cc
index cb677b3..cac162e 100644 (file)
--- a/test/cctest/test-assembler-arm.cc
+++ b/test/cctest/test-assembler-arm.cc
@@ -1418,4 +1418,25 @@ TEST(16) {
   CHECK_EQ(0x11121313, t.dst4);
 }
 
+
+TEST(17) {
+  // Test generating labels at high addresses.
+  // Should not assert.
+  CcTest::InitializeVM();
+  Isolate* isolate = Isolate::Current();
+  HandleScope scope(isolate);
+
+  // Generate a code segment that will be longer than 2^24 bytes.
+  Assembler assm(isolate, NULL, 0);
+  for (size_t i = 0; i < 1 << 23 ; ++i) {  // 2^23
+    __ nop();
+  }
+
+  Label target;
+  __ b(eq, &target);
+  __ bind(&target);
+  __ nop();
+}
+
+
 #undef __