Re: CVS commit: src/lib/libc/tls

[Takeshi Nakayama <nakayama%NetBSD.org@localhost>]

>>> Joerg Sonnenberger <joerg%bec.de@localhost> wrote

> 
> On Fri, Nov 22, 2019 at 08:53:25AM +0900, Takeshi Nakayama wrote:
> > >>> Takeshi Nakayama <nakayama%NetBSD.org@localhost> wrote
> > 
> > > >>> Joerg Sonnenberger <joerg%bec.de@localhost> wrote
> > > 
> > > > On Thu, Nov 21, 2019 at 11:06:16PM +0000, Takeshi Nakayama wrote:
> > > > > Module Name:	src
> > > > > Committed By:	nakayama
> > > > > Date:		Thu Nov 21 23:06:16 UTC 2019
> > > > > 
> > > > > Modified Files:
> > > > > 	src/lib/libc/tls: tls.c
> > > > > 
> > > > > Log Message:
> > > > > Fix PR/54074 and PR/54093 completely.
> > > > > 
> > > > > More similar to the ld.elf_so logic, it is necessary to align with
> > > > > p_align first.  Also, invert the #ifdef condition for consistency.
> > > > 
> > > > This commit just wastes space without reason.
> > > 
> > > No, without this commit the TLS offset calculation is mismatch if
> > > alignof(max_align_t)) != p_align.
> > 
> > Ah, this is wrong.
> > Correctly, the TLS offset calculation is mismatch with what ld(1)
> > expected if p_memsz is not aligned with p_align.
> 
> For TLS variant I, it literally just adds padding at the end of the
> allocation. For TLS variant II, it is redundant, because the rounding is
> already done in with max_align_t. We do not support p_align > malloc
> alignment and the patch is certainly nowhere near enough to change that.
> It is actively harmful in that it can make people believe that it could
> ever work.

I don't want to do anything about the case of p_align > malloc
alignment.  I just want to fix an usual 32-bit sparc statically
linked binaries not working properly.

The new jemalloc now uses TLS, and the 32-bit sparc statically
linked binary ELF header looks like this:

% readelf -l rescue/sh | egrep 'MemSiz|TLS'     
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  TLS            0x6dc000 0x0075c000 0x0075c000 0x00b10 0x00b14 R   0x8

The point is when MemSiz (p_memsz) is not aligned with Align
(p_align) as above.

In tls.c rev 1.13:
  tls_size = p_memsz --> 0x00b14
  tls_allocation = roundup2(tls_size, alignof(max_align_t)) --> 0x00b18
  tcb = calloc() + tls_allocation --> calloc() + 0x00b18
  p = tcb - tls_size --> calloc() + 4
  memcpy(p, tls_initaddr, tls_initsize)

So, the TLS initialization image is copied to calloc() + 4.
However, ld(1) assumes that the TLS area starts with:
  tcb - roundup2(p_memsz, p_align) = tcb - 0x00b18 --> calloc()

Since this is 4 bytes different, the initial value of the thread
local variable is not set correctly.

To fix this problem, I added the same thing as this line in ld.elf_so:

  https://nxr.netbsd.org/xref/src/libexec/ld.elf_so/tls.c#236

* obj->tlssize = p_memsz, obj->tlsalign = p_align

-- Takeshi Nakayama