Page MenuHomePhabricator

[ELF][test] Add some tests for versioned symbols in object files
ClosedPublic

Authored by MaskRay on Nov 27 2020, 9:30 PM.

Details

Summary

Test the symbol resolution related to defined foo@@v1 and foo@v1 in object files/shared objects, undefined foo@v1, and the interaction with --wrap.

Diff Detail

Event Timeline

MaskRay created this revision.Nov 27 2020, 9:30 PM
MaskRay requested review of this revision.Nov 27 2020, 9:30 PM
MaskRay updated this revision to Diff 308238.Nov 29 2020, 3:45 PM

Add one more duplicate definition test

jhenderson added inline comments.Nov 30 2020, 12:53 AM
lld/test/ELF/symver.s
2

Perhaps worth a top-level comment saying what this test is intended to cover?

16
23
28

(I think this is more traditional punctuation for this style of message)

90–91

Do you need a similar test-case for --wrap=foo@v1 (and/or --wrap=foo@@v1)? There's an interesting interaction between the two things with your follow-up patch, which probably needs exploring in tests.

94–97

Not that it really matters, but in all the other cases, the prefixes are CHECK*. Should this be CHECK6 for consistency?

MaskRay updated this revision to Diff 308393.Nov 30 2020, 9:15 AM
MaskRay marked 5 inline comments as done.

Add more tests

MaskRay edited the summary of this revision. (Show Details)Nov 30 2020, 9:23 AM
MaskRay updated this revision to Diff 308401.Nov 30 2020, 9:28 AM

Improve --wrap comments

As noted elsewhere, my symbol versioning knowledge is lacking. I'm happy with what I've seen, but I could easily have missed things that don't make sense, so best to get someone else to sign off.

lld/test/ELF/symver.s
96

Nit: it looks weird that -shared has only one dash, when all other options have two. Probably should standardise on one or the other?

Same applies elsewhere.

MaskRay added inline comments.Dec 1 2020, 12:49 AM
lld/test/ELF/symver.s
96

The idea is that -shared is the most common form (the compiler driver option is -shared) so I use it. For -pie, I prefer the form as well. For many other long options, I use two dashes.

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

LGTM

This revision is now accepted and ready to land.Dec 1 2020, 1:12 AM