RFR: 8225035: Thread stack size issue caused by large TLS size

[Florian Weimer]

* Jiangli Zhou:

> Hi Florian,
>
> On Thu, Jun 27, 2019 at 8:14 AM Florian Weimer <fweimer at redhat.com> wrote:
>>
>> * Jiangli Zhou:
>>
>> > On Thu, Jun 27, 2019 at 12:57 AM Florian Weimer <fweimer at redhat.com> wrote:
>> >>
>> >> * Thomas Stüfe:
>> >>
>> >> >> I think
>> >> >>
>> >> >>   stack_adjust_size += guard_size;
>> >> >>
>> >> >> is redundant with the get_minstack-based adjustment because it is either
>> >> >> included in it directly in the get_minstack value, or indirectly in the
>> >> >> updated stack size computation in glibc (where it adds the guard size
>> >> >> internally).
>> >> >
>> >> > Please do not change this!
>> >> >
>> >> > To me and I guess many others it is very important that nothing at all
>> >> > changes if this workaround is off.
>> >>
>> >> That has already happened because recent glibc adds the guard size to
>> >> the stack size internally now.
>> >>
>> >> But what I meant: only perform the += adjustment above for the
>> >> !AdjustStackSizeForTLS case.
>> >
>> > I'm reluctant to do that, because it adds additional dependencies on
>> > the glibc implementation. Subtracting the non-TLS size from the value
>> > returned by __pthread_get_minstack may not be forward compatible in
>> > the future when __pthread_get_minstack changes. I agree with Thomas to
>> > keep the workaround as simple as possible.
>>
>> Even though it is an internal interface, __pthread_get_minstack will not
>> change its competition.  It will be removed if glibc switches to
>> allocating the TCB and TLS data off the stack.  But that that point, the
>> size adjustment for TLS will no loger be necessary.
>>
>> PTHREAD_MIN_STACK could theoretically change (it is rather small on some
>> architectures), but we have been extremely reluctant to make that
>> change, due to dependencies like this one.  Rather, we have made the
>> effective available stack size larger by other means, for the same value
>> of PTHREAD_MIN_STACK.  (By not subtracting the guard size, in
>> particular, and we also avoid calls in to the dynamic loader in more
>> cases, which tend to require lots of stack space on several
>> architectures.)
>
> I think the issue is if we subtract PTHREAD_STACK_MIN, do we want to
> also subtract the _dl_pagesize? For glibc implementation older than
> 2.27, do we need to handle the case for the guardsize separately? If
> we do that, we would need conditional code to handle different glibc
> versions. That seems to be worse than the approach of going with
> _dl_get_tls_static_info??

I think you can handle the guard size in this way:

  pthread_attr_setguardsize(&attr, guard_size);

  size_t stack_adjust_size = 0;
  if (AdjustStackSizeForTLS) {
    size_t minstack_size = get_minstack(&attr);
    size_t tls_size = minstack_size - vm_page_size() - PTHREAD_STACK_MIN;
    // In glibc before 2.27, tls_size still includes guard_size.
    // In glibc 2.27 and later, guard_size is automatically
    // added to the stack size by pthread_create.
    // In both cases, the guard size is taken into account.
    stack_adjust_size += tls_size;
  } else {
    stack_adjust_size += guard_size;
  }

Thanks,
Florian