[PATCH] D60927: [WIP][llvm-objdump] Switch between ARM/Thumb based on mapping symbols.

Tue Apr 23 12:04:50 PDT 2019

efriedma marked an inline comment as done.
efriedma added inline comments.

================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:576

+static bool isArm32Elf(const ObjectFile *Obj) {
+  return (Obj->isELF() &&
----------------
peter.smith wrote:
> MaskRay wrote:
> > efriedma wrote:
> > > MaskRay wrote:
> > > > Since you are adding `isArm32Elf`, can you add `isArm64Elf` and delete `isArmElf` if that orthogonal property is desired?
> > > 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"?
> > SG
> 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.
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?

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60927