[PATCH] D80059: [ELF] Parse SHT_GNU_verneed and respect versioned undefined symbols in shared objects

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 21 04:50:17 PDT 2020


grimar added inline comments.


================
Comment at: lld/ELF/InputFiles.cpp:1225
+
+  const uint8_t *verneedBuf = obj.base() + sec->sh_offset;
+  const uint8_t *bufEnd =
----------------
grimar wrote:
> grimar wrote:
> > This looks a bit cleaner probably:
> > 
> > ```
> >   ArrayRef<uint8_t> data =
> >       CHECK(obj.template getSectionContentsAsArray<uint8_t>(sec), this);
> >   const uint8_t *verneedBuf = data.begin();
> > ```
> > 
> > And then you can just inline `bufEnd` as `data.end()` I think. See below.
> You've missed this one?
> Using of `getSectionContentsAsArray` has benefits:
> 
> It would check the sh_offset/sh_size early and might provide a better error message:
> 
> ```
> ELFFile<ELFT>::getSectionContentsAsArray(const Elf_Shdr *Sec) const {
> ...
>   uintX_t Offset = Sec->sh_offset;
>   uintX_t Size = Sec->sh_size;
> 
> ...
>   if (std::numeric_limits<uintX_t>::max() - Offset < 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");
>   if (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 is greater than the file size (0x" +
>                        Twine::utohexstr(Buf.size()) + ")");
> ```
> 
> It also hides the calculations done here like `obj.base() + sec->sh_offset` and
> `obj.base() + std::min<uint64_t>(sec->sh_offset + sec->sh_size, obj.getBufSize());`,
> what is good.
Ah, and you can actually `getSectionContents`:

```
template <class ELFT>
Expected<ArrayRef<uint8_t>>
ELFFile<ELFT>::getSectionContents(const Elf_Shdr *Sec) const {
  return getSectionContentsAsArray<uint8_t>(Sec);
}
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80059





More information about the llvm-commits mailing list