This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Initial support for wildcard in symbol versions
ClosedPublic

Authored by davide on Jun 25 2016, 11:14 PM.

Details

Summary
VERSION_1.0 {
  global: foo*; 
  local: *; }

currently doesn't add any symbol to the dynamic symbol table because the code has no logic for wildcard(s).
This patch is an attempt to fix, only for the "easy" case when the wildcard matching is at the end of the word.

The patch is motivated by an example I found in the wild. In fact, libreoffice has a version script which looks like this:

PRIVATE_textenc.1 { # LibreOffice 3.6
    global:
        _ZN3sal6detail7textenc20convertCharToUnicode*;
        _ZN3sal6detail7textenc20convertUnicodeToChar*;
        _ZN3sal6detail7textenc32handleUndefinedUnicodeToTextChar*;
        _ZN3sal6detail7textenc37handleBadInputTextToUnicodeConversion*;
        _ZN3sal6detail7textenc37handleBadInputUnicodeToTextConversion*;
};

This patch implements the matching in the most naive way possible, i.e. through a linear scan of the symbol table for each requested matching. It could be probably improved switching symtab to a data structure that preserves ordering of elements, but that will slow down the common case, so I'm not entirely sure it's worth it.

Comments welcome.

Diff Detail

Repository
rL LLVM

Event Timeline

davide updated this revision to Diff 61901.Jun 25 2016, 11:14 PM
davide retitled this revision from to [ELF] Initial support for wildcard in symbol versions.
davide updated this object.
davide added reviewers: grimar, rafael, ruiu.
davide added a subscriber: llvm-commits.

FWIW, I think we should bit the bullet and implement also generalized wildcard matching.
When I was taking a shot at the libreoffice version scripts I found:

_ZThn*_N4cppu30WeakAggComponentImplHelperBase7acquireEv;
_ZThn*_N4cppu30WeakAggComponentImplHelperBase7disposeEv;
_ZThn*_N4cppu30WeakAggComponentImplHelperBase7releaseEv;
_ZThn*_N4cppu16OComponentHelper14queryInterfaceERKN3com3sun4star3uno4TypeE;
_ZThn*_N4cppu16OComponentHelper16addEventListenerERKN3com3sun4star3uno9ReferenceINS3_4lang14XEventListenerEEE;
_ZThn*_N4cppu16OComponentHelper19removeEventListenerERKN3com3sun4star3uno9ReferenceINS3_4lang14XEventListenerEEE;
_ZThn*_N4cppu16OComponentHelper7acquireEv;
_ZThn*_N4cppu16OComponentHelper7disposeEv;
_ZThn*_N4cppu16OComponentHelper7releaseEv;
_ZThn*_N4cppu18OPropertySetHelper14queryInterfaceERKN3com3sun4star3uno4TypeE;
_ZThn*_N4cppu18OPropertySetHelper20getFastPropertyValueE?;
_ZThn*_N4cppu18OPropertySetHelper20setFastPropertyValueE?RKN3com3sun4star3uno3AnyE;
_ZThn*_N4cppu18OPropertySetHelper16getPropertyValueERKN3rtl8OUStringE;

There are also some of them which use ? matching, e.g.:

# Introducing a question mark at the end because of
# marginal type discrepancy there is a difference in the
# mangled name between Linux and Mac OS X, see #i69351#
_ZN9salhelper21SimpleReferenceObjectnwE?; # salhelper::SimpleReferenceObject::operator new (std::size_t)
ruiu edited edge metadata.Jun 26 2016, 2:22 AM

So a natural question to ask is how slow it is if you have symbol patterns that contain "*"? I'd appreciate if you run a benchmark.

Also, how slow is it if you use matchStr function defined in LinkerScript.cpp?

ELF/SymbolTable.cpp
435 ↗(On Diff #61901)

This parameter is not actually a prefix but a pattern that could end with "*". So I'd name this Pattern.

438–440 ↗(On Diff #61901)
if (SymbolBody *B = find(Prefix))
  Result.push_back(B);
grimar edited edge metadata.Jun 27 2016, 12:43 AM

Not sure how much crazy that idea is, but I was thinking about splitting the list of globals somehow.
One list can contain symbols without wildcards and can be processed in a tight loop, and the second
list with wildcards that probably should be processed using matchStr to support more complex cases.

davide updated this revision to Diff 62146.Jun 28 2016, 4:02 PM
davide edited edge metadata.

Rebase

ruiu added inline comments.Jun 28 2016, 6:47 PM
ELF/LinkerScript.h
101 ↗(On Diff #62146)

This should be defined in lld::elf namespace.

ELF/SymbolTable.cpp
468–469 ↗(On Diff #62146)

I think you can do

if (!HasWildcards)
  return {find(Pattern)};
ELF/SymbolTable.h
88 ↗(On Diff #62146)

s/Prefix/Pattern/

davide updated this revision to Diff 62166.Jun 28 2016, 7:16 PM

Rui's comments.

ruiu added a comment.Jun 28 2016, 7:28 PM

Stylistic comments.

ELF/LinkerScript.cpp
292 ↗(On Diff #62166)

I think you can remove lld::.

ELF/LinkerScript.h
99 ↗(On Diff #62166)

Please move this line at beginning of this elf {} namespace.

ELF/SymbolTable.cpp
462 ↗(On Diff #62166)

Naturally you want to move this line after the if.

davide updated this revision to Diff 62170.Jun 28 2016, 7:42 PM

Another round of comments.

ruiu accepted this revision.Jun 28 2016, 7:46 PM
ruiu edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jun 28 2016, 7:46 PM
ruiu added inline comments.Jun 28 2016, 7:48 PM
ELF/SymbolTable.h
88 ↗(On Diff #62170)

One more thing: please make this a private function.

This revision was automatically updated to reflect the committed changes.