This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Apply version script patterns to non-default version symbols

Authored by MaskRay on Jul 31 2021, 6:50 PM.



Currently version script patterns are ignored for .symver produced
non-default version (single @) symbols. This makes such symbols
not localizable by local:, e.g.

.symver foo3_v1,foo3@v1
.globl foo_v1

ld.lld --version-script=a.ver -shared a.o
# In a.out, foo3@v1 is incorrectly exported.

This patch adds the support:

  • Move config->versionDefinitions[VER_NDX_LOCAL].patterns to config->versionDefinitions[versionId].localPatterns
  • Rename config->versionDefinitions[versionId].patterns to config->versionDefinitions[versionId].nonLocalPatterns
  • Allow findAllByVersion to find non-default version symbols when includeNonDefault is true. (Note: symtab keys do not have @@)
  • Make each pattern check both the unversioned and the versioned ${}@${}
  • localPatterns can localize ${}@${}. nonLocalPatterns can prevent localization by assigning verdefIndex (before parseSymbolVersion).

If a user notices new undefined symbol errors with a version script containing
local: *;, the issue is likely due to a missing global: pattern.

Diff Detail

Event Timeline

MaskRay created this revision.Jul 31 2021, 6:50 PM
MaskRay requested review of this revision.Jul 31 2021, 6:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2021, 6:50 PM

I struggled a bit to understand the implementation, probably not helped by the complexity of symbol version scripts. I can agree with what you are trying to achieve though. Do we need any more tests? For example the error message.


Is it worth renaming this to nonLocalPatterns (globalPatterns might work, but may not be strictly accurate).


Suggest adding
// return false if no symbol definition matches ver.


I'm struggling a bit to understand this part. IIUC if we have a script like:

v1 { global: foo; }

then we'll call

assignExactVersion({"foo@v1", /*pat.isExternCpp*/ false, /*hasWildcard*/ false}, v1 id, "v1");

However in assignExactVersion we have:

// Get a list of symbols which we need to assign the version to.
  std::vector<Symbol *> syms = findByVersion(ver);

Which I'd expect to return symbols matching "foo@v1";
but later on in assignExactVersion there is:

if (versionId != VER_NDX_LOCAL && sym->getName().contains('@'))

which I'd assume foo@v1 would always end up in continue.

If I'm right, the only affect of calling assignExactVersion here is to update found


Would be useful to have /* hasWildcard */ false)


The body of the loop is almost identical, may be worth extracting into a lambda or a function? Just a mild suggestion, difficult to know if it will be an improvement or not.


Would be useful to have /* hasWildcard */ false)


/* checkAt */ false


/* hasWildcard */ true


/* checkAt */ true


Instead of "checkAt" I suggest "includeNonDefault" or "includeNonDefaultVer".


Same as above, I suggest "includeNonDefault"

MaskRay updated this revision to Diff 363784.Aug 3 2021, 9:49 AM
MaskRay marked 11 inline comments as done.

Address comments.

Added a diagnostic test to version-script-noundef.s
The coverage should be quite good now.
version-script-symver.s & version-script-symver2.s check various complex cases.

verneed-* and other version-script-* cover misc ad-hoc cases.
(I was struggling with reorganizing them.)


Updating found is one use.

The other use is to allow a global: to override local: *. A symbol's version is unassigned when verdexIndex == UINT32_C(-1).
If a global: pattern assigns verdexIndex, a subsequent local: pattern won't be able to localize the symbol.

These tests would fail if I remove the assignExactVersion({"foo@v1" function call:

Failed Tests (5):
  lld :: ELF/partition-synthetic-sections.s
  lld :: ELF/verdef-defaultver.s
  lld :: ELF/verneed.s
  lld :: ELF/version-script-symver2.s
  lld :: ELF/version-symbol-undef.s

I picked /*checkAt=*/true.

clang-format will delete space before true if it recognizes this form.

clang-tidy -checks='-*,bugprone-argument-comment' recognizes the = form and can suggest argument renames.

MaskRay edited the summary of this revision. (Show Details)Aug 3 2021, 9:54 AM
MaskRay updated this revision to Diff 363802.Aug 3 2021, 10:18 AM

Add extern "C++" tests

peter.smith accepted this revision.Aug 4 2021, 2:44 AM

Thanks for the update LGTM.

This revision is now accepted and ready to land.Aug 4 2021, 2:44 AM