Differential D19072

Fix initial exec TLS mode for dynamically loaded shared objects.
ClosedPublic
Actions

Authored by kib on Feb 3 2019, 9:11 PM.

Details

Reviewers: None
Commits: rS345703: Fix initial exec TLS mode for dynamically loaded shared objects.

Summary

If the DF_STATIC_TLS dynamic flag is set, rtld tries to allocate TLS in static space. If there is no space left, the dlopen(3) fails. If space if allocated, initial content from PT_TLS segment is distributed to all threads' pcbs.

See https://www.redhat.com/archives/phil-list/2003-February/msg00077.html for plain language explanation.
I am unsure about https://reviews.llvm.org/D33041, I did observed that lld 6.0.1 does not set DF_STATIC_TLS. IMO this is a bug.

The test case is https://github.com/dumbbell/test-tls-initial-exec. I only compiled and somewhat tested patch on amd64, and patch have untested bits for i386.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 22411

Event Timeline

kib created this revision.Feb 3 2019, 9:11 PM

Herald added subscribers: bdragon, imp. · View Herald TranscriptFeb 3 2019, 9:11 PM

Harbormaster completed remote builds in B22333: Diff 53566.Feb 3 2019, 9:11 PM

Should we encourage lld to revive the https://reviews.llvm.org/D33041 patch?

In D19072#407798, @emaste wrote:

Should we encourage lld to revive the https://reviews.llvm.org/D33041 patch?

Lets discuss it. They did not stated why DF_STATIC_TLS was considered as 'not needed'.

In D19072#407799, @kib wrote:

Lets discuss it. They did not stated why DF_STATIC_TLS was considered as 'not needed'.

See the ongoing discussion in the LLVM thread - glibc and musl do not use the flag

glibc used to use it, but doesn't since 2002
(2430d57a13f4f10312e13c58962cd9104e6428fd).

As far as I can tell glibc originally used the flag only to signal that the .so should not be opened by dlopen, and the referenced commit changed it to checking explicitly for TLS use. Later changes for dlopen w/ static TLS did not check the flag.

Fix quite a bug in the loading of depended objects.
Restructure to avoid coding current thread offset calculation twice.

Harbormaster completed remote builds in B22411: Diff 53726.Feb 9 2019, 11:42 AM

Provide implementations for all arches.

Herald added a subscriber: andrew. · View Herald TranscriptFeb 14 2019, 2:17 PM

Harbormaster completed remote builds in B22500: Diff 53919.Feb 14 2019, 2:17 PM

llvm commit in https://reviews.llvm.org/D57821, @dim reasonable to backport in your opinion?

In D19072#411770, @emaste wrote:

llvm commit in https://reviews.llvm.org/D57821, @dim reasonable to backport in your opinion?

and https://reviews.llvm.org/D57749

In D19072#411771, @emaste wrote:

In D19072#411770, @emaste wrote:

llvm commit in https://reviews.llvm.org/D57821, @dim reasonable to backport in your opinion?

and https://reviews.llvm.org/D57749

Yes, those should be fine; can be imported independently of this revision.

aryeeteygerald_rogers.com added a subscriber: aryeeteygerald_rogers.com.Mar 28 2019, 1:08 AM

• ian added a subscriber: • ian.Mar 29 2019, 3:13 PM

This comment was removed by • ian.

I haven't reviewed this change, but I did run-test it (on armv7 only). I updated to r345693 then applied the patch from this review, and ran the test code from https://github.com/emaste/test-tls-initial-exec. The test failed before patching and worked afterwards.

This revision was not accepted when it landed; it landed in state Needs Review.Mar 29 2019, 5:53 PM

Closed by commit rS345703: Fix initial exec TLS mode for dynamically loaded shared objects. (authored by kib). · Explain Why

This revision was automatically updated to reflect the committed changes.