Message ID | 20170316073012.22763-1-vapier@gentoo.org |
---|---|
State | New, archived |
Headers |
Received: (qmail 106922 invoked by alias); 16 Mar 2017 07:30:37 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 106275 invoked by uid 89); 16 Mar 2017 07:30:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=recover, breathing X-HELO: smtp.gentoo.org From: Mike Frysinger <vapier@gentoo.org> To: libc-alpha@sourceware.org Cc: adhemerval.zanella@linaro.org Subject: [PATCH] posix_spawn: use a larger min stack for -fstack-check [BZ #21253] Date: Thu, 16 Mar 2017 00:30:12 -0700 Message-Id: <20170316073012.22763-1-vapier@gentoo.org> |
Commit Message
Mike Frysinger
March 16, 2017, 7:30 a.m. UTC
When glibc is built with -fstack-check, trying to use posix_spawn can lead to segfaults due to gcc internally probing stack memory too far. The new spawn API will allocate a minimum of 1 page, but the stack checking logic might probe a couple of pages. When it tries to walk them, everything falls apart. The gcc internal docs [1] state the default interval checking is one page. Which means we need two pages (the current one, and the next probed). No target currently defines it larger. Further, it mentions that the default minimum stack size needed to recover from an overflow is 4/8KiB for sjlj or 8/12KiB for others. But some Linux targets (like mips and ppc) go up to 16KiB (and some non-Linux targets go up to 24KiB). Let's create each child with a minimum of 32KiB to support them all, and give us future breathing room. No test is added as existing ones crash. Even a simple call is enough to trigger the problem: char *argv[] = { "/bin/ls", NULL }; posix_spawn(&pid, "/bin/ls", NULL, NULL, argv, NULL); [1] https://gcc.gnu.org/onlinedocs/gcc-6.3.0/gccint/Stack-Checking.html --- sysdeps/unix/sysv/linux/spawni.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Comments
On 03/16/2017 08:30 AM, Mike Frysinger wrote: > When glibc is built with -fstack-check, trying to use posix_spawn can > lead to segfaults due to gcc internally probing stack memory too far. > The new spawn API will allocate a minimum of 1 page, but the stack > checking logic might probe a couple of pages. When it tries to walk > them, everything falls apart. > > The gcc internal docs [1] state the default interval checking is one > page. Which means we need two pages (the current one, and the next > probed). No target currently defines it larger. GCC miscomputes the offsets in some cases, so I would not rely on this. Would it be possible compile the functions involved without -fstack-check instead? > /* Add a slack area for child's stack. */ > size_t argv_size = (argc * sizeof (void *)) + 512; > - size_t stack_size = ALIGN_UP (argv_size, GLRO(dl_pagesize)); > + /* We need at least a few pages in case the compiler's stack checking is > + enabled. In some configs, it is known to use at least 24KiB. */ > + size_t stack_size = ALIGN_UP (argv_size, 32 * 1024); 64 KiB pages are common, so this reduces the stack size in many cases. Thanks, Florian
On 16/03/2017 05:17, Florian Weimer wrote: > On 03/16/2017 08:30 AM, Mike Frysinger wrote: >> When glibc is built with -fstack-check, trying to use posix_spawn can >> lead to segfaults due to gcc internally probing stack memory too far. >> The new spawn API will allocate a minimum of 1 page, but the stack >> checking logic might probe a couple of pages. When it tries to walk >> them, everything falls apart. >> >> The gcc internal docs [1] state the default interval checking is one >> page. Which means we need two pages (the current one, and the next >> probed). No target currently defines it larger. > > GCC miscomputes the offsets in some cases, so I would not rely on this. > > Would it be possible compile the functions involved without -fstack-check instead? There is some old bug reports about this GCC option which states it somewhat unreliable in some cases [1]. However what really worries me is the bug report [2] stating that the probe check range can wrap around and thus totally infective in some cases (although this case indeed might be not common). Anyway, I am with Florian, we should not rely on this. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=13182 [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66479 > >> /* Add a slack area for child's stack. */ >> size_t argv_size = (argc * sizeof (void *)) + 512; >> - size_t stack_size = ALIGN_UP (argv_size, GLRO(dl_pagesize)); >> + /* We need at least a few pages in case the compiler's stack checking is >> + enabled. In some configs, it is known to use at least 24KiB. */ >> + size_t stack_size = ALIGN_UP (argv_size, 32 * 1024); > > 64 KiB pages are common, so this reduces the stack size in many cases. > > Thanks, > Florian >
On 16 Mar 2017 09:17, Florian Weimer wrote: > On 03/16/2017 08:30 AM, Mike Frysinger wrote: > > When glibc is built with -fstack-check, trying to use posix_spawn can > > lead to segfaults due to gcc internally probing stack memory too far. > > The new spawn API will allocate a minimum of 1 page, but the stack > > checking logic might probe a couple of pages. When it tries to walk > > them, everything falls apart. > > > > The gcc internal docs [1] state the default interval checking is one > > page. Which means we need two pages (the current one, and the next > > probed). No target currently defines it larger. > > GCC miscomputes the offsets in some cases, so I would not rely on this. > > Would it be possible compile the functions involved without > -fstack-check instead? i mentioned in the bug that it's not feasible: compiling this one file doesn't help as it calls other glibc funcs which will have checking enabled. so we'd have to manually track the full call stack here and disable it on all the files which is a fairly fragile/burdensome process. > > /* Add a slack area for child's stack. */ > > size_t argv_size = (argc * sizeof (void *)) + 512; > > - size_t stack_size = ALIGN_UP (argv_size, GLRO(dl_pagesize)); > > + /* We need at least a few pages in case the compiler's stack checking is > > + enabled. In some configs, it is known to use at least 24KiB. */ > > + size_t stack_size = ALIGN_UP (argv_size, 32 * 1024); > > 64 KiB pages are common, so this reduces the stack size in many cases. common where ? are you talking about large pages ? going by PAGE_SHIFT in the kernel, looks to me like very few targets allow using 64 KiB at all, let alone being common. alpha: 8 KiB arc: kconfig allows 4, 8 (default), or 16 KiB arm: 4 KiB arm64: kconfig allows 4, 16, or 64 KiB (default depends on # of VMA bits) avr32: 4 KiB cris: 8 KiB frv: 16 KiB hexagon: kconfig allows 4 (default), 16, 64, 256 KiB, or 1 MiB ia64: kconfig allows 4, 8, 16 (default), or 64 KiB m32r: 4 KiB m68k: 4 (non-coldfire) or 8 KiB (coldfire) meta: kconfig allows 4 (default), 8, or 16 KiB microblaze: kconfig allows 4 (default), 16, or 64 KiB mips: kconfig allows 4 (default for most, but gets tricky), 8, 16, 32, or 64 KiB mn10300: 4 KiB nios2: 4 KiB openrisc: 8 KiB parisc: kconfig allows 4 (default), 16, or 64 KiB powerpc: kconfig allows 4 (default), 16, 64, or 256 KiB s390: 4 KiB score: 4 KiB sh: 4 KiB (for no-mmu, you have other choices) sparc32: 4 KiB sparc64: 8 KiB tile: kconfig allows 4, 16, or 64 KiB (default) unicore32: 4 KiB x86: 4 KiB xtensa: 4 KiB i picked 32 KiB as a trade off of it being more than big enough and being a nice multiple of 4, 8, and 16, while not overtly allocating too much otherwise unused. -mike
On Thu, Mar 16, 2017 at 2:52 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On 16 Mar 2017 09:17, Florian Weimer wrote: >> On 03/16/2017 08:30 AM, Mike Frysinger wrote: >> > When glibc is built with -fstack-check, trying to use posix_spawn can >> > lead to segfaults due to gcc internally probing stack memory too far. >> > The new spawn API will allocate a minimum of 1 page, but the stack >> > checking logic might probe a couple of pages. When it tries to walk >> > them, everything falls apart. >> > >> > The gcc internal docs [1] state the default interval checking is one >> > page. Which means we need two pages (the current one, and the next >> > probed). No target currently defines it larger. >> >> GCC miscomputes the offsets in some cases, so I would not rely on this. >> >> Would it be possible compile the functions involved without >> -fstack-check instead? > > i mentioned in the bug that it's not feasible: compiling this one file > doesn't help as it calls other glibc funcs which will have checking > enabled. so we'd have to manually track the full call stack here and > disable it on all the files which is a fairly fragile/burdensome process. > >> > /* Add a slack area for child's stack. */ >> > size_t argv_size = (argc * sizeof (void *)) + 512; >> > - size_t stack_size = ALIGN_UP (argv_size, GLRO(dl_pagesize)); >> > + /* We need at least a few pages in case the compiler's stack checking is >> > + enabled. In some configs, it is known to use at least 24KiB. */ >> > + size_t stack_size = ALIGN_UP (argv_size, 32 * 1024); >> >> 64 KiB pages are common, so this reduces the stack size in many cases. > > common where ? are you talking about large pages ? > > going by PAGE_SHIFT in the kernel, looks to me like very few > targets allow using 64 KiB at all, let alone being common. RHEL on ARM64 defaults to 64k. For Cavium Octeon SDK, the mips kernel defaults to 64k page size. Thanks, Andrew > > alpha: 8 KiB > arc: kconfig allows 4, 8 (default), or 16 KiB > arm: 4 KiB > arm64: kconfig allows 4, 16, or 64 KiB (default depends on # of VMA bits) > avr32: 4 KiB > cris: 8 KiB > frv: 16 KiB > hexagon: kconfig allows 4 (default), 16, 64, 256 KiB, or 1 MiB > ia64: kconfig allows 4, 8, 16 (default), or 64 KiB > m32r: 4 KiB > m68k: 4 (non-coldfire) or 8 KiB (coldfire) > meta: kconfig allows 4 (default), 8, or 16 KiB > microblaze: kconfig allows 4 (default), 16, or 64 KiB > mips: kconfig allows 4 (default for most, but gets tricky), 8, 16, 32, or 64 KiB > mn10300: 4 KiB > nios2: 4 KiB > openrisc: 8 KiB > parisc: kconfig allows 4 (default), 16, or 64 KiB > powerpc: kconfig allows 4 (default), 16, 64, or 256 KiB > s390: 4 KiB > score: 4 KiB > sh: 4 KiB (for no-mmu, you have other choices) > sparc32: 4 KiB > sparc64: 8 KiB > tile: kconfig allows 4, 16, or 64 KiB (default) > unicore32: 4 KiB > x86: 4 KiB > xtensa: 4 KiB > > i picked 32 KiB as a trade off of it being more than big enough and > being a nice multiple of 4, 8, and 16, while not overtly allocating > too much otherwise unused. > -mike
On 16 Mar 2017 14:56, Andrew Pinski wrote: > On Thu, Mar 16, 2017 at 2:52 PM, Mike Frysinger wrote: > > On 16 Mar 2017 09:17, Florian Weimer wrote: > >> On 03/16/2017 08:30 AM, Mike Frysinger wrote: > >> > When glibc is built with -fstack-check, trying to use posix_spawn can > >> > lead to segfaults due to gcc internally probing stack memory too far. > >> > The new spawn API will allocate a minimum of 1 page, but the stack > >> > checking logic might probe a couple of pages. When it tries to walk > >> > them, everything falls apart. > >> > > >> > The gcc internal docs [1] state the default interval checking is one > >> > page. Which means we need two pages (the current one, and the next > >> > probed). No target currently defines it larger. > >> > >> GCC miscomputes the offsets in some cases, so I would not rely on this. > >> > >> Would it be possible compile the functions involved without > >> -fstack-check instead? > > > > i mentioned in the bug that it's not feasible: compiling this one file > > doesn't help as it calls other glibc funcs which will have checking > > enabled. so we'd have to manually track the full call stack here and > > disable it on all the files which is a fairly fragile/burdensome process. > > > >> > /* Add a slack area for child's stack. */ > >> > size_t argv_size = (argc * sizeof (void *)) + 512; > >> > - size_t stack_size = ALIGN_UP (argv_size, GLRO(dl_pagesize)); > >> > + /* We need at least a few pages in case the compiler's stack checking is > >> > + enabled. In some configs, it is known to use at least 24KiB. */ > >> > + size_t stack_size = ALIGN_UP (argv_size, 32 * 1024); > >> > >> 64 KiB pages are common, so this reduces the stack size in many cases. > > > > common where ? are you talking about large pages ? > > > > going by PAGE_SHIFT in the kernel, looks to me like very few > > targets allow using 64 KiB at all, let alone being common. > > RHEL on ARM64 defaults to 64k. > For Cavium Octeon SDK, the mips kernel defaults to 64k page size. i think it's fair to say that those userbases do not constitute anywhere close to a majority, or even a signficiantly large presence. from what i can tell, gcc does not expose any CPP defines we can use to see if stack checking is enabled. so we don't have a way of turning the extra allocations on conditionally. wouldn't be easy to add a configure check either since -fstack-check can be turned on via default specs. -mike
On 16/03/17 08:17, Florian Weimer wrote: > On 03/16/2017 08:30 AM, Mike Frysinger wrote: >> When glibc is built with -fstack-check, trying to use posix_spawn can >> lead to segfaults due to gcc internally probing stack memory too far. >> The new spawn API will allocate a minimum of 1 page, but the stack >> checking logic might probe a couple of pages. When it tries to walk >> them, everything falls apart. >> >> The gcc internal docs [1] state the default interval checking is one >> page. Which means we need two pages (the current one, and the next >> probed). No target currently defines it larger. > > GCC miscomputes the offsets in some cases, so I would not rely on this. > > Would it be possible compile the functions involved without -fstack-check instead? > +1 for compiling with -fno-stack-check >> /* Add a slack area for child's stack. */ >> size_t argv_size = (argc * sizeof (void *)) + 512; >> - size_t stack_size = ALIGN_UP (argv_size, GLRO(dl_pagesize)); >> + /* We need at least a few pages in case the compiler's stack checking is >> + enabled. In some configs, it is known to use at least 24KiB. */ >> + size_t stack_size = ALIGN_UP (argv_size, 32 * 1024); > > 64 KiB pages are common, so this reduces the stack size in many cases. > i think mmap will align up, so there is no actual reduction, but i don't think the magic value is justified.
On Mär 16 2017, Mike Frysinger <vapier@gentoo.org> wrote: > common where ? SLES on POWER uses 64K pages. > i picked 32 KiB as a trade off ??? Just use the max of pagesize and 32k. Andreas.
On 03/16/2017 10:52 PM, Mike Frysinger wrote: > i picked 32 KiB as a trade off of it being more than big enough and > being a nice multiple of 4, 8, and 16, while not overtly allocating > too much otherwise unused. Depending on what constant page size GCC uses, this will not fix the -fstack-check bug on all architectures. I doubt there is a good solution to this, except fixing the GCC bugs in this area and allocating an additional page in glibc. Thanks, Florian
On 20 Mar 2017 09:56, Florian Weimer wrote: > On 03/16/2017 10:52 PM, Mike Frysinger wrote: > > i picked 32 KiB as a trade off of it being more than big enough and > > being a nice multiple of 4, 8, and 16, while not overtly allocating > > too much otherwise unused. > > Depending on what constant page size GCC uses, this will not fix the > -fstack-check bug on all architectures. do you know of one today ? i grepped through the latest gcc source to come up with the numbers that i used, and i think 32 KiB should cover them all. > I doubt there is a good solution to this, except fixing the GCC bugs in > this area and allocating an additional page in glibc. what gcc bugs do you mean ? -mike
On 03/20/2017 10:04 AM, Mike Frysinger wrote: > On 20 Mar 2017 09:56, Florian Weimer wrote: >> On 03/16/2017 10:52 PM, Mike Frysinger wrote: >>> i picked 32 KiB as a trade off of it being more than big enough and >>> being a nice multiple of 4, 8, and 16, while not overtly allocating >>> too much otherwise unused. >> >> Depending on what constant page size GCC uses, this will not fix the >> -fstack-check bug on all architectures. > > do you know of one today ? i grepped through the latest gcc source to > come up with the numbers that i used, and i think 32 KiB should cover > them all. GCC stack probing assumes a 4 KiB stack size, which should be a conservative assumption almost everywhere. But the first probe is off by a few bytes. This seems to be a common issue across many target, e.g. on GCC 6 for aarch64: void external(void *); void int1 (void) { int a; external (&a); } .text .align 2 .p2align 3,,7 .global int1 .type int1, %function int1: .LFB0: .cfi_startproc sub x9, sp, #8192 str xzr, [x9, 4064] stp x29, x30, [sp, -32]! .cfi_def_cfa_offset 32 .cfi_offset 29, -32 .cfi_offset 30, -24 add x29, sp, 0 .cfi_def_cfa_register 29 add x0, x29, 28 bl external ldp x29, x30, [sp], 32 .cfi_restore 30 .cfi_restore 29 .cfi_def_cfa 31, 0 ret .cfi_endproc .LFE0: .size int1, .-int1 Or ppc64le: .abiversion 2 .section ".text" .align 2 .p2align 4,,15 .globl int1 .type int1, @function int1: .LCF0: 0: addis 2,12,.TOC.-.LCF0@ha addi 2,2,.TOC.-.LCF0@l .localentry int1,.-int1 mflr 0 std 0,-16432(1) std 0,16(1) stdu 1,-48(1) addi 3,1,32 bl external nop addi 1,1,48 ld 0,16(1) mtlr 0 blr .long 0 .byte 0,0,0,1,128,0,0,0 .size int1,.-int1 Or x86_64: .text .p2align 4,,15 .globl int1 .type int1, @function int1: .LFB0: .cfi_startproc subq $4152, %rsp orq $0, (%rsp) addq $4128, %rsp .cfi_def_cfa_offset 32 leaq 12(%rsp), %rdi call external addq $24, %rsp .cfi_def_cfa_offset 8 ret .cfi_endproc .LFE0: .size int1, .-int1 .p2align 4,,15 I know x86-64 best, so I'm going to explain the issue there. Suppose that at the time of the call instruction, %rsp (the stack pointer) ends with …3000. Then on function entry, %rsp == …3008. Decrementing %rsp by 4152 results in …1FD0. The 8-byte stack probe touches the bytes from …1FD0 to …1FD7. This skips over the guard page at …2000 (which extends to …2FFF). The other problem is that GCC does not take the implied stack probe by the call instruction into account, or other forms of linear stack access. This leads to grossly inefficient code with -fstack-check. I actually think that compiling with -fstack-check is a good idea, but GCC really needs fixes first. We do not know the root cause of these issues (note the 3 * 4096 probe offset on POWER), and just throwing random workarounds into glibc does not seem right. Thanks, Florian
On 20/03/2017 10:26, Florian Weimer wrote: > On 03/20/2017 10:04 AM, Mike Frysinger wrote: >> On 20 Mar 2017 09:56, Florian Weimer wrote: >>> On 03/16/2017 10:52 PM, Mike Frysinger wrote: >>>> i picked 32 KiB as a trade off of it being more than big enough and >>>> being a nice multiple of 4, 8, and 16, while not overtly allocating >>>> too much otherwise unused. >>> >>> Depending on what constant page size GCC uses, this will not fix the >>> -fstack-check bug on all architectures. >> >> do you know of one today ? i grepped through the latest gcc source to >> come up with the numbers that i used, and i think 32 KiB should cover >> them all. > > GCC stack probing assumes a 4 KiB stack size, which should be a conservative assumption almost everywhere. But the first probe is off by a few bytes. This seems to be a common issue across many target, e.g. on GCC 6 for aarch64: > > void external(void *); > > void > int1 (void) > { > int a; > external (&a); > } > > > .text > .align 2 > .p2align 3,,7 > .global int1 > .type int1, %function > int1: > .LFB0: > .cfi_startproc > sub x9, sp, #8192 > str xzr, [x9, 4064] > stp x29, x30, [sp, -32]! > .cfi_def_cfa_offset 32 > .cfi_offset 29, -32 > .cfi_offset 30, -24 > add x29, sp, 0 > .cfi_def_cfa_register 29 > add x0, x29, 28 > bl external > ldp x29, x30, [sp], 32 > .cfi_restore 30 > .cfi_restore 29 > .cfi_def_cfa 31, 0 > ret > .cfi_endproc > .LFE0: > .size int1, .-int1 > > Or ppc64le: > > .abiversion 2 > .section ".text" > .align 2 > .p2align 4,,15 > .globl int1 > .type int1, @function > int1: > .LCF0: > 0: addis 2,12,.TOC.-.LCF0@ha > addi 2,2,.TOC.-.LCF0@l > .localentry int1,.-int1 > mflr 0 > std 0,-16432(1) > std 0,16(1) > stdu 1,-48(1) > addi 3,1,32 > bl external > nop > addi 1,1,48 > ld 0,16(1) > mtlr 0 > blr > .long 0 > .byte 0,0,0,1,128,0,0,0 > .size int1,.-int1 > > > Or x86_64: > > .text > .p2align 4,,15 > .globl int1 > .type int1, @function > int1: > .LFB0: > .cfi_startproc > subq $4152, %rsp > orq $0, (%rsp) > addq $4128, %rsp > .cfi_def_cfa_offset 32 > leaq 12(%rsp), %rdi > call external > addq $24, %rsp > .cfi_def_cfa_offset 8 > ret > .cfi_endproc > .LFE0: > .size int1, .-int1 > .p2align 4,,15 > > I know x86-64 best, so I'm going to explain the issue there. Suppose that at the time of the call instruction, %rsp (the stack pointer) ends with …3000. Then on function entry, %rsp == …3008. Decrementing %rsp by 4152 results in …1FD0. The 8-byte stack probe touches the bytes from …1FD0 to …1FD7. This skips over the guard page at …2000 (which extends to …2FFF). > > The other problem is that GCC does not take the implied stack probe by the call instruction into account, or other forms of linear stack access. This leads to grossly inefficient code with -fstack-check. > > I actually think that compiling with -fstack-check is a good idea, but GCC really needs fixes first. We do not know the root cause of these issues (note the 3 * 4096 probe offset on POWER), and just throwing random workarounds into glibc does not seem right. > > Thanks, > Florian Should also -fstack-check check for probe value underflow/overflow? The cases should be rare (if valid), but at least gcc has one bug report about it [1]. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66479
On 03/20/2017 02:44 PM, Adhemerval Zanella wrote: > Should also -fstack-check check for probe value underflow/overflow? The cases should be > rare (if valid), but at least gcc has one bug report about it [1]. > > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66479 I think what we want is a well-controlled page fault. How we get there does not really matter. Florian
On 20 Mar 2017 14:26, Florian Weimer wrote: > On 03/20/2017 10:04 AM, Mike Frysinger wrote: > > On 20 Mar 2017 09:56, Florian Weimer wrote: > >> On 03/16/2017 10:52 PM, Mike Frysinger wrote: > >>> i picked 32 KiB as a trade off of it being more than big enough and > >>> being a nice multiple of 4, 8, and 16, while not overtly allocating > >>> too much otherwise unused. > >> > >> Depending on what constant page size GCC uses, this will not fix the > >> -fstack-check bug on all architectures. > > > > do you know of one today ? i grepped through the latest gcc source to > > come up with the numbers that i used, and i think 32 KiB should cover > > them all. > > GCC stack probing assumes a 4 KiB stack size, which should be a > conservative assumption almost everywhere. But the first probe is off > by a few bytes. This seems to be a common issue across many target, > e.g. on GCC 6 for aarch64: > > void external(void *); > > void > int1 (void) > { > int a; > external (&a); > } > > > .text > .align 2 > .p2align 3,,7 > .global int1 > .type int1, %function > int1: > .LFB0: > .cfi_startproc > sub x9, sp, #8192 > str xzr, [x9, 4064] > stp x29, x30, [sp, -32]! > .cfi_def_cfa_offset 32 > .cfi_offset 29, -32 > .cfi_offset 30, -24 > add x29, sp, 0 > .cfi_def_cfa_register 29 > add x0, x29, 28 > bl external > ldp x29, x30, [sp], 32 > .cfi_restore 30 > .cfi_restore 29 > .cfi_def_cfa 31, 0 > ret > .cfi_endproc > .LFE0: > .size int1, .-int1 > > Or ppc64le: > > .abiversion 2 > .section ".text" > .align 2 > .p2align 4,,15 > .globl int1 > .type int1, @function > int1: > .LCF0: > 0: addis 2,12,.TOC.-.LCF0@ha > addi 2,2,.TOC.-.LCF0@l > .localentry int1,.-int1 > mflr 0 > std 0,-16432(1) > std 0,16(1) > stdu 1,-48(1) > addi 3,1,32 > bl external > nop > addi 1,1,48 > ld 0,16(1) > mtlr 0 > blr > .long 0 > .byte 0,0,0,1,128,0,0,0 > .size int1,.-int1 > > > Or x86_64: > > .text > .p2align 4,,15 > .globl int1 > .type int1, @function > int1: > .LFB0: > .cfi_startproc > subq $4152, %rsp > orq $0, (%rsp) > addq $4128, %rsp > .cfi_def_cfa_offset 32 > leaq 12(%rsp), %rdi > call external > addq $24, %rsp > .cfi_def_cfa_offset 8 > ret > .cfi_endproc > .LFE0: > .size int1, .-int1 > .p2align 4,,15 > > I know x86-64 best, so I'm going to explain the issue there. Suppose > that at the time of the call instruction, %rsp (the stack pointer) ends > with …3000. Then on function entry, %rsp == …3008. Decrementing %rsp > by 4152 results in …1FD0. The 8-byte stack probe touches the bytes from > …1FD0 to …1FD7. This skips over the guard page at …2000 (which extends > to …2FFF). > > The other problem is that GCC does not take the implied stack probe by > the call instruction into account, or other forms of linear stack > access. This leads to grossly inefficient code with -fstack-check. > > I actually think that compiling with -fstack-check is a good idea, but > GCC really needs fixes first. We do not know the root cause of these > issues (note the 3 * 4096 probe offset on POWER), and just throwing > random workarounds into glibc does not seem right. i don't think this qualifies as random workarounds. we know gcc is going to be probing the stack, by design, past the edge. atm, we only give it 512 bytes of breathing room. regardless of the efficiency or specific distance, 512 is never going to be enough. therefore, glibc is broken. now we have to figure out how much slack will gcc want to see w/out triggering a false positive. based on my research of the gcc tree, 32 KiB gets us enough space. yes, there might be some fudge factor in there, but it isn't like adding only 4 KiB would ever be the correct size. -mike
diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c index bb3eecfde1a5..e206f069bc5a 100644 --- a/sysdeps/unix/sysv/linux/spawni.c +++ b/sysdeps/unix/sysv/linux/spawni.c @@ -329,7 +329,9 @@ __spawnix (pid_t * pid, const char *file, /* Add a slack area for child's stack. */ size_t argv_size = (argc * sizeof (void *)) + 512; - size_t stack_size = ALIGN_UP (argv_size, GLRO(dl_pagesize)); + /* We need at least a few pages in case the compiler's stack checking is + enabled. In some configs, it is known to use at least 24KiB. */ + size_t stack_size = ALIGN_UP (argv_size, 32 * 1024); void *stack = __mmap (NULL, stack_size, prot, MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0); if (__glibc_unlikely (stack == MAP_FAILED))