[PATCH] D143841: [Propeller] Make decoding BBAddrMaps trace through relocations

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, barring typo (and also possibly extra test case, though I'm uncertain about that one).



================
Comment at: llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test:147
+
+## Check that if we have a missing relocatied section, we fail and give the
+## appropriate warning.
----------------
Still got a typo :)


================
Comment at: llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test:174
+
+## Check that if we have a missing relocation section, getSectionAndRelocations
+## fails and we get the appropriate warning.
----------------
aidengrossman wrote:
> jhenderson wrote:
> > ?
> > 
> > Is it worth the following test case: an ET_EXEC or ET_DYN file with a .rela.dyn section that doesn't reference a particular section? To show that we don't get this warning in that situation?
> Fixed the typo. Thank you. When creating a shared object/exectuable, the addresses for the functions are already populated in the SHT_LLVM_BB_ADDR_MAP` sections, so we don't need to do any relocations. The new paths introduced by this patch also only run on `ET_REL` files.
My thinking is mostly along the lines of "what do we have that ensures that the `if (ET_REL)` check is actually there?" In other words, if that check disappeared, would this warning potentially pop up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143841