[PATCH] D74595: [llvm-readobj] - Report warnings instead of errors for broken relocations.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/relocation-errors.test:47
+    Relocations:
+## Case 1: There is no a symbol with index of 0xFEFEFEFE. Also, check
+##         that warnings we report are uniqified.
----------------
no a symbol -> no symbol
index of -> index


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3326-3327
+    this->reportUniqueWarning(createError(
+        "error dumping the relocation at offset 0x" +
+        Twine::utohexstr(R.r_offset) + ": " + toString(Target.takeError())));
+  else
----------------
I'm not sure this error message quite makes sense. To me, I read it as dumping the relocation at file offset 0x1234 (i.e. where the relocation is physically stored in the file), whereas the offset is the section-relative offset.

Perhaps this error should print the index of the relocation and its section, i.e. something like "unable to print the relocation with index 4 in .rela.text: ...". I'd also recommend the phrasing I've used there regarding "unable to print" instead of "error dumping".

Same comment goes for LLVM style.

By the way, would it not make sense to print something like "<?>" or "<corrupt>" etc for the invalid values?


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

https://reviews.llvm.org/D74595