[PATCH] D70495: [llvm-readobj/llvm-readelf] - Reimplement dumping of the SHT_GNU_verdef section.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D70495#1759592 <https://reviews.llvm.org/D70495#1759592>, @MaskRay wrote:

> LGTM, but please make sure @jhenderson is happy about the untested part.


I'd prefer the yaml2obj change land first if possible. However, if it's a sizeable change (I doubt it is, but just in case), I don't think it's worth blocking this change from landing if there's a particular need for it. I would like the error message improvement discussed inline though.



================
Comment at: llvm/test/tools/llvm-readobj/elf-versioninfo.test:321
+
+# INVALID-STRING-TABLE: warning: '[[FILE]]': SHT_GNU_verdef section: can't use the string table linked: invalid sh_type for string table section [index 0]: expected SHT_STRTAB, but got SHT_NULL
+
----------------
grimar wrote:
> jhenderson wrote:
> > I'd change this to:
> > 
> > sh_link field of SHT_GNU_verdef section at index N is invalid: the section at index 0 is not a string table: expected SHT_STRTAB but got SHT_NULL
> It comes from:
> 
> ```
> Expected<StringRef> StrTabOrErr = Obj->getStringTable(*StrTabSecOrErr);
>   if (!StrTabOrErr)
>     return createError(
>         "SHT_GNU_verdef section: can't use the string table linked: " +
>         toString(StrTabOrErr.takeError()));
> ```
> 
> i.e. we know that `getStringTable` failed, but can't do any additional assumptions about details. `getStringTable()` reports the reason and it's text is
> out of the area of this patch. I've changed the prefix part.
The "can't get the string table linked" is the bit I'm most concerned about, as it is not good English and is part of this patch. How about this instead: "invalid string table linked to SHT_GNU_verdef secion with index 1: invalid sh_type ..." (or simply "invalid section linked to...")?

Same goes for the similar message above.


================
Comment at: llvm/test/tools/llvm-readobj/elf-versioninfo.test:338
+
+## Check that we report a warning when can't read a content of the SHT_GNU_verdef section.
+
----------------
can't read a content -> we can't read the content


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

https://reviews.llvm.org/D70495