Thread stack size issue related to glibc TLS bug

[Jiangli Zhou]

Hi Florian,

On Fri, May 24, 2019 at 1:50 PM Florian Weimer <fweimer at redhat.com> wrote:
>
> * Jiangli Zhou:
>
> > Hi Florian,
> >
> > On Fri, May 24, 2019 at 2:46 AM Florian Weimer <fweimer at redhat.com> wrote:
> >>
> >> * Jiangli Zhou:
> >>
> >> > [3] change: http://cr.openjdk.java.net/~jiangli/tls_size/webrev/
> >> > (contributed by Jeremy Manson)
> >>
> >> _dl_get_tls_static_info is an internal symbol (it carries a
> >> GLIBC_PRIVATE symbol version).  Its implementation can change at any
> >> time.  Please do not do this.
> >
> > Point taken. Thanks.
> >
> > It appears that asan is already carrying the same type of fix:
> >
> > https://github.com/gcc-mirror/gcc/blob/master/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc
> >
> > As the issue has not been able to be addressed in the glibc layer, all
> > the others have to workaround it (and possibly by using the glibc
> > private APIs, _dl_get_tls_static_info or __pthread_get_minstack). That
> > would cause more dependencies on the private APIs.
>
> _dl_get_tls_static_info changed ABI on i386 in glibc 2.27 already, so
> you really don't want to use that (in case someone backported that
> cleanup to an earlier version, although that might be bit unlikely).
>
> The sanitizers are special and have a much shorter shelf life than the
> OpenJDK binaries.
>
> > One alternative (besides fixing glibc) may be making
> > _dl_get_tls_static_info public. Would that be possible?
>
> For __pthread_get_minstack, I can add a comment to the glibc sources
> that if the ABI changes (or if TLS allocations are no longer considered
> part of the stack), we should rename the function.  Then, as long as you
> use a weak reference or dlsym (the latter is preferable for the sack of
> RPM-based distributions which require special steps to reference
> GLIBC_PRIVATE symbols directly), old binaries would keep working if we
> make further changes.

Sounds good. Really appreciate for your help from the glibc layer. If
__pthread_get_minstack has more 'stability', I will change the patch
to use it. Jeremy also suggested that when we discussed yesterday.

>
> Since _dl_get_tls_static_info already changed ABI once, I really can't
> recommend its use.  Especially if you can work with
> __pthread_get_minstack instead.
>
> The value of PTHREAD_STACK_MIN is rather problematic on x86-64 (for the
> reasons I explained earlier), but it's not likely we are going to change
> the value of the constant any time soon.  It's more likely that we are
> going to work around the AVX-512 register file issue by providing
> exactly four usable stack pages with PTHREAD_STACK_MIN, and not less
> than two as we did until recently.  So you can assume that it's indeed
> possible to use PTHREAD_STACK_MIN and sysconf (_SC_PAGESIZE) to compute
> the static TLS area size.

Ok. That sounds reassuring.

Best,

Jiangli

>
> Thanks,
> Florian