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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
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});
----------------
grimar wrote:
> 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.
Added a helper to `[ELF] Make foo@@v1 resolve undefined foo at v1`


================
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
----------------
grimar wrote:
> Something is wrong with rebasing it seems? D92258 has much more llvm-mc lines here.
I had added more tests in D92258 but did not rebase this patch...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92260