From a4c072ed47e14ab64cb8b228eb1409718cca5163 Mon Sep 17 00:00:00 2001 From: "bmeurer@chromium.org" Date: Thu, 1 Aug 2013 08:13:08 +0000 Subject: [PATCH] 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 . git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15997 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- AUTHORS | 1 + src/arm/assembler-arm.cc | 59 ++++++++++++------------------- src/arm/assembler-arm.h | 1 - test/cctest/test-assembler-arm.cc | 21 +++++++++++ 4 files changed, 44 insertions(+), 38 deletions(-) diff --git a/AUTHORS b/AUTHORS index 1a927c4573..46e3a14bc1 100644 --- 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 Alexander Botero-Lowry diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc index ba0dc4b81d..a9db5a5994 100644 --- 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 496eb3e880..f647848de5 100644 --- 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 cb677b3bb6..cac162e018 100644 --- 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 __