Page MenuHomeFreeBSD

Fix initial exec TLS mode for dynamically loaded shared objects.
Needs ReviewPublic

Authored by kib on Sun, Feb 3, 9:11 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
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
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 22500

Event Timeline

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

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

kib added a comment.Sun, Feb 3, 10:25 PM

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'.

emaste added a comment.Mon, Feb 4, 9:56 PM
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.

kib updated this revision to Diff 53726.Sat, Feb 9, 11:42 AM

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

kib updated this revision to Diff 53919.Thu, Feb 14, 2:17 PM

Provide implementations for all arches.

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

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

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

dim added a comment.Tue, Feb 19, 9:30 PM

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.