[PATCH] D94907: [llvm-nm][ELF] - Make -D display symbol versions.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-nm/dynamic.test:153
+
+## Case 2: check we report a warning when we are unable to read a SHT_GNU_versym section entry.
+## In this case it has a size that is not a multiple of its sh_entsize.
----------------
grimar wrote:
> jhenderson wrote:
> > Here the thing you can't read is the section, not the section entry, right? If so, delete the "entry" in this sentence.
> The reported warning is:
> 
> ```
> # VERSION-ERR2: warning: unable to read symbol versions: unable to read an entry with index 1 from SHT_GNU_versym section with index 1: section [index 1] has an invalid sh_size (255) which is not a multiple of its sh_entsize (2)
> ```
> 
> I.e. I can't read a particular entry because there is an issue with extracting all section entries.
> 
> The test case tries to trigger the following code in `readSymbolVersionsELF`:
> 
> ```
>     if (!VerEntryOrErr)
>       return createError("unable to read an entry with index " + Twine(I) +
>                          " from " + describe(Obj, *SymVerSec) + ": " +
>                          toString(VerEntryOrErr.takeError()));
> ```
> 
> 
The next line of this comment refers to "it" which would mean the unreadable "section entry" mentioned at the end of the previous comment, so you either need to change the first line to talk about the section, not the section entry, or the second line to not use "it" (e.g. "In this case, the section has...")


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

https://reviews.llvm.org/D94907