[PATCH] D92260: [ELF] Error for undefined foo at v1

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

Sounds reasonable.



================
Comment at: lld/ELF/Relocations.cpp:952
+  // 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});
----------------
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.


================
Comment at: lld/test/ELF/symver.s:9
 # 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
----------------
Something is wrong with rebasing it seems? D92258 has much more llvm-mc lines here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92260