[PATCH] D68705: [llvm-readelf/llvm-readobj] - Improve dumping of broken versioning sections.
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 28 07:58:43 PDT 2019
jhenderson added inline comments.
================
Comment at: llvm/test/tools/llvm-readobj/elf-invalid-versioning.test:154
+## Here we have a SHT_GNU_verneed section aligned by 1 byte.
+## This makes the first auxiliary record offset % 4 be != 0.
+
----------------
I found it slightly hard to parse this last sentence. Perhaps change `!= 0` to `non-zero`.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:671
+ if ((ptrdiff_t)VernauxBuf % sizeof(uint32_t) != 0)
+ report_fatal_error("an auxiliary record found is misaligned");
+
----------------
This and the other errors here shouldn't use `report_fatal_error`. They should use `reportError` or similar. I'm okay with that being a separate change, but perhaps it should be first to avoid adding another bad usage.
The message could include more context, for example giving the offset or similar, so that a user might be able to identify the bad entry.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3927-3928
+ StringRef File = StringTable.size() >= Verneed->vn_file
+ ? StringTable.drop_front(Verneed->vn_file)
+ : "<invalid>";
----------------
You probably need a test case for `vn_file > StringTable.size()` (but the string table exists) and `vn_file == StringTable.size()`.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3941-3943
+ StringRef Name = StringTable.size() >= Vernaux->vna_name
+ ? StringTable.drop_front(Vernaux->vna_name)
+ : "<invalid>";
----------------
You probably need a test case for `vna_name == StringTable.size()`.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5720-5723
+ StringRef FileName = StringTable.size() >= Verneed->vn_file
+ ? StringTable.drop_front(Verneed->vn_file)
+ : "<invalid>";
+ W.printString("FileName", FileName.data());
----------------
Same comment as GNU style.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5735-5738
+ StringRef Name = StringTable.size() >= Vernaux->vna_name
+ ? StringTable.drop_front(Vernaux->vna_name)
+ : "<invalid>";
+ W.printString("Name", Name.data());
----------------
Same comment as GNU style.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68705/new/
https://reviews.llvm.org/D68705
More information about the llvm-commits
mailing list