Bug 40718 - Invalid code produced with -foptimize-sibling-calls
Summary: Invalid code produced with -foptimize-sibling-calls
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.5.0
: P3 normal
Target Milestone: 4.3.5
Assignee: Uroš Bizjak
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-11 00:06 UTC by Dmitry Gorbachev
Modified: 2009-08-29 10:26 UTC (History)
2 users (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build: i686-pc-linux-gnu
Known to work:
Known to fail: 4.3.3 4.4.1
Last reconfirmed: 2009-08-22 13:38:01


Attachments
patch to fix the failure (456 bytes, patch)
2009-08-22 13:43 UTC, Uroš Bizjak
Details | Diff
Updated patch (457 bytes, patch)
2009-08-23 09:08 UTC, Uroš Bizjak
Details | Diff
Additional patch (466 bytes, patch)
2009-08-23 11:57 UTC, Uroš Bizjak
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Gorbachev 2009-07-11 00:06:09 UTC
The following program will fail.

gcc -S -O1 -foptimize-sibling-calls bug.c

bug.c:
============================================
#define stdcall __attribute__((__stdcall__))

struct S {
    void (stdcall *f)(struct S *);
    int i;
};

void stdcall foo(struct S *s)
{
    s->i++;
}

void stdcall bar(struct S *s)
{
    foo(s);
    s->f(s);
}

int main(void)
{
    struct S s = { foo, 0 };
    bar(&s);
}
============================================

This is what the compiler produces:

bar:
        pushl   %ebp
        movl    %esp, %ebp
        pushl   %ebx
        subl    $20, %esp
        movl    8(%ebp), %ebx
        movl    %ebx, (%esp)
        call    foo
        subl    $4, %esp
        movl    %ebx, 8(%ebp)
        movl    -4(%ebp), %ebx
        leave
        jmp     *(%ebx)
Comment 1 Sergei Trofimovich 2009-08-22 12:09:28 UTC
I can confirm gcc-4.4.1 errors the same way.

I think stdcall is offender:
My sample is more complicated, but has the same features: stdcall+tailcall.

Produces SIGSEGV for me https://bugs.gentoo.org/show_bug.cgi?id=282189
Comment 2 Uroš Bizjak 2009-08-22 13:38:01 UTC
Have a patch.
Comment 3 Uroš Bizjak 2009-08-22 13:43:10 UTC
Created attachment 18413 [details]
patch to fix the failure

Sibcalls of any kind should be done through call-clobbered regs only.

This patch fixes the testcase.

Sergei, can you please test it if it fixes your original problem?
Comment 4 Uroš Bizjak 2009-08-22 13:51:45 UTC
Patched gcc:

bar:
	pushl	%ebp
	movl	%esp, %ebp
	pushl	%ebx
	subl	$20, %esp
	movl	8(%ebp), %ebx
	movl	%ebx, (%esp)
	call	foo
	subl	$4, %esp
	movl	%ebx, 8(%ebp)
	movl	(%ebx), %eax
	movl	-4(%ebp), %ebx
	leave
	jmp	*%eax
Comment 5 Sergei Trofimovich 2009-08-22 18:38:18 UTC
(In reply to comment #3)
> Created an attachment (id=18413) [edit]
> patch to fix the failure
> 
> Sibcalls of any kind should be done through call-clobbered regs only.
> 
> This patch fixes the testcase.
> 
> Sergei, can you please test it if it fixes your original problem?
> 

Sorry, it does not compile on gcc-4.4.1:

build/genoutput /tmp/paludis/sys-devel-gcc-4.4.1-r1/work/gcc-4.4.1/gcc/config/i386/i386.md \
          insn-conditions.md > tmp-output.c
/tmp/paludis/sys-devel-gcc-4.4.1-r1/work/gcc-4.4.1/gcc/config/i386/i386.md:14984: wrong number of alternatives in operand 2
make[3]: *** [s-output] Error 1
make[3]: *** Waiting for unfinished jobs....
Comment 6 Uroš Bizjak 2009-08-23 09:08:47 UTC
Created attachment 18415 [details]
Updated patch

This patch is bootstrapped and regression tested.
Comment 7 uros 2009-08-23 09:46:17 UTC
Subject: Bug 40718

Author: uros
Date: Sun Aug 23 09:46:00 2009
New Revision: 151028

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=151028
Log:
	PR target/40718
	* config/i386/i386.c (*call_pop_1): Disable for sibling calls.
	(*sibcall_pop_1): New insn pattern.

testsuite/ChangeLog:

	PR target/40718
	* gcc.target/i386/pr40718.c: New test.


Added:
    trunk/gcc/testsuite/gcc.target/i386/pr40718.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.md
    trunk/gcc/testsuite/ChangeLog

Comment 8 Sergei Trofimovich 2009-08-23 11:12:19 UTC
(In reply to comment #7)
> Subject: Bug 40718
> 
> Author: uros
> Date: Sun Aug 23 09:46:00 2009
> New Revision: 151028
> 
> URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=151028
> Log:
>         PR target/40718
>         * config/i386/i386.c (*call_pop_1): Disable for sibling calls.
>         (*sibcall_pop_1): New insn pattern.
> 
> testsuite/ChangeLog:
> 
>         PR target/40718
>         * gcc.target/i386/pr40718.c: New test.
> 
> 
> Added:
>     trunk/gcc/testsuite/gcc.target/i386/pr40718.c
> Modified:
>     trunk/gcc/ChangeLog
>     trunk/gcc/config/i386/i386.md
>     trunk/gcc/testsuite/ChangeLog
> 

This patch fixes for me Dmitry's sample, but does not fix mine. Still SIGSEGVs. I've managed to
place whole testcase in one file:

$ g++ -O1 -foptimize-sibling-calls -m32 -DCALLTYPE="__attribute__((stdcall))" main.cc -o show_the_bug 
$ ./show_the_bug
Segmentation fault
$ cat main.cc 
#define CALLTYPE __attribute__((stdcall))

struct Base {
    virtual unsigned long CALLTYPE base_do1(unsigned long, unsigned long) __attribute__((noinline))
    {
        return 4;
    }
};

static Base bi;
Base * glo_ptr_to_base = &bi;

struct Stuff {
    void CALLTYPE do_stuff(unsigned long param1, unsigned long param2) __attribute__((noinline))
    {
        if (param1 == 0xFFFFFFFE)
        {
            return;
        }
        glo_ptr_to_base->base_do1(param1, param2);
    }
};

int
main()
{
    Stuff o;
    o.do_stuff(1, 32);
    return 0;
}

-----------------------------------------------
_ZN5Stuff8do_stuffEmm:
.LFB1:
        .cfi_startproc
        .cfi_personality 0x0,__gxx_personality_v0
        pushl   %ebp
        .cfi_def_cfa_offset 8
        movl    %esp, %ebp
        .cfi_offset 5, -8
        .cfi_def_cfa_register 5
        pushl   %ebx
        subl    $4, %esp
        movl    12(%ebp), %eax
        cmpl    $-2, %eax
        je      .L5
        .cfi_offset 3, -12
        movl    glo_ptr_to_base, %edx
        movl    %edx, 8(%ebp)
        movl    -4(%ebp), %ebx
        leave
        jmp     *(%ebx)
.L5:
        movl    -4(%ebp), %ebx
        leave
        ret     $12
        .cfi_endproc
Comment 9 Uroš Bizjak 2009-08-23 11:41:33 UTC
(In reply to comment #8)

> This patch fixes for me Dmitry's sample, but does not fix mine. Still SIGSEGVs.
> I've managed to
> place whole testcase in one file:

Ah, the same cure should be applied to "*call_value_pop_1" pattern too.

Wait a minute, I'll craft a patch ASAP.
Comment 10 Uroš Bizjak 2009-08-23 11:57:59 UTC
Created attachment 18416 [details]
Additional patch

Additional patch to fix call_value_pop_1.
Comment 11 Uroš Bizjak 2009-08-23 12:08:29 UTC
Patched gcc compiles testcase from comment 8 to:

_ZN5Stuff8do_stuffEmm:
.LFB4:
	.cfi_startproc
	.cfi_personality 0x0,__gxx_personality_v0
	pushl	%ebp
	.cfi_def_cfa_offset 8
	movl	%esp, %ebp
	.cfi_offset 5, -8
	.cfi_def_cfa_register 5
	pushl	%ebx
	subl	$4, %esp
	movl	12(%ebp), %eax
	cmpl	$-2, %eax
	je	.L2
	.cfi_offset 3, -12
	movl	glo_ptr_to_base, %edx
	movl	(%edx), %ebx
	movl	%edx, 8(%ebp)
	movl	(%ebx), %edx
	movl	-4(%ebp), %ebx
	leave
	.cfi_remember_state
	.cfi_restore 5
	.cfi_def_cfa 4, 4
	.cfi_restore 3
	jmp	*%edx
.L2:
	.cfi_restore_state
	movl	-4(%ebp), %ebx
	leave
	.cfi_restore 5
	.cfi_def_cfa 4, 4
	.cfi_restore 3
	ret	$12
	.cfi_endproc
Comment 12 uros 2009-08-23 12:14:39 UTC
Subject: Bug 40718

Author: uros
Date: Sun Aug 23 12:14:26 2009
New Revision: 151029

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=151029
Log:
	PR target/40718
	* config/i386/i386.c (*call_pop_1): Disable for sibling calls.
	(*call_value_pop_1): Ditto.
	(*sibcall_pop_1): New insn pattern.
	(*sibcall_value_pop_1): Ditto.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.md

Comment 13 uros 2009-08-23 12:38:08 UTC
Subject: Bug 40718

Author: uros
Date: Sun Aug 23 12:37:53 2009
New Revision: 151030

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=151030
Log:
	PR target/40718
	* config/i386/i386.c (*call_pop_1): Disable for sibling calls.
	(*call_value_pop_1): Ditto.
	(*sibcall_pop_1): New insn pattern.
	(*sibcall_value_pop_1): Ditto.

testsuite/ChangeLog:

	PR target/40718
	* gcc.target/i386/pr40718.c: New test.


Added:
    branches/gcc-4_4-branch/gcc/testsuite/gcc.target/i386/pr40718.c
Modified:
    branches/gcc-4_4-branch/gcc/ChangeLog
    branches/gcc-4_4-branch/gcc/config/i386/i386.md
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog

Comment 14 uros 2009-08-23 13:03:54 UTC
Subject: Bug 40718

Author: uros
Date: Sun Aug 23 13:03:39 2009
New Revision: 151033

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=151033
Log:
	PR target/40718
	* config/i386/i386.c (*call_pop_1): Disable for sibling calls.
	(*call_value_pop_1): Ditto.
	(*sibcall_pop_1): New insn pattern.
	(*sibcall_value_pop_1): Ditto.

testsuite/ChangeLog:

	PR target/40718
	* gcc.target/i386/pr40718.c: New test.


Added:
    branches/gcc-4_3-branch/gcc/testsuite/gcc.target/i386/pr40718.c
Modified:
    branches/gcc-4_3-branch/gcc/ChangeLog
    branches/gcc-4_3-branch/gcc/config/i386/i386.md
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog

Comment 15 Uroš Bizjak 2009-08-23 13:04:38 UTC
Fixed everywhere.
Comment 16 Sergei Trofimovich 2009-08-29 10:26:15 UTC
(In reply to comment #15)
> Fixed everywhere.
> 

Just tested on my sample (got this only revision from 4_4 branch and applied to gentoo gcc sources).
All works as expected.

Thanks!