Re: [PR19826] fix non-LE TLS in static programs

[Alexandre Oliva <aoliva at redhat dot com>]

On Dec  2, 2016, Florian Weimer <fweimer@redhat.com> wrote:

> On 09/29/2016 09:04 PM, Carlos O'Donell wrote:
>> On 09/20/2016 10:21 PM, Alexandre Oliva wrote:
>>> Ok to install?
>>> 
>>> for  ChangeLog
>>> 
>>> [BZ #19826]
>>> * elf/dl-tls.c (_dl_allocate_tls_init): Restore DTV early
>>> initialization of static TLS entries.
>>> * elf/dl-reloc.c (_dl_nothread_init_static_tls): Likewise.
>>> * nptl/allocatestack.c (init_one_static_tls): Likewise.
>> 
>> Alex is going to hand this patch off to me to champion and finish
>> iterating on with the rest of the interested developers (Andreas, Torvald,
>> Florian).
>> 
>> I'll work up a version 2 right now and post it shortly.

> What's the status here?  It seems that Alexandre committed it on
> September 22nd, despite Torvald's objections.

No, I committed the *first* of two patches, that I'd posted on 2016-09-20.
https://sourceware.org/ml/libc-alpha/2016-09/msg00365.html

That patch was approved by Andreas on 2016-09-21.
https://sourceware.org/ml/libc-alpha/2016-09/msg00378.html

Not having got any objections, I pushed it on 2016-09-22.
https://sourceware.org/bugzilla/show_bug.cgi?id=19826#c14

On 2016-09-24, *I* was the first and only person to object to this first
patch, when I realized it reintroduced the bug that an earlier (zeroth)
patch of mine had fixed back in March, 2015.  Along with the objection,
I posted a second patch in this thread, containing a comment that
explained why that part of the first patch I'd installed was incorrect,
in more detail than what I'd incorrently taken out in the first patch,
so that we wouldn't make the same mistake again.
https://sourceware.org/ml/libc-alpha/2016-09/msg00512.html

Here's the zeroth patch.
https://sourceware.org/ml/libc-alpha/2015-03/msg00563.html

Also on 2016-09-24, Torvald objected to the *second* patch, because he
wasn't happy with the wording of the comments, and that I wasn't adding
documentation for all concurrency issues in the TLS implementation along
with it, and later than I wasn't revamping it into using atomics, and
instead I just wanted to revert this small change that I had mistakenly
made but later realized why I shouldn't have.
https://sourceware.org/ml/libc-alpha/2016-09/msg00513.html

When Carlos announced he'd pick that up, after I asked him for help
because the thread was turning into yet another infinite distraction, he
responded to the first patch, rather than the second.  I suppose that's
the source of your confusion.  Maybe he was already confused about which
patch was pending back then, or he just went for the top of the thread
or something.
https://sourceware.org/ml/libc-alpha/2016-09/msg00593.html

> This patch broke non-optimized global-dynamic TLS on aarch64 (for
> shared builds), as shown by my new tst-tls-manydynamic test case.

Would you please try the second patch and confirm that it fixes the
problem?  What you're hitting is most likely the DTV resizing race
condition described in the comments there, at the '(**)' note.

It's quite unfortunate that this known race has been left unfixed for so
long because of objections to the wording of comments.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer