Re: Gnulib's alloca.h used even when there is a system header

[Eli Zaretskii]

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 <address@hidden>
> CC: address@hidden, address@hidden,
>       Keith Marshall <address@hidden>
> 
> > From: Bruno Haible <address@hidden>
> > Cc: Eli Zaretskii <address@hidden>, Paul Eggert <address@hidden>
> > 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
> 
>