[PATCH] D144783: [ELF] Move getSectionAndRelocations to ELF.cpp from ELFDumper.cpp

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/Object/ELF.h:401
+  /// retrieve the content section of any relocation section.
+  Expected<llvm::MapVector<const Elf_Shdr *, const Elf_Shdr *>>
+  getSectionAndRelocations(
----------------
Maybe a dumb question, but why are we qualifying `MapVector` with the `llvm` namespace here and elsewhere?


================
Comment at: llvm/lib/Object/ELF.cpp:718-719
+    if (!DoesSectionMatch) {
+      return DoesSectionMatch.takeError();
+      continue;
+    }
----------------
Do you want to try again with these two lines ;)

I'm assuming one of the two lines is spurious...


================
Comment at: llvm/lib/Object/ELF.cpp:732-735
+      return createError(describe(*this, Sec) +
+                         ": failed to get a relocated section: " +
+                         toString(RelSecOrErr.takeError()));
+      continue;
----------------
Same comment as above.


================
Comment at: llvm/lib/Object/ELF.cpp:740-741
+    if (!DoesRelTargetMatch) {
+      return DoesRelTargetMatch.takeError();
+      continue;
+    }
----------------
And again.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/stack-sizes.test:874
-# INVALID-TARGET: warning: '[[FILE]]': SHT_RELA section with index 1: failed to get a relocated section: invalid section index: 255
-# INVALID-TARGET: warning: '[[FILE]]': SHT_RELA section with index 2: failed to get a relocated section: invalid section index: 255
 
----------------
It would be nice to not lost the second warning, since it's for a different section, and therefore indicates that there's more than one problem. I think you'd have to use `joinErrors` in your loop, to gather all errors into a single one, and then handle them at the caller end, individually.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:6211
+  if (!StackSizeRelocMapOrErr) {
+    reportUniqueWarning("Unable to get stack size map section(s): " +
+                        toString(StackSizeRelocMapOrErr.takeError()));
----------------



================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:7047
+  if (!SecToRelocMapOrErr) {
+    this->reportUniqueWarning("Unable to get CG Profile section(s): " +
+                              toString(SecToRelocMapOrErr.takeError()));
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144783