This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Error for undefined foo@v1
ClosedPublic

Authored by MaskRay on Nov 27 2020, 10:29 PM.

Details

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

Diff Detail

Event Timeline

MaskRay created this revision.Nov 27 2020, 10:29 PM
MaskRay requested review of this revision.Nov 27 2020, 10:29 PM
MaskRay edited the summary of this revision. (Show Details)Nov 27 2020, 10:37 PM

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 added inline comments.Nov 30 2020, 12:55 AM
lld/test/ELF/lto/version-script2.ll
13
MaskRay updated this revision to Diff 308431.Nov 30 2020, 11:23 AM
MaskRay marked 3 inline comments as done.

Comments. Rebase

MaskRay added inline comments.Nov 30 2020, 11:27 AM
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...

No more comments from me, thanks.

grimar accepted this revision.Dec 1 2020, 1:15 AM

LGTM too.

This revision is now accepted and ready to land.Dec 1 2020, 1:15 AM
MaskRay updated this revision to Diff 308679.Dec 1 2020, 8:59 AM

Fix an assertion failure

This revision was landed with ongoing or failed builds.Dec 1 2020, 9:00 AM
This revision was automatically updated to reflect the committed changes.