This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Combine foo@v1 and foo with the same versionId if both are defined
ClosedPublic

Authored by MaskRay on Jul 31 2021, 10:48 PM.

Details

Summary

Due to an assembler design flaw (IMO), .symver foo,foo@v1 produces two symbols`foo` and foo@v1 if foo is defined.

  • v1 {}; produces both foo and foo@v1 but GNU ld only produces foo@v1
  • v1 { foo; }; produces both foo@@v1 and foo@v1 but GNU ld only produces foo@v1
  • v2 { foo; }; produces both foo@@v2 and foo@v1, matching GNU ld. (Tested by symver.s)

This patch implements the GNU ld behavior by reusing the symbol redirection mechanism
in D92259. The new test symver-non-default.s checks the first two cases.

Without the patch, the second case will produce foo@v1 and foo@@v1 which
looks weird and makes foo unnecessarily default versioned.

Note: .symver foo,foo@v1,remove exists but the unfortunate foo will not go
away anytime soon.

Diff Detail

Event Timeline

MaskRay created this revision.Jul 31 2021, 10:48 PM
MaskRay requested review of this revision.Jul 31 2021, 10:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2021, 10:48 PM
MaskRay retitled this revision from [ELF] Combine foo@v1 and foo with the same versionId if both are defined Due to an assembler design flaw (IMO), `.symver foo,foo@v1` produces two symbols `foo` and `foo@v1` if `foo` is defined. * `v1 {};` keeps `foo` as is but GNU ld will... to [ELF] Combine foo@v1 and foo with the same versionId if both are defined.Jul 31 2021, 10:49 PM
MaskRay edited the summary of this revision. (Show Details)
MaskRay edited the summary of this revision. (Show Details)Jul 31 2021, 10:52 PM
MaskRay edited the summary of this revision. (Show Details)
peter.smith accepted this revision.Aug 3 2021, 6:03 AM

LGTM. One quick suggestion to improve the comment and one typo in the comment.

lld/ELF/Driver.cpp
2073

I think this comment is now incomplete. Perhaps worth changing to something like:

For version symbol foo@v1 check the existing symbol foo.
We have two special cases to handle:

  • There is a definition of foo@v1 and foo@@v1
  • There is a definition of foo@v1 and foo
2094

typo elimiates -> eliminates

This revision is now accepted and ready to land.Aug 3 2021, 6:03 AM
MaskRay updated this revision to Diff 363766.Aug 3 2021, 9:09 AM
MaskRay marked 2 inline comments as done.

Thanks for the review!

Address comments...

peter.smith accepted this revision.Aug 4 2021, 1:26 AM

Thanks for the update. The elimiates typo on line 2097 looks like it is still there, but that is easily fixed up.