[llvm-objdump] Switch between ARM/Thumb based on mapping symbols.

[efriedma]

Summary

The ARMDisassembler changes allow changing between ARM and Thumb mode based on the MCSubtargetInfo, rather than the Target, which simplifies the other changes a bit.

I'm not really happy with adding more target-specific logic to tools/llvm-objdump/, but there any easy way around it: the logic in question specifically applies to disassembling an object file, and that code simply isn't located in lib/Target, at least at the moment.

[MaskRay]

I'm not really happy about putting the switching logic into llvm-objdump.cpp; it seems like there should be a better way to refactor the interface so the target-specific logic is somewhere inside the target.

I do feel the monolithic disassembleObject is hard to reason about. Do you have idea how to simplify it? On the other hand, I think the new addition of ThumbMappingSymsAddr ARMMappingSymsAddr should be ok, because the ARM specific logic is already there.

tools/llvm-objdump/llvm-objdump.cpp

619

Since you are adding isArm32Elf, can you add isArm64Elf and delete isArmElf if that orthogonal property is desired?

[srhines]

I added Peter and Oliver since they might have some suggestions here too.

lib/Target/ARM/Disassembler/ARMDisassembler.cpp

550

Add logic to check if you are defaulting to Thumb mode here (i.e. that was the triple that was passed).

1044

Wouldn't this be easier to fix by retaining a createThumbDisassembler that calls a shared createARMThumbDisassembler with a flag to denote the default case for disassembly?

[efriedma]

In terms of how to generally improve disassembleObject, I guess there are a few different ways to think about it...

Currently, MCDisassembler::getInstruction is a "stateless" API which takes some bytes, and outputs a single instruction or instruction bundle. (We currently cheat a little on Thumb: the Thumb disassembler has a "mutable" member that keeps track of "it" blocks on Thumb. But that's mostly orthogonal.) That has some nice properties: essentially, it's easy to understand what it's doing. But it seems like it isn't rich enough for disassembling object files, so we have a bunch of code in llvm-objdump which has to deal with target-specific aspects of object files. We could instead add a disassembler API which is intended specifically for disassembling object files, and formally has state: essentially, the llvm-objdump would just pass the symbols and raw bits, and the API would handle the various target-specific aspects of disassembly.

Not sure it's worth the effort, though; I'm not sure there would be any use for it outside of llvm-objdump itself.

lib/Target/ARM/Disassembler/ARMDisassembler.cpp

550

Not sure how that would work; getFeatureBits() only returns "on" or "off". There's no reasonable way to check the default. We should inherit the right default anyway when the target is used to construct the subtarget.

tools/llvm-objdump/llvm-objdump.cpp

619

In some places the property we want is "can this object file have mapping symbols", which is currently equivalent to isArmElf. I guess I can add a separate helper for that, and make it check "isArmElf32 || isArmElf64"?

[MaskRay]

tools/llvm-objdump/llvm-objdump.cpp

619

SG

[peter.smith]

I've not got any great ideas about how to move the target switching logic out of llvm-objdump. I think that we would have to either construct a map (Arm/Thumb/Data) in llvm-objdump that we can pass to the disassembler and it can tell from the index which SubTarget to use assuming the disassembler also has the index, or we'd have to make the disassembler aware of symbols so it could look them up.

I think Stephen's suggestion of passing in a parameter of the initial Primary/Secondary based on the triple could be useful. In general for completely stripped binaries with no symbols getting any sensible disassembly is difficult. The only heuristics I know there is to see if the Entry Point has bit 0 set (Thumb) so we say if entry Thumb state then assume a Thumb triple.

tools/llvm-objdump/llvm-objdump.cpp

619

Could we use isAArch64Elf() and isArmElf(), I think that this would be more consistent with the ABI documentation and the current implementation?

Rather than name all the subtargets it might be neater to check the e_machine field is EM_ARM or EM_AARCH64, although handling the elf class and endian may prove to be as complicated.

1143–1144

Rather than 4 separate vectors, would it be better to have a single one that stores a pair of Index, Letter. In a valid object there is only one mapping symbol at a particular address so each index would match a unique entry. The search code would need to pass in a query function to obtain a symbol of the right letter, but this might be a bit easier to read than 4 separate ones?

1301–1302

Worth updating the comment to "Arm and AArch64 ELF binaries"?

1340

A way to extend this for Arm/Thumb in the case of an Object with no mapping symbols (Strip?) but other symbols are present then if there is a symbol at (Index & 0xfffffffe) then:

• Symbol Type == STT_FUNC and bit 0 of symbol value is 1 then switch to Secondary STI
• Symbol Type == STT_FUNC and bit 0 of symbol value is 0 then switch to Primary STI

In most cases this won't add much over just mapping symbols though and could easily be added as a follow up patch.

1496

Would it be worth moving this up to line 1452 as the part that adds +thumb-mode is quite far from the part that does -thumb-mode. It took me longer than I'd like to work out why --triple=thumbv7a-linux-gnueabi didn't make the primary and secondary subtargets Thumb. Alternatively maybe worth a comment on 1432 saying something like Primary STI is always ARM secondary is always Thumb regardless of Triple.

[efriedma]

tools/llvm-objdump/llvm-objdump.cpp

619

I don't think there's any convenient way to access the e_machine field from here... ELFObjectFileBase has a getEMachine() method, but it's private, and the only other reasonable way to access the field is to downcast all the way to ELF32LEObjectFile/ELF32BEObjectFile. I guess we could change that?

[efriedma]

Partially addresses review comments. Still need to think about the mapping symbol vectors a bit more.

[peter.smith]

tools/llvm-objdump/llvm-objdump.cpp

619

Yes, it looks like there is some prior art in llvm-objdump to get the e_type field. In printDynamicRelocations there is:

const auto *Elf = dyn_cast<ELFObjectFileBase>(Obj);
  if (!Elf || Elf->getEType() != ELF::ET_DYN) {

where ElfObjectFileBase::getEType() is very similar to ElfObjectFileBase::getEType() so it could be possible to make it public if it made sense to do so. We test against e_machine quite a bit in LLD as we often don't need to worry about the specific subtarget.

[efriedma]

Switched to a single vector for mapping symbols. Switched isArmElf() to use getEMachine(). Updated title/commit message.

I think this addresses all the review comments.

[peter.smith]

Thanks for the update. Looks good to me. I've made one comment about the v7r test but not knowing what the original intention behind the test was I'm not sure if it is worth changing.

test/tools/llvm-objdump/ARM/v7r-subfeatures.s

3

I think it depends on whether the dissassembly is supposed to be architecturally aware or not. It seems like GNU arm-none-eabi-objdump is not and will disassemble the instruction, whereas llvm-objdump is.

Strictly speaking the build-attributes are lying to objdump here as there should be:

.eabi_attribute	Tag_DIV_use, 2 // This code was permitted to use SDIV and UDIV in the ARM and Thumb ISAs

If --arm-add-attributes is used then llvm-mc will put this in. With this attribute then llvm-objdump will disassemble it. I'm not entirely sure what the intention of the test is though.

include/llvm/Object/ELFObjectFile.h

 class ELFObjectFileBase : public ObjectFile {
   friend class ELFRelocationRef;
   friend class ELFSectionRef;
   friend class ELFSymbolRef;
 
 protected:
   ELFObjectFileBase(unsigned int Type, MemoryBufferRef Source);
 
-  virtual uint16_t getEMachine() const = 0;
   virtual uint64_t getSymbolSize(DataRefImpl Symb) const = 0;
   virtual uint8_t getSymbolBinding(DataRefImpl Symb) const = 0;
   virtual uint8_t getSymbolOther(DataRefImpl Symb) const = 0;
   virtual uint8_t getSymbolELFType(DataRefImpl Symb) const = 0;
 
   virtual uint32_t getSectionType(DataRefImpl Sec) const = 0;
   virtual uint64_t getSectionFlags(DataRefImpl Sec) const = 0;
   virtual uint64_t getSectionOffset(DataRefImpl Sec) const = 0;
public:
   SubtargetFeatures getARMFeatures() const;
 
   SubtargetFeatures getRISCVFeatures() const;
 
   void setARMSubArch(Triple &TheTriple) const override;
 
   virtual uint16_t getEType() const = 0;
 
+  virtual uint16_t getEMachine() const = 0;
+
   std::vector<std::pair<DataRefImpl, uint64_t>> getPltAddresses() const;
 };
 
 class ELFSectionRef : public SectionRef {
 public:
   ELFSectionRef(const SectionRef &B) : SectionRef(B) {
     assert(isa<ELFObjectFileBase>(SectionRef::getObject()));
   }

lib/Target/ARM/Disassembler/ARMDisassembler.cpp

public:
   }
 
   ~ARMDisassembler() override = default;
 
   DecodeStatus getInstruction(MCInst &Instr, uint64_t &Size,
                               ArrayRef<uint8_t> Bytes, uint64_t Address,
                               raw_ostream &VStream,
                               raw_ostream &CStream) const override;
-};
-
-/// Thumb disassembler for all Thumb platforms.
-class ThumbDisassembler : public MCDisassembler {
-public:
-  ThumbDisassembler(const MCSubtargetInfo &STI, MCContext &Ctx) :
-    MCDisassembler(STI, Ctx) {
-  }
 
-  ~ThumbDisassembler() override = default;
+private:
+  DecodeStatus getARMInstruction(MCInst &Instr, uint64_t &Size,
+                                 ArrayRef<uint8_t> Bytes, uint64_t Address,
+                                 raw_ostream &VStream,
+                                 raw_ostream &CStream) const;
 
-  DecodeStatus getInstruction(MCInst &Instr, uint64_t &Size,
+  DecodeStatus getThumbInstruction(MCInst &Instr, uint64_t &Size,
                                    ArrayRef<uint8_t> Bytes, uint64_t Address,
                                    raw_ostream &VStream,
-                              raw_ostream &CStream) const override;
+                                   raw_ostream &CStream) const;
 
-private:
   mutable ITStatus ITBlock;
   mutable VPTStatus VPTBlock;
 
   DecodeStatus AddThumbPredicate(MCInst&) const;
   void UpdateThumbVFPPredicate(DecodeStatus &, MCInst&) const;
 };
 
 } // end anonymous namespace

 #include "ARMGenDisassemblerTables.inc"
 
 static MCDisassembler *createARMDisassembler(const Target &T,
                                              const MCSubtargetInfo &STI,
                                              MCContext &Ctx) {
   return new ARMDisassembler(STI, Ctx);
 }
 
-static MCDisassembler *createThumbDisassembler(const Target &T,
-                                               const MCSubtargetInfo &STI,
-                                               MCContext &Ctx) {
-  return new ThumbDisassembler(STI, Ctx);
-}
-
 // Post-decoding checks
 static DecodeStatus checkDecodedInstruction(MCInst &MI, uint64_t &Size,
                                             uint64_t Address, raw_ostream &OS,
                                             raw_ostream &CS,
                                             uint32_t Insn,
                                             DecodeStatus Result) {
   switch (MI.getOpcode()) {
     case ARM::HVC: {
switch (MI.getOpcode()) {
     default: return Result;
   }
 }
 
 DecodeStatus ARMDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
                                              ArrayRef<uint8_t> Bytes,
                                              uint64_t Address, raw_ostream &OS,
                                              raw_ostream &CS) const {
+  if (STI.getFeatureBits()[ARM::ModeThumb])
+    return getThumbInstruction(MI, Size, Bytes, Address, OS, CS);
+  return getARMInstruction(MI, Size, Bytes, Address, OS, CS);
+}
+
+DecodeStatus ARMDisassembler::getARMInstruction(MCInst &MI, uint64_t &Size,
+                                                ArrayRef<uint8_t> Bytes,
+                                                uint64_t Address,
+                                                raw_ostream &OS,
+                                                raw_ostream &CS) const {
   CommentStream = &CS;
 
   assert(!STI.getFeatureBits()[ARM::ModeThumb] &&
          "Asked to disassemble an ARM instruction but Subtarget is in Thumb "
          "mode!");
 
   // We want to read exactly 4 bytes of data.
   if (Bytes.size() < 4) {
static bool isVectorPredicable(unsigned Opcode) {
   return false;
 }
 
 // Most Thumb instructions don't have explicit predicates in the
 // encoding, but rather get their predicates from IT context.  We need
 // to fix up the predicate operands using this context information as a
 // post-pass.
 MCDisassembler::DecodeStatus
-ThumbDisassembler::AddThumbPredicate(MCInst &MI) const {
+ARMDisassembler::AddThumbPredicate(MCInst &MI) const {
   MCDisassembler::DecodeStatus S = Success;
 
   const FeatureBitset &FeatureBits = getSubtargetInfo().getFeatureBits();
 
   // A few instructions actually have predicates encoded in them.  Don't
   // try to overwrite it if we're seeing one of those.
   switch (MI.getOpcode()) {
     case ARM::tBcc:
ARMDisassembler::AddThumbPredicate(MCInst &MI) const {
   return S;
 }
 
 // Thumb VFP instructions are a special case.  Because we share their
 // encodings between ARM and Thumb modes, and they are predicable in ARM
 // mode, the auto-generated decoder will give them an (incorrect)
 // predicate operand.  We need to rewrite these operands based on the IT
 // context as a post-pass.
-void ThumbDisassembler::UpdateThumbVFPPredicate(
+void ARMDisassembler::UpdateThumbVFPPredicate(
   DecodeStatus &S, MCInst &MI) const {
   unsigned CC;
   CC = ITBlock.getITCC();
   if (CC == 0xF)
     CC = ARMCC::AL;
   if (ITBlock.instrInITBlock())
     ITBlock.advanceITState();
   else if (VPTBlock.instrInVPTBlock()) {
if (OpInfo[i].isPredicate() ) {
         I->setReg(0);
       else
         I->setReg(ARM::CPSR);
       return;
     }
   }
 }
 
-DecodeStatus ThumbDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
+DecodeStatus ARMDisassembler::getThumbInstruction(MCInst &MI, uint64_t &Size,
                                                   ArrayRef<uint8_t> Bytes,
                                                   uint64_t Address,
                                                   raw_ostream &OS,
                                                   raw_ostream &CS) const {
   CommentStream = &CS;
 
   assert(STI.getFeatureBits()[ARM::ModeThumb] &&
          "Asked to disassemble in Thumb mode but Subtarget is in ARM mode!");
 
   // We want to read exactly 2 bytes of data.
   if (Bytes.size() < 2) {
     Size = 0;

 }
 
 extern "C" void LLVMInitializeARMDisassembler() {
   TargetRegistry::RegisterMCDisassembler(getTheARMLETarget(),
                                          createARMDisassembler);
   TargetRegistry::RegisterMCDisassembler(getTheARMBETarget(),
                                          createARMDisassembler);
   TargetRegistry::RegisterMCDisassembler(getTheThumbLETarget(),
-                                         createThumbDisassembler);
+                                         createARMDisassembler);
   TargetRegistry::RegisterMCDisassembler(getTheThumbBETarget(),
-                                         createThumbDisassembler);
+                                         createARMDisassembler);
 }
 
 static const uint16_t GPRDecoderTable[] = {
   ARM::R0, ARM::R1, ARM::R2, ARM::R3,
   ARM::R4, ARM::R5, ARM::R6, ARM::R7,
   ARM::R8, ARM::R9, ARM::R10, ARM::R11,
   ARM::R12, ARM::SP, ARM::LR, ARM::PC
 };

test/CodeGen/ARM/inlineasm-switch-mode.ll

-;RUN: llc -mtriple=thumbv7-linux-gnueabi < %s | llvm-mc -triple=thumbv7-linux-gnueabi -filetype=obj > %t
-; Two pass decoding needed because llvm-objdump does not respect mapping symbols
-;RUN: llvm-objdump -triple=armv7   -d %t | FileCheck %s --check-prefix=ARM
-;RUN: llvm-objdump -triple=thumbv7 -d %t | FileCheck %s --check-prefix=THUMB
+;RUN: llc -mtriple=thumbv7-linux-gnueabi < %s | llvm-mc -triple=thumbv7-linux-gnueabi -filetype=obj | llvm-objdump -d - | FileCheck %s
 
 define hidden i32 @bah(i8* %start) #0 align 2 {
   %1 = ptrtoint i8* %start to i32
   %2 = tail call i32 asm sideeffect "@ Enter ARM Mode  \0A\09adr r3, 1f \0A\09bx  r3 \0A\09.align 2 \0A\09.code 32 \0A1:  push {r7} \0A\09mov r7, $4 \0A\09svc 0x0 \0A\09pop {r7} \0A\09@ Enter THUMB Mode\0A\09adr r3, 2f+1 \0A\09bx  r3 \0A\09.code 16 \0A2: \0A\09", "={r0},{r0},{r1},{r2},r,~{r3}"(i32 %1, i32 %1, i32 0, i32 983042) #3
   %3 = add i32 %1, 1
   ret i32 %3
 }
 
-; ARM: $a
-; ARM-NEXT: 04 70 2d e5     str     r7, [sp, #-4]!
-; ARM: $t
-; ARM-NEXT: 48 1c
-
-; THUMB: $a{{.*}}:
-; THUMB-NEXT: 04 70
-; THUMB-NEXT: 2d e5
-; THUMB: $t{{.*}}:
-; THUMB-NEXT: 48 1c   adds    r0, r1, #1
+; CHECK: $a{{.*}}:
+; CHECK-NEXT: 04 70 2d e5     str     r7, [sp, #-4]!
+; CHECK: $t{{.*}}:
+; CHECK-NEXT: 48 1c   adds    r0, r1, #1

test/tools/llvm-objdump/ARM/v7r-subfeatures.s

-@ RUN: llvm-mc < %s -triple armv7r -mattr=+hwdiv-arm -filetype=obj | llvm-objdump -triple=thumb -d - | FileCheck %s
-@ RUN: llvm-mc < %s -triple armv7r -mattr=+hwdiv-arm -filetype=obj | llvm-objdump -triple=arm -d - | FileCheck %s --check-prefix=CHECK-ARM
+@ RUN: llvm-mc < %s -triple armv7r -mattr=+hwdiv-arm -filetype=obj | llvm-objdump -d - | FileCheck %s
+@ v7r implies Thumb hwdiv, but ARM hwdiv is optional
+@ FIXME: Does that imply we should actually refuse to disassemble it?
 
 .eabi_attribute Tag_CPU_arch, 10 // v7
 .eabi_attribute Tag_CPU_arch_profile, 0x52 // 'R' profile
 
 .arm
 div_arm:
   udiv r0, r1, r2
 
 @CHECK-LABEL: div_arm
-@CHECK-NOT: udiv r0, r1, r2
-@CHECK-ARM-NOT: udiv r0, r1, r2
+@CHECK: 11 f2 30 e7 <unknown>
 
 .thumb
 div_thumb:
   udiv r0, r1, r2
 
 @CHECK-LABEL: div_thumb
 @CHECK: b1 fb f2 f0 udiv r0, r1, r2

tools/llvm-objdump/llvm-objdump.cpp

if (LineBuffer != LineCache.end()) {
         return;
       // Vector begins at 0, line numbers are non-zero
       OS << Delimiter << LineBuffer->second[LineInfo.Line - 1] << '\n';
     }
   }
   OldLineInfo = LineInfo;
 }
 
+static bool isAArch64Elf(const ObjectFile *Obj) {
+  const auto *Elf = dyn_cast<ELFObjectFileBase>(Obj);
+  return Elf && Elf->getEMachine() == ELF::EM_AARCH64;
+}
+
 static bool isArmElf(const ObjectFile *Obj) {
-  return (Obj->isELF() &&
-          (Obj->getArch() == Triple::aarch64 ||
-           Obj->getArch() == Triple::aarch64_be ||
-           Obj->getArch() == Triple::arm || Obj->getArch() == Triple::armeb ||
-           Obj->getArch() == Triple::thumb ||
-           Obj->getArch() == Triple::thumbeb));
+  const auto *Elf = dyn_cast<ELFObjectFileBase>(Obj);
+  return Elf && Elf->getEMachine() == ELF::EM_ARM;
+}
+
+static bool hasMappingSymbols(const ObjectFile *Obj) {
+  return isArmElf(Obj) || isAArch64Elf(Obj);
 }
 
 static void printRelocation(const RelocationRef &Rel, uint64_t Address,
                             bool Is64Bits) {
   StringRef Fmt = Is64Bits ? "\t\t%016" PRIx64 ":  " : "\t\t\t%08" PRIx64 ":  ";
   SmallString<16> Name;
   SmallString<32> Val;
   Rel.getTypeName(Name);
   error(getRelocationValueString(Rel, Val));
   outs() << format(Fmt.data(), Address) << Name << "\t" << Val << "\n";
 }

 // TODO: implement for other file formats.
 static bool shouldAdjustVA(const SectionRef &Section) {
   const ObjectFile *Obj = Section.getObject();
   if (isa<object::ELFObjectFileBase>(Obj))
     return ELFSectionRef(Section).getFlags() & ELF::SHF_ALLOC;
   return false;
 }
 
+
+typedef std::pair<uint64_t, char> MappingSymbolPair;
+static char getMappingSymbolKind(ArrayRef<MappingSymbolPair> MappingSymbols,
+                                 uint64_t Address) {
+  auto Sym = bsearch(MappingSymbols, [Address](const MappingSymbolPair &Val) {
+      return Val.first > Address;
+  });
+  // Return zero for any address before the first mapping symbol; this means
+  // we should use the default disassembly mode, depending on the target.
+  if (Sym == MappingSymbols.begin())
+    return '\x00';
+  return (Sym - 1)->second;
+}
+
 static uint64_t
 dumpARMELFData(uint64_t SectionAddr, uint64_t Index, uint64_t End,
                const ObjectFile *Obj, ArrayRef<uint8_t> Bytes,
-               const std::vector<uint64_t> &TextMappingSymsAddr) {
+               ArrayRef<MappingSymbolPair> MappingSymbols) {
   support::endianness Endian =
       Obj->isLittleEndian() ? support::little : support::big;
   while (Index < End) {
     outs() << format("%8" PRIx64 ":", SectionAddr + Index);
     outs() << "\t";
     if (Index + 4 <= End) {
       dumpBytes(Bytes.slice(Index, 4), outs());
       outs() << "\t.word\t"
              << format_hex(
                     support::endian::read32(Bytes.data() + Index, Endian), 10);
       Index += 4;
     } else if (Index + 2 <= End) {
       dumpBytes(Bytes.slice(Index, 2), outs());
       outs() << "\t\t.short\t"
              << format_hex(
                     support::endian::read16(Bytes.data() + Index, Endian), 6);
       Index += 2;
     } else {
       dumpBytes(Bytes.slice(Index, 1), outs());
       outs() << "\t\t.byte\t" << format_hex(Bytes[0], 4);
       ++Index;
     }
     outs() << "\n";
-    if (std::binary_search(TextMappingSymsAddr.begin(),
-                           TextMappingSymsAddr.end(), Index))
+    if (getMappingSymbolKind(MappingSymbols, Index) != 'd')
       break;
   }
   return Index;
 }
 
 static void dumpELFData(uint64_t SectionAddr, uint64_t Index, uint64_t End,
                         ArrayRef<uint8_t> Bytes) {
   // print out data up to 8 bytes at a time in hex and ascii
if (NumBytes == 8) {
       outs() << reinterpret_cast<char *>(AsciiData);
       outs() << '\n';
       NumBytes = 0;
     }
   }
 }
 
 static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,
-                              MCContext &Ctx, MCDisassembler *DisAsm,
+                              MCContext &Ctx, MCDisassembler *PrimaryDisAsm,
+                              MCDisassembler *SecondaryDisAsm,
                               const MCInstrAnalysis *MIA, MCInstPrinter *IP,
-                              const MCSubtargetInfo *STI, PrettyPrinter &PIP,
+                              const MCSubtargetInfo *PrimarySTI,
+                              const MCSubtargetInfo *SecondarySTI,
+                              PrettyPrinter &PIP,
                               SourcePrinter &SP, bool InlineRelocs) {
+  const MCSubtargetInfo *STI = PrimarySTI;
+  MCDisassembler *DisAsm = PrimaryDisAsm;
+  bool PrimaryIsThumb = false;
+  if (isArmElf(Obj))
+    PrimaryIsThumb = STI->checkFeatures("+thumb-mode");
+
   std::map<SectionRef, std::vector<RelocationRef>> RelocMap;
   if (InlineRelocs)
     RelocMap = getRelocsMap(*Obj);
   bool Is64Bits = Obj->getBytesInAddress() > 4;
 
   // Create a mapping from virtual address to symbol name.  This is used to
   // pretty print the symbols while disassembling.
   std::map<SectionRef, SectionSymbolsTy> AllSymbols;
if (FilterSections.empty() && !DisassembleAll &&
       continue;
 
     uint64_t SectionAddr = Section.getAddress();
     uint64_t SectSize = Section.getSize();
     if (!SectSize)
       continue;
 
     // Get the list of all the symbols in this section.
     SectionSymbolsTy &Symbols = AllSymbols[Section];
-    std::vector<uint64_t> DataMappingSymsAddr;
-    std::vector<uint64_t> TextMappingSymsAddr;
-    if (isArmElf(Obj)) {
+    std::vector<MappingSymbolPair> MappingSymbols;
+    if (hasMappingSymbols(Obj)) {
       for (const auto &Symb : Symbols) {
         uint64_t Address = std::get<0>(Symb);
         StringRef Name = std::get<1>(Symb);
         if (Name.startswith("$d"))
-          DataMappingSymsAddr.push_back(Address - SectionAddr);
+          MappingSymbols.emplace_back(Address - SectionAddr, 'd');
         if (Name.startswith("$x"))
-          TextMappingSymsAddr.push_back(Address - SectionAddr);
+          MappingSymbols.emplace_back(Address - SectionAddr, 'x');
         if (Name.startswith("$a"))
-          TextMappingSymsAddr.push_back(Address - SectionAddr);
+          MappingSymbols.emplace_back(Address - SectionAddr, 'a');
         if (Name.startswith("$t"))
-          TextMappingSymsAddr.push_back(Address - SectionAddr);
+          MappingSymbols.emplace_back(Address - SectionAddr, 't');
       }
     }
 
-    llvm::sort(DataMappingSymsAddr);
-    llvm::sort(TextMappingSymsAddr);
+    llvm::sort(MappingSymbols);
 
     if (Obj->isELF() && Obj->getArch() == Triple::amdgcn) {
       // AMDGPU disassembler uses symbolizer for printing labels
       std::unique_ptr<MCRelocationInfo> RelInfo(
         TheTarget->createMCRelocationInfo(TripleName, Ctx));
       if (RelInfo) {
         std::unique_ptr<MCSymbolizer> Symbolizer(
           TheTarget->createMCSymbolizer(
#endif
       // only disassembling text (applicable all architectures), we are in a
       // situation where we must print the data and not disassemble it.
       if (Obj->isELF() && std::get<2>(Symbols[SI]) == ELF::STT_OBJECT &&
           !DisassembleAll && Section.isText()) {
         dumpELFData(SectionAddr, Index, End, Bytes);
         Index = End;
       }
 
-      bool CheckARMELFData = isArmElf(Obj) &&
+      bool CheckARMELFData = hasMappingSymbols(Obj) &&
                              std::get<2>(Symbols[SI]) != ELF::STT_OBJECT &&
                              !DisassembleAll;
       while (Index < End) {
-        // AArch64 ELF binaries can interleave data and text in the same
-        // section. We rely on the markers introduced to understand what we
-        // need to dump. If the data marker is within a function, it is
+        // ARM and AArch64 ELF binaries can interleave data and text in the
+        // same section. We rely on the markers introduced to understand what
+        // we need to dump. If the data marker is within a function, it is
         // denoted as a word/short etc.
         if (CheckARMELFData &&
-            std::binary_search(DataMappingSymsAddr.begin(),
-                               DataMappingSymsAddr.end(), Index)) {
+            getMappingSymbolKind(MappingSymbols, Index) == 'd') {
           Index = dumpARMELFData(SectionAddr, Index, End, Obj, Bytes,
-                                 TextMappingSymsAddr);
+                                 MappingSymbols);
           continue;
         }
 
         // When -z or --disassemble-zeroes are given we always dissasemble
         // them. Otherwise we might want to skip zero bytes we see.
         if (!DisassembleZeroes) {
           uint64_t MaxOffset = End - Index;
           // For -reloc: print zero blocks patched by relocations, so that
           // relocations can be shown in the dump.
           if (RelCur != RelEnd)
             MaxOffset = RelCur->getOffset() - Index;
 
           if (size_t N =
                   countSkippableZeroBytes(Bytes.slice(Index, MaxOffset))) {
             outs() << "\t\t..." << '\n';
             Index += N;
             continue;
           }
         }
 
+        if (SecondarySTI) {
+          if (getMappingSymbolKind(MappingSymbols, Index) == 'a') {
+            STI = PrimaryIsThumb ? SecondarySTI : PrimarySTI;
+            DisAsm = PrimaryIsThumb ? SecondaryDisAsm : PrimaryDisAsm;
+          } else if (getMappingSymbolKind(MappingSymbols, Index) == 't') {
+            STI = PrimaryIsThumb ? PrimarySTI : SecondarySTI;
+            DisAsm = PrimaryIsThumb ? PrimaryDisAsm : SecondaryDisAsm;
+          }
+        }
+
         // Disassemble a real instruction or a data when disassemble all is
         // provided
         MCInst Inst;
         bool Disassembled = DisAsm->getInstruction(
             Inst, Size, Bytes.slice(Index), SectionAddr + Index, DebugOut,
             CommentStream);
         if (Size == 0)
           Size = 1;
 
static void disassembleObject(const ObjectFile *Obj, bool InlineRelocs) {
   // FIXME: for now initialize MCObjectFileInfo with default values
   MOFI.InitMCObjectFileInfo(Triple(TripleName), false, Ctx);
 
   std::unique_ptr<MCDisassembler> DisAsm(
       TheTarget->createMCDisassembler(*STI, Ctx));
   if (!DisAsm)
     report_error(Obj->getFileName(),
                  "no disassembler for target " + TripleName);
 
+  // If we have an ARM object file, we need a second disassembler, because
+  // ARM CPUs have two different instruction sets: ARM mode, and Thumb mode.
+  // We use mapping symbols to switch between the two assemblers, where
+  // appropriate.
+  std::unique_ptr<MCDisassembler> SecondaryDisAsm;
+  std::unique_ptr<const MCSubtargetInfo> SecondarySTI;
+  if (isArmElf(Obj) && !STI->checkFeatures("+mclass")) {
+    if (STI->checkFeatures("+thumb-mode"))
+      Features.AddFeature("-thumb-mode");
+    else
+      Features.AddFeature("+thumb-mode");
+    SecondarySTI.reset(TheTarget->createMCSubtargetInfo(TripleName, MCPU,
+                                                        Features.getString()));
+    SecondaryDisAsm.reset(TheTarget->createMCDisassembler(*SecondarySTI, Ctx));
+  }
+
   std::unique_ptr<const MCInstrAnalysis> MIA(
       TheTarget->createMCInstrAnalysis(MII.get()));
 
   int AsmPrinterVariant = AsmInfo->getAssemblerDialect();
   std::unique_ptr<MCInstPrinter> IP(TheTarget->createMCInstPrinter(
       Triple(TripleName), AsmPrinterVariant, *AsmInfo, *MII, *MRI));
   if (!IP)
     report_error(Obj->getFileName(),
                  "no instruction printer for target " + TripleName);
   IP->setPrintImmHex(PrintImmHex);
 
   PrettyPrinter &PIP = selectPrettyPrinter(Triple(TripleName));
   SourcePrinter SP(Obj, TheTarget->getName());
 
   for (StringRef Opt : DisassemblerOptions)
     if (!IP->applyTargetSpecificCLOption(Opt))
       error("Unrecognized disassembler option: " + Opt);
 
-  disassembleObject(TheTarget, Obj, Ctx, DisAsm.get(), MIA.get(), IP.get(),
-                    STI.get(), PIP, SP, InlineRelocs);
+  disassembleObject(TheTarget, Obj, Ctx, DisAsm.get(), SecondaryDisAsm.get(),
+                    MIA.get(), IP.get(), STI.get(), SecondarySTI.get(), PIP,
+                    SP, InlineRelocs);
 }
 
 void printRelocations(const ObjectFile *Obj) {
   StringRef Fmt = Obj->getBytesInAddress() > 4 ? "%016" PRIx64 :
                                                  "%08" PRIx64;
   // Regular objdump doesn't print relocations in non-relocatable object
   // files.
   if (!Obj->isRelocatableObject())