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

[Jiangli Zhou]

Hi Florian and Thomas,

Thank you both for the quick turnaround! I've incorporated the
suggestions from you and updated the webrev below. Hopefully that I've
captured all of those without missing any. Please let me know if
there's anything that I missed.

http://cr.openjdk.java.net/~jiangli/8225035/webrev.02/

- Removed the use of pthread_, _pthread, or __pthread from new VM symbols
- Removed weak symbol reference
- Replaced RTLD_NEXT with RTLD_DEFAULT
- Incorporated Thomas' comment suggestion with minor tweaks
- Add comments (brief) for the __thread variable in the test executable.

Please see more comments below.

On Wed, Jun 26, 2019 at 7:59 AM Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>
> Hi Florian,
>
> On Wed, Jun 26, 2019 at 4:47 PM Florian Weimer <fweimer at redhat.com> wrote:
>>
>> * Thomas Stüfe:
>>
>> > About the attr pointer to pthread_get_minstack(): Since this function
>> > is undocumented, we have no idea what it does to attr. We know that
>> > the implementation Florian posted seems not to do anything with it,
>> > but is that true for all glibc implementations, over all past versions
>> > we want to function on, and on all platforms?
>>
>> Ahh, sorry, you are right.  The historic implementation looks like this:
>>
>> size_t
>> __pthread_get_minstack (const pthread_attr_t *attr)
>> {
>>   struct pthread_attr *iattr = (struct pthread_attr *) attr;
>>
>>   return (GLRO(dl_pagesize) + __static_tls_size + PTHREAD_STACK_MIN
>>          + iattr->guardsize);
>> }
>>
>> So you need to pass the attribute you use during thread creation.
>
>
> But we have not yet called pthread_attr_setguardsize() by the time we call __pthread_get_minstack. So attr contains whatever is the default set by pthread_attr_init(). Also, some lines below we add the guard size manually to the stack size, for similar reasons we now want to add the TLS size...
>
> All this outguessing the glibc makes my head hurt :)
>
>>
>> __pthread_get_minstack was changed when once the guardsize was no longer
>> subtracted from the available stack size, so you should not subtract the
>> guard size in OpenJDK, only sysconf (_SC_PAGESIZE) and
>> PTHREAD_STACK_MIN.
>>
>
> Which versions of glibc have this historic implementation? Do we need to care?

That's for glibc 2.27 and older versions. That reminded me why I was
getting bogus values from __pthread_get_minstack() when passing dummy
'attr' initially. I was testing with a glibc 2.24 at the time (a few
weeks back). Now I'm testing with glibc 2.28, which doesn't include
the 'iattr->guardsize'. Passing either NULL or dummy 'attr' doesn't
seem to be safe.

With older glibc (<2.27), an extra page (default value) for guardsize
(as well as PTHREAD_STACK_MIN) would be included and added to the
stack space when we enable the stack size adjustment with
AdjustStackSizeForTLS. That's okay as long as the adjustment size is
large enough to accommodate the on-stack TLS blocks.

It's not perfect. Before the issue can be addressed in the glibc or a
public API is available for user code to obtain the on-stack TLS size,
our current approach seems to be acceptable to me.

Best regards,

Jiangli

>
> ..Thomas
>
>>
>> Thanks,
>> Florian