[PATCH] D63266: [llvm-readobj] - Do not fail to dump the object which has wrong type of .shstrtab.

Fri Jun 14 04:33:51 PDT 2019

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM.

================
Comment at: tools/llvm-readobj/ELFDumper.cpp:3012
+  if (!Index || Index >= Sections.size())
+    report_fatal_error("invalid section index");
+  StringRef Data = toStringRef(
----------------
grimar wrote:
> jhenderson wrote:
> > Could this call `reportError(Twine)` at least? Ideally, I'd prefer it if it called a function that included the file name too. I think there's one in llvm-readobj.cpp that would be useful: `reportError(StringRef Input, Error Err)`. I don't see a reason that this couldn't be exposed to the rest of llvm-readobj.
> I changed to `reportError(Twine)`. It looks a bit problematic to get a file name here and I am not sure it is very useful to do this change right now, because actually, I think we do not need this error at all here. I introduced it to minimize the change of the existent logic (i.e. we already had this fatal error at deeper levels), but I think `getSectionName` actually should never fail and terminate the proccess. It probably might return a `Error` instead or a pair of section name + warning message, or something else, but idea is that if we can`t get the section name because of any reason we should probably continue dumping and just show something like `<section name unknown>` instead of the section name.
The easiest way would be to pass in ELFObjectFile rather than ELFFile, I think, but maybe that doesn't make sense.

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

https://reviews.llvm.org/D63266