[PATCH] D130357: [MC,llvm-objdump,ARM] Target-dependent disassembly resync policy.

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham added inline comments.


================
Comment at: llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp:758
+      if (Bytes.size() >= 2) {
+        uint16_t Insn16 = (Bytes[1] << 8) | Bytes[0];
+        return Insn16 < 0xE800 ? 2 : 4;
----------------
simon_tatham wrote:
> DavidSpickett wrote:
> > Is the in memory layout of thumb instructions the same between endians?
> In the modern (BE8-style) Arm architecture, yes: instructions are stored little-endian regardless of the data endianness. In A32 that means little-endian 32-bit words, and in T32 it means little-endian 16-bit halfwords.
> 
> In older versions of the architecture that was different, if I remember rightly. But the rest of LLVM doesn't support those versions either, because this code is taken directly from the code in the same file that extracts the halfword for actual disassembly.
Hmmm, actually, now I go back and check more carefully ...

I'd forgotten the wrinkle that in the AArch32 ABI, ELF //objects// are supposed to have their instructions stored in endianness matching the ELF header. But ELF //images// have them stored in BE8, and the linker is supposed to byte-swap the right parts of the code sections based on the mapping symbols. So actually, you're right: the places in this patch and in D130358 where I've added always-little-endian accesses into the code section won't work everywhere.

On the other hand, (a) LLD doesn't support that byte-reversal, and (b) the MC disassembler //also// reads instructions as little-endian unconditionally, so even before this patch, `llvm-objdump` would mis-disassemble an object file of that kind.

So I'm not introducing any more big-endian incompatibility than was already there. And if we make major changes to fix that at a later date, then I think these extra little-endian accesses won't be forgotten about, because the llvm-objdump tests touched by these patches will fail and remind whoever is doing it!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130357/new/

https://reviews.llvm.org/D130357