[Bug target/91779] New: [9 regression] Unbalanced stack manipulation

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] New: [9 regression] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

            Bug ID: 91779
           Summary: [9 regression] Unbalanced stack manipulation
           Product: gcc
           Version: 9.2.1
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: [hidden email]
  Target Milestone: ---
            Target: i586-*-*

Created attachment 46887
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46887&action=edit
Preprocessed input

When compiled with -m32 -O2, the first call to _fprintf in main

    _fprintf(out,"->{'%c'}\n",Cr.x);

saves the format string on the stack at $fp-100:

        pushl   %eax
        leal    .LC164@GOTOFF(%ebx), %eax
        pushl   %eax
        pushl   (%esi)
        movl    %eax, -100(%ebp)
        call    _fprintf

This is then reused by the second call:

        pushl   %eax
        pushl   -100(%ebp)
        pushl   (%esi)
        call    _fprintf

but here pushl %eax overwrites the value in $fp-100.

The bug appears to be here (after first call to fflush):

        pushl   (%esi)
        call    fflush@PLT
        leal    -84(%ebp), %eax
        popl    %edx
        popl    %ecx

The second popl is causing a stack frame underflow.
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

Andreas Schwab <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[9 regression] Unbalanced   |Unbalanced stack
                   |stack manipulation          |manipulation
      Known to fail|                            |7.4.1, 9.2.1

--- Comment #1 from Andreas Schwab <[hidden email]> ---
Not a regression, also fails with 7.4.1.
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

--- Comment #2 from Uroš Bizjak <ubizjak at gmail dot com> ---
Created attachment 46892
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46892&action=edit
libffi.so.7

Needed shared library to run the test.
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

--- Comment #3 from Uroš Bizjak <ubizjak at gmail dot com> ---
The test needs -fpie and attached libffi.so.7 (plus a symbolic link from
libffi.so to libffi.so.7).

$gcc -O2 -m32 -fpie -L. -lffi pr91779.i
$ LD_LIBRARY_PATH=. ./a.out
Char f(Char,double,Char):({'A'},0.2,{'C'})->{'B'}
Segmentation fault
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

--- Comment #4 from Uroš Bizjak <ubizjak at gmail dot com> ---
As explained in the #Comment 0, we have the following situation in main:

        leal    .LC164@GOTOFF(%ebx), %eax       # 32    [c=5 l=6]  *leasi
        pushl   %eax    # 33    [c=4 l=1]  *pushsi2
        pushl   (%esi)  # 35    [c=4 l=2]  *pushsi2
        movl    %eax, -100(%ebp)        # 159   [c=4 l=3]  *movsi_internal/1
        call    _fprintf        # 36    [c=9 l=5]  *call_value

...

        pushl   %eax    # 123   [c=4 l=1]  *pushsi2
        pushl   -100(%ebp)      # 125   [c=8 l=3]  *pushsi2
        pushl   (%esi)  # 127   [c=4 l=2]  *pushsi2
        call    _fprintf        # 128   [c=9 l=5]  *call_value

where (insn 159) pushes value on the stack, which is later read in (insn 125).

Before LRA, we have the following sequence:

_.ira:

(insn 32 31 33 2 (set (reg/f:SI 116)
        (plus:SI (reg:SI 82)
            (const:SI (unspec:SI [
                        (symbol_ref/f:SI ("*.LC164") [flags 0x2]  <var_decl
0x7fee8274b990 *.LC164>)
                    ] UNSPEC_GOTOFF))))
"../../testsuite/libffi.bhaible/test-callback.c":2310:5 186 {*leasi}
     (expr_list:REG_EQUIV (symbol_ref/f:SI ("*.LC164") [flags 0x2]  <var_decl
0x7fee8274b990 *.LC164>)
        (nil)))
(insn 33 32 35 2 (set (mem/f:SI (pre_dec:SI (reg/f:SI 7 sp)) [2  S4 A32])
        (reg/f:SI 116)) "../../testsuite/libffi.bhaible/test-callback.c":2310:5
43 {*pushsi2}
     (expr_list:REG_ARGS_SIZE (const_int 28 [0x1c])
        (nil)))
(insn 35 33 36 2 (set (mem/f:SI (pre_dec:SI (reg/f:SI 7 sp)) [3  S4 A32])
        (mem/f/c:SI (reg/f:SI 109) [3 out+0 S4 A32]))
"../../testsuite/libffi.bhaible/test-callback.c":2310:5 43 {*pushsi2}
     (expr_list:REG_ARGS_SIZE (const_int 32 [0x20])
        (nil)))
(call_insn 36 35 37 2 (set (reg:SI 0 ax)
        (call (mem:QI (symbol_ref:SI ("_fprintf") [flags 0x3]  <function_decl
0x7fee8398d500 _fprintf>) [0 _fprintf S1 A8])
            (const_int 32 [0x20])))
"../../testsuite/libffi.bhaible/test-callback.c":2310:5 676 {*call_value}
     (expr_list:REG_UNUSED (reg:SI 0 ax)
        (expr_list:REG_CALL_DECL (symbol_ref:SI ("_fprintf") [flags 0x3]
<function_decl 0x7fee8398d500 _fprintf>)
            (expr_list:REG_EH_REGION (const_int 0 [0])
                (nil))))
    (nil))

...

(insn 123 122 125 6 (set (mem:SI (pre_dec:SI (reg/f:SI 7 sp)) [1  S4 A32])
        (reg:SI 136 [ Cr ]))
"../../testsuite/libffi.bhaible/test-callback.c":2328:5 43 {*pushsi2}
     (expr_list:REG_DEAD (reg:SI 136 [ Cr ])
        (expr_list:REG_ARGS_SIZE (const_int 8 [0x8])
            (nil))))
(insn 125 123 127 6 (set (mem/f:SI (pre_dec:SI (reg/f:SI 7 sp)) [2  S4 A32])
        (reg/f:SI 116)) "../../testsuite/libffi.bhaible/test-callback.c":2328:5
43 {*pushsi2}
     (expr_list:REG_DEAD (reg/f:SI 116)
        (expr_list:REG_ARGS_SIZE (const_int 12 [0xc])
            (nil))))
(insn 127 125 128 6 (set (mem/f:SI (pre_dec:SI (reg/f:SI 7 sp)) [3  S4 A32])
        (mem/f/c:SI (reg/f:SI 109) [3 out+0 S4 A32]))
"../../testsuite/libffi.bhaible/test-callback.c":2328:5 43 {*pushsi2}
     (expr_list:REG_ARGS_SIZE (const_int 16 [0x10])
        (nil)))
(call_insn 128 127 129 6 (set (reg:SI 0 ax)
        (call (mem:QI (symbol_ref:SI ("_fprintf") [flags 0x3]  <function_decl
0x7fee8398d500 _fprintf>) [0 _fprintf S1 A8])
            (const_int 16 [0x10])))
"../../testsuite/libffi.bhaible/test-callback.c":2328:5 676 {*call_value}
     (expr_list:REG_UNUSED (reg:SI 0 ax)
        (expr_list:REG_CALL_DECL (symbol_ref:SI ("_fprintf") [flags 0x3]
<function_decl 0x7fee8398d500 _fprintf>)
            (expr_list:REG_EH_REGION (const_int 0 [0])
                (nil))))
    (nil))

Please note (reg 116), which is kept alive all the way to (insn 127), where it
is pushed on stack.

LRA pass stores the value in (reg 116) to a frame with:

(insn 159 32 33 2 (set (mem/c:SI (plus:SI (reg/f:SI 6 bp)
                (const_int -100 [0xffffffffffffff9c])) [35 %sfp+-76 S4 A32])
        (reg/f:SI 0 ax [116]))
"../../testsuite/libffi.bhaible/test-callback.c":2310:5 67 {*movsi_internal}
     (nil))

and substitutes (reg 116) in (insn 125) with a new memory location:

(insn 125 123 127 7 (set (mem/f:SI (pre_dec:SI (reg/f:SI 7 sp)) [2  S4 A32])
        (mem/c:SI (plus:SI (reg/f:SI 6 bp)
                (const_int -100 [0xffffffffffffff9c])) [35 %sfp+-76 S4 A32]))
"../../testsuite/libffi.bhaible/test-callback.c":2328:5 43 {*pushsi2}
     (expr_list:REG_ARGS_SIZE (const_int 12 [0xc])
        (nil)))

The frame is correctly constructed using:

        subl    $100, %esp      # 185   [c=4 l=3]
pro_epilogue_adjust_stack_si_add/0

It looks to me that LRA creates too small frame, perhpas it doesn't account for
preceeding (insn 123) for some reason.
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

--- Comment #5 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Uroš Bizjak from comment #4)

> Please note (reg 116), which is kept alive all the way to (insn 127), where
> it is pushed on stack.

... all the way to (insn 125), ...
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

--- Comment #6 from Uroš Bizjak <ubizjak at gmail dot com> ---
Some debugging & tracing throug main shows the following:

There is an indirect call where we enter with stack pointer 0xffffce60, and
return with a stack pointer of 0xffffce80. Later, pushl misaligns the stack
pointer to 0xffffce7c.

        movzbl  C1@GOTOFF(%ebx), %eax   # 111   [c=9 l=7]  *movqi_internal/4
        pushl   4+d2@GOTOFF(%ebx)       # 166   [c=9 l=6]  *pushsi2
        pushl   d2@GOTOFF(%ebx) # 167   [c=9 l=6]  *pushsi2
        pushl   %eax    # 112   [c=4 l=1]  *pushqi2
        pushl   -96(%ebp)       # 113   [c=8 l=3]  *pushsi2
ce60    call    *-84(%ebp)      # 115   [c=4 l=3]  *call_pop
ce80    movzbl  -89(%ebp), %eax # 116   [c=8 l=4]  *movqi_internal/4
        pushl   %edi    # 118   [c=4 l=1]  *pushsi2
ce7c    movb    %al, -96(%ebp)  # 157   [c=4 l=3]  *movqi_internal/8
        call    ffi_closure_free@PLT    # 120   [c=0 l=5]  *call
ce7c    movsbl  -96(%ebp), %eax # 122   [c=12 l=4]  extendqisi2

The offending indirect call is defined as:

(call_insn 115 114 116 7 (parallel [
            (call (mem:QI (mem/f/c:SI (plus:SI (reg/f:SI 6 bp)
                            (const_int -84 [0xffffffffffffffac])) [23
callback_code+0 S4 A32]) [0 *callback_code.865_10 S1 A8])
                (const_int 32 [0x20]))
            (set (reg/f:SI 7 sp)
                (plus:SI (reg/f:SI 7 sp)
                    (const_int 20 [0x14])))
        ]) "../../testsuite/libffi.bhaible/test-callback.c":2325:13 673
{*call_pop}
     (expr_list:REG_ARGS_SIZE (const_int 12 [0xc])
        (expr_list:REG_CALL_DECL (nil)
            (nil)))
    (nil))

So, the function should pop 20 bytes from the stack, which is not the case.
According to the debug session, it pops 32 bytes.

The indirectly called function is:

ffi_closure_STDCALL () from ./libffi.so.7

So, the above function misaligns the stack.
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

Uroš Bizjak <ubizjak at gmail dot com> changed:

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

--- Comment #7 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Uroš Bizjak from comment #6)
> The indirectly called function is:
>
> ffi_closure_STDCALL () from ./libffi.so.7
>
> So, the above function misaligns the stack.

Not our problem.
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

Andreas Schwab <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
   Last reconfirmed|                            |2019-09-17
         Resolution|INVALID                     |---
     Ever confirmed|0                           |1

--- Comment #8 from Andreas Schwab <[hidden email]> ---
Nope, don't use a broken libffi.
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

Andreas Schwab <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #46892|0                           |1
        is obsolete|                            |

--- Comment #9 from Andreas Schwab <[hidden email]> ---
Created attachment 46893
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46893&action=edit
libfi.so.7
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

--- Comment #10 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Andreas Schwab from comment #9)
> Created attachment 46893 [details]
> libfi.so.7

This attached library has exactly the same problem.

Put a breakpoint at:

   0x08049265 <+341>:   pushl  -0x60(%ebp)
*  0x08049268 <+344>:   call   *-0x54(%ebp)
   0x0804926b <+347>:   movzbl -0x59(%ebp),%eax

then:

Breakpoint 1, 0x08049268 in main ()
(gdb) p $sp
$1 = (void *) 0xffffce60
(gdb) ni
Char f(Char,double,Char):({'A'},0.2,{'C'})->{'B'}
0x0804926b in main ()
(gdb) p $sp
$2 = (void *) 0xffffce80

The called STDCALL function should adjust stack by 0x14 here, this is what
compiler claims:

(call_insn 115 114 116 7 (parallel [
            (call (mem:QI (mem/f/c:SI (plus:SI (reg/f:SI 6 bp)
                            (const_int -84 [0xffffffffffffffac])) [23
callback_code+0 S4 A32]) [0 *callback_code.865_10 S1 A8])
                (const_int 32 [0x20]))
            (set (reg/f:SI 7 sp)
                (plus:SI (reg/f:SI 7 sp)
                    (const_int 20 [0x14])))
        ]) "../../testsuite/libffi.bhaible/test-callback.c":2325:13 673
{*call_pop}
     (expr_list:REG_ARGS_SIZE (const_int 12 [0xc])
        (expr_list:REG_CALL_DECL (nil)
            (nil)))
    (nil))
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

--- Comment #11 from Uroš Bizjak <ubizjak at gmail dot com> ---
Also:

Breakpoint 1, 0x08049268 in main ()
(gdb) ni
Char f(Char,double,Char):({'A'},0.2,{'C'})->{'B'}
0x0804926b in main ()
(gdb) p $sp
$5 = (void *) 0xffffce80
(gdb) set $sp = 0xffffce74
(gdb) c
Continuing.
Char f(Char,double,Char):({'A'},0.2,{'C'})->{'B'}
[Inferior 1 (process 7985) exited normally]
Reply | Threaded
Open this post in threaded view
|

[Bug target/91779] Unbalanced stack manipulation

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91779

Andreas Schwab <[hidden email]> changed:

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

--- Comment #12 from Andreas Schwab <[hidden email]> ---
Ok, I was fooled by the extra insns for stack alignment.