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

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Object/ELF.cpp:648
+  // reference to the function in the bbaddrmap section) to the addend
+  // (the location of the function in the text section)
+
----------------
Nit.

Also, since this comment is tied to the following variable, there should not be a blank line between them (there might want to be one before the comment, but that's up to you).


================
Comment at: llvm/lib/Object/ELF.cpp:705-710
+      if (FOTIterator != FunctionOffsetTranslations.end())
+        Address = FOTIterator->second;
+      else
+        return createError("Failed to get relocation data for offset: " +
+                           Twine(SectionOffset) + " in section " +
+                           describe(*this, Sec));
----------------
You could turn this into a plain `if` without an `else` if you flip the two parts around, i.e. make the if clause `FOTIterator == FunctionOffsetTranslations.end()`.


================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml:5
+
+# Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
+# UNSUPPORTED: system-windows
----------------



================
Comment at: llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml:5
+
+# UNSUPPORTED: system-windows
+
----------------
jhenderson wrote:
> Why is this unsupported on Windows?
Given we have an open ticket for this, I wonder if this should be an XFAIL, not UNSUPPORTED. The idea being that when the issue is fixed, the test will start x-passing, and not be forgotten about.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test:5
+## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
+## UNSUPPORTED: system-windows
+
----------------
Also, see above re. XFAIL vs UNSUPPORTED


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:7102
+  llvm::MapVector<const Elf_Shdr *, const Elf_Shdr *> Sections;
+  this->getSectionAndRelocations(IsMatch, Sections);
+  for (auto const& [Sec, RelocSec] : Sections) {
----------------
This (currently) returns an `Error`, so that return value needs capturing and checking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143841