[ELF] Error for undefined foo@v1

[MaskRay]

Summary

If an object file has an undefined foo@v1, we emit a dynamic symbol foo.
This is incorrect if at runtime a shared object provides the non-default version foo@v1
(the undefined foo may bind to foo@@v2, for example).

GNU ld issues an error for this case, even if foo@v1 is undefined weak
(https://sourceware.org/bugzilla/show_bug.cgi?id=3351). This behavior makes
sense because to represent an undefined foo@v1, we have to construct a Verneed
entry. However, without knowing the defining filename, we cannot construct a
Verneed entry (Verneed::vn_file is unavailable).

This patch implements the error.

Depends on D92258

[grimar]

Sounds reasonable.

lld/ELF/Relocations.cpp

952

We have the similar hacky check in

std::string lld::toString(const elf::Symbol &sym) {
  StringRef name = sym.getName();
  std::string ret = demangle(name);

  // If sym has a non-default version, its name may have been truncated at '@'
  // by Symbol::parseSymbolVersion(). Add the trailing part. This check is safe
  // because every symbol name ends with '\0'.
  if (name.data()[name.size()] == '@')
    ret += name.data() + name.size();
  return ret;
}

I think we should introduce Symbol::isVersioned or alike method to have this logic in a single place.

lld/test/ELF/symver.s

9

Something is wrong with rebasing it seems? D92258 has much more llvm-mc lines here.

[jhenderson]

lld/test/ELF/lto/version-script2.ll

13

[MaskRay]

Comments. Rebase

[MaskRay]

lld/ELF/Relocations.cpp

952

Added a helper to [ELF] Make foo@@v1 resolve undefined foo@v1

lld/test/ELF/symver.s

9

I had added more tests in D92258 but did not rebase this patch...

[jhenderson]

No more comments from me, thanks.

[grimar]

LGTM too.

[MaskRay]

Fix an assertion failure

lld/ELF/Relocations.cpp

if (!it.value().locs.empty())
       reportUndefinedSymbol<ELFT>(it.value(), it.index() < 2);
   undefs.clear();
 }
 
 // Report an undefined symbol if necessary.
 // Returns true if the undefined symbol will produce an error message.
 static bool maybeReportUndefined(Symbol &sym, InputSectionBase &sec,
                                  uint64_t offset) {
-  if (!sym.isUndefined() || sym.isWeak())
+  if (!sym.isUndefined())
+    return false;
+  // If versioned, issue an error (even if the symbol is weak) because we don't
+  // know the defining filename which is required to construct a Verneed entry.
+  if (sym.getName().data()[sym.getName().size()] == '@') {
+    undefs.push_back({&sym, {{&sec, offset}}, false});
+    return true;
+  }
+  if (sym.isWeak())
     return false;
 
   bool canBeExternal = !sym.isLocal() && sym.visibility == STV_DEFAULT;
   if (config->unresolvedSymbols == UnresolvedPolicy::Ignore && canBeExternal)
     return false;
 
   // clang (as of 2019-06-12) / gcc (as of 8.2.1) PPC64 may emit a .rela.toc
   // which references a switch table in a discarded .rodata/.text section. The

lld/test/ELF/lto/version-script2.ll

 ; REQUIRES: x86
 
 ;; For non-relocatable output, test we parse symbol versions after LTO,
 ;; otherwise we may get a symbol named "foo@@VER1", but not "foo" with the
 ;; version VER1.
 
-; RUN: llvm-as %s -o %t.o
+; RUN: split-file %s %t
+; RUN: llvm-as %t/ir -o %t.o
+; RUN: llvm-mc -filetype=obj -triple=x86_64 %t/asm -o %tbar.o
+; RUN: ld.lld %tbar.o -shared --soname=tbar --version-script %t.script -o %tbar.so
 ; RUN: echo "VER1 {};" > %t.script
-; RUN: ld.lld %t.o -o %t.so -shared --version-script %t.script
+
+;; Emit an error is bar@VER1 is not defined.
+; RUN: not ld.lld %t.o -o /dev/null -shared --version-script %t.script 2>&1 | FileCheck %s --check-prefix=UNDEF
+
+; UNDEF: error: undefined symbol: bar@VER1
+
+; RUN: ld.lld %t.o %tbar.so -o %t.so -shared --version-script %t.script
 ; RUN: llvm-readelf --dyn-syms %t.so | FileCheck %s
 
-;; For non-relocatable output, @ in symbol names has no effect on undefined symbols.
-; CHECK:      UND       bar{{$}}
+; CHECK:      UND       bar@VER1
 ; CHECK-NEXT: {{[1-9]}} foo@@VER1
 
 ;; For relocatable output, @ should be retained in the symbol name.
 ;; Don't parse and drop `@VER1`. Also check that --version-script is ignored.
 ; RUN: ld.lld %t.o -o %t.ro -r --version-script %t.script
 ; RUN: llvm-readelf -s %t.ro | FileCheck %s --check-prefix=RELOCATABLE
 
 ; RELOCATABLE:      {{[1-9]}} foo@@VER1
 ; RELOCATABLE-NEXT: UND       bar@VER1
 
+;--- ir
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 module asm ".global foo"
 module asm "foo: call bar"
 module asm ".symver foo,foo@@@VER1"
 module asm ".symver bar,bar@@@VER1"
+
+;--- asm
+.globl bar
+.symver bar,bar@@@VER1
+bar:
+  ret

lld/test/ELF/symver.s

 # REQUIRES: x86
 # RUN: split-file %s %t
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %t/ref.s -o %t/ref.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %t/ref1.s -o %t/ref1.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/ref1w.s -o %t/ref1w.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %t/def1.s -o %t/def1.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %t/hid1.s -o %t/hid1.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %t/def2.s -o %t/def2.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %t/wrap.s -o %t/wrap.o
 # RUN: ld.lld -shared --soname=def1.so --version-script=%t/ver %t/def1.o -o %t/def1.so
 # RUN: ld.lld -shared --soname=hid1.so --version-script=%t/ver %t/hid1.o -o %t/hid1.so
 
 # Report an duplicate definition error for foo@v1 and foo@@v1.
 # RUN: not ld.lld -shared --version-script=%t/ver %t/def1.o %t/hid1.o -o /dev/null 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=DUP
 
 # DUP:      error: duplicate symbol: foo@v1
 # DUP-NEXT: >>> defined at {{.*}}/hid1.o:(.text+0x0)
 # DUP-NEXT: >>> defined at {{.*}}/def1.o:(.text+0x0)
 
+# RUN: not ld.lld -shared --version-script=%t/ver %t/ref1w.o -o /dev/null 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=UNDEF
+
+# UNDEF:      error: undefined symbol: foo@v1
+# UNDEF-NEXT: >>> referenced by {{.*}}.o:(.text+0x1)
+
 ## foo@@v1 resolves both undefined foo and foo@v1. There is one single definition.
 ## Note, set soname so that the name string referenced by .gnu.version_d is fixed.
 # RUN: ld.lld -shared --soname=t1 --version-script=%t/ver %t/ref.o %t/ref1.o %t/def1.o -o %t1
 # RUN: llvm-readelf -r --dyn-syms %t1 | FileCheck %s
 
 # CHECK:       Relocation section '.rela.plt' at offset {{.*}} contains 1 entries:
 # CHECK-NEXT:  {{.*}} Type               {{.*}}
 # CHECK-NEXT:  {{.*}} R_X86_64_JUMP_SLOT {{.*}} foo@@v1 + 0
 
 # CHECK:       1: {{.*}} NOTYPE GLOBAL DEFAULT {{[1-9]}} foo@@v1
 # CHECK-EMPTY:
 
 ## foo@@v2 does not resolve undefined foo@v1.
-## TODO Undefined 'foo@v1' should be errored.
-# RUN: ld.lld -shared --soname=t2 --version-script=%t/ver %t/ref.o %t/ref1.o %t/def2.o -o %t2
-# RUN: llvm-readelf --dyn-syms %t2 | FileCheck %s --check-prefix=CHECK2
-
-# CHECK2:       1: {{.*}} NOTYPE GLOBAL DEFAULT UND       foo{{$}}
-# CHECK2-NEXT:  2: {{.*}} NOTYPE GLOBAL DEFAULT {{[1-9]}} foo@@v2
-# CHECK2-EMPTY:
+# RUN: not ld.lld -shared --soname=t2 --version-script=%t/ver %t/ref.o %t/ref1.o %t/def2.o \
+# RUN:   -o /dev/null 2>&1 | FileCheck %s --check-prefix=UNDEF2
+
+# UNDEF2: error: undefined symbol: foo@v1
 
 ## foo@@v2 resolves undefined foo while foo@v1 resolves undefined foo@v1.
 # RUN: ld.lld -shared --soname=t3 --version-script=%t/ver %t/ref.o %t/ref1.o %t/hid1.o %t/def2.o -o %t3
 # RUN: llvm-readelf -r --dyn-syms %t3 | FileCheck %s --check-prefix=CHECK3
 # RUN: llvm-objdump -d --no-show-raw-insn %t3 | FileCheck %s --check-prefix=DIS3
 
 # CHECK3:       00000000000034b8 {{.*}} R_X86_64_JUMP_SLOT {{.*}} foo@@v2 + 0
 # CHECK3-NEXT:  00000000000034c0 {{.*}} R_X86_64_JUMP_SLOT {{.*}} foo@v1 + 0

 
 #--- ref.s
 call foo
 
 #--- ref1.s
 .symver foo, foo@@@v1
 call foo
 
+#--- ref1w.s
+.weak foo
+.symver foo, foo@@@v1
+call foo
+
 #--- def1.s
 .globl foo
 .symver foo, foo@@@v1
 foo:
 
 #--- hid1.s
 .globl foo_v1
 .symver foo_v1, foo@v1