From 8af3f965e08414c95165d9014c2f5397932aad23 Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Mon, 6 Apr 2015 18:44:42 +0000 Subject: [PATCH] ARM: do not relax Thumb1 -> Thumb2 if only Thumb1 is available. After recognising that a certain narrow instruction might need a relocation to be represented, we used to unconditionally relax it to a Thumb2 instruction to permit this. Unfortunately, some CPUs (e.g. v6m) don't even have most Thumb2 instructions, so we end up emitting a completely invalid instruction. Theoretically, ELF does have relocations for these situations; but they are fairly unusable with such short ranges and the ABI document even says they're documented "for completeness". So an error is probably better there too. rdar://20391953 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@234195 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp | 12 +++++++----- lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h | 2 ++ .../ARM/MCTargetDesc/ARMELFObjectWriter.cpp | 8 ++++++-- .../ARM/MCTargetDesc/ARMMachObjectWriter.cpp | 7 +------ test/MC/ARM/thumb1-relax-adr.s | 9 +++++++++ test/MC/ARM/thumb1-relax-bcc.s | 12 ++++++++++++ test/MC/ARM/thumb1-relax-br.s | 19 +++++++++++++++++++ test/MC/ARM/thumb1-relax-ldrlit.s | 9 +++++++++ 8 files changed, 65 insertions(+), 13 deletions(-) create mode 100644 test/MC/ARM/thumb1-relax-adr.s create mode 100644 test/MC/ARM/thumb1-relax-bcc.s create mode 100644 test/MC/ARM/thumb1-relax-br.s create mode 100644 test/MC/ARM/thumb1-relax-ldrlit.s diff --git a/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp index 9dcfdc0f5d5..904d25632c8 100644 --- a/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp +++ b/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp @@ -153,18 +153,20 @@ void ARMAsmBackend::handleAssemblerFlag(MCAssemblerFlag Flag) { } } // end anonymous namespace -static unsigned getRelaxedOpcode(unsigned Op) { +unsigned ARMAsmBackend::getRelaxedOpcode(unsigned Op) const { + bool HasThumb2 = STI->getFeatureBits() & ARM::FeatureThumb2; + switch (Op) { default: return Op; case ARM::tBcc: - return ARM::t2Bcc; + return HasThumb2 ? ARM::t2Bcc : Op; case ARM::tLDRpci: - return ARM::t2LDRpci; + return HasThumb2 ? ARM::t2LDRpci : Op; case ARM::tADR: - return ARM::t2ADR; + return HasThumb2 ? ARM::t2ADR : Op; case ARM::tB: - return ARM::t2B; + return HasThumb2 ? ARM::t2B : Op; case ARM::tCBZ: return ARM::tHINT; case ARM::tCBNZ: diff --git a/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h b/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h index f4f10821037..4fa8c79e5af 100644 --- a/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h +++ b/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h @@ -47,6 +47,8 @@ public: void applyFixup(const MCFixup &Fixup, char *Data, unsigned DataSize, uint64_t Value, bool IsPCRel) const override; + unsigned getRelaxedOpcode(unsigned Op) const; + bool mayNeedRelaxation(const MCInst &Inst) const override; bool fixupNeedsRelaxation(const MCFixup &Fixup, uint64_t Value, diff --git a/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp b/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp index a821a6b0b53..e06e2efdbe3 100644 --- a/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp +++ b/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp @@ -81,7 +81,9 @@ unsigned ARMELFObjectWriter::GetRelocTypeInner(const MCValue &Target, unsigned Type = 0; if (IsPCRel) { switch ((unsigned)Fixup.getKind()) { - default: llvm_unreachable("Unimplemented"); + default: + report_fatal_error("unsupported relocation on symbol"); + return ELF::R_ARM_NONE; case FK_Data_4: switch (Modifier) { default: llvm_unreachable("Unsupported Modifier"); @@ -147,7 +149,9 @@ unsigned ARMELFObjectWriter::GetRelocTypeInner(const MCValue &Target, } } else { switch ((unsigned)Fixup.getKind()) { - default: llvm_unreachable("invalid fixup kind!"); + default: + report_fatal_error("unsupported relocation on symbol"); + return ELF::R_ARM_NONE; case FK_Data_1: switch (Modifier) { default: llvm_unreachable("unsupported Modifier"); diff --git a/lib/Target/ARM/MCTargetDesc/ARMMachObjectWriter.cpp b/lib/Target/ARM/MCTargetDesc/ARMMachObjectWriter.cpp index 3187d36f751..e1c17c689ed 100644 --- a/lib/Target/ARM/MCTargetDesc/ARMMachObjectWriter.cpp +++ b/lib/Target/ARM/MCTargetDesc/ARMMachObjectWriter.cpp @@ -88,6 +88,7 @@ static bool getARMFixupKindMachOInfo(unsigned Kind, unsigned &RelocType, case ARM::fixup_arm_ldst_pcrel_12: case ARM::fixup_arm_pcrel_10: case ARM::fixup_arm_adr_pcrel_12: + case ARM::fixup_arm_thumb_br: return false; // Handle 24-bit branch kinds. @@ -101,12 +102,6 @@ static bool getARMFixupKindMachOInfo(unsigned Kind, unsigned &RelocType, Log2Size = llvm::Log2_32(4); return true; - // Handle Thumb branches. - case ARM::fixup_arm_thumb_br: - RelocType = unsigned(MachO::ARM_THUMB_RELOC_BR22); - Log2Size = llvm::Log2_32(2); - return true; - case ARM::fixup_t2_uncondbranch: case ARM::fixup_arm_thumb_bl: case ARM::fixup_arm_thumb_blx: diff --git a/test/MC/ARM/thumb1-relax-adr.s b/test/MC/ARM/thumb1-relax-adr.s new file mode 100644 index 00000000000..80b93ec8eae --- /dev/null +++ b/test/MC/ARM/thumb1-relax-adr.s @@ -0,0 +1,9 @@ +@ RUN: not llvm-mc -triple thumbv6m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s +@ RUN: not llvm-mc -triple thumbv7m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s +@ RUN: not llvm-mc -triple thumbv7m-none-eabi -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s + + .global func1 +_func1: + adr r0, _func2 +@ CHECK-ERROR: unsupported relocation on symbol + diff --git a/test/MC/ARM/thumb1-relax-bcc.s b/test/MC/ARM/thumb1-relax-bcc.s new file mode 100644 index 00000000000..02fde2e040e --- /dev/null +++ b/test/MC/ARM/thumb1-relax-bcc.s @@ -0,0 +1,12 @@ +@ RUN: not llvm-mc -triple thumbv6m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s +@ RUN: not llvm-mc -triple thumbv7m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s +@ RUN: llvm-mc -triple thumbv7m-none-eabi -filetype=obj -o %t %s +@ RUN: llvm-objdump -d -r -triple thumbv7m-none-eabi %t | FileCheck --check-prefix=CHECK-ELF %s + + .global func1 +_func1: + bne _func2 +@ CHECK-ERROR: unsupported relocation on symbol + +@ CHECK-ELF: 7f f4 fe af bne.w #-4 +@ CHECK-ELF-NEXT: R_ARM_THM_JUMP24 _func2 diff --git a/test/MC/ARM/thumb1-relax-br.s b/test/MC/ARM/thumb1-relax-br.s new file mode 100644 index 00000000000..92a82752738 --- /dev/null +++ b/test/MC/ARM/thumb1-relax-br.s @@ -0,0 +1,19 @@ +@ RUN: not llvm-mc -triple thumbv6m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s +@ RUN: llvm-mc -triple thumbv7m-none-macho -filetype=obj -o %t %s +@ RUN: llvm-objdump -d -r -triple thumbv7m-none-macho %t | FileCheck --check-prefix=CHECK-MACHO %s +@ RUN: llvm-mc -triple thumbv7m-none-eabi -filetype=obj -o %t %s +@ RUN: llvm-objdump -d -r -triple thumbv7m-none-eabi %t | FileCheck --check-prefix=CHECK-ELF %s + + .global func1 +_func1: + @ There is no MachO relocation for Thumb1's unconditional branch, so + @ this is unrepresentable. FIXME: I think ELF could represent this. + b _func2 + +@ CHECK-ERROR: unsupported relocation on symbol + +@ CHECK-MACHO: ff f7 fe bf b.w #-4 +@ CHECK-MACHO-NEXT: ARM_THUMB_RELOC_BR22 + +@ CHECK-ELF: ff f7 fe bf b.w #-4 +@ CHECK-ELF-NEXT: R_ARM_THM_JUMP24 _func2 diff --git a/test/MC/ARM/thumb1-relax-ldrlit.s b/test/MC/ARM/thumb1-relax-ldrlit.s new file mode 100644 index 00000000000..96c5aa036f2 --- /dev/null +++ b/test/MC/ARM/thumb1-relax-ldrlit.s @@ -0,0 +1,9 @@ +@ RUN: not llvm-mc -triple thumbv6m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s +@ RUN: not llvm-mc -triple thumbv7m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s +@ RUN: not llvm-mc -triple thumbv7m-none-eabi -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s + + .global func1 +_func1: + ldr r0, _func2 +@ CHECK-ERROR: unsupported relocation on symbol + -- 2.34.1