This is an archive of the discontinued LLVM Phabricator instance.

[lld][ELF][test] Add testing for IE/LD TLS weak undef references
ClosedPublic

Authored by jhenderson on Jan 21 2021, 3:17 AM.

Details

Summary

Whilst migrating/retiring some downstream testing, I came across a test for weak undef IE and LD TLS references, but was unable to find any equivalent in LLD's upstream testing. There does seem to be some slight subtle differences that could be worth testing compared to LE TLS references, in particular that IE can be relaxed to LE in this case, hence this change.

Diff Detail

Event Timeline

jhenderson created this revision.Jan 21 2021, 3:17 AM
jhenderson requested review of this revision.Jan 21 2021, 3:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 21 2021, 3:17 AM
grimar added inline comments.Jan 21 2021, 4:16 AM
lld/test/ELF/tls-weak-undef.s
14

Where %t.o comes from? Doesn't seem the test creates it currently?

Fix reference to stale object and missing check-prefix. Not sure how I didn't spot the latter...

lld/test/ELF/tls-weak-undef.s
14

tls-le-weak-undef.s used %t.o as its input file and the object was still kicking around in my test output directory.

missing check-prefix. Not sure how I didn't spot the latter...

The CHECK line is still missing.

Reupload diff. Previous diff somehow was missing one of the changes.

grimar accepted this revision.Jan 21 2021, 4:54 AM

LGTM with a nit, please wait for others.

lld/test/ELF/tls-weak-undef.s
14

I'd suggest to create an output in this case too. MIght be usefull for debugging.

This revision is now accepted and ready to land.Jan 21 2021, 4:54 AM
jhenderson marked 2 inline comments as done.

Change to use separate output file to aid debugging.

MaskRay accepted this revision.Jan 21 2021, 11:15 AM

LGTM.

lld/test/ELF/tls-weak-undef.s
12

A TLS undefined weak symbol

undefined is important.
TLS as a modifier is also useful.

24

Copy relocations are specific to executables. I think COPYRELOC may be misguiding.

I'll land this on Monday, as I'm coming to the end of my workday now.

lld/test/ELF/tls-weak-undef.s
12

I've gone with "An undefined weak TLS symbol" as I feel like it reads nicer.

24

COPYRELOC was the name in the pre-existing test my change is essentially updating. I'm not particularly familiar with this aspect of code. I'll go with ERROR since it's testing the error message, unless anybody else has got a better suggestion?

jhenderson marked 2 inline comments as done.

Address MaskRay's review comments.