Page MenuHomePhabricator

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

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



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

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


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


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.


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.


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

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


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