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

[Rahman Lavaee via Phabricator via llvm-commits]

rahmanl added a comment.

No major comments. Looks good.



================
Comment at: llvm/lib/Object/ELFObjectFile.cpp:692
+      return false;
     if (TextSectionIndex) {
       Expected<const Elf_Shdr *> TextSecOrErr = EF.getSection(Sec.sh_link);
----------------
Flip this for more readability, i.e,
```
if (!TextSectionIndex) return true;

Expected<const Elf_Shdr *> TextSecOrErr = EF.getSection(Sec.sh_link);
...
```


================
Comment at: llvm/lib/Object/ELFObjectFile.cpp:712
+    if (IsRelocatable)
+      RelaSec = *RelocSec;
+    Expected<std::vector<BBAddrMap>> BBAddrMapOrErr =
----------------
This can be `nullptr`. 


================
Comment at: llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test:101-108
+          - ID:              2
+            AddressOffset:   0x0
+            Size:            0x6
+            Metadata:        0x8
+          - ID:              3
+            AddressOffset:   0x0
+            Size:            0x5
----------------
Maybe remove these too.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:7092
+  using Elf_Shdr = typename ELFT::Shdr;
+  auto IsMatch = [&](const Elf_Shdr &Sec) -> bool {
+    return Sec.sh_type == ELF::SHT_LLVM_BB_ADDR_MAP ||
----------------
No need to capture anything here.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:7109
     ListScope L(W, "BBAddrMap");
+    std::optional<Elf_Shdr> RelaSec = {};
+    if(IsRelocatable)
----------------
Change this consistently with the other code.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:7111
+    if(IsRelocatable)
+      RelaSec = *RelocSec;
     Expected<std::vector<BBAddrMap>> BBAddrMapOrErr =
----------------
Again, handle the case where this is `nullptr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143841