SerenityOS / serenity

The Serenity Operating System 🐞
https://serenityos.org
BSD 2-Clause "Simplified" License
30.47k stars 3.18k forks source link

DynamicLoader: TLS doesn't work properly in some use-cases #6609

Open itamar8910 opened 3 years ago

itamar8910 commented 3 years ago

First use case is defining TLS variables as static.

This code works:

// lib.cpp
__thread uint32_t tls_var;
int func()
{
    return ++tls_var;
}

A R_386_TLS_TPOFF relocation is generated, as expected (relevant code in the loader).

# readelf -r lib.so
...
00001ff8  0000090e R_386_TLS_TPOFF   00000000   tls_var
...

However, if we also define tls_var as static, then there is a R_386_TLS_TPOFF without a value for the relocation's symbol:

...
00001fe8  0000000e R_386_TLS_TPOFF  
...

In this case we won't perform the relocation and access to this TLS variable will be incorrect.

@gunnarbeutner suggested switching from -ftls-model=initial-exec to -ftls-mode=global-dynamic to fix this.

Second use case is defining TLS variables in the main program (i.e not in a shared lib). Example:

//main.cpp
__thread int a;
int main()
{
    return a;
}

When we do this, the TLS offset when accessing a is hardcoded to -4:

00000430 <main>:
 430:   e8 21 02 00 00          call   656 <__x86.get_pc_thunk.ax>
 435:   05 97 1b 00 00          add    eax,0x1b97
 43a:   65 8b 15 00 00 00 00    mov    edx,DWORD PTR gs:0x0 # get end of TLS block
 441:   c7 c0 fc ff ff ff       mov    eax,0xfffffffc # offset = -4
 447:   8b 04 02                mov    eax,DWORD PTR [edx+eax*1] # load value of `a`
 44a:   c3                      ret    
 44b:   66 90                   xchg   ax,ax
 44d:   66 90                   xchg   ax,ax
 44f:   90                      nop

Our current implementation assumes that every access to TLS data first gets its offset from the GOT, which we set up when handling TLS relocations. It seems that the compiler assumes that we put the main program's TLS data at the end of the TLS block, so a possible fix would be to actually do that (Currently, we put the main program's TLS at the beginning of the TLS block).

also cc @ADKaster

gunnarbeutner commented 3 years ago

https://github.com/gunnarbeutner/serenity/tree/shlibs-tls has some initial work towards implementing -ftls-mode=global-dynamic for shared libraries.

Also, yeah, we should move the main program's TLS data towards the end of the TLS block.