[Bug target/35124] New: Method _alloca is defined different by MSVCRT as by gcc.

[Bug target/35124] New: Method _alloca is defined different by MSVCRT as by gcc.

[ktietz at gcc dot gnu dot org]

The method _alloca assemble implementation is written that way, that it does
not return the stack pointer. I just reserves and probes the stack space.
The MSVCRT variant declares it as a synonym for POSIX alloca().
Here are three problems by the gcc variant:
1) The stack pointer isn't returned in (eax/rax).
2) If the _alloca function is called by a value not equal to the stack
boundary,
  the stack pointer (esp/rsp) gets unaligned and OS will raise an exception for
  the next instruction pushing to the stack.
3) For target x86_64-pc-mingw32 the calling convention is treated correctly.
  For ms abi, the argument gets reserved room on stack, too. Like for the 32
  bit variant.

For the x86_64-pc-mingw32 target I attached a patch for this.
Index: gcc/gcc/config/i386/cygwin.asm
===================================================================
--- gcc.orig/gcc/config/i386/cygwin.asm
+++ gcc/gcc/config/i386/cygwin.asm
@@ -72,15 +72,44 @@ Ldone:
        pushl   %eax
        ret
 #else
-/* __alloca is a normal function call, which uses %rcx as the argument.  */
+/* __alloca is a normal function call, which uses %rcx as the argument.  And
stack space
+   for the argument is saved.  */
 __alloca:
        movq    %rcx, %rax
-       /* FALLTHRU */
+       addq    $0x7, %rax
+       andq    $0xfffffffffffffff8, %rax
+       popq    %rcx            /* pop return address */
+       popq    %r10            /* Pop the reserved stack space.  */
+       movq    %rsp, %r10      /* get sp */
+       cmpq    $0x1000, %rax   /* > 4k ?*/
+       jb      Ldone_alloca
+
+Lprobe_alloca:
+       subq    $0x1000, %r10           /* yes, move pointer down 4k*/
+       orq     $0x0, (%r10)            /* probe there */
+       subq    $0x1000, %rax           /* decrement count */
+       cmpq    $0x1000, %rax
+       ja      Lprobe_alloca                   /* and do it again */
+
+Ldone_alloca:
+       subq    %rax, %r10
+       orq     $0x0, (%r10)    /* less than 4k, just peek here */
+       movq    %r10, %rax
+       subq    $0x8, %r10      /* Reserve argument stack space.  */
+       movq    %r10, %rsp      /* decrement stack */
+
+       /* Push the return value back.  Doing this instead of just
+          jumping to %rcx preserves the cached call-return stack
+          used by most modern processors.  */
+       pushq   %rcx
+       ret

 /* ___chkstk is a *special* function call, which uses %rax as the argument.
    We avoid clobbering the 4 integer argument registers, %rcx, %rdx, 
    %r8 and %r9, which leaves us with %rax, %r10, and %r11 to use.  */
 ___chkstk:
+       addq    $0x7, %rax      /* Make sure stack is on alignment of 8.  */
+       andq    $0xfffffffffffffff8, %rax
        popq    %r11            /* pop return address */
        movq    %rsp, %r10      /* get sp */
        cmpq    $0x1000, %rax   /* > 4k ?*/


-- 
           Summary: Method _alloca is defined different by MSVCRT as by gcc.
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: critical
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: ktietz at gcc dot gnu dot org
 GCC build triplet: *-*-mingw32
  GCC host triplet: *-*-mingw32
GCC target triplet: *-*-mingw32


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35124

[Bug target/35124] Method _alloca is defined different by MSVCRT as by gcc.

[nightstrike at gmail dot com]

------- Comment #1 from nightstrike at gmail dot com  2008-02-12 02:39 -------
http://gcc.gnu.org/ml/gcc-patches/2008-02/msg00350.html


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35124

[Bug target/35124] Method _alloca is defined different by MSVCRT as by gcc.

[nightstrike at gmail dot com]

------- Comment #2 from nightstrike at gmail dot com  2008-02-14 01:17 -------
This bug needs to be finished off before 4.3.0 closes...


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35124

[Bug target/35124] Method _alloca is defined different by MSVCRT as by gcc.

[pinskia at gcc dot gnu dot org]

------- Comment #3 from pinskia at gcc dot gnu dot org  2008-02-14 01:20 -------
(In reply to comment #2)
> This bug needs to be finished off before 4.3.0 closes...

Why?  it has been a bug in GCC for a while now.  And x86_64-pc-mingw32 is new.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|critical                    |normal


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35124

[Bug target/35124] Method _alloca is defined different by MSVCRT as by gcc.

[dannysmith at users dot sourceforge dot net]

------- Comment #4 from dannysmith at users dot sourceforge dot net  2008-02-14 01:43 -------
Actually, I see this as unfortunate choice of name for the undocumented
__alloca label rather than as a serious bug. 

__alloca is an internal symbol that should have been named _alloca_probe for
consistency with MSVCT.    

Certainly it is not a regression.  It has been that way since 1996.  The mingw
header malloc.h  does this to get a MSVCRT-consistent alloca:
#ifdef __GNUC__
#define _alloca(x) __builtin_alloca((x))
#endif 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35124

[Bug target/35124] Method _alloca is defined different by MSVCRT as by gcc.

[dannysmith at users dot sourceforge dot net]

------- Comment #5 from dannysmith at users dot sourceforge dot net  2008-02-14 08:14 -------
And just think of the havoc that will be caused with old mingw32 and cygwin
libs that depend on the _chkstk meaning of __alloca if you change __alloca
semantics.
eg there are 69 undefined references to __alloca in  mingw32-gcc-4.2.1 built
libgmp.a 

In my opinion it is a bug that 64 bit mingw defines __alloca differently to 32
bit mingw.  


Danny



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35124

[Bug target/35124] Method _alloca is defined different by MSVCRT as by gcc.

[ktietz at gcc dot gnu dot org]

------- Comment #6 from ktietz at gcc dot gnu dot org  2008-02-14 09:00 -------
I agree, that the havoc for 32-bit backward compatibility is to avoid.
But the havoc for windows sources using -fno-builtin and using _alloca () for
stack allocation produces in future even more troubles IMHO.

We allready defined the macro _alloca as 32-bit mingw does. The next source
tarball will inherit this patch.

But if we simply add a return type in eax for 32-bit world, does this really
harm anybody?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35124

[Bug target/35124] Method _alloca is defined different by MSVCRT as by gcc.

[dannysmith at users dot sourceforge dot net]

------- Comment #7 from dannysmith at users dot sourceforge dot net  2008-02-14 17:46 -------
(In reply to comment #6)
> I agree, that the havoc for 32-bit backward compatibility is to avoid.
> But the havoc for windows sources using -fno-builtin and using _alloca () for
> stack allocation produces in future even more troubles IMHO.

Why should windows sources need to use __alloca.

If someone really wants an MSCV compatible (one underscore)  _alloca they just
add this to their srcs
void *_alloca (size_t size) {return __builtin_alloca (size));


MSDN documentation says that "This function is deprecated because a more secure
version is available; see _malloca."


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35124

[Bug target/35124] Method _alloca is defined different by MSVCRT as by gcc.

[dannysmith at users dot sourceforge dot net]

------- Comment #8 from dannysmith at users dot sourceforge dot net  2008-02-14 20:10 -------
(In reply to comment #7)

> If someone really wants an MSCV compatible (one underscore)  _alloca they just
> add this to their srcs
> void *_alloca (size_t size) {return __builtin_alloca (size));
> 
Ugh. Sorry, I wasn't thinking clearly. Currently, that will lead to circular
reference.  I'll change gen_allocate_stack_worker to use __chkstk for 32-bits
when 4.4. opens.
Just #define _alloca __builtin_alloca,  That isn't affected by -fno-builtin
anyway
Danny

> 
> MSDN documentation says that "This function is deprecated because a more secure
> version is available; see _malloca."
> 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35124

[Bug target/35124] Method _alloca is defined different by MSVCRT as by gcc.

[ktietz at gcc dot gnu dot org]

------- Comment #9 from ktietz at gcc dot gnu dot org  2009-09-01 16:20 -------
As the initial reason of this bug is solved, I close it. In fact is the
__chkstk function here incompatible to VC version, but this should be part of a
feature request.


-- 

ktietz at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35124