[PATCH] D41247: Handle a VersymIndex of 0 as an error

[Peter Collingbourne via Phabricator via llvm-commits]

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

It looks like bfd and gold use this value for undefined symbols in DSOs that do not have version definitions. But at this point we are only dealing with defined symbols.

The Solaris documentation: https://docs.oracle.com/cd/E19120-01/open.solaris/819-0690/chapter6-31/index.html
also talks about this value being used for "A global symbol defined within an object that does not have a SHT_SUNW_verdef version definition section."
but that seems to be specific to their ABI.

Aside from that, I can't see a reason why a linker would deliberately use VER_NDX_LOCAL for a dynamic symbol. It could appear there because of a bug similar to the case where an STB_LOCAL appears in the dynsym. But unless we encounter an example of such a DSO I think we'd be fine erroring out on it.

This LGTM with one nit.



================
Comment at: ELF/InputFiles.cpp:811
     const Elf_Verdef *Ver = nullptr;
-    if (VersymIndex != VER_NDX_GLOBAL) {
-      if (VersymIndex >= Verdefs.size()) {
+    if (Versym && VersymIndex != VER_NDX_GLOBAL) {
+      if (VersymIndex >= Verdefs.size() || VersymIndex == VER_NDX_LOCAL) {
----------------
Can we set `VersymIndex` to `VER_NDX_GLOBAL` instead of 0 at the start of the loop body? Then I think this can just be `if (VersymIndex != VER_NDX_GLOBAL)`.


https://reviews.llvm.org/D41247