* 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 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
* 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
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).