[PATCH] D68705: [llvm-readelf/llvm-readobj] - Improve dumping of broken versioning sections.

[James Henderson via Phabricator via llvm-commits]

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