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

Thu Mar 9 10:19:25 PST 2023

aidengrossman added a comment.

@rahmanl Do you mind reviewing my most recent changes and accepting this (assuming there are no further comments) when you get a chance? Thank you.

================
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.
----------------
jhenderson wrote:
> 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?
Ah, that logic makes a lot of sense. I've added an `ET_DYN` test with a `.rela.dyn` section to test this. Thanks for the suggestion!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143841