[PATCH] [BZ 18034] [AArch64] Lazy TLSDESC relocation data race fix

[Szabolcs Nagy]

On 22/04/15 18:14, Szabolcs Nagy wrote:
> On 22/04/15 17:08, Torvald Riegel wrote:
>> Given that you have looked at the code, could you give a rough estimate
>> of how much churn it would be to make the TLS code data-race-free?
> 
> elf/tlsdeschtab.h:
> _dl_tlsdesc_resolve_early_return_p
> sysdep/{x86_64,i386,arm,aarch64}/tlsdesc.c:
> _dl_tlsdesc_resolve_rela_fixup
> _dl_tlsdesc_resolve_hold_fixup
> 

i think these all need to use atomics for accessing
td->entry for c11 correctness, but it's hard to tell
what's right since c11 only talks about synchronizing
with c code, not asm.

the td->entry can be in 3 states during lazy resolution:

* init: retries the call of entry until caller==entry
  (using a double-checked locking mechanism, then it
  - grabs GL(dl_load_lock)
  - changes the entry to the hold state
  - does the resolver work
  - changes arg and entry to the final value
  - releases the lock to wake the waiters
  - calls the new entry)
* hold: waits on the GL(dl_load_lock)
  (then retries calling the entry when it's woken up)
* final state: (static, dynamic or undefweak callback)
  calculates the tls offset based on arg
  (currently without any locks which is bad on weak
  memory models)

the code for tls access is generated by the compiler so
there is no atomics there: the loaded entry can be in
init, hold or final state, the guarantee about the state
of arg must come from the target arch memory model.

and to answer my earlier question about the hold state:
it is needed to avoid the spinning in the double-checked
locking in init.

>> It also looks as if the x86_64 variant of tlsdesc.c is, before your
>> changes and ignoring some additional comments, very similar to the
>> aarch64 variant.  Can we get one proper tlsdesc.c (data-race-free and
>> using the C11 model) that can be used on several archs?  This would
>> certainly decrease maintenance overhead.
>>
> 
> should be possible i think: these functions are called
> from asm to do the lazy resolution
> 
> but i have to check the arch specific details.

looking at this, but it seems to be significant work:

* i386 and arm determine the caller differently
* i386 uses special calling convention from asm
* arm handles local symbols specially

i think the way to move forward is

* fix the correctness bug now on aarch64
* decide if lazy static tls resolver is worth it
* do the refactoring so tlsdesc is common code
* use c11 atomics

the fix for arm is a lot harder (because atomics are
different on different versions of arm, an ifunc based
dispatch could in principle solve the dmb vs no-dmb
issue for the lazy resolvers).

is this ok?