[llvm-objdump,ARM] Fix big-endian AArch32 disassembly.

[simon_tatham]

Summary

The ABI for big-endian AArch32, as specified by AAELF32, is above-
averagely complicated. Relocatable object files are expected to store
instruction encodings in byte order matching the ELF file's endianness
(so, big-endian for a BE ELF file). But executable images can
either do that or store instructions little-endian regardless
of data and ELF endianness (to support BE32 and BE8 platforms
respectively). They signal the latter by setting the EF_ARM_BE8 flag
in the ELF header.

(In the case of the Thumb instruction set, this all means that each
16-bit halfword of a Thumb instruction is stored in one or other
endianness. The two halfwords of a 32-bit Thumb instruction must
appear in the same order no matter what, because the first halfword is
the one that must avoid overlapping the encoding of any 16-bit Thumb
instruction.)

llvm-objdump was unconditionally expecting Arm instructions to be
stored little-endian. So it would correctly disassemble a BE8 image,
but if you gave it a BE32 image or a BE object file, it would retrieve
every instruction in byte-swapped form and disassemble it to
nonsense. (Even an object file output by LLVM itself, because
ARMMCCodeEmitter outputs instructions big-endian in big-endian mode,
which is correct for writing an object file.)

This patch allows llvm-objdump to correctly disassemble all three of
those classes of Arm ELF file. It does it by introducing a new
SubtargetFeature for big-endian instructions, setting it from the ELF
image type and flags during llvm-objdump setup, and teaching both
ARMDisassembler and llvm-objdump itself to pay attention to it when
retrieving instruction data from a section being disassembled.

[simon_tatham]

Updated with arc lint style fixes.

(I didn't apply them ahead of time because I was confused by output from arc that appeared to want to reformat most of ARMDisassembler.cpp! But it turns out that was some kind of intermediate output, and not part of the final patch it wanted to apply.)

[DavidSpickett]

llvm/lib/Target/ARM/ARM.td

748

When you say "in memory" here, does it refer to "memory" on the device or "memory" in the object file? I think from the last sentence, you mean inside the object file, which is what objdump cares about.

Because if I had an object file that was going to be byte reversed by the linker then I'd need to disassemble the object file in one endian, but if I was disassembling the content of memory on the device. I'd have to use the opposite endian.

[simon_tatham]

llvm/lib/Target/ARM/ARM.td

748

Hmmm. It's true, of course, that when you read a relocatable object file, the endianness in memory after linking is not yet specified, because the linker may or may not choose to byte-reverse the instructions and set the BE8 flag. But I had imagined it would be clear that "in memory" here means in the memory that the disassembler is reading from, not the memory on the target!

Do you have a suggested alternative wording that doesn't end up being a whole paragraph? Perhaps just deleting "in memory" would be simplest?

[MaskRay]

llvm/tools/llvm-objdump/llvm-objdump.cpp

683

printInst is called for every instruction. Calling checkFeatures here may be too expensive.

[MaskRay]

llvm/test/tools/llvm-objdump/ELF/ARM/be32-image-disasm.test

4 ↗

(On Diff #449016)

Add -NEXT whenever appropriate

45 ↗

(On Diff #449016)

delete ...

llvm/test/tools/llvm-objdump/ELF/ARM/be8-image-disasm.test

1 ↗

(On Diff #449016)

Do we need two files? Grep yaml2obj .*-D and use it to combine tests into one file.

3 ↗

(On Diff #449016)

Add -NEXT whenever appropriate

45 ↗

(On Diff #449016)

delete ...

[DavidSpickett]

llvm/lib/Target/ARM/ARM.td

748

Perhaps just deleting "in memory" would be simplest?

Sounds good to me.

[simon_tatham]

Addressed all review comments, I think.

I couldn't find any convenient way to make
STI.getFeatureBits()[ARM::ModeBigEndianInstructions] work in
llvm-objdump.cpp, because that tool doesn't have the right include
path to include the generated file where those enums are defined. So
I've made the prettyprinter have its own endianness flag that's set
directly at the same time as setting that feature in the
SubtargetInfo, and that will save having to check anything in the STI
at all.

[DavidSpickett]

LGTM if @MaskRay is cool with it.

I remembered that this patch https://reviews.llvm.org/D48811 was posted ages ago and never went anywhere. Do you have an opinion on that? It makes objdump disassemble as big endian if there is eb in the target name. From what I read here I don't think it's needed because the important thing is the format in the object file. Is that correct?

llvm/lib/Target/ARM/ARM.td

733

// Endianness of instruction encodings

[simon_tatham]

I didn't know about that previous patch at all, but off the top of my head, yes, I don't think it should be necessary at least for ELF files, because the combination of ELF header endianness and EF_ARM_BE8 flag is enough to autodetect the right code endianness in all cases.

For other Arm object file formats, on the other hand, I can't say the same. I know nothing at all about big-endian support in Arm COFF, for example, including whether it has any BE support at all!

I wouldn't rule out the possibility that at some point an instance of MCDisassembler might need to have its code endianness manually set to the right state – but the facility provided in this commit can be reused for that, if and when the use case shows up, because all you have to do is to set the new big-endian-instructions subtarget feature.

[MaskRay]

llvm/test/tools/llvm-objdump/ELF/ARM/be-image-disasm.test

1 ↗

(On Diff #449257)

You can place be-image-disasm.test and be-object-disasm.test in one file.

Grep docnum on existing tests.

4 ↗

(On Diff #449257)

binary utility test directories use ## for non-RUN non-CHECK comments. They make comments stand out and they may be highlit with different styles in an editor.

[MaskRay]

LGTM

[simon_tatham]

Oh, oops, caught out again :-( I really must stop assuming that a Phab comment saying "LGTM" actually marks the patch as accepted. Sorry about that. Will revert if you need me to.

llvm/include/llvm/BinaryFormat/ELF.h

 };
 
 // ARM Specific e_flags
 enum : unsigned {
   EF_ARM_SOFT_FLOAT = 0x00000200U,     // Legacy pre EABI_VER5
   EF_ARM_ABI_FLOAT_SOFT = 0x00000200U, // EABI_VER5
   EF_ARM_VFP_FLOAT = 0x00000400U,      // Legacy pre EABI_VER5
   EF_ARM_ABI_FLOAT_HARD = 0x00000400U, // EABI_VER5
+  EF_ARM_BE8 = 0x00800000U,
   EF_ARM_EABI_UNKNOWN = 0x00000000U,
   EF_ARM_EABI_VER1 = 0x01000000U,
   EF_ARM_EABI_VER2 = 0x02000000U,
   EF_ARM_EABI_VER3 = 0x03000000U,
   EF_ARM_EABI_VER4 = 0x04000000U,
   EF_ARM_EABI_VER5 = 0x05000000U,
   EF_ARM_EABIMASK = 0xFF000000U
 };

llvm/lib/ObjectYAML/ELFYAML.cpp

case ELF::EM_ARM:
     BCase(EF_ARM_SOFT_FLOAT);
     BCase(EF_ARM_VFP_FLOAT);
     BCaseMask(EF_ARM_EABI_UNKNOWN, EF_ARM_EABIMASK);
     BCaseMask(EF_ARM_EABI_VER1, EF_ARM_EABIMASK);
     BCaseMask(EF_ARM_EABI_VER2, EF_ARM_EABIMASK);
     BCaseMask(EF_ARM_EABI_VER3, EF_ARM_EABIMASK);
     BCaseMask(EF_ARM_EABI_VER4, EF_ARM_EABIMASK);
     BCaseMask(EF_ARM_EABI_VER5, EF_ARM_EABIMASK);
+    BCaseMask(EF_ARM_BE8, EF_ARM_BE8);
     break;
   case ELF::EM_MIPS:
     BCase(EF_MIPS_NOREORDER);
     BCase(EF_MIPS_PIC);
     BCase(EF_MIPS_CPIC);
     BCase(EF_MIPS_ABI2);
     BCase(EF_MIPS_32BITMODE);
     BCase(EF_MIPS_FP64);

llvm/lib/Target/ARM/ARM.td

def FeatureHardenSlsBlr : SubtargetFeature<"harden-sls-blr",
   "HardenSlsBlr", "true",
   "Harden against straight line speculation across indirect calls">;
 /// Generate thunk code for SLS mitigation in the normal text section.
 def FeatureHardenSlsNoComdat : SubtargetFeature<"harden-sls-nocomdat",
   "HardenSlsNoComdat", "true",
   "Generate thunk code for SLS mitigation in the normal text section">;
 
 //===----------------------------------------------------------------------===//
+// Endianness of instruction encodings in memory.
+//
+// In the current Arm architecture, this is usually little-endian regardless of
+// data endianness. But before Armv7 it was typical for instruction endianness
+// to match data endianness, so that a big-endian system was consistently big-
+// endian. And Armv7-R can be configured to use big-endian instructions.
+//
+// Additionally, even when targeting Armv7-A, big-endian instructions can be
+// found in relocatable object files, because the Arm ABI specifies that the
+// linker byte-reverses them depending on the target architecture.
+//
+// So we have a feature here to indicate that instructions are stored big-
+// endian, which you can set when instantiating an MCDisassembler.
+def ModeBigEndianInstructions : SubtargetFeature<"big-endian-instructions",
+    "BigEndianInstructions", "true",
+     "Expect instructions to be stored big-endian.">;
+
+//===----------------------------------------------------------------------===//
 // ARM Processor subtarget features.
 //
 
 def ProcA5      : SubtargetFeature<"a5", "ARMProcFamily", "CortexA5",
                                    "Cortex-A5 ARM processors", []>;
 def ProcA7      : SubtargetFeature<"a7", "ARMProcFamily", "CortexA7",
                                    "Cortex-A7 ARM processors", []>;
 def ProcA8      : SubtargetFeature<"a8", "ARMProcFamily", "CortexA8",

llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp

private:
       SmallVector<unsigned char, 4> VPTStates;
   };
 
 /// ARM disassembler for all ARM platforms.
 class ARMDisassembler : public MCDisassembler {
 public:
   ARMDisassembler(const MCSubtargetInfo &STI, MCContext &Ctx) :
     MCDisassembler(STI, Ctx) {
+    InstructionEndianness = STI.getFeatureBits()[ARM::ModeBigEndianInstructions]
+                                ? llvm::support::big
+                                : llvm::support::little;
   }
 
   ~ARMDisassembler() override = default;
 
   DecodeStatus getInstruction(MCInst &Instr, uint64_t &Size,
                               ArrayRef<uint8_t> Bytes, uint64_t Address,
                               raw_ostream &CStream) const override;
 
DecodeStatus getThumbInstruction(MCInst &Instr, uint64_t &Size,
                                    ArrayRef<uint8_t> Bytes, uint64_t Address,
                                    raw_ostream &CStream) const;
 
   mutable ITStatus ITBlock;
   mutable VPTStatus VPTBlock;
 
   DecodeStatus AddThumbPredicate(MCInst&) const;
   void UpdateThumbVFPPredicate(DecodeStatus &, MCInst&) const;
+
+  llvm::support::endianness InstructionEndianness;
 };
 
 } // end anonymous namespace
 
 static bool Check(DecodeStatus &Out, DecodeStatus In) {
   switch (In) {
     case MCDisassembler::Success:
       // Out stays the same.
uint64_t ARMDisassembler::suggestBytesToSkip(ArrayRef<uint8_t> Bytes,
   // second half as something else.
   //
   // If we don't have the instruction data available, we just have to
   // recommend skipping the minimum sensible distance, which is 2
   // bytes.
   if (Bytes.size() < 2)
     return 2;
 
-  uint16_t Insn16 = (Bytes[1] << 8) | Bytes[0];
+  uint16_t Insn16 = llvm::support::endian::read<uint16_t>(
+      Bytes.data(), InstructionEndianness);
   return Insn16 < 0xE800 ? 2 : 4;
 }
 
 DecodeStatus ARMDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
                                              ArrayRef<uint8_t> Bytes,
                                              uint64_t Address,
                                              raw_ostream &CS) const {
   if (STI.getFeatureBits()[ARM::ModeThumb])
assert(!STI.getFeatureBits()[ARM::ModeThumb] &&
          "mode!");
 
   // We want to read exactly 4 bytes of data.
   if (Bytes.size() < 4) {
     Size = 0;
     return MCDisassembler::Fail;
   }
 
-  // Encoded as a small-endian 32-bit word in the stream.
-  uint32_t Insn =
-      (Bytes[3] << 24) | (Bytes[2] << 16) | (Bytes[1] << 8) | (Bytes[0] << 0);
+  // Encoded as a 32-bit word in the stream.
+  uint32_t Insn = llvm::support::endian::read<uint32_t>(Bytes.data(),
+                                                        InstructionEndianness);
 
   // Calling the auto-generated decoder function.
   DecodeStatus Result =
       decodeInstruction(DecoderTableARM32, MI, Insn, Address, this, STI);
   if (Result != MCDisassembler::Fail) {
     Size = 4;
     return checkDecodedInstruction(MI, Size, Address, CS, Insn, Result);
   }
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;
     return MCDisassembler::Fail;
   }
 
-  uint16_t Insn16 = (Bytes[1] << 8) | Bytes[0];
+  uint16_t Insn16 = llvm::support::endian::read<uint16_t>(
+      Bytes.data(), InstructionEndianness);
   DecodeStatus Result =
       decodeInstruction(DecoderTableThumb16, MI, Insn16, Address, this, STI);
   if (Result != MCDisassembler::Fail) {
     Size = 2;
     Check(Result, AddThumbPredicate(MI));
     return Result;
   }
 
DecodeStatus ARMDisassembler::getThumbInstruction(MCInst &MI, uint64_t &Size,
 
   // We want to read exactly 4 bytes of data.
   if (Bytes.size() < 4) {
     Size = 0;
     return MCDisassembler::Fail;
   }
 
   uint32_t Insn32 =
-      (Bytes[3] << 8) | (Bytes[2] << 0) | (Bytes[1] << 24) | (Bytes[0] << 16);
+      (uint32_t(Insn16) << 16) | llvm::support::endian::read<uint16_t>(
+                                     Bytes.data() + 2, InstructionEndianness);
 
   Result =
       decodeInstruction(DecoderTableMVE32, MI, Insn32, Address, this, STI);
   if (Result != MCDisassembler::Fail) {
     Size = 4;
 
     // Nested VPT blocks are UNPREDICTABLE. Must be checked before we add
     // the VPT predicate.

llvm/test/tools/llvm-objdump/ELF/ARM/be-disasm.test

+# RUN: yaml2obj --docnum=1 -DCONTENT=FA000002E59F100CE0800001E12FFF1E4802EB00308047703141592627182818 %s | llvm-objdump -d --triple=armv7r - | FileCheck %s
+# RUN: yaml2obj --docnum=1 -DCONTENT=020000FA0C109FE5010080E01EFF2FE1024800EB803070473141592627182818 -DFLAG=,EF_ARM_BE8 %s | llvm-objdump -d --triple=armv7r - | FileCheck %s
+# RUN: yaml2obj --docnum=2 -DCONTENT=FA000002E59F100CE0800001E12FFF1E4802EB00308047703141592627182818 %s | llvm-objdump -d --triple=armv7r - | FileCheck %s
+
+## Test llvm-objdump disassembly of all three kinds of
+## AAELF32-compliant big-endian ELF file.
+##
+## In image files, by default AArch32 ELF stores the instructions
+## big-endian ('BE32' style), unless the EF_ARM_BE8 flag is set in the
+## ELF header, which indicates that instructions are stored
+## little-endian ('BE8' style). llvm-objdump should detect the flag and
+## handle both types, using the $a, $t and $d mapping symbols to
+## distinguish Arm instructions, Thumb instructions, and data.
+##
+## Relocatable object files always use the BE32 style. (The linker is
+## expected to byte-swap code sections, using the same the mapping
+## symbols to decide how, if it's going to generate an image with BE8
+## instruction endianness and the BE8 flag set.)
+##
+## This test checks all three cases of this. It provides llvm-objdump
+## with the BE32 and BE8 versions of the same image file, with the code
+## section byte-swapped, and the EF_ARM_BE8 flag absent and present
+## respectively to indicate that. We also provide a matching object
+## file. We expect the identical disassembly from both, apart from the
+## detail that addresses in the ELF images start at 0x8000 and section
+## offsets in the object start at 0.
+
+# CHECK:             0: fa000002      blx
+# CHECK-NEXT:        4: e59f100c      ldr     r1, [pc, #12]
+# CHECK-NEXT:        8: e0800001      add     r0, r0, r1
+# CHECK-NEXT:        c: e12fff1e      bx      lr
+# CHECK:            10: 4802          ldr     r0, [pc, #8]
+# CHECK-NEXT:       12: eb00 3080     add.w   r0, r0, r0, lsl #14
+# CHECK-NEXT:       16: 4770          bx      lr
+# CHECK:            18: 31 41 59 26   .word   0x31415926
+# CHECK-NEXT:       1c: 27 18 28 18   .word   0x27182818
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2MSB
+  Type:            ET_EXEC
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_EABI_UNKNOWN[[FLAG=]] ]
+  Entry:           0x8000
+ProgramHeaders:
+  - Type:            PT_LOAD
+    Flags:           [ PF_X, PF_R ]
+    FirstSec:        .text
+    LastSec:         .text
+    VAddr:           0x8000
+    Align:           0x4
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x8000
+    AddressAlign:    0x4
+    Content:         [[CONTENT]]
+Symbols:
+  - Name:            '$a'
+    Section:         .text
+    Value:           0x8000
+  - Name:            '$t'
+    Section:         .text
+    Value:           0x8010
+  - Name:            '$d'
+    Section:         .text
+    Value:           0x8018
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2MSB
+  Type:            ET_REL
+  Machine:         EM_ARM
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x4
+    Content:         [[CONTENT]]
+Symbols:
+  - Name:            '$a'
+    Section:         .text
+  - Name:            '$t'
+    Section:         .text
+    Value:           0x10
+  - Name:            '$d'
+    Section:         .text
+    Value:           0x18

llvm/tools/llvm-objdump/llvm-objdump.cpp

void printInst(MCInstPrinter &IP, const MCInst *MI, ArrayRef<uint8_t> Bytes,
                  object::SectionedAddress Address, formatted_raw_ostream &OS,
                  StringRef Annot, MCSubtargetInfo const &STI, SourcePrinter *SP,
                  StringRef ObjectFilename, std::vector<RelocationRef> *Rels,
                  LiveVariablePrinter &LVP) override {
     if (SP && (PrintSource || PrintLines))
       SP->printSourceLine(OS, Address, ObjectFilename, LVP);
     LVP.printBetweenInsts(OS, false);
 
     size_t Start = OS.tell();
     if (LeadingAddr)
       OS << format("%8" PRIx64 ":", Address.Address);
     if (ShowRawInsn) {
       size_t Pos = 0, End = Bytes.size();
       if (STI.checkFeatures("+thumb-mode")) {
         for (; Pos + 2 <= End; Pos += 2)
           OS << ' '
              << format_hex_no_prefix(
                     llvm::support::endian::read<uint16_t>(
-                        Bytes.data() + Pos, llvm::support::little),
+                        Bytes.data() + Pos, InstructionEndianness),
                     4);
       } else {
         for (; Pos + 4 <= End; Pos += 4)
           OS << ' '
              << format_hex_no_prefix(
                     llvm::support::endian::read<uint32_t>(
-                        Bytes.data() + Pos, llvm::support::little),
+                        Bytes.data() + Pos, InstructionEndianness),
                     8);
       }
       if (Pos < End) {
         OS << ' ';
         dumpBytes(Bytes.slice(Pos), OS);
       }
     }
 
     AlignToInstStartColumn(Start, STI, OS);
 
     if (MI) {
       IP.printInst(MI, Address.Address, "", STI, OS);
     } else
       OS << "\t<unknown>";
   }
+
+  void setInstructionEndianness(llvm::support::endianness Endianness) {
+    InstructionEndianness = Endianness;
+  }
+
+private:
+  llvm::support::endianness InstructionEndianness = llvm::support::little;
 };
 ARMPrettyPrinter ARMPrettyPrinterInst;
 
 class AArch64PrettyPrinter : public PrettyPrinter {
 public:
   void printInst(MCInstPrinter &IP, const MCInst *MI, ArrayRef<uint8_t> Bytes,
                  object::SectionedAddress Address, formatted_raw_ostream &OS,
                  StringRef Annot, MCSubtargetInfo const &STI, SourcePrinter *SP,
std::unique_ptr<const MCAsmInfo> AsmInfo(
       TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
   if (!AsmInfo)
     reportError(Obj->getFileName(),
                 "no assembly info for target " + TripleName);
 
   if (MCPU.empty())
     MCPU = Obj->tryGetCPUName().value_or("").str();
 
+  if (isArmElf(*Obj)) {
+    // When disassembling big-endian Arm ELF, the instruction endianness is
+    // determined in a complex way. In relocatable objects, AAELF32 mandates
+    // that instruction endianness matches the ELF file endianness; in
+    // executable images, that's true unless the file header has the EF_ARM_BE8
+    // flag, in which case instructions are little-endian regardless of data
+    // endianness.
+    //
+    // We must set the big-endian-instructions SubtargetFeature to make the
+    // disassembler read the instructions the right way round, and also tell
+    // our own prettyprinter to retrieve the encodings the same way to print in
+    // hex.
+    const auto *Elf32BE = dyn_cast<ELF32BEObjectFile>(Obj);
+
+    if (Elf32BE && (Elf32BE->isRelocatableObject() ||
+                    !(Elf32BE->getPlatformFlags() & ELF::EF_ARM_BE8))) {
+      Features.AddFeature("+big-endian-instructions");
+      ARMPrettyPrinterInst.setInstructionEndianness(llvm::support::big);
+    } else {
+      ARMPrettyPrinterInst.setInstructionEndianness(llvm::support::little);
+    }
+  }
+
   std::unique_ptr<const MCSubtargetInfo> STI(
       TheTarget->createMCSubtargetInfo(TripleName, MCPU, Features.getString()));
   if (!STI)
     reportError(Obj->getFileName(),
                 "no subtarget info for target " + TripleName);
   std::unique_ptr<const MCInstrInfo> MII(TheTarget->createMCInstrInfo());
   if (!MII)
     reportError(Obj->getFileName(),