* Gnulib's alloca.h used even when there is a system header @ 2019-02-17 17:20 Eli Zaretskii 2019-02-17 20:06 ` Paul Eggert 2019-02-17 23:28 ` Bruno Haible 0 siblings, 2 replies; 15+ messages in thread From: Eli Zaretskii @ 2019-02-17 17:20 UTC (permalink / raw) To: bug-gnulib Building the latest version of Texinfo, I see that Gnulib decides to generate its own alloca.h header even though there's a system header alloca.h (and HAVE_ALLOCA_H is 1 in config.h). Why does it do that? Isn't that dangerous? e.g., it could conflict with how the system header defies 'alloca', no? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-02-17 17:20 Gnulib's alloca.h used even when there is a system header Eli Zaretskii @ 2019-02-17 20:06 ` Paul Eggert 2019-02-17 20:24 ` Eli Zaretskii 2019-02-17 23:28 ` Bruno Haible 1 sibling, 1 reply; 15+ messages in thread From: Paul Eggert @ 2019-02-17 20:06 UTC (permalink / raw) To: Eli Zaretskii; +Cc: bug-gnulib Eli Zaretskii wrote: > Gnulib decides to > generate its own alloca.h header even though there's a system header > alloca.h (and HAVE_ALLOCA_H is 1 in config.h). Why does it do that? > Isn't that dangerous? e.g., it could conflict with how the system > header defies 'alloca', no? This stuff is rickety, as it's built atop Autoconf's AC_FUNC_ALLOCA and that stuff hasn't changed in at least a decade. In the old days, some alloca implementations were broken and when in doubt it was better to replace the system alloca than to use it. Nowadays I doubt whether it matters much. In any event Autoconf and/or Gnulib patches to be more cautious about replacing alloca.h would be welcome. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-02-17 20:06 ` Paul Eggert @ 2019-02-17 20:24 ` Eli Zaretskii 2019-02-17 21:09 ` Paul Eggert 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2019-02-17 20:24 UTC (permalink / raw) To: Paul Eggert; +Cc: bug-gnulib > Cc: bug-gnulib@gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Sun, 17 Feb 2019 12:06:53 -0800 > > This stuff is rickety, as it's built atop Autoconf's AC_FUNC_ALLOCA and that > stuff hasn't changed in at least a decade. In the old days, some alloca > implementations were broken and when in doubt it was better to replace the > system alloca than to use it. Nowadays I doubt whether it matters much. In any > event Autoconf and/or Gnulib patches to be more cautious about replacing > alloca.h would be welcome. Thanks. I'm currently testing the below, will get back when I'm sure it's TRT. But if you think it might not be right for reasons related to other platforms or some global Gnulib considerations, please tell. --- gnulib/lib/alloca.h~0 2019-02-17 18:00:40.589250000 +0200 +++ gnulib/lib/alloca.h 2019-02-17 18:22:50.933000000 +0200 @@ -37,7 +37,11 @@ #ifndef alloca # ifdef __GNUC__ -# define alloca __builtin_alloca +# if HAVE_ALLOCA_H +# include_next <alloca.h> +# else +# define alloca __builtin_alloca +# endif # elif defined _AIX # define alloca __alloca # elif defined _MSC_VER ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-02-17 20:24 ` Eli Zaretskii @ 2019-02-17 21:09 ` Paul Eggert 2019-02-17 23:32 ` Bruno Haible 2019-02-18 16:59 ` Eli Zaretskii 0 siblings, 2 replies; 15+ messages in thread From: Paul Eggert @ 2019-02-17 21:09 UTC (permalink / raw) To: Eli Zaretskii; +Cc: bug-gnulib Eli Zaretskii wrote: > #ifndef alloca > # ifdef __GNUC__ > -# define alloca __builtin_alloca > +# if HAVE_ALLOCA_H > +# include_next <alloca.h> > +# else > +# define alloca __builtin_alloca > +# endif > # elif defined _AIX Why do the 'include_next' only for GCC? Why not do it for all compilers? Also, what platforms don't work with the current Gnulib alloca module, and why? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-02-17 21:09 ` Paul Eggert @ 2019-02-17 23:32 ` Bruno Haible 2019-02-18 16:59 ` Eli Zaretskii 1 sibling, 0 replies; 15+ messages in thread From: Bruno Haible @ 2019-02-17 23:32 UTC (permalink / raw) To: bug-gnulib; +Cc: Eli Zaretskii, Paul Eggert Hi Paul, > Eli Zaretskii wrote: > > #ifndef alloca > > # ifdef __GNUC__ > > -# define alloca __builtin_alloca > > +# if HAVE_ALLOCA_H > > +# include_next <alloca.h> > > +# else > > +# define alloca __builtin_alloca > > +# endif > > # elif defined _AIX > > Why do the 'include_next' only for GCC? Why not do it for all compilers? 'include_next' needs absolute file name for some compilers, and our mechanics for determining the absolute file name of a .h file does not work when the .h file defines only macros. See m4/include_next.m4: # Note: This macro assumes that the header file is not empty after # preprocessing, i.e. it does not only define preprocessor macros but also # provides some type/enum definitions or function/variable declarations. alloca.h unfortunately may be in this category. > Also, what platforms don't work with the current Gnulib alloca module, and why? Yes, Eli, I'd like to know this as well: what is the proposed change doing to fix/improve? Bruno ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-02-17 21:09 ` Paul Eggert 2019-02-17 23:32 ` Bruno Haible @ 2019-02-18 16:59 ` Eli Zaretskii 2019-02-18 23:32 ` Bruno Haible 1 sibling, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2019-02-18 16:59 UTC (permalink / raw) To: Paul Eggert; +Cc: bug-gnulib > Cc: bug-gnulib@gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Sun, 17 Feb 2019 13:09:06 -0800 > > Eli Zaretskii wrote: > > #ifndef alloca > > # ifdef __GNUC__ > > -# define alloca __builtin_alloca > > +# if HAVE_ALLOCA_H > > +# include_next <alloca.h> > > +# else > > +# define alloca __builtin_alloca > > +# endif > > # elif defined _AIX > > Why do the 'include_next' only for GCC? Why not do it for all compilers? I didn't know other compilers supported include_next. > Also, what platforms don't work with the current Gnulib alloca module, and why? I bumped into this with mingw.org's MinGW, which introduced alloca.h in its recent versions. (MinGW64 doesn't have this header.) This new MinGW alloca.h does this: __CRT_ALIAS void *alloca( size_t __n ){ return __builtin_alloca( __n ); } which makes alloca an always-inline function by virtue of # define __CRT_INLINE extern __inline__ # define __CRT_ALIAS __CRT_INLINE __attribute__((__always_inline__)) The other part of the puzzle is that stdlib.h does this: # include "alloca.h" so Gnulib's alloca.h is bypassed. Which causes this when compiling Gnulib's vasnprintf.c: In file included from vasnprintf.c:57:0: d:\usr\include\alloca.h: In function 'vasnprintf': ./alloca.h:40:18: error: inlining failed in call to always_inline '__builtin_alloca': function not considered for inlining # define alloca __builtin_alloca ^ In file included from d:\usr\include\stdlib.h:499:0, from vasnprintf.c:71: d:\usr\include\alloca.h:62:48: note: called from here __CRT_ALIAS void *alloca( size_t __n ){ return __builtin_alloca( __n ); } ^~~~~~~~~~~~~~~~~~~~~~~ Makefile:945: recipe for target `vasnprintf.lo' failed ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-02-18 16:59 ` Eli Zaretskii @ 2019-02-18 23:32 ` Bruno Haible 2019-02-19 17:18 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Bruno Haible @ 2019-02-18 23:32 UTC (permalink / raw) To: bug-gnulib; +Cc: Eli Zaretskii, Paul Eggert Hi Eli, > ./alloca.h:40:18: error: inlining failed in call to always_inline '__builtin_alloca': function not considered for inlining > # define alloca __builtin_alloca > ^ GCC never inlines a function that calls alloca() or __builtin_alloca(). The reason is that if you call this function in a loop, then without inlining it will consume a bounded amount of stack whereas with inlining it might cause a stack overflow. The mingw people have declared their new alloca() function as "always inline", and GCC is decent enough to point us to the dilemma that it cannot resolve. Of course, removing the always-inline marker would not help: alloca(N) would then return an address that is approximately N bytes below the stack pointer, that is, which points to an area that gets randomly overwritten (when a signal comes in). The upshot is: you can't define an alloca() function this way. Either define it as a macro - which is what the gnulib override does, if it gets activated -, or define it as a function in assembly-language. Since you say that this is a fresh bug from mingw, it's probably best that it gets reported to the mingw people. Then we don't need a Gnulib workaround. Bruno ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-02-18 23:32 ` Bruno Haible @ 2019-02-19 17:18 ` Eli Zaretskii 2019-02-28 18:40 ` Eli Zaretskii 2019-03-09 19:25 ` Keith Marshall 0 siblings, 2 replies; 15+ messages in thread From: Eli Zaretskii @ 2019-02-19 17:18 UTC (permalink / raw) To: Bruno Haible; +Cc: Keith Marshall, eggert, bug-gnulib > From: Bruno Haible <bruno@clisp.org> > Cc: Eli Zaretskii <eliz@gnu.org>, Paul Eggert <eggert@cs.ucla.edu> > Date: Tue, 19 Feb 2019 00:32:24 +0100 > > Hi Eli, > > > ./alloca.h:40:18: error: inlining failed in call to always_inline '__builtin_alloca': function not considered for inlining > > # define alloca __builtin_alloca > > ^ > > GCC never inlines a function that calls alloca() or __builtin_alloca(). > The reason is that if you call this function in a loop, then without > inlining it will consume a bounded amount of stack whereas with inlining > it might cause a stack overflow. > > The mingw people have declared their new alloca() function as "always > inline", and GCC is decent enough to point us to the dilemma that it > cannot resolve. The GCC diagnostics is somewhat cryptic, so it's hard for me to be sure what it means. My original interpretation was that due to Gnulib's #define, what GCC sees is this: extern inline __attribute__((__always_inline__)) void *__builtin_alloca( size_t __n ){ return __builtin_alloca( __n ); } which AFAIU cannot possibly compile. As for not ever inlining, please see a toy function I compiled and its disassembly at the end of this message. Does this confirm what you say above? I'm not sure. > The upshot is: you can't define an alloca() function this way. > Either define it as a macro - which is what the gnulib override does, > if it gets activated -, or define it as a function in assembly-language. > > Since you say that this is a fresh bug from mingw, it's probably best > that it gets reported to the mingw people. Then we don't need a Gnulib > workaround. CC'ing Keith, who maintains the MinGW runtime. Regardless of whether there is or isn't a bug in MinGW, I urge you to reconsider whether unconditionally overriding a system header in such a way is a good idea, at least for platforms that use GCC. I think this is a fragile arrangement, even if it seems to work. Here's the toy source code I used: #include <alloca.h> #include <string.h> char * make_string (int c, size_t n) { char *local_string = alloca (n + 1); int i; for (i = 0; i < n; i++) local_string[i] = c; local_string[n] = '\0'; return strdup (local_string); } And here's the disassembly of the object file generated by MinGW GCC: al.o: file format pe-i386 Disassembly of section .text: 00000000 <_make_string>: #include <alloca.h> #include <string.h> char * make_string (int c, size_t n) { 0: 55 push %ebp 1: 89 e5 mov %esp,%ebp 3: 83 ec 28 sub $0x28,%esp char *local_string = alloca (n + 1); 6: 8b 45 0c mov 0xc(%ebp),%eax 9: 83 c0 01 add $0x1,%eax c: 89 45 ec mov %eax,-0x14(%ebp) #if defined _GNU_SOURCE || ! defined _NO_OLDNAMES /* This is the GNU standard API; it is also compatible with Microsoft's * original, but now deprecated, naming convention. */ __CRT_ALIAS void *alloca( size_t __n ){ return __builtin_alloca( __n ); } f: 8b 45 ec mov -0x14(%ebp),%eax 12: 8d 50 0f lea 0xf(%eax),%edx 15: b8 10 00 00 00 mov $0x10,%eax 1a: 83 e8 01 sub $0x1,%eax 1d: 01 d0 add %edx,%eax 1f: b9 10 00 00 00 mov $0x10,%ecx 24: ba 00 00 00 00 mov $0x0,%edx 29: f7 f1 div %ecx 2b: 6b c0 10 imul $0x10,%eax,%eax 2e: e8 00 00 00 00 call 33 <_make_string+0x33> 33: 29 c4 sub %eax,%esp 35: 8d 44 24 04 lea 0x4(%esp),%eax 39: 83 c0 0f add $0xf,%eax 3c: c1 e8 04 shr $0x4,%eax 3f: c1 e0 04 shl $0x4,%eax 42: 89 45 f0 mov %eax,-0x10(%ebp) int i; for (i = 0; i < n; i++) 45: c7 45 f4 00 00 00 00 movl $0x0,-0xc(%ebp) 4c: eb 11 jmp 5f <_make_string+0x5f> local_string[i] = c; 4e: 8b 55 f4 mov -0xc(%ebp),%edx 51: 8b 45 f0 mov -0x10(%ebp),%eax 54: 01 d0 add %edx,%eax 56: 8b 55 08 mov 0x8(%ebp),%edx 59: 88 10 mov %dl,(%eax) make_string (int c, size_t n) { char *local_string = alloca (n + 1); int i; for (i = 0; i < n; i++) 5b: 83 45 f4 01 addl $0x1,-0xc(%ebp) 5f: 8b 45 f4 mov -0xc(%ebp),%eax 62: 39 45 0c cmp %eax,0xc(%ebp) 65: 77 e7 ja 4e <_make_string+0x4e> local_string[i] = c; local_string[n] = '\0'; 67: 8b 55 f0 mov -0x10(%ebp),%edx 6a: 8b 45 0c mov 0xc(%ebp),%eax 6d: 01 d0 add %edx,%eax 6f: c6 00 00 movb $0x0,(%eax) return strdup (local_string); 72: 8b 45 f0 mov -0x10(%ebp),%eax 75: 89 04 24 mov %eax,(%esp) 78: e8 00 00 00 00 call 7d <_make_string+0x7d> } 7d: c9 leave 7e: c3 ret 7f: 90 nop ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-02-19 17:18 ` Eli Zaretskii @ 2019-02-28 18:40 ` Eli Zaretskii 2019-03-09 19:25 ` Keith Marshall 1 sibling, 0 replies; 15+ messages in thread From: Eli Zaretskii @ 2019-02-28 18:40 UTC (permalink / raw) To: bruno; +Cc: keith, eggert, bug-gnulib Ping! This discussion seems to have stalled, without reaching any firms conclusion. Can we please finish discussing this? Or maybe it is OK to install the patch I proposed up-thread? TIA > Date: Tue, 19 Feb 2019 19:18:15 +0200 > From: Eli Zaretskii <eliz@gnu.org> > CC: bug-gnulib@gnu.org, eggert@cs.ucla.edu, > Keith Marshall <keith@users.osdn.me> > > > From: Bruno Haible <bruno@clisp.org> > > Cc: Eli Zaretskii <eliz@gnu.org>, Paul Eggert <eggert@cs.ucla.edu> > > Date: Tue, 19 Feb 2019 00:32:24 +0100 > > > > Hi Eli, > > > > > ./alloca.h:40:18: error: inlining failed in call to always_inline '__builtin_alloca': function not considered for inlining > > > # define alloca __builtin_alloca > > > ^ > > > > GCC never inlines a function that calls alloca() or __builtin_alloca(). > > The reason is that if you call this function in a loop, then without > > inlining it will consume a bounded amount of stack whereas with inlining > > it might cause a stack overflow. > > > > The mingw people have declared their new alloca() function as "always > > inline", and GCC is decent enough to point us to the dilemma that it > > cannot resolve. > > The GCC diagnostics is somewhat cryptic, so it's hard for me to be > sure what it means. My original interpretation was that due to > Gnulib's #define, what GCC sees is this: > > extern inline __attribute__((__always_inline__)) > void *__builtin_alloca( size_t __n ){ return __builtin_alloca( __n ); } > > which AFAIU cannot possibly compile. > > As for not ever inlining, please see a toy function I compiled and its > disassembly at the end of this message. Does this confirm what you > say above? I'm not sure. > > > The upshot is: you can't define an alloca() function this way. > > Either define it as a macro - which is what the gnulib override does, > > if it gets activated -, or define it as a function in assembly-language. > > > > Since you say that this is a fresh bug from mingw, it's probably best > > that it gets reported to the mingw people. Then we don't need a Gnulib > > workaround. > > CC'ing Keith, who maintains the MinGW runtime. > > Regardless of whether there is or isn't a bug in MinGW, I urge you to > reconsider whether unconditionally overriding a system header in such > a way is a good idea, at least for platforms that use GCC. I think > this is a fragile arrangement, even if it seems to work. > > Here's the toy source code I used: > > #include <alloca.h> > #include <string.h> > > char * > make_string (int c, size_t n) > { > char *local_string = alloca (n + 1); > int i; > > for (i = 0; i < n; i++) > local_string[i] = c; > local_string[n] = '\0'; > return strdup (local_string); > } > > And here's the disassembly of the object file generated by MinGW GCC: > > > al.o: file format pe-i386 > > > Disassembly of section .text: > > 00000000 <_make_string>: > #include <alloca.h> > #include <string.h> > > char * > make_string (int c, size_t n) > { > 0: 55 push %ebp > 1: 89 e5 mov %esp,%ebp > 3: 83 ec 28 sub $0x28,%esp > char *local_string = alloca (n + 1); > 6: 8b 45 0c mov 0xc(%ebp),%eax > 9: 83 c0 01 add $0x1,%eax > c: 89 45 ec mov %eax,-0x14(%ebp) > > #if defined _GNU_SOURCE || ! defined _NO_OLDNAMES > /* This is the GNU standard API; it is also compatible with Microsoft's > * original, but now deprecated, naming convention. > */ > __CRT_ALIAS void *alloca( size_t __n ){ return __builtin_alloca( __n ); } > f: 8b 45 ec mov -0x14(%ebp),%eax > 12: 8d 50 0f lea 0xf(%eax),%edx > 15: b8 10 00 00 00 mov $0x10,%eax > 1a: 83 e8 01 sub $0x1,%eax > 1d: 01 d0 add %edx,%eax > 1f: b9 10 00 00 00 mov $0x10,%ecx > 24: ba 00 00 00 00 mov $0x0,%edx > 29: f7 f1 div %ecx > 2b: 6b c0 10 imul $0x10,%eax,%eax > 2e: e8 00 00 00 00 call 33 <_make_string+0x33> > 33: 29 c4 sub %eax,%esp > 35: 8d 44 24 04 lea 0x4(%esp),%eax > 39: 83 c0 0f add $0xf,%eax > 3c: c1 e8 04 shr $0x4,%eax > 3f: c1 e0 04 shl $0x4,%eax > 42: 89 45 f0 mov %eax,-0x10(%ebp) > int i; > > for (i = 0; i < n; i++) > 45: c7 45 f4 00 00 00 00 movl $0x0,-0xc(%ebp) > 4c: eb 11 jmp 5f <_make_string+0x5f> > local_string[i] = c; > 4e: 8b 55 f4 mov -0xc(%ebp),%edx > 51: 8b 45 f0 mov -0x10(%ebp),%eax > 54: 01 d0 add %edx,%eax > 56: 8b 55 08 mov 0x8(%ebp),%edx > 59: 88 10 mov %dl,(%eax) > make_string (int c, size_t n) > { > char *local_string = alloca (n + 1); > int i; > > for (i = 0; i < n; i++) > 5b: 83 45 f4 01 addl $0x1,-0xc(%ebp) > 5f: 8b 45 f4 mov -0xc(%ebp),%eax > 62: 39 45 0c cmp %eax,0xc(%ebp) > 65: 77 e7 ja 4e <_make_string+0x4e> > local_string[i] = c; > local_string[n] = '\0'; > 67: 8b 55 f0 mov -0x10(%ebp),%edx > 6a: 8b 45 0c mov 0xc(%ebp),%eax > 6d: 01 d0 add %edx,%eax > 6f: c6 00 00 movb $0x0,(%eax) > return strdup (local_string); > 72: 8b 45 f0 mov -0x10(%ebp),%eax > 75: 89 04 24 mov %eax,(%esp) > 78: e8 00 00 00 00 call 7d <_make_string+0x7d> > } > 7d: c9 leave > 7e: c3 ret > 7f: 90 nop > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-02-19 17:18 ` Eli Zaretskii 2019-02-28 18:40 ` Eli Zaretskii @ 2019-03-09 19:25 ` Keith Marshall 2019-03-10 19:03 ` Bruno Haible 1 sibling, 1 reply; 15+ messages in thread From: Keith Marshall @ 2019-03-09 19:25 UTC (permalink / raw) To: Eli Zaretskii, Bruno Haible; +Cc: eggert, bug-gnulib [-- Attachment #1.1: Type: text/plain, Size: 2427 bytes --] On 19/02/19 17:18, Eli Zaretskii wrote: >> From: Bruno Haible <bruno@clisp.org> >> Cc: Eli Zaretskii <eliz@gnu.org>, Paul Eggert <eggert@cs.ucla.edu> >> Date: Tue, 19 Feb 2019 00:32:24 +0100 >> >> Hi Eli, >> >>> ./alloca.h:40:18: error: inlining failed in call to always_inline '__builtin_alloca': function not considered for inlining >>> # define alloca __builtin_alloca >>> ^ >> >> GCC never inlines a function that calls alloca() or >> __builtin_alloca(). With respect, while that may be true of ancient historical versions of GCC, my own testing shows it to be untrue for modern releases; I cannot reproduce[1] anything resembling the above diagnostic, unless I resort to archaeological investigation, compiling with mingw32-gcc-3.4.5. The next version, which I still have available, is mingw32-gcc-4.8.2; with that, and with all subsequent releases up to mingw32-gcc-8.2.0, other than at -O0, GCC generates identical code for alloca() expressed as an in-line function, to that which is generated when it is expressed as a macro. (Furthermore, the difference in code generated at -O0 is inconsequential, and will have no effect on behaviour). >> The reason is that if you call this function in a loop, then >> without inlining it will consume a bounded amount of stack whereas >> with inlining it might cause a stack overflow. I'm sorry to say this, but that just seems disingenuous. For every version of GCC which I have available, *including* mingw32-gcc-3.4.5, if I place a call to alloca(), *implemented as a macro*, within a loop, then the stack grows on each iteration of the loop. The growth is not bounded, even for your macro implementation, with mingw32-gcc-3.4.5, but for all versions from mingw32-gcc-4.8.2 onwards it *is* bounded, (by calling __chkstk_ms(), before adjusting the stack pointer), for *both* your macro implementation, and for in-line function expansion. [1] Well, I actually can reproduce it, even with mingw32-gcc-8.2.0, but *only* when I deliberately introduce the very bug which Eli has noted, *and* I compile at -O0; at any other optimization level, this gnulib bug seems to be optimized away, with __builtin_alloca() itself being expanded to in-line intrinsic code, as intended. -- Regards, Keith. Public key available from keys.gnupg.net Key fingerprint: C19E C018 1547 DE50 E1D4 8F53 C0AD 36C6 347E 5A3F [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-03-09 19:25 ` Keith Marshall @ 2019-03-10 19:03 ` Bruno Haible 2019-03-10 19:39 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Bruno Haible @ 2019-03-10 19:03 UTC (permalink / raw) To: Keith Marshall; +Cc: Eli Zaretskii, eggert, bug-gnulib Hi Keith, 1) > >> GCC never inlines a function that calls alloca() or > >> __builtin_alloca(). > > With respect, while that may be true of ancient historical versions of > GCC, my own testing shows it to be untrue for modern releases Indeed. My statement above is true for gcc 2.95.3, but wrong for gcc 3.1 to 8.3.0. Tested with this program: ============================================================= #include <alloca.h> #include <stdio.h> #include <stdlib.h> static __attribute__((__always_inline__)) int square (int x) { alloca (1); return x * x; } int main (int argc, char *argv[]) { int n = atoi (argv[1]); int x = atoi (argv[2]); int i; for (i = 0; i < n; i++) printf ("%d -> %d\n", x, square (x)); return 0; } ============================================================= compiled with "-S -Winline -O -finline-functions foo.c". 2) > >> The reason is that if you call this function in a loop, then > >> without inlining it will consume a bounded amount of stack whereas > >> with inlining it might cause a stack overflow. > > I'm sorry to say this, but that just seems disingenuous. Don't call me "disingenuous", because this is a personal attack. Just because I make a statement based on my understanding of the semantics of inline functions, and you make a different statement based on testing with GCC, that just makes my statement wrong. Don't assume bad intentions!!! > For every > version of GCC which I have available, *including* mingw32-gcc-3.4.5, if > I place a call to alloca(), *implemented as a macro*, within a loop, > then the stack grows on each iteration of the loop. The stack grows at each iteration of the loop, if - alloca is implemented as a macro, - or if alloca is implemented as an inline function AND the compiler is GCC. The stack does NOT grow at each iteration of the loop, if - alloca is implemented as an inline function AND the compiler is clang. Tested with this program: =========================================================== #include <alloca.h> #include <stdio.h> #include <stdlib.h> #ifdef AS_MACRO #define my_alloca(x) __builtin_alloca (x) #else static __attribute__((__always_inline__)) inline char * my_alloca (int x) { return __builtin_alloca (x); } #endif int main (int argc, char *argv[]) { int n = atoi (argv[1]); int i; for (i = 0; i < n; i++) { char *y = my_alloca (1000); printf ("%p\n", y); } return 0; } =========================================================== These invocations show 10 different values for variables on the stack: $ gcc-version 8.3.0 -m32 -Winline -O2 -finline-functions foo.c && ./a.out 10 $ gcc-version 8.3.0 -m32 -Winline -O2 -finline-functions -DAS_MACRO foo.c && ./a.out 10 $ gcc-version 4.2.4 -m32 -Winline -O2 -finline-functions foo.c && ./a.out 10 $ gcc-version 4.2.4 -m32 -Winline -O2 -finline-functions -DAS_MACRO foo.c && ./a.out 10 $ gcc-version 8.3.0 -m32 -Winline -O0 foo.c && ./a.out 10 $ gcc-version 8.3.0 -m32 -Winline -O0 -DAS_MACRO foo.c && ./a.out 10 $ gcc-version 4.2.4 -m32 -Winline -O0 foo.c && ./a.out 10 $ gcc-version 4.2.4 -m32 -Winline -O0 -DAS_MACRO foo.c && ./a.out 10 $ clang -m32 -O0 -DAS_MACRO foo.c && ./a.out 10 $ clang -m32 -O2 -finline-functions -DAS_MACRO foo.c && ./a.out 10 On the other hand: $ clang -m32 -O0 foo.c && ./a.out 10 0xff992010 0xff992010 0xff992010 0xff992010 0xff992010 0xff992010 0xff992010 0xff992010 0xff992010 0xff992010 $ clang -m32 -O2 -finline-functions foo.c && ./a.out 10 0xffeb513c 0xffeb513c 0xffeb513c 0xffeb513c 0xffeb513c 0xffeb513c 0xffeb513c 0xffeb513c 0xffeb513c 0xffeb513c In summary, your inline-function based implementation of alloca works as long as the compiler is GCC. It will break when someone attempts to use clang with mingw. 3) Back to Eli's problem. > [1] Well, I actually can reproduce it, even with mingw32-gcc-8.2.0, but > *only* when I deliberately introduce the very bug which Eli has noted, > *and* I compile at -O0; at any other optimization level, this gnulib bug > seems to be optimized away, with __builtin_alloca() itself being > expanded to in-line intrinsic code, as intended. Thanks for the additional detail that it depends on the optimization level. > The other part of the puzzle is that stdlib.h does this: > > # include "alloca.h" > > so Gnulib's alloca.h is bypassed. So, Gnulib's alloca.h and the system's alloca.h are both included, and the system's alloca.h comes last. To avoid this kind of trouble, we need to make use of '#include_next <alloca.h>'. I think this patch should do it. Can you please review it, Eli? (Since it includes <alloca.h> only when GCC or clang is present, it can assume include_next. Since it can assume include_next, it does not need the absolute file name of the system's <alloca.h>, and therefore it is irrelevant whether the file is empty after preprocessing or not.) diff --git a/lib/alloca.in.h b/lib/alloca.in.h index 8aaf64d..84d92e6 100644 --- a/lib/alloca.in.h +++ b/lib/alloca.in.h @@ -36,6 +36,12 @@ #ifndef alloca # ifdef __GNUC__ + /* Some version of mingw have an <alloca.h> that causes trouble when + included after 'alloca' gets defined as a macro. As a workaround, include + this <alloca.h> first and define 'alloca' as a macro afterwards. */ +# if (defined _WIN32 && ! defined __CYGWIN__) && @HAVE_ALLOCA_H@ +# include_next <alloca.h> +# endif # define alloca __builtin_alloca # elif defined _AIX # define alloca __alloca diff --git a/m4/alloca.m4 b/m4/alloca.m4 index 46d60f9..29bd289 100644 --- a/m4/alloca.m4 +++ b/m4/alloca.m4 @@ -1,4 +1,4 @@ -# alloca.m4 serial 14 +# alloca.m4 serial 15 dnl Copyright (C) 2002-2004, 2006-2007, 2009-2019 Free Software Foundation, dnl Inc. dnl This file is free software; the Free Software Foundation @@ -37,6 +37,13 @@ AC_DEFUN([gl_FUNC_ALLOCA], fi AC_SUBST([ALLOCA_H]) AM_CONDITIONAL([GL_GENERATE_ALLOCA_H], [test -n "$ALLOCA_H"]) + + if test $ac_cv_working_alloca_h = yes; then + HAVE_ALLOCA_H=1 + else + HAVE_ALLOCA_H=0 + fi + AC_SUBST([HAVE_ALLOCA_H]) ]) # Prerequisites of lib/alloca.c. diff --git a/modules/alloca-opt b/modules/alloca-opt index d4468de..53bb28d 100644 --- a/modules/alloca-opt +++ b/modules/alloca-opt @@ -21,7 +21,7 @@ if GL_GENERATE_ALLOCA_H alloca.h: alloca.in.h $(top_builddir)/config.status $(AM_V_GEN)rm -f $@-t $@ && \ { echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \ - cat $(srcdir)/alloca.in.h; \ + sed -e 's|@''HAVE_ALLOCA_H''@|$(HAVE_ALLOCA_H)|g' < $(srcdir)/alloca.in.h; \ } > $@-t && \ mv -f $@-t $@ else ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-03-10 19:03 ` Bruno Haible @ 2019-03-10 19:39 ` Eli Zaretskii 2019-03-10 19:54 ` Bruno Haible 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2019-03-10 19:39 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib, eggert, keith > From: Bruno Haible <bruno@clisp.org> > Cc: Eli Zaretskii <eliz@gnu.org>, bug-gnulib@gnu.org, eggert@cs.ucla.edu > Date: Sun, 10 Mar 2019 20:03:33 +0100 > > So, Gnulib's alloca.h and the system's alloca.h are both included, and > the system's alloca.h comes last. To avoid this kind of trouble, we > need to make use of '#include_next <alloca.h>'. I think this patch > should do it. Can you please review it, Eli? It LGTM, since I already succeeded to build that package with this: > +# if (defined _WIN32 && ! defined __CYGWIN__) && @HAVE_ALLOCA_H@ replaced by just "#if HAVE_ALLOCA_H", and because the 2 additional conditions you propose are obviously true for MinGW. > + if test $ac_cv_working_alloca_h = yes; then > + HAVE_ALLOCA_H=1 > + else > + HAVE_ALLOCA_H=0 > + fi > + AC_SUBST([HAVE_ALLOCA_H]) > ]) This cannot hurt, but in the case in point the package was already testing for alloca.h, and defined HAVE_ALLOCA_H by itself. Thanks. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-03-10 19:39 ` Eli Zaretskii @ 2019-03-10 19:54 ` Bruno Haible 2019-03-10 20:00 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Bruno Haible @ 2019-03-10 19:54 UTC (permalink / raw) To: Eli Zaretskii; +Cc: bug-gnulib, eggert, keith Eli Zaretskii wrote: > It LGTM, since I already succeeded to build that package with this: Thanks for the confirmation. > > +# if (defined _WIN32 && ! defined __CYGWIN__) && @HAVE_ALLOCA_H@ > > replaced by just "#if HAVE_ALLOCA_H" Right, AC_FUNC_ALLOCA already defines HAVE_ALLOCA_H as a preprocessor macro, under the same conditions. But we have the habit, when we create a .h file from a .in.h file, to substitute values found at configure time directly. This - makes it possible to reference the same variables in the module description, - increases the probability that the .h file works even when the user has forgotten to #include <config.h> or adds or removes a macro definition through #define or #undef, - increases transparency in case of a compilation error. Pushed. Bruno ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-03-10 19:54 ` Bruno Haible @ 2019-03-10 20:00 ` Eli Zaretskii 0 siblings, 0 replies; 15+ messages in thread From: Eli Zaretskii @ 2019-03-10 20:00 UTC (permalink / raw) To: Bruno Haible; +Cc: bug-gnulib, eggert, keith > From: Bruno Haible <bruno@clisp.org> > Cc: keith@users.osdn.me, bug-gnulib@gnu.org, eggert@cs.ucla.edu > Date: Sun, 10 Mar 2019 20:54:20 +0100 > > Pushed. Thank you. And thanks to Keith for his thorough research. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Gnulib's alloca.h used even when there is a system header 2019-02-17 17:20 Gnulib's alloca.h used even when there is a system header Eli Zaretskii 2019-02-17 20:06 ` Paul Eggert @ 2019-02-17 23:28 ` Bruno Haible 1 sibling, 0 replies; 15+ messages in thread From: Bruno Haible @ 2019-02-17 23:28 UTC (permalink / raw) To: bug-gnulib; +Cc: Eli Zaretskii Hi Eli, > Building the latest version of Texinfo, I see that Gnulib decides to > generate its own alloca.h header even though there's a system header > alloca.h (and HAVE_ALLOCA_H is 1 in config.h). Why does it do that? Generally speaking, we (gnulib developers) put effort into generating zero code in the object files on platforms which implement the POSIX standard plus the desired GNU extensions. We don't put as much effort into using avoiding a gnulib-generated .h file because - whether the gnulib-generated .h file exists or not, the code size in the generated binaries is the same, - when we have extra code that decides when to omit the .h file, we have to update it each time we add a new workaround for one of the types/macros/functions defined in this .h file. In other words, it costs maintenance effort and provides little gain (except for purists). > Isn't that dangerous? e.g., it could conflict with how the system > header defies 'alloca', no? One of the problems the current code avoids is that on old AIX versions, <alloca.h> had a #pragma which was only effective when <alloca.h> was the first system include file to get included. This was a very annoying restriction (must always include <alloca.h> first, after <config.h>), and IIRC this is one of the things that the current gnulib code fixes. Bruno ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-03-10 20:00 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-17 17:20 Gnulib's alloca.h used even when there is a system header Eli Zaretskii 2019-02-17 20:06 ` Paul Eggert 2019-02-17 20:24 ` Eli Zaretskii 2019-02-17 21:09 ` Paul Eggert 2019-02-17 23:32 ` Bruno Haible 2019-02-18 16:59 ` Eli Zaretskii 2019-02-18 23:32 ` Bruno Haible 2019-02-19 17:18 ` Eli Zaretskii 2019-02-28 18:40 ` Eli Zaretskii 2019-03-09 19:25 ` Keith Marshall 2019-03-10 19:03 ` Bruno Haible 2019-03-10 19:39 ` Eli Zaretskii 2019-03-10 19:54 ` Bruno Haible 2019-03-10 20:00 ` Eli Zaretskii 2019-02-17 23:28 ` Bruno Haible
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).