[ARM] Fix for branch label dissassembly for Thumb targets

[avieira]

Summary

different MCInstrAnalysis classes for arm and thumb mode each with their own evaluateBranch implementation.

I added a test case and fixed the coff-relocations test to use '<label>:' rather than '<label>' in the CHECK-LABEL entries, since the ones without the colon would match branch targets. Might be worth noticing that llvm-objdump does not lookup the relocation and thus assigns it a target depending on the encoded immediate which is #0, so it thinks it branches to the next instruction.

[samparker]

Hi Andre,

Thanks for working on this. It would be be nice to have isConditonalBranch and isUnconditionalBranch implemented but, there as there was no specific thumb handling previously, it can definitely wait. LGTM with just the small comment change, no need to re-review.

cheers,
sam

lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp

279 ↗

(On Diff #91728)

thumb mode

[rengolin]

Nice!

llvm/trunk/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp

if (Inst.getOpcode() == ARM::Bcc && Inst.getOperand(1).getImm()==ARMCC::AL)
       return false;
     return MCInstrAnalysis::isConditionalBranch(Inst);
   }
 
   bool evaluateBranch(const MCInst &Inst, uint64_t Addr,
                       uint64_t Size, uint64_t &Target) const override {
     // We only handle PCRel branches for now.
     if (Info->get(Inst.getOpcode()).OpInfo[0].OperandType!=MCOI::OPERAND_PCREL)
-      return false;
-
-    int64_t Imm = Inst.getOperand(0).getImm();
-    // FIXME: This is not right for thumb.
-    Target = Addr+Imm+8; // In ARM mode the PC is always off by 8 bytes.
-    return true;
-  }
-};
-
-}
-
-static MCInstrAnalysis *createARMMCInstrAnalysis(const MCInstrInfo *Info) {
-  return new ARMMCInstrAnalysis(Info);
-}
-
-// Force static initialization.
-extern "C" void LLVMInitializeARMTargetMC() {
-  for (Target *T : {&getTheARMLETarget(), &getTheARMBETarget(),
+      return false;
+
+    int64_t Imm = Inst.getOperand(0).getImm();
+    Target = Addr+Imm+8; // In ARM mode the PC is always off by 8 bytes.
+    return true;
+  }
+};
+
+class ThumbMCInstrAnalysis : public ARMMCInstrAnalysis {
+public:
+  ThumbMCInstrAnalysis(const MCInstrInfo *Info) : ARMMCInstrAnalysis(Info) {}
+
+  bool evaluateBranch(const MCInst &Inst, uint64_t Addr,
+                      uint64_t Size, uint64_t &Target) const override {
+    // We only handle PCRel branches for now.
+    if (Info->get(Inst.getOpcode()).OpInfo[0].OperandType!=MCOI::OPERAND_PCREL)
+      return false;
+
+    int64_t Imm = Inst.getOperand(0).getImm();
+    Target = Addr+Imm+4; // In Thumb mode the PC is always off by 4 bytes.
+    return true;
+  }
+};
+
+}
+
+static MCInstrAnalysis *createARMMCInstrAnalysis(const MCInstrInfo *Info) {
+  return new ARMMCInstrAnalysis(Info);
+}
+
+static MCInstrAnalysis *createThumbMCInstrAnalysis(const MCInstrInfo *Info) {
+  return new ThumbMCInstrAnalysis(Info);
+}
+
+// Force static initialization.
+extern "C" void LLVMInitializeARMTargetMC() {
+  for (Target *T : {&getTheARMLETarget(), &getTheARMBETarget(),
                     &getTheThumbLETarget(), &getTheThumbBETarget()}) {
     // Register the MC asm info.
     RegisterMCAsmInfoFn X(*T, createARMMCAsmInfo);
 
     // Register the MC instruction info.
     TargetRegistry::RegisterMCInstrInfo(*T, createARMMCInstrInfo);
 
     // Register the MC register info.
     TargetRegistry::RegisterMCRegInfo(*T, createARMMCRegisterInfo);
 
     // Register the MC subtarget info.
-    TargetRegistry::RegisterMCSubtargetInfo(*T,
-                                            ARM_MC::createARMMCSubtargetInfo);
-
-    // Register the MC instruction analyzer.
-    TargetRegistry::RegisterMCInstrAnalysis(*T, createARMMCInstrAnalysis);
-
-    TargetRegistry::RegisterELFStreamer(*T, createELFStreamer);
-    TargetRegistry::RegisterCOFFStreamer(*T, createARMWinCOFFStreamer);
-    TargetRegistry::RegisterMachOStreamer(*T, createARMMachOStreamer);
+    TargetRegistry::RegisterMCSubtargetInfo(*T,
+                                            ARM_MC::createARMMCSubtargetInfo);
+
+    TargetRegistry::RegisterELFStreamer(*T, createELFStreamer);
+    TargetRegistry::RegisterCOFFStreamer(*T, createARMWinCOFFStreamer);
+    TargetRegistry::RegisterMachOStreamer(*T, createARMMachOStreamer);
 
     // Register the obj target streamer.
     TargetRegistry::RegisterObjectTargetStreamer(*T,
                                                  createARMObjectTargetStreamer);
 
     // Register the asm streamer.
     TargetRegistry::RegisterAsmTargetStreamer(*T, createARMTargetAsmStreamer);
 
     // Register the null TargetStreamer.
     TargetRegistry::RegisterNullTargetStreamer(*T, createARMNullTargetStreamer);
 
     // Register the MCInstPrinter.
     TargetRegistry::RegisterMCInstPrinter(*T, createARMMCInstPrinter);
 
     // Register the MC relocation info.
-    TargetRegistry::RegisterMCRelocationInfo(*T, createARMMCRelocationInfo);
-  }
-
-  // Register the MC Code Emitter
-  for (Target *T : {&getTheARMLETarget(), &getTheThumbLETarget()})
-    TargetRegistry::RegisterMCCodeEmitter(*T, createARMLEMCCodeEmitter);
+    TargetRegistry::RegisterMCRelocationInfo(*T, createARMMCRelocationInfo);
+  }
+
+  // Register the MC instruction analyzer.
+  for (Target *T : {&getTheARMLETarget(), &getTheARMBETarget()})
+    TargetRegistry::RegisterMCInstrAnalysis(*T, createARMMCInstrAnalysis);
+  for (Target *T : {&getTheThumbLETarget(), &getTheThumbBETarget()})
+    TargetRegistry::RegisterMCInstrAnalysis(*T, createThumbMCInstrAnalysis);
+
+  // Register the MC Code Emitter
+  for (Target *T : {&getTheARMLETarget(), &getTheThumbLETarget()})
+    TargetRegistry::RegisterMCCodeEmitter(*T, createARMLEMCCodeEmitter);
   for (Target *T : {&getTheARMBETarget(), &getTheThumbBETarget()})
     TargetRegistry::RegisterMCCodeEmitter(*T, createARMBEMCCodeEmitter);
 
   // Register the asm backend.
   TargetRegistry::RegisterMCAsmBackend(getTheARMLETarget(),
                                        createARMLEAsmBackend);
   TargetRegistry::RegisterMCAsmBackend(getTheARMBETarget(),
                                        createARMBEAsmBackend);
   TargetRegistry::RegisterMCAsmBackend(getTheThumbLETarget(),
                                        createThumbLEAsmBackend);
   TargetRegistry::RegisterMCAsmBackend(getTheThumbBETarget(),
                                        createThumbBEAsmBackend);
 }

llvm/trunk/test/MC/ARM/branch-disassemble.s

+@ RUN: llvm-mc -mcpu=cortex-a9 -triple armv7.arm-none-eabi -filetype obj -o - %s \
+@ RUN:   | llvm-objdump -mcpu=cortex-a9 -triple armv7.arm-none-eabi -d - \
+@ RUN:   | FileCheck %s -check-prefix CHECK-ARM
+
+@ RUN: llvm-mc -mcpu=cortex-m3 -triple thumbv7.arm-none-eabi -filetype obj -o - %s \
+@ RUN:   | llvm-objdump -mcpu=cortex-m3 -triple thumbv7.arm-none-eabi -d - \
+@ RUN:   | FileCheck %s -check-prefix CHECK-THUMB
+
+b.w .Lbranch
+@ CHECK-ARM: b #4 <$a.0+0xC>
+@ CHECK-THUMB: b.w #8 <$t.0+0xC>
+adds r0, r1, #42
+adds r1, r2, #42
+.Lbranch:
+movs r2, r3

llvm/trunk/test/MC/ARM/coff-relocations.s

 @ RUN: llvm-mc -triple thumbv7-windows-itanium -filetype obj -o - %s \
 @ RUN:   | llvm-readobj -r - | FileCheck %s -check-prefix CHECK-RELOCATION
 
 @ RUN: llvm-mc -triple thumbv7-windows-itanium -filetype obj -o - %s \
 @ RUN:   | llvm-objdump -d - | FileCheck %s -check-prefix CHECK-ENCODING
 
   .syntax unified
   .text
   .thumb
 
   .global target
 
   .thumb_func
-branch24t:
-  b target
-
-@ CHECK-ENCODING-LABEL: branch24t
-@ CHECK-ENCODING-NEXT: b.w #0
-
-  .thumb_func
-branch20t:
-  bcc target
-
-@ CHECK-ENCODING-LABEL: branch20t
-@ CHECK-ENCODING-NEXT: blo.w #0
-
-  .thumb_func
-blx23t:
-  bl target
-
-@ CHECK-ENCODING-LABEL: blx23t
-@ CHECK-ENCODING-NEXT: bl #0
-
-  .thumb_func
+branch24t:
+  b target
+
+@ CHECK-ENCODING-LABEL: branch24t:
+@ CHECK-ENCODING-NEXT: b.w #0
+
+  .thumb_func
+branch20t:
+  bcc target
+
+@ CHECK-ENCODING-LABEL: branch20t:
+@ CHECK-ENCODING-NEXT: blo.w #0
+
+  .thumb_func
+blx23t:
+  bl target
+
+@ CHECK-ENCODING-LABEL: blx23t:
+@ CHECK-ENCODING-NEXT: bl #0
+
+  .thumb_func
 mov32t:
   movw r0, :lower16:target
-  movt r0, :upper16:target
-  blx r0
-
-@ CHECK-ENCODING-LABEL: mov32t
-@ CHECK-ENCODING-NEXT: movw r0, #0
-@ CHECK-ENCODING-NEXT: movt r0, #0
-@ CHECK-ENCODING-NEXT: blx r0
+  movt r0, :upper16:target
+  blx r0
+
+@ CHECK-ENCODING-LABEL: mov32t:
+@ CHECK-ENCODING-NEXT: movw r0, #0
+@ CHECK-ENCODING-NEXT: movt r0, #0
+@ CHECK-ENCODING-NEXT: blx r0
 
   .thumb_func
 addr32:
   ldr r0, .Laddr32
   bx r0
   trap
-.Laddr32:
-  .long target
-
-@ CHECK-ENCODING-LABEL: addr32
-@ CHECK-ENCODING-NEXT: ldr r0, [pc, #4]
-@ CHECK-ENCODING-NEXT: bx r0
-@ CHECK-ENCODING-NEXT: trap
+.Laddr32:
+  .long target
+
+@ CHECK-ENCODING-LABEL: addr32:
+@ CHECK-ENCODING-NEXT: ldr r0, [pc, #4]
+@ CHECK-ENCODING-NEXT: bx r0
+@ CHECK-ENCODING-NEXT: trap
 @ CHECK-ENCODING-NEXT: movs r0, r0
 @ CHECK-ENCODING-NEXT: movs r0, r0
 
   .thumb_func
 addr32nb:
   ldr r0, .Laddr32nb
   bx r0
   trap
-.Laddr32nb:
-  .long target(imgrel)
-
-@ CHECK-ENCODING-LABEL: addr32nb
-@ CHECK-ENCODING-NEXT: ldr.w r0, [pc, #4]
-@ CHECK-ENCODING-NEXT: bx r0
-@ CHECK-ENCODING-NEXT: trap
+.Laddr32nb:
+  .long target(imgrel)
+
+@ CHECK-ENCODING-LABEL: addr32nb:
+@ CHECK-ENCODING-NEXT: ldr.w r0, [pc, #4]
+@ CHECK-ENCODING-NEXT: bx r0
+@ CHECK-ENCODING-NEXT: trap
 @ CHECK-ENCODING-NEXT: movs r0, r0
 @ CHECK-ENCODING-NEXT: movs r0, r0
 
        .thumb_func
 secrel:
   ldr r0, .Lsecrel
   bx r0
   trap
-.Lsecrel:
-  .long target(secrel32)
-
-@ CHECK-ENCODING-LABEL: secrel
-@ CHECK-ENCODING-NEXT: ldr.w r0, [pc, #4]
-@ CHECK-ENCODING-NEXT: bx r0
-@ CHECK-ENCODING-NEXT: trap
+.Lsecrel:
+  .long target(secrel32)
+
+@ CHECK-ENCODING-LABEL: secrel:
+@ CHECK-ENCODING-NEXT: ldr.w r0, [pc, #4]
+@ CHECK-ENCODING-NEXT: bx r0
+@ CHECK-ENCODING-NEXT: trap
 @ CHECK-ENCODING-NEXT: movs r0, r0
 @ CHECK-ENCODING-NEXT: movs r0, r0
 
 @ CHECK-RELOCATION: Relocations [
 @ CHECK-RELOCATION:   Section (1) .text {
 @ CHECK-RELOCATION:     0x0 IMAGE_REL_ARM_BRANCH24T
 @ CHECK-RELOCATION:     0x4 IMAGE_REL_ARM_BRANCH20T
 @ CHECK-RELOCATION:     0x8 IMAGE_REL_ARM_BLX23T
 @ CHECK-RELOCATION:     0xC IMAGE_REL_ARM_MOV32T
 @ CHECK-RELOCATION:     0x1C IMAGE_REL_ARM_ADDR32
 @ CHECK-RELOCATION:     0x28 IMAGE_REL_ARM_ADDR32NB
 @ CHECK-RELOCATION:     0x34 IMAGE_REL_ARM_SECREL
 @ CHECK-RELOCATION:   }
 @ CHECK-RELOCATION: ]