[Bug c/113596] New: Stack memory leakage caused by inline alloca

[Bug c/113596] New: Stack memory leakage caused by inline alloca

[sanpeqf at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

            Bug ID: 113596
           Summary: Stack memory leakage caused by inline alloca
           Product: gcc
           Version: 8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sanpeqf at gmail dot com
  Target Milestone: ---

I discovered this issue while using inline alloca. I have tried changing to a
different version of gcc and found that this issue was introduced in gcc 8.0. I
have also tried changing to different architectures (on aarch64 and x86) and
can reproduce this issue.

The following is the code that triggers the error:

#include <string.h>

static inline __attribute__((always_inline))
int test_alloca(void)
{
    void *block;

    block = __builtin_alloca(0x100);
    memset(block, 0, 0x100);
    (void)block;

    return 0;
}

int main(int argc, const char *argv[])
{
    for (;;)
        test_alloca();
    return 0;
}

The following tests were conducted on this machine:

# gcc issue.c
# ./a.out
Segmentation fault (core dumped)

# gcc -fsanitize=address issue.c
# ./a.out
AddressSanitizer:DEADLYSIGNAL
=================================================================
==1108774==ERROR: AddressSanitizer: stack-overflow on address 0x7ffe6b152f78
(pc 0x7fd7bfa5a8ab bp 0x7ffe6b1537d0 sp 0x7ffe6b152f80 T0)
    #0 0x7fd7bfa5a8ab in memset (/lib64/libasan.so.8+0x5a8ab) (BuildId:
7fcb7759bc17ef47f9682414b6d99732d6a6ab0c)
    #1 0x4011c8 in main (/disk/files/workspace/gcc-issue/a.out+0x4011c8)
(BuildId: 66d839c602ac12950f3212be978c5c471e7c06e3)
    #2 0x7fd7bf846149 in __libc_start_call_main (/lib64/libc.so.6+0x28149)
(BuildId: 788cdd41a15985bf8e0a48d213a46e07d58822df)
    #3 0x7fd7bf84620a in __libc_start_main_impl (/lib64/libc.so.6+0x2820a)
(BuildId: 788cdd41a15985bf8e0a48d213a46e07d58822df)
    #4 0x401094 in _start (/disk/files/workspace/gcc-issue/a.out+0x401094)
(BuildId: 66d839c602ac12950f3212be978c5c471e7c06e3)

SUMMARY: AddressSanitizer: stack-overflow (/lib64/libasan.so.8+0x5a8ab)
(BuildId: 7fcb7759bc17ef47f9682414b6d99732d6a6ab0c) in memset
==1108774==ABORTING

# gcc -v
...
gcc version 13.2.1 20231205 (Red Hat 13.2.1-6) (GCC)

# objdump -d ./a.out
...
0000000000401126 <main>:
main():
  401126:       55                      push   %rbp
  401127:       48 89 e5                mov    %rsp,%rbp
  40112a:       48 83 ec 20             sub    $0x20,%rsp
  40112e:       89 7d ec                mov    %edi,-0x14(%rbp)
  401131:       48 89 75 e0             mov    %rsi,-0x20(%rbp)
  401135:       b8 10 00 00 00          mov    $0x10,%eax
  40113a:       48 83 e8 01             sub    $0x1,%rax
  40113e:       48 05 08 01 00 00       add    $0x108,%rax
  401144:       b9 10 00 00 00          mov    $0x10,%ecx
  401149:       ba 00 00 00 00          mov    $0x0,%edx
  40114e:       48 f7 f1                div    %rcx
  401151:       48 6b c0 10             imul   $0x10,%rax,%rax
  401155:       48 29 c4                sub    %rax,%rsp
  401158:       48 89 e0                mov    %rsp,%rax
  40115b:       48 83 c0 0f             add    $0xf,%rax
  40115f:       48 c1 e8 04             shr    $0x4,%rax
  401163:       48 c1 e0 04             shl    $0x4,%rax
  401167:       48 89 45 f8             mov    %rax,-0x8(%rbp)
  40116b:       48 8b 45 f8             mov    -0x8(%rbp),%rax
  40116f:       ba 00 01 00 00          mov    $0x100,%edx
  401174:       be 00 00 00 00          mov    $0x0,%esi
  401179:       48 89 c7                mov    %rax,%rdi
  40117c:       e8 af fe ff ff          call   401030 <memset@plt>
  401181:       90                      nop
  401182:       eb b1                   jmp    401135 <main+0xf>

[Bug middle-end/113596] Stack memory leakage caused by inline alloca

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Why do you think this is a bug?

You are forcing a function which calls alloca to be inlined, GCC DOES not
normally inline functions which call alloca due to not restoring the stack
pointer after the return of the inlined function.

[Bug middle-end/113596] Stack memory leakage caused by inline alloca

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
You could instead use VLA which is done correctly though.

[Bug middle-end/113596] Stack memory leakage caused by inline alloca

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |documentation

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The documentation should be more clear on this though.

[Bug middle-end/113596] Stack memory leakage caused by inline alloca

[sanpeqf at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

--- Comment #4 from John Sanpe <sanpeqf at gmail dot com> ---
(In reply to Andrew Pinski from comment #1)
> Why do you think this is a bug?
> 
> You are forcing a function which calls alloca to be inlined, GCC DOES not
> normally inline functions which call alloca due to not restoring the stack
> pointer after the return of the inlined function.

I think most people's logic should be that exiting the function will release
the memory of alloca, and clang does the same.

[Bug middle-end/113596] Stack memory leakage caused by inline alloca

[sanpeqf at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

--- Comment #5 from John Sanpe <sanpeqf at gmail dot com> ---
(In reply to Andrew Pinski from comment #3)
> The documentation should be more clear on this though.

But adding a hint would also be reasonable

[Bug middle-end/113596] Stack memory leakage caused by inline alloca

[sanpeqf at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

John Sanpe <sanpeqf at gmail dot com> changed:

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

--- Comment #6 from John Sanpe <sanpeqf at gmail dot com> ---
Resolved

[Bug middle-end/113596] Stack memory leakage caused by inline alloca

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
In theory, if somebody really wanted it, we could replace alloca with
__builtin_stack_save/restore during inlining (not sure if it would
simply work, and be efficient, by just putting save at the start of the
function and restore at the end).

We could also warn when (forced-)inlining a function calling alloca.

[Bug middle-end/113596] Stack memory leakage caused by inline alloca

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> In theory, if somebody really wanted it, we could replace alloca with
> __builtin_stack_save/restore during inlining (not sure if it would
> simply work, and be efficient, by just putting save at the start of the
> function and restore at the end).

Not replace, just emit __builtin_stack_save/restore around any inlined function
which calls alloca.  That looks like a good idea to me.
I'll have a look, perhaps we should do it for now just without further changes,
but in the future might also reconsider making functions which call alloca
non-inlinable.

[Bug middle-end/113596] Stack memory leakage caused by inline alloca

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 57215
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57215&action=edit
gcc14-pr113596.patch

Untested patch to do that.
The disadvantage of doing that is that it may penalize inline calls which just
use VLAs, because calls_alloca covers even those functions.  For simple:
static inline __attribute__((always_inline)) void
foo (int n)
{
  char p[n];
  bar (p, n);
}
the fab1 pass actually removes redundant pair of stack_save/stack_restore, but
bet if it would be something like { call (); { char p[n]; bar (p, n); } call
(); } then it wouldn't.
Anyway, this isn't a regression, so I think it is stage1 material for GCC 15.

[Bug middle-end/113596] Stack memory leakage caused by inline alloca

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #9)
> Created attachment 57215 [details]
> gcc14-pr113596.patch
> 
> Untested patch to do that.
> The disadvantage of doing that is that it may penalize inline calls which
> just use VLAs, because calls_alloca covers even those functions.  For simple:
> static inline __attribute__((always_inline)) void
> foo (int n)
> {
>   char p[n];
>   bar (p, n);
> }
> the fab1 pass actually removes redundant pair of stack_save/stack_restore,
> but
> bet if it would be something like { call (); { char p[n]; bar (p, n); } call
> (); } then it wouldn't.
> Anyway, this isn't a regression, so I think it is stage1 material for GCC 15.

Most definitely.  We can make ->calls_alloca more precise though of course
we usually also do not want to inline functions with VLAs.  IIRC a VLA
forces a frame pointer for the caller then.

[Bug middle-end/113596] Stack memory leakage caused by inline alloca

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
In reply to Richard Biener from comment #10)
> We can make ->calls_alloca more precise though of course
> we usually also do not want to inline functions with VLAs.

Yes.  Or remember while inlining whether we've seen a non-VLA alloca call in
the id structure and then just emit both calls afterwards to the corresponding
blocks.

> IIRC a VLA forces a frame pointer for the caller then.

The patch doesn't change whether to inline such functions or not, that would be
for incremental discussions.  Supposedly sometimes it could be worth it even
when needing a frame pointer because of that, but it should be taken into
account in the cost heuristics or something.

[Bug middle-end/113596] Stack memory leakage caused by inline alloca

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113596

--- Comment #12 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:7117e1f6bf6de25c1ff26c4d7abcc79b407ca221

commit r15-125-g7117e1f6bf6de25c1ff26c4d7abcc79b407ca221
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri May 3 09:44:30 2024 +0200

    tree-inline: Add __builtin_stack_{save,restore} pair about inline calls
with calls to alloca [PR113596]

    The following patch adds save_NNN = __builtin_stack_save (); ...
    __builtin_stack_restore (save_NNN);
    pair around inline calls which call alloca (alloca calls because of
    VLA vars are ignored in that decision).
    The patch doesn't change anything on whether we try to inline such calls or
    not, it just fixes the behavior when we inline them despite those checks.
    The stack save/restore restores the behavior that alloca acquired regions
    are freed at the end of the containing call.

    2024-05-03  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/113596
            * tree-inline.cc (expand_call_inline): Emit __builtin_stack_save
            and __builtin_stack_restore calls around inlined functions which
            call alloca.

            * gcc.dg/pr113596.c: New test.
            * gcc.dg/tree-ssa/pr113596.c: New test.