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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added inline comments.


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:576
 
+static bool isArm32Elf(const ObjectFile *Obj) {
+  return (Obj->isELF() &&
----------------
efriedma wrote:
> 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?
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.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60927