From c29003813ab9bd6ea7b6de40ea8f1fe21979f13f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 7 Feb 2020 08:38:12 -0500 Subject: [PATCH] Revert "[LLD][ELF][ARM] Do not substitute BL/BLX for non STT_FUNC symbols." There are still problems after the fix in "[ELF][ARM] Fix regression of BL->BLX substitution after D73542" so let's revert to get trunk back to green while we investigate. See https://reviews.llvm.org/D73542 This reverts commit 5461fa2b1fcfcfcd8e28e3ac3383d2245d5d90bf. This reverts commit 0b4a047bfbd11fe1f5abda8da0e2391c1918162a. --- lld/ELF/Arch/ARM.cpp | 46 +++++++---------- lld/test/ELF/arm-thumb-interwork-notfunc.s | 80 +++++------------------------- lld/test/ELF/arm-thumb-interwork-shared.s | 59 ++++++++++------------ lld/test/ELF/arm-thumb-undefined-weak.s | 1 - lld/test/ELF/arm-undefined-weak.s | 1 - 5 files changed, 57 insertions(+), 130 deletions(-) diff --git a/lld/ELF/Arch/ARM.cpp b/lld/ELF/Arch/ARM.cpp index 447b5d8..7f18eff 100644 --- a/lld/ELF/Arch/ARM.cpp +++ b/lld/ELF/Arch/ARM.cpp @@ -402,15 +402,11 @@ void ARM::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const { checkInt(loc, val, 31, rel); write32le(loc, (read32le(loc) & 0x80000000) | (val & ~0x80000000)); break; - case R_ARM_CALL: { - // R_ARM_CALL is used for BL and BLX instructions, for symbols of type - // STT_FUNC we choose whether to write a BL or BLX depending on the - // value of bit 0 of Val. With bit 0 == 1 denoting Thumb. If the symbol is - // not of type STT_FUNC then we must preserve the original instruction. - // PLT entries are always ARM state so we know we don't need to interwork. - bool isBlx = (read32le(loc) & 0xfe000000) == 0xfa000000; - bool interwork = rel.sym && rel.sym->isFunc() && rel.expr != R_PLT_PC; - if (interwork ? val & 1 : isBlx) { + case R_ARM_CALL: + // R_ARM_CALL is used for BL and BLX instructions, depending on the + // value of bit 0 of Val, we must select a BL or BLX instruction + if (val & 1) { + // If bit 0 of Val is 1 the target is Thumb, we must select a BLX. // The BLX encoding is 0xfa:H:imm24 where Val = imm24:H:'1' checkInt(loc, val, 26, rel); write32le(loc, 0xfa000000 | // opcode @@ -418,11 +414,11 @@ void ARM::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const { ((val >> 2) & 0x00ffffff)); // imm24 break; } - // BLX (always unconditional) instruction to an ARM Target, select an - // unconditional BL. - write32le(loc, 0xeb000000 | (read32le(loc) & 0x00ffffff)); + if ((read32le(loc) & 0xfe000000) == 0xfa000000) + // BLX (always unconditional) instruction to an ARM Target, select an + // unconditional BL. + write32le(loc, 0xeb000000 | (read32le(loc) & 0x00ffffff)); // fall through as BL encoding is shared with B - } LLVM_FALLTHROUGH; case R_ARM_JUMP24: case R_ARM_PC24: @@ -447,23 +443,16 @@ void ARM::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const { ((val >> 5) & 0x2000) | // J1 ((val >> 1) & 0x07ff)); // imm11 break; - case R_ARM_THM_CALL: { - // R_ARM_THM_CALL is used for BL and BLX instructions, for symbols of type - // STT_FUNC we choose whether to write a BL or BLX depending on the - // value of bit 0 of Val. With bit 0 == 0 denoting ARM, if the symbol is - // not of type STT_FUNC then we must preserve the original instruction. - // PLT entries are always ARM state so we know we need to interwork. - bool isBlx = (read16le(loc + 2) & 0x1000) == 0; - bool interwork = (rel.sym && rel.sym->isFunc()) || rel.expr == R_PLT_PC; - if (interwork ? (val & 1) == 0 : isBlx) { - // We are writing a BLX. Ensure BLX destination is 4-byte aligned. As - // the BLX instruction may only be two byte aligned. This must be done - // before overflow check. + case R_ARM_THM_CALL: + // R_ARM_THM_CALL is used for BL and BLX instructions, depending on the + // value of bit 0 of Val, we must select a BL or BLX instruction + if ((val & 1) == 0) { + // Ensure BLX destination is 4-byte aligned. As BLX instruction may + // only be two byte aligned. This must be done before overflow check val = alignTo(val, 4); - write16le(loc + 2, read16le(loc + 2) & ~0x1000); - } else { - write16le(loc + 2, (read16le(loc + 2) & ~0x1000) | 1 << 12); } + // Bit 12 is 0 for BLX, 1 for BL + write16le(loc + 2, (read16le(loc + 2) & ~0x1000) | (val & 1) << 12); if (!config->armJ1J2BranchEncoding) { // Older Arm architectures do not support R_ARM_THM_JUMP24 and have // different encoding rules and range due to J1 and J2 always being 1. @@ -477,7 +466,6 @@ void ARM::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const { ((val >> 1) & 0x07ff)); // imm11 break; } - } // Fall through as rest of encoding is the same as B.W LLVM_FALLTHROUGH; case R_ARM_THM_JUMP24: diff --git a/lld/test/ELF/arm-thumb-interwork-notfunc.s b/lld/test/ELF/arm-thumb-interwork-notfunc.s index 49a55f2..4f29fdf 100644 --- a/lld/test/ELF/arm-thumb-interwork-notfunc.s +++ b/lld/test/ELF/arm-thumb-interwork-notfunc.s @@ -23,9 +23,7 @@ thumb_func_with_explicit_notype: /// All the symbols that are targets of the branch relocations do not have /// type STT_FUNC. LLD should not insert interworking thunks as non STT_FUNC /// symbols have no state information, the ABI assumes the user has manually -/// done the interworking. For the BL and BLX instructions LLD should -/// preserve the original instruction instead of writing out the correct one -/// for the assumed state at the target. +/// done the interworking. .section .arm_caller, "ax", %progbits .balign 4 .arm @@ -37,24 +35,10 @@ _start: b .thumb_target b thumb_func_with_notype b thumb_func_with_explicit_notype - bl .arm_target - bl arm_func_with_notype - bl arm_func_with_explicit_notype - bl .thumb_target - bl thumb_func_with_notype - bl thumb_func_with_explicit_notype - blx .arm_target - blx arm_func_with_notype - blx arm_func_with_explicit_notype - blx .thumb_target - blx thumb_func_with_notype - blx thumb_func_with_explicit_notype .section .thumb_caller, "ax", %progbits .thumb .balign 4 - .global thumb_caller -thumb_caller: b.w .arm_target b.w arm_func_with_notype b.w arm_func_with_explicit_notype @@ -67,18 +51,6 @@ thumb_caller: beq.w .thumb_target beq.w thumb_func_with_notype beq.w thumb_func_with_explicit_notype - bl .arm_target - bl arm_func_with_notype - bl arm_func_with_explicit_notype - bl .thumb_target - bl thumb_func_with_notype - bl thumb_func_with_explicit_notype - blx .arm_target - blx arm_func_with_notype - blx arm_func_with_explicit_notype - blx .thumb_target - blx thumb_func_with_notype - blx thumb_func_with_explicit_notype // CHECK: 00012008 _start: // CHECK-NEXT: 12008: b #-16 @@ -87,41 +59,15 @@ thumb_caller: // CHECK-NEXT: 12014: b #-24 // CHECK-NEXT: 12018: b #-28 // CHECK-NEXT: 1201c: b #-32 -// CHECK-NEXT: 12020: bl #-40 -// CHECK-NEXT: 12024: bl #-44 -// CHECK-NEXT: 12028: bl #-48 -// CHECK-NEXT: 1202c: bl #-48 -// CHECK-NEXT: 12030: bl #-52 -// CHECK-NEXT: 12034: bl #-56 -// CHECK-NEXT: 12038: blx #-64 -// CHECK-NEXT: 1203c: blx #-68 -// CHECK-NEXT: 12040: blx #-72 -// CHECK-NEXT: 12044: blx #-72 -// CHECK-NEXT: 12048: blx #-76 -// CHECK-NEXT: 1204c: blx #-80 - -// CHECK: 00012050 thumb_caller: -// CHECK-NEXT: 12050: b.w #-84 -// CHECK-NEXT: 12054: b.w #-88 -// CHECK-NEXT: 12058: b.w #-92 -// CHECK-NEXT: 1205c: b.w #-92 -// CHECK-NEXT: 12060: b.w #-96 -// CHECK-NEXT: 12064: b.w #-100 -// CHECK-NEXT: 12068: beq.w #-108 -// CHECK-NEXT: 1206c: beq.w #-112 -// CHECK-NEXT: 12070: beq.w #-116 -// CHECK-NEXT: 12074: beq.w #-116 -// CHECK-NEXT: 12078: beq.w #-120 -// CHECK-NEXT: 1207c: beq.w #-124 -// CHECK-NEXT: 12080: bl #-132 -// CHECK-NEXT: 12084: bl #-136 -// CHECK-NEXT: 12088: bl #-140 -// CHECK-NEXT: 1208c: bl #-140 -// CHECK-NEXT: 12090: bl #-144 -// CHECK-NEXT: 12094: bl #-148 -// CHECK-NEXT: 12098: blx #-156 -// CHECK-NEXT: 1209c: blx #-160 -// CHECK-NEXT: 120a0: blx #-164 -// CHECK-NEXT: 120a4: blx #-164 -// CHECK-NEXT: 120a8: blx #-168 -// CHECK-NEXT: 120ac: blx #-172 +// CHECK: 12020: b.w #-36 +// CHECK-NEXT: 12024: b.w #-40 +// CHECK-NEXT: 12028: b.w #-44 +// CHECK-NEXT: 1202c: b.w #-44 +// CHECK-NEXT: 12030: b.w #-48 +// CHECK-NEXT: 12034: b.w #-52 +// CHECK-NEXT: 12038: beq.w #-60 +// CHECK-NEXT: 1203c: beq.w #-64 +// CHECK-NEXT: 12040: beq.w #-68 +// CHECK-NEXT: 12044: beq.w #-68 +// CHECK-NEXT: 12048: beq.w #-72 +// CHECK-NEXT: 1204c: beq.w #-76 diff --git a/lld/test/ELF/arm-thumb-interwork-shared.s b/lld/test/ELF/arm-thumb-interwork-shared.s index e8464eb..0b699bdc 100644 --- a/lld/test/ELF/arm-thumb-interwork-shared.s +++ b/lld/test/ELF/arm-thumb-interwork-shared.s @@ -1,7 +1,7 @@ // REQUIRES: arm // RUN: llvm-mc -arm-add-build-attributes -filetype=obj -triple=thumbv7a-none-linux-gnueabi %s -o %t // RUN: ld.lld %t --shared -o %t.so -// RUN: llvm-objdump -d --no-show-raw-insn -triple=thumbv7a-none-linux-gnueabi %t.so | FileCheck %s +// RUN: llvm-objdump -d -triple=thumbv7a-none-linux-gnueabi %t.so | FileCheck %s .syntax unified .global sym1 .global elsewhere @@ -10,51 +10,46 @@ sym1: b.w elsewhere b.w weakref - bl elsewhere - bl weakref - // Check that we generate a thunk for an undefined symbol called via a plt // entry. // CHECK: Disassembly of section .text: // CHECK-EMPTY: // CHECK-NEXT: sym1: -// CHECK-NEXT: 11e0: b.w #12 <__ThumbV7PILongThunk_elsewhere> -// CHECK-NEXT: b.w #20 <__ThumbV7PILongThunk_weakref> -// CHECK-NEXT: blx #68 -// CHECK-NEXT: blx #80 +// CHECK-NEXT: 11e0: 00 f0 02 b8 b.w #4 <__ThumbV7PILongThunk_elsewhere> +// CHECK-NEXT: 11e4: 00 f0 06 b8 b.w #12 <__ThumbV7PILongThunk_weakref> // CHECK: __ThumbV7PILongThunk_elsewhere: -// CHECK-NEXT: 11f0: movw r12, #52 -// CHECK-NEXT: movt r12, #0 -// CHECK-NEXT: add r12, pc -// CHECK-NEXT: bx r12 +// CHECK-NEXT: 11e8: 40 f2 2c 0c movw r12, #44 +// CHECK-NEXT: 11ec: c0 f2 00 0c movt r12, #0 +// CHECK-NEXT: 11f0: fc 44 add r12, pc +// CHECK-NEXT: 11f2: 60 47 bx r12 // CHECK: __ThumbV7PILongThunk_weakref: -// CHECK-NEXT: 11fc: movw r12, #56 -// CHECK-NEXT: movt r12, #0 -// CHECK-NEXT: add r12, pc -// CHECK-NEXT: bx r12 +// CHECK-NEXT: 11f4: 40 f2 30 0c movw r12, #48 +// CHECK-NEXT: 11f8: c0 f2 00 0c movt r12, #0 +// CHECK-NEXT: 11fc: fc 44 add r12, pc +// CHECK-NEXT: 11fe: 60 47 bx r12 // CHECK: Disassembly of section .plt: // CHECK-EMPTY: // CHECK-NEXT: $a: -// CHECK-NEXT: 1210: str lr, [sp, #-4]! -// CHECK-NEXT: add lr, pc, #0, #12 -// CHECK-NEXT: add lr, lr, #8192 -// CHECK-NEXT: ldr pc, [lr, #148]! +// CHECK-NEXT: 1200: 04 e0 2d e5 str lr, [sp, #-4]! +// CHECK-NEXT: 1204: 00 e6 8f e2 add lr, pc, #0, #12 +// CHECK-NEXT: 1208: 02 ea 8e e2 add lr, lr, #8192 +// CHECK-NEXT: 120c: 94 f0 be e5 ldr pc, [lr, #148]! // CHECK: $d: -// CHECK-NEXT: 1220: d4 d4 d4 d4 .word 0xd4d4d4d4 -// CHECK-NEXT: .word 0xd4d4d4d4 -// CHECK-NEXT: .word 0xd4d4d4d4 -// CHECK-NEXT: .word 0xd4d4d4d4 +// CHECK-NEXT: 1210: d4 d4 d4 d4 .word 0xd4d4d4d4 +// CHECK-NEXT: 1214: d4 d4 d4 d4 .word 0xd4d4d4d4 +// CHECK-NEXT: 1218: d4 d4 d4 d4 .word 0xd4d4d4d4 +// CHECK-NEXT: 121c: d4 d4 d4 d4 .word 0xd4d4d4d4 // CHECK: $a: -// CHECK-NEXT: 1230: add r12, pc, #0, #12 -// CHECK-NEXT: add r12, r12, #8192 -// CHECK-NEXT: ldr pc, [r12, #124]! +// CHECK-NEXT: 1220: 00 c6 8f e2 add r12, pc, #0, #12 +// CHECK-NEXT: 1224: 02 ca 8c e2 add r12, r12, #8192 +// CHECK-NEXT: 1228: 7c f0 bc e5 ldr pc, [r12, #124]! // CHECK: $d: -// CHECK-NEXT: 123c: d4 d4 d4 d4 .word 0xd4d4d4d4 +// CHECK-NEXT: 122c: d4 d4 d4 d4 .word 0xd4d4d4d4 // CHECK: $a: -// CHECK-NEXT: 1240: add r12, pc, #0, #12 -// CHECK-NEXT: add r12, r12, #8192 -// CHECK-NEXT: ldr pc, [r12, #112]! +// CHECK-NEXT: 1230: 00 c6 8f e2 add r12, pc, #0, #12 +// CHECK-NEXT: 1234: 02 ca 8c e2 add r12, r12, #8192 +// CHECK-NEXT: 1238: 70 f0 bc e5 ldr pc, [r12, #112]! // CHECK: $d: -// CHECK-NEXT: 124c: d4 d4 d4 d4 .word 0xd4d4d4d4 +// CHECK-NEXT: 123c: d4 d4 d4 d4 .word 0xd4d4d4d4 diff --git a/lld/test/ELF/arm-thumb-undefined-weak.s b/lld/test/ELF/arm-thumb-undefined-weak.s index 82069a7..b6812b8 100644 --- a/lld/test/ELF/arm-thumb-undefined-weak.s +++ b/lld/test/ELF/arm-thumb-undefined-weak.s @@ -10,7 +10,6 @@ .syntax unified .weak target - .type target, %function .text .global _start diff --git a/lld/test/ELF/arm-undefined-weak.s b/lld/test/ELF/arm-undefined-weak.s index c7e3f5f..2d47263 100644 --- a/lld/test/ELF/arm-undefined-weak.s +++ b/lld/test/ELF/arm-undefined-weak.s @@ -12,7 +12,6 @@ .syntax unified .weak target - .type target, %function .text .global _start -- 2.7.4