[PATCH] D92636: [llvm-readelf/obj] - Improve diagnostics when printing NT_FILE notes.
Jordan Rupprecht via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 4 14:08:52 PST 2020
rupprecht added a comment.
Thanks!
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5338
+ return createError(
+ "unable to read the file mappings number: the note of size 0x" +
+ Twine::utohexstr(Desc.size()) + " is truncated");
----------------
This is checking more than just the file mappings number, it also includes the page size (note the check above is for 2 bytes).
How about: "the note of size 0x1 is too short, expected at least 0x2"?
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5349
+ return createError(
+ "unable to read start/end/offset tripples: the note of size 0x" +
+ Twine::utohexstr(Desc.size()) + " is truncated");
----------------
I'd probably just say "unable to read file mappings".
If you prefer this, there's a typo: tripples -> triples.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5350
+ "unable to read start/end/offset tripples: the note of size 0x" +
+ Twine::utohexstr(Desc.size()) + " is truncated");
----------------
Maybe this could also say how many bytes it expected it to be? It should be `(2 + 3 * FileCount) * Bytes` IIRC.
And maybe say what `FileCount` is? That may give indication to a developer debugging a bad note whether the note is really too short or if the number of files is bogus.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5363
+ return createError(
+ "unable to read the file name for the file mapping with index " +
+ Twine(I) + ": the note of size 0x" + Twine::utohexstr(Desc.size()) +
----------------
"file name for the file mapping" feels a little redundant -- maybe just "file name for the mapping"?
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5576
+ for (const typename ELFT::Note Note : Obj.notes(P, Err)) {
+ ++Index;
+ if (Error E = ProcessNoteFn(Note))
----------------
`Index` should probably be incremented at the end, otherwise it reports the first note as index 1, not index 0.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92636/new/
https://reviews.llvm.org/D92636
More information about the llvm-commits
mailing list