This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Async signal safe TLS accesses
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Alexandre Oliva <aoliva at redhat dot com>, Andrew Hunter <ahh at google dot com>
- Cc: Paul Pluzhnikov <ppluzhnikov at google dot com>, "Joseph S. Myers" <joseph at codesourcery dot com>, GLIBC Devel <libc-alpha at sourceware dot org>
- Date: Fri, 06 Dec 2013 16:45:16 -0500
- Subject: Re: [PATCH] Async signal safe TLS accesses
- Authentication-results: sourceware.org; auth=none
- References: <1380830518-16721-1-git-send-email-ahh at google dot com> <1382477766-15770-1-git-send-email-ahh at google dot com> <Pine dot LNX dot 4 dot 64 dot 1310222145490 dot 13258 at digraph dot polyomino dot org dot uk> <CALoOobMsO6X86JFD4J7F-EL-J+xOTEOVbzH=6mwrvfCnFvw57Q at mail dot gmail dot com> <Pine dot LNX dot 4 dot 64 dot 1311052233090 dot 30260 at digraph dot polyomino dot org dot uk> <CALoOobM70-mix+=1zuDnSoK7SRqQChbL=03xBkUcFf1fyS1Mjw at mail dot gmail dot com> <CALoOobP7kdpZCZA0a7MZWCtONu81fW4H_qAWOEfpfvzxJgG_=Q at mail dot gmail dot com> <CALoOobP6rTDosadvLKhHY+deDsU-FtvyO8QX_Y4dZy716e2ATQ at mail dot gmail dot com> <CALoOobOCT-inwMZkzEr+JYT4c8qwpN-EGMPFu_kHQTpc2icj0g at mail dot gmail dot com> <CALoOobPHo7+jG0nfiZp9afC2rArLUMRYZEag21W+78UBTZF=CQ at mail dot gmail dot com> <orvbzckoif dot fsf at livre dot home> <CADroS=7gw+zXDbaPR=bnR7hSe2yxBYgi6Ao14pwa03=Ra-t=7A at mail dot gmail dot com> <orvbz1n8vq dot fsf at livre dot home>
On 12/06/2013 01:15 PM, Alexandre Oliva wrote:
> On Dec 5, 2013, Andrew Hunter <ahh@google.com> wrote:
>
>> On Thu, Nov 28, 2013 at 5:03 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
>
>>> The lock is recursive, but I don't see anything in the patch besides
>>> the CAS loops that avoids inconsistent state, particularly in the
>>> case of lazy TLS relocation (introduced with GNU2 TLS). Was that not
>>> taken into account?
>
>> glibc recursive locks aren't async-signal-safe. We avoid problems
>> with reentrancy via dl_mask_all_signals.
>
> But that doesn't avoid *concurrency* problems, does it?
>
> Besides, GNU2 TLS provides for lazy TLS relocation (as mentioned above),
> and that most certainly takes the DL lock. I don't see anything in the
> patch that would avoid signal-safety problems in that path.
>
> GNU2 will probably be made default on x86 and ARM eventually, and it
> already is the default on a number of other recent architectures;
> AArch64 is one of them IIRC. So, if this issue was not addressed by
> this patch, I would recommend being careful with the messaging: it may
> fail to make TLS AS-safe in general. Some qualifier would be necessary
> to make the statement correct.
Alex is correct here.
I had not considered TLS descriptors in my initial review.
Echoing Alex's comment though in that this is not yet the default,
you have to explicitly request `-mtls-dialect=gnu2', but the benefits
of the descriptor are make it worth it.
I just looked at the TLSDESC code, which applies to x86, x86_64, arm,
and aarch64, and it looks like you will take dl_load_lock and that
could cause a deadlock in the signal handler if the TLS variable also
uses TLSDESC.
e.g.
DSO function
-> Signal handler
-> var@plt
-> _dl_tlsdesc_resolve_rela_fixup (sysdeps/x86_64/tlsdesc.c)
-> _dl_tlsdesc_resolve_early_return_p (elf/tlsdeschtab.h)
-> __rtld_lock_lock_recursive (GL(dl_load_lock));
I think it's entirely possible to rewrite this to be AS-safe.
I'm not requiring it, but I agree again with Alex that it must be
explicitly stated that GNU2 dialect TLS is not yet signal-safe with
your patch.
I assume __rtld_lock_lock_recursive is not AS-safe because there
a space of time between taking the lock and recording the ownership
where if the signal arrived would cause the code to attempt to
retake the lock and deadlock.
Cheers,
Carlos.