Revision: 15997
Author: [email protected]
Date: Thu Aug 1 01:13:08 2013
Log: 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
[email protected], [email protected]
Review URL: https://codereview.chromium.org/17116006
Patch from Kimmo Kinnunen <[email protected]>.
http://code.google.com/p/v8/source/detail?r=15997
Modified:
/branches/bleeding_edge/AUTHORS
/branches/bleeding_edge/src/arm/assembler-arm.cc
/branches/bleeding_edge/src/arm/assembler-arm.h
/branches/bleeding_edge/test/cctest/test-assembler-arm.cc
=======================================
--- /branches/bleeding_edge/AUTHORS Mon Apr 22 08:01:45 2013
+++ /branches/bleeding_edge/AUTHORS Thu Aug 1 01:13:08 2013
@@ -10,6 +10,7 @@
Igalia, S.L.
Joyent, Inc.
Bloomberg Finance L.P.
+NVIDIA Corporation
Akinori MUSHA <[email protected]>
Alexander Botero-Lowry <[email protected]>
=======================================
--- /branches/bleeding_edge/src/arm/assembler-arm.cc Thu Jul 25 08:04:38
2013
+++ /branches/bleeding_edge/src/arm/assembler-arm.cc Thu Aug 1 01:13:08
2013
@@ -764,10 +764,13 @@
// 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 @@
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));
@@ -884,27 +887,6 @@
if (pos > last_bound_pos_)
last_bound_pos_ = 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) {
@@ -916,7 +898,9 @@
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 @@
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 @@
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));
}
=======================================
--- /branches/bleeding_edge/src/arm/assembler-arm.h Thu Jul 25 08:04:38 2013
+++ /branches/bleeding_edge/src/arm/assembler-arm.h Thu Aug 1 01:13:08 2013
@@ -1548,7 +1548,6 @@
// 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 {
=======================================
--- /branches/bleeding_edge/test/cctest/test-assembler-arm.cc Thu Jul 25
08:04:38 2013
+++ /branches/bleeding_edge/test/cctest/test-assembler-arm.cc Thu Aug 1
01:13:08 2013
@@ -1417,5 +1417,26 @@
CHECK_EQ(0x00000003, t.dst3);
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 __
--
--
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.
