[PATCH] D70826: [llvm-readobj/llvm-readelf] - Reimplement dumping of the SHT_GNU_verneed section.

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: llvm/test/tools/llvm-readobj/elf-verneed-invalid.test:372
+    ShOffset: 0xFFFFFFFF
+    Dependencies:
+      - Version: 1
----------------
grimar wrote:
> jhenderson wrote:
> > grimar wrote:
> > > jhenderson wrote:
> > > > Ditto
> > > Done.
> > Huh, just realised: how come 0xFFFFFFFF + 0 can't be represented?
> It happens because section offset (0xFFFFFFFF) + Size (0) > file size (560):
> 
> ```
> ELFFile<ELFT>::getSectionContentsAsArray(const Elf_Shdr *Sec) const {
> ...
>   if ((std::numeric_limits<uintX_t>::max() - Offset < Size) ||
>       Offset + Size > Buf.size())
>     return createError("section " + getSecIndexForError(this, Sec) +
>                        " has a sh_offset (0x" + Twine::utohexstr(Offset) +
>                        ") + sh_size (0x" + Twine::utohexstr(Size) +
>                        ") that cannot be represented");
> ```
Okay, I think I must have missed that in a previous review: "cannot be represented" means the value can't fit in the appropriate uintX_t, whereas if an offset is greater than the file size, it should be a different error message e.g. "section (W) has a sh_offset (X) + sh_size (Y) that is greater than the file size (Z)". I think this should be fixed in a follow-up.


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

https://reviews.llvm.org/D70826