[PATCH] D64714: [Object/llvm-readelf/llvm-readobj] - Improve error reporting when e_shstrndx is broken.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I wonder if it would make sense if the file name was included in the error message, at least in the llvm-readobj/llvm-readelf case? In that case, there can be more than one file, or indeed it could be an archive member that is broken, so reporting the file name would make sense.

Aside from that, do we have a way yet of testing the e_shstrndx == SHN_XINDEX case yet for this error? I think that deserves its own test case once we do, so probably should have a TODO note somewhere around the relevant bits of code.



================
Comment at: include/llvm/Object/ELF.h:471
+    return createError(
+        "e_shstrndx/SHN_XINDEX refers to a section that doesn't exist: " +
+        Twine(Index));
----------------
I'm not sure "SHN_XINDEX" in this context makes sense, since that isn't the thing that's doing the referring. How about simply "section header string table index <N> does not exist"?


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:3021
+    reportError(
+        "e_shstrndx/SHN_XINDEX refers to a section that doesn't exist: " +
+        Twine(Index));
----------------
Same comment applies here.


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

https://reviews.llvm.org/D64714