We use the larger value on all other 64-bit platforms except powerpc64.
As a side effect, PTHREAD_STACK_MIN grows as well. The current value is
too small and we run out of stack space when exiting; see PR 234775.
Add a test to act as a canary.
Differential D18824
Raise __MINSIGSTKSZ on amd64. markj on Jan 11 2019, 7:29 PM. Authored by Tags None Referenced Files
Subscribers None
Details
We use the larger value on all other 64-bit platforms except powerpc64. Add a test to act as a canary.
Diff Detail
Event TimelineComment Actions I should have noted that this breaks the ABI, so I was planning to bump __FreeBSD_version. Alternately, we might instead decouple PTHREAD_STACK_MIN from MINSIGSTKSZ and just hardcode it to 4096. Comment Actions Indeed this changes the ABI and while I do not see why not change PTHREAD_STACK_MIN value, I would prefer to keep the library accepting the old min stack value. Otherwise we break old programs which use PTHREAD_STACK_MIN, and such programs exist even in base. Also, it is not clear to me how changing this value impacts allocation of actual stack mapping, since mmap() would round up to the full page size anyway ? Can you show ktrace output with and without your change ? Comment Actions Hmm, I don't see any actual uses? Indeed, there are some references in ntpd, but the code there only makes sure that its internal minimum stack size of 64KB is not smaller than PTHREAD_STACK_MIN. And, I do not see how any dynamically linked programs can successfully use a stack size of PTHREAD_STACK_MIN today.
Without the change: https://reviews.freebsd.org/P244 Indeed, we map a full page for the stack, but the requested stack size is passed to thr_new(2) and is used to initialize %rsp. Comment Actions They can, if they do not exit threads, but terminate the whole process e.g. from the main thread. And from my experience, this is very popular usage model. I looked at the PTHREAD_STACK_MIN usage on debian code search tool. Unfortunately, it is not very useful because the search results are overloaded with hits from glibc and gcc. Most other uses are to ensure that if some user provided a stack size less that PTHREAD_STACK_MIN than PTHREAD_STACK_MIN is specified. Worrying part is that such code exists e.g. in the perl xs processor, which means that lazy users which pass 0 as a stack size suddently stop work after we require 4096 min stack size.
I see. thanks for the explanation. Comment Actions I see. I can think of a few ways to deal with this:
Comment Actions Isn't this is what was proposed by you before ? Except without a compat symbol, and I do not see how compat symbol is helpful.
Comment Actions The PTHREAD_STACK_MIN limit is enforced by pthread_attr_setstacksize(); as you pointed out, applications that internally round up the stack size to PTHREAD_STACK_MIN before calling pthread_attr_setstacksize() would be broken if we blindly raised PTHREAD_STACK_MIN. A compat version of the symbol could be used to preserve the old limit for applications that are linked against the old libthr. Comment Actions No, I mean that such compat version is not too useful because apps really cannot distinguish between old and new compilation environments. Whether the app is linked to old or new symbol is purely defined by the timing of the base system install and not due to any changes in the app code _OR_ depended headers. So if you compile the binary on old system (before the STACK_MIN was raised) and run it on new system, it would magically work. But then you take the same sources and compile them on new system and it is unexpectedly broken. |