Handle a VersymIndex of 0 as an error

[• rafael]

Summary

Hi Peter,

I just noticed that the continue this patch deletes was not tested. Trying to add a test I realized that we never put a VER_NDX_LOCAL symbol in the dynamic symbol table. Is there a reason why a linker ever would?

[pcc]

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.

ELF/InputFiles.cpp

811

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).

[• espindola]

r320817

ELF/InputFiles.cpp

       continue;
     }
 
-    // Ignore local symbols.
-    if (Versym && VersymIndex == VER_NDX_LOCAL)
-      continue;
     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) {
         error("corrupt input file: version definition index " +
               Twine(VersymIndex) + " for symbol " + Name +
               " is out of bounds\n>>> defined in " + toString(this));

test/ELF/corrupted-version-reference.s

 # CHECK: error: corrupt input file: version definition index 9 for symbol __cxa_finalize is out of bounds
 # CHECK: >>> defined in {{.+}}/corrupt-version-reference.so
 
+# CHECK:      error: corrupt input file: version definition index 0 for symbol _Jv_RegisterClasses is out of bounds
+# CHECK-NEXT: >>> defined in {{.*}}/corrupt-version-reference.so
+
 .globl __start
 __start:
     dla $a0, __cxa_finalize