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

[George Rimar via Phabricator via llvm-commits]

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

LGTM, thanks!



================
Comment at: lld/ELF/InputFiles.cpp:1226
+  if (!dataOrErr)
+    return {};
+
----------------
MaskRay wrote:
> grimar wrote:
> > This doesn't look right to me:
> > 
> > 1) `obj.getSectionContents(sec)` returns `Expected<ArrayRef<uint8_t>>` and you do not handle an error,
> > the code will fail with `LLVM_ENABLE_ABI_BREAKING_CHECKS`. Do we have a test for this case?
> > 
> > 2) Not sure why you return no entries. This invocation might fail when `sh_size` or `sh_offset` is broken.
> > Shouldn't we call `fatal()` too?
> Sorry, missed this. The first test (`ShOffset: 0xFFFFFFFF) tested this scenario. It will indeed break `LLVM_ENABLE_ABI_BREAKING_CHECKS` builds (which I can't test now because of some unrelated llvm-tblgen problems)
nit: you probably might want to use `CHECK` as it often used in LLD:
(looks shorter, and I think will provide exactly the same error output)

`ArrayRef<uint8_t> data = CHECK(obj.getSectionContents(sec), this);`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80059