Fix disassembling ARM instructions as big-endian

[comex]

Summary

ARMv6 and earlier include optional support for big-endian instruction
encoding (in three all encoding variants: ARM, Thumb, and Thumb-2).
LLVM already supports assembling ARM code as big-endian by specifying
'armbe' as the target architecture; however, the disassembler
currently ignores the endian choice and always disassembles as
little-endian. This is a simple patch to fix that, plus a test.

[javed.absar]

Is this applicable only for ARMv6 case? From the patch it does not seem so and so may be good to add cases for ARMv7 as well.

test/MC/Disassembler/ARM/endian.txt

2

Please consider adding a line description of the test. Kind of obvious from reading the test but may still be useful.

[comex]

Fix disassembling ARM instructions as big-endian

ARMv6 and earlier (and ARMv7-R) include optional support for
big-endian instruction encoding, in all three encoding variants:
ARM, Thumb, and Thumb-2. LLVM already supports assembling these
as big-endian by adding 'eb' to the architecture name in the triple
(e.g. 'armeb', 'armv6eb', 'thumbv7eb'); however, the disassembler
currently ignores the endian choice and always disassembles as
little-endian. This is a simple patch to fix that, plus a test.

[Sidenote: ARM supports configuring data and instruction endianness
separately, and ARMv7 does not deprecate big-endian *data* encoding.
Thus, the ARM proprietary toolchain and GNU toolchain support targets
where instructions are encoded is little-endian but data accesses
are big-endian. This is known as BE8 mode (--be8) and is implemented
in a hacky way: the compiler emits big-endian instructions and the
linker swaps them! LLVM does not currently support BE8 at all. In
theory, when parsing an ELF file with EF_ARM_BE8 in e_flags, the
disassembler should disassemble as little-endian, i.e. the old
behavior. However, I think that's out of scope for this patch,
which is just a bugfix.]

[alanphipps]

I just ran into what I think may be the same issue here in that llvm-objdump seems to ignore machine data (endian, possible mode) when disassembling ARM instructions on ARMv7. Is there any interest in completing this review and committing this particular fix (and making sure it works for ARMv7 as well)?

[alanphipps]

Just checking in again -- is there interest in completing this review? I'd be happy to assist, or I could post an updated version of it.

[DavidSpickett]

I would use support::endian::endianness instead of a bool. Then you can remove the /*isLittleEndian=*/ comments and use the read calls that take an endianness. E.g. uint16_t read16(const void *P, endianness E);.

I assume armv7eb would work out of the box, given that llvm-objdump already knows the file is big endian. (I thought there might be some ELF attribute we needed to look for)

lib/Target/ARM/Disassembler/ARMDisassembler.cpp

public:
       }
 
     private:
       std::vector<unsigned char> ITStates;
   };
 
 /// ARM disassembler for all ARM platforms.
 class ARMDisassembler : public MCDisassembler {
+  bool IsLittleEndian;
 public:
-  ARMDisassembler(const MCSubtargetInfo &STI, MCContext &Ctx) :
-    MCDisassembler(STI, Ctx) {
+  ARMDisassembler(const MCSubtargetInfo &STI, MCContext &Ctx, bool IsLittleEndian) :
+    MCDisassembler(STI, Ctx), IsLittleEndian(IsLittleEndian) {
   }
 
   ~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 {
+  bool IsLittleEndian;
 public:
-  ThumbDisassembler(const MCSubtargetInfo &STI, MCContext &Ctx) :
-    MCDisassembler(STI, Ctx) {
+  ThumbDisassembler(const MCSubtargetInfo &STI, MCContext &Ctx, bool IsLittleEndian) :
+    MCDisassembler(STI, Ctx), IsLittleEndian(IsLittleEndian) {
   }
 
   ~ThumbDisassembler() override = default;
 
   DecodeStatus getInstruction(MCInst &Instr, uint64_t &Size,
                               ArrayRef<uint8_t> Bytes, uint64_t Address,
                               raw_ostream &VStream,
                               raw_ostream &CStream) const override;

 static DecodeStatus DecodeForVMRSandVMSR(MCInst &Inst, unsigned Val,
                                          uint64_t Address, const void *Decoder);
 
 #include "ARMGenDisassemblerTables.inc"
 
 static MCDisassembler *createARMDisassembler(const Target &T,
                                              const MCSubtargetInfo &STI,
                                              MCContext &Ctx) {
-  return new ARMDisassembler(STI, Ctx);
+  return new ARMDisassembler(STI, Ctx, /*IsLittleEndian=*/true);
+}
+
+static MCDisassembler *createARMBEDisassembler(const Target &T,
+                                               const MCSubtargetInfo &STI,
+                                               MCContext &Ctx) {
+  return new ARMDisassembler(STI, Ctx, /*IsLittleEndian=*/false);
 }
 
 static MCDisassembler *createThumbDisassembler(const Target &T,
                                                const MCSubtargetInfo &STI,
                                                MCContext &Ctx) {
-  return new ThumbDisassembler(STI, Ctx);
+  return new ThumbDisassembler(STI, Ctx, /*IsLittleEndian=*/true);
+}
+
+static MCDisassembler *createThumbBEDisassembler(const Target &T,
+                                                 const MCSubtargetInfo &STI,
+                                                 MCContext &Ctx) {
+  return new ThumbDisassembler(STI, Ctx, /*IsLittleEndian=*/false);
 }
 
 // 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) {
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);
+  uint32_t Insn = IsLittleEndian ? support::endian::read32le(Bytes.data())
+                                 : support::endian::read32be(Bytes.data());
 
   // 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, OS, 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 = IsLittleEndian ? support::endian::read16le(Bytes.data())
+                                   : support::endian::read16be(Bytes.data());
+
   DecodeStatus Result =
       decodeInstruction(DecoderTableThumb16, MI, Insn16, Address, this, STI);
   if (Result != MCDisassembler::Fail) {
     Size = 2;
     Check(Result, AddThumbPredicate(MI));
     return Result;
   }
 
DecodeStatus ThumbDisassembler::getInstruction(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);
+  uint16_t Second16 = IsLittleEndian ? support::endian::read16le(Bytes.data() + 2)
+                                     : support::endian::read16be(Bytes.data() + 2);
+  uint32_t Insn32 = Second16 | ((uint32_t)Insn16 << 16);
+
   Result =
       decodeInstruction(DecoderTableThumb32, MI, Insn32, Address, this, STI);
   if (Result != MCDisassembler::Fail) {
     Size = 4;
     bool InITBlock = ITBlock.instrInITBlock();
     Check(Result, AddThumbPredicate(MI));
     AddThumb1SBit(MI, InITBlock);
     return Result;
DecodeStatus ThumbDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
   Size = 0;
   return MCDisassembler::Fail;
 }
 
 extern "C" void LLVMInitializeARMDisassembler() {
   TargetRegistry::RegisterMCDisassembler(getTheARMLETarget(),
                                          createARMDisassembler);
   TargetRegistry::RegisterMCDisassembler(getTheARMBETarget(),
-                                         createARMDisassembler);
+                                         createARMBEDisassembler);
   TargetRegistry::RegisterMCDisassembler(getTheThumbLETarget(),
                                          createThumbDisassembler);
   TargetRegistry::RegisterMCDisassembler(getTheThumbBETarget(),
-                                         createThumbDisassembler);
+                                         createThumbBEDisassembler);
 }
 
 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/MC/Disassembler/ARM/endian.txt

+# Test that the disassembler can understand both big- and little-endian encoding of all three instruction sets:
+
+# ARM instruction set (32-bit swap):
+# RUN: echo "0x01 0x00 0xa0 0xe1" | llvm-mc -triple=armv6-none -disassemble | FileCheck %s -check-prefix=ARM
+# RUN: echo "0xe1 0xa0 0x00 0x01" | llvm-mc -triple=armv6eb-none -disassemble | FileCheck %s -check-prefix=ARM
+# ARM: mov r0, r1
+
+# Thumb instruction set (16-bit swap):
+# RUN: echo "0x48 0x46" | llvm-mc -triple=thumbv6t2-none -disassemble | FileCheck %s -check-prefix=THUMB
+# RUN: echo "0x46 0x48" | llvm-mc -triple=thumbv6t2eb-none -disassemble | FileCheck %s -check-prefix=THUMB
+# THUMB: mov r0, r9
+
+# Thumb-2 instruction set (two 16-bit words swapped individually!):
+# RUN: echo "0x4f 0xea 0x09 0x00" | llvm-mc -triple=thumbv6t2-none -disassemble | FileCheck %s -check-prefix=THUMB2
+# RUN: echo "0xea 0x4f 0x00 0x09" | llvm-mc -triple=thumbv6t2eb-none -disassemble | FileCheck %s -check-prefix=THUMB2
+# THUMB2: mov.w r0, r9