#46 Correct TLS alignment.

Unstable (example)
closed-fixed
nobody
CRT (12)
5
2014-03-12
2012-01-16
No

Moved from Bugs. https://sourceforge.net/tracker/?func=detail&aid=3474498&group_id=202880&atid=983354.

_tls_used offsets the start of the tls section by adding 1 to _tls_start.

I feel this is incorrect in two respects. The first is that _tls_start is defined as a char so adding 1 only increments by 1 byte.

Second secrel32 generates an offset from the start of the section. By offsetting from _tls_start the secrel32 offset becomes invalid and has the potential to allow access to invalid memory.

I also see no reason _tls_used should be placed in the TLS section. Other compilers treat is as read only and place it in .rdata.

See the included patch for my suggested changes.

Discussion

  • Daniel Green

    Daniel Green - 2012-01-16

    Correct TLS alignment.

     
  • Kai Tietz

    Kai Tietz - 2012-01-17

    Hmm, I am aware that __tls_used should be in .rodata section. I will see what side-effects we get, if we move out of tls-section. The alignment of __tls_start had a reason, I will look into this tomorrow.
    I forgot about why, so it is good for taking a closer look into this, and if necessary adding some comments for it.

     
  • Daniel Green

    Daniel Green - 2012-01-17

    Windows, Win7 at least, will initialize the TLS section by copying the contents from the executable. This created problems when using secrel32 relocation to generate symbol offsets because of the +1.

    I referenced http://www.nynaeve.net/?p=183 when working on TLS.

     
  • Kai Tietz

    Kai Tietz - 2012-01-18

    So I looked into this. To move __tls_used into rodata is also proper in terms of pe-coff-specification, but has a side-effect in binutils AFAICS. The issue is that binutils sets the PE header-field TLS_DIRECTORY to the start of TLS-section. So we can't use this patch, as information in produced executables would be wrong about this directory-entry.
    The alignment issue mentioned here is true, but should be solved in different way. You patch would lead to the issue that start and end will have always a difference of 1, which might lead to issues here. Additionally we would have here an stray 0 at beginning of table, which is wrong too.

    So I would suggest to change instead simply the types of _tls_start and _tls_end to uintptr_t. This should fix the alignment issue, too

     
  • Daniel Green

    Daniel Green - 2012-01-18

    Removing the stray 0 could break the use of secrel32 to generate TLS offsets. Looking at the pe-coff specification secrel32 appears to be the recommended(only?) way of doing so. To initialize TLS data Windows will copy the TLS section as defined by _tls_used. Since secrel32 will generate an offset from the start of the section and _tls_start is at the start of the section. When windows is told to load _tls_used+1, the secrel32 offset becomes invalid. I'll verify this occurs when the offset is 4 bytes and not 1. I have verified that if the offset is removed secrel32 will produce correct data.

    __declspec (thread) int tlsFlag[2] = {4, 8}; // secrel32 of tlsFlag will point to index 1(value = 8) instead of index 0(value = 4)

    Is it possible that TLS_DIRECTORY and _tls_used are less related. Section 5.7 of the pe-coff specification Revision 8.2 provided by Microsoft states that the linker will use the data in __tls_used to create the TLS Directory.

    That seems to suggest that the linker is merely using __tls_used to help create the TLS directory. Do you have any idea what kind side-effects would be generated. It may be luck but I've tested TLS access with both 32-bit and 64-bit executables and have not noticed any side effects.

    The D language makes heavy use of TLS and the testsuite has ran successfully with the GDC version of the compiler utilizing a patched mingw-w64 runtime.

     
  • Daniel Green

    Daniel Green - 2012-01-18

    Offsetting __tls_start does indeed break secrel32 offsets.

    I would like to cite http://www.nynaeve.net/?p=183 .

    "Thread Local Storage, part 3". You will find a definition for _tls_used that doesn't offset _tls_start.

    "Thread Local Storage, part 4". You will find generated assembly to access a TLS variable as well as a dump of what the linker does. The stray 0 is there and the offset of their example variable is 4. The offset that would be generated by secrel32 is 4.

    These reasons leads me to believe that the stray 0 at the start is correct and essential.

    I am happy to provide any assistance needed in assessing how the changes made here will affect TLS. GCC doesn't at this time support native TLS. So it will need to be done with assembler or with the D compiler I referenced earlier.

     
  • Kai Tietz

    Kai Tietz - 2014-03-12
    • status: open --> closed-fixed
    • Group: --> Unstable (example)
     
  • Kai Tietz

    Kai Tietz - 2014-03-12

    Issue got already fixed. If problem still exists please don't hesitate to reopen this issue.