While compling program below (3 files) with O2 or O3 option, code is generated not properly (SF). /******************************************************************************/ file 1: gcc_bug.c #include <stdio.h> #include "function.h" static void my_listener( int a, int b, int c) { printf("It works!!!\n"); } static struct data_t data; int main() { data.listener = my_listener; data.a = 11; data.b = 22; data.c = 33; /* with O2 compiling, function_calling_listener try call function with address 33, not my_listener address */ data.d = 44; function_calling_listener(data); return 0; } /******************************************************************************/ file 2: function.h typedef void (*listener_fun)( int a, int b, int c); struct data_t { int a; listener_fun listener; int b; int c; int d; }; void function_calling_listener (struct data_t data); /******************************************************************************/ file 3: function.c #include "function.h" void function_calling_listener (struct data_t data) { data.listener(data.a, data.c, data.d); } /******************************************************************************/ GCC VERSION: gcc --version gcc (GCC) 4.1.2 20070626 (Red Hat 4.1.2-13) Copyright (C) 2006 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> gcc (GCC) 4.1.2 20070626 (Red Hat 4.1.2-13) Please try 4.2.3 or 4.3.0. Also you should have reported this to RedHat first as it is their modified compiler.
The same problem on that version. I don't think that is Red Hat specific problem... Using built-in specs. Target: i486-linux-gnu Configured with: ../src/configure -v --enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr --enable-shared -- Thread model: posix gcc version 4.1.2 20061028 (prerelease) (Debian 4.1.1-19)
Ok. Now version 4.3.0 from Debian. The same problem.... gcc-4.3 --version gcc-4.3 (Debian 4.3.0-1) 4.3.1 20080309 (prerelease) Copyright (C) 2008 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Can you include the contents of function.h, it seems to be missing ?
All is fine. My friend copy that program from here, compile on gcc-4.3 and give me result that is in comment #3.... But ok, i copy one more time what is in function.h file: typedef void (*listener_fun)( int a, int b, int c); struct data_t { int a; listener_fun listener; int b; int c; int d; }; void function_calling_listener (struct data_t data);
This looks like a target issue.
asm dump with GCC: (GNU) 4.4.0 20080318 (experimental) and -O2 -fomit-frame-pointer: function_calling_listener: movl 20(%esp), %eax # data.d, data.d movl 16(%esp), %ecx # data.c, data.c movl %eax, 12(%esp) # data.d, movl %ecx, 8(%esp) # data.c, jmp *%ecx # data.c -O0: function_calling_listener: pushl %ebx # subl $24, %esp #, movl 36(%esp), %ebx # data.listener, D.1190 movl 48(%esp), %eax # data.d, D.1191 movl 44(%esp), %edx # data.c, D.1192 movl 32(%esp), %ecx # data.a, D.1193 movl %eax, 8(%esp) # D.1191, movl %edx, 4(%esp) # D.1192, movl %ecx, (%esp) # D.1193, call *%ebx # D.1190 addl $24, %esp #, popl %ebx # ret
It works when function.c is compiled with -O2 -fno-optimize-sibling-calls.
The stack modifications before the sibcall look indeed interesting. Micha, maybe you want to poke into this somewhat?
The tree dumps already look wrong. from .130t.final_cleanup: function_calling_listener (data) { <bb 2>: data (data.a, data.c, data.d) [tail call]; return; } Note how the function pointer is replaced with the whole "data" object. Huh?
cse1 pass somehow figures out that: (insn 11 10 12 2 function.c:5 (set (reg/f:SI 61) (mem/s/f/c:SI (plus:SI (reg/f:SI 16 argp) (const_int 4 [0x4])) [4 data.listener+0 S4 A32])) 41 {*movsi_1} (nil)) (call_insn/j 12 11 0 2 function.c:5 (call (mem:QI (reg/f:SI 61) [0 S1 A8]) (const_int 12 [0xc])) 423 {*sibcall_1} (nil) (nil)) can be substituted with: (insn 11 10 12 2 function.c:5 (set (reg/f:SI 61 [ data.listener ]) (reg:SI 59 [ data.c ])) 41 {*movsi_1} (nil)) (call_insn/j 12 11 0 2 function.c:5 (call (mem:QI (reg:SI 59 [ data.c ]) [0 S1 A 8]) (const_int 12 [0xc])) 423 {*sibcall_1} (nil) (nil))
Uros: the problem isn't cse. It's already expand which creates broken code (it reads from and writes to (mem (plus (virtual-incoming-regs) (4)). But it does that only because the input tree is completely wrong (TER breaks this).
Actually the trees are correct, just the dumping of function names in call exprs is "interesting". Still expand is the primary suspect here.
(In reply to comment #12) > Uros: the problem isn't cse. It's already expand which creates broken code > (it reads from and writes to (mem (plus (virtual-incoming-regs) (4)). > But it does that only because the input tree is completely wrong (TER breaks > this). Yes, looking a bit more into the expand dump, it looks that gcc tries to fix outgoing call stack by copying 'c' and 'd' into the position of 'listener' and 'b'. It accidentally overwrites 'listener' (and 'b', but this isn't used anywhere). The code would work OK if 'listener' would be copied to a temporary first.
We can either force expanding the call address before the arguments (if it overlaps with them) or simply validate the thing after the fact. Validating seems a bit easier (and is in line with what is done already for the expansion of overlapping arguments). Proof of concept patch: Index: calls.c =================================================================== --- calls.c (revision 133304) +++ calls.c (working copy) @@ -2756,8 +2756,15 @@ expand_call (tree exp, rtx target, int i use_reg (&call_fusage, struct_value); } - funexp = prepare_call_address (funexp, static_chain_value, + { + rtx before_arg = get_last_insn (); + + funexp = prepare_call_address (funexp, static_chain_value, &call_fusage, reg_parm_seen, pass == 0); + if (pass == 0 + && check_sibcall_argument_overlap (before_arg, 0, 0)) + sibcall_failure = 1; + } load_register_parameters (args, num_actuals, &call_fusage, flags, pass == 0, &sibcall_failure);
Subject: Bug 35616 Author: matz Date: Wed Mar 19 19:15:03 2008 New Revision: 133348 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=133348 Log: PR middle-end/35616 * calls.c (expand_call): Check overlap of arguments with call address for sibcalls. * gcc.dg/pr35616.c: New test. Added: trunk/gcc/testsuite/gcc.dg/pr35616.c Modified: trunk/gcc/ChangeLog trunk/gcc/calls.c trunk/gcc/testsuite/ChangeLog
Subject: Bug 35616 Author: matz Date: Wed Mar 19 19:37:48 2008 New Revision: 133350 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=133350 Log: Backport from mainline: PR middle-end/35616 * calls.c (expand_call): Check overlap of arguments with call address for sibcalls. * gcc.dg/pr35616.c: New test. Added: branches/gcc-4_3-branch/gcc/testsuite/gcc.dg/pr35616.c - copied unchanged from r133348, trunk/gcc/testsuite/gcc.dg/pr35616.c Modified: branches/gcc-4_3-branch/gcc/ChangeLog branches/gcc-4_3-branch/gcc/calls.c branches/gcc-4_3-branch/gcc/testsuite/ChangeLog
trunk and 4.3 are fixed.
Closing 4.1 branch.
Closing 4.2 branch, fixed for 4.3.1 and 4.4.