On the following test case compiled with GCC 4.4.1 release version and the following command line gcc -S -O2 -finline-functions-called-once -fdump-tree-all-details -fdump-ipa-all fail.c typedef struct SEntry { unsigned char num; } TEntry; typedef struct STable { TEntry data[2]; } TTable; TTable *init (); int fake_expect (int, int); void fake_assert (int); void expect_func (int a, unsigned char *b) __attribute__ ((noinline)); static inline void inlined_wrong (TEntry *entry_p, int flag); void inlined_wrong (TEntry *entry_p, int flag) { unsigned char index; entry_p->num = 0; if (!flag) fake_assert (0); for (index = 0; index < 1; index++) entry_p->num++; asm ("before"); if (entry_p->num) { fake_assert(0); asm ("#here"); } } void expect_func (int a, unsigned char *b) { if (fake_expect ((a == 0), 0)) fake_assert (0); if (fake_expect ((b == 0), 0)) fake_assert (0); } void broken () { unsigned char index = 0; TTable *table_p = init(); inlined_wrong (&(table_p->data[1]), 1); expect_func (0, &index); inlined_wrong ((TEntry *)0xf00f, 1); LocalFreeMemory (&table_p); } we get after FRE: broken () { unsigned char index; unsigned char D.1321; unsigned char D.1320; unsigned char index; unsigned char D.1316; unsigned char D.1315; struct TTable * table_p; unsigned char index; struct TEntry * D.1281; struct TTable * table_p.1; struct TTable * table_p.0; <bb 2>: index = 0; table_p.0_1 = init (); table_p = table_p.0_1; table_p.1_2 = table_p.0_1; D.1281_3 = &table_p.1_2->data[1]; table_p.1_2->data[1].num = 0; goto <bb 4>; <bb 3>: D.1315_4 = D.1281_3->num; D.1316_5 = D.1315_4 + 1; D.1281_3->num = D.1316_5; index_7 = index_6 + 1; <bb 4>: # index_6 = PHI <0(2), index_7(3)> if (index_6 == 0) goto <bb 3>; else goto <bb 5>; <bb 5>: __asm__ __volatile__("before"); D.1315_8 = 0; expect_func (0, &index); 61455B->num ={v} 0; goto <bb 7>; <bb 6>: D.1320_10 ={v} 61455B->num; D.1321_11 = D.1320_10 + 1; 61455B->num ={v} D.1321_11; index_13 = index_12 + 1; <bb 7>: # index_12 = PHI <0(5), index_13(6)> if (index_12 == 0) goto <bb 6>; else goto <bb 8>; <bb 8>: __asm__ __volatile__("before"); D.1320_14 ={v} 61455B->num; if (D.1320_14 != 0) goto <bb 9>; else goto <bb 10>; <bb 9>: fake_assert (0); __asm__ __volatile__("#here"); <bb 10>: LocalFreeMemory (&table_p); return; } Note the check "if (entry_p->num)" and associated block is completely eliminated. The dumps indicate: Replaced table_p with table_p.0_1 in table_p.1_2 = table_p; Replaced table_p.1_2->data[1].num with 0 in D.1315_8 = table_p.1_2->data[1].num; Removing basic block 6 ;; basic block 6, loop depth 0, count 0 ;; prev block 5, next block 7 ;; pred: 5 [39.0%] (true,exec) ;; succ: 7 [100.0%] (fallthru,exec) <bb 6>: fake_assert (0); __asm__ __volatile__("#here"); If the same code is compiled with the function "inlined_wrong" declared as static inline void inlined_wrong (TEntry *entry_p, int flag) __attribute__ ((always_inline)); The generated code is correct with the check in place, suggesting ipa-inline is troublesome while early inlining works okay?
Can you massage this into a runtime testcase that calls abort () when miscompiled?
This code should do it: --- extern void *malloc(int); extern void abort(void); extern void free(void *); typedef struct SEntry { unsigned char num; } TEntry; typedef struct STable { TEntry data[2]; } TTable; TTable *init () { return malloc(sizeof(TTable)); } void expect_func (int a, unsigned char *b) __attribute__ ((noinline)); static inline void inlined_wrong (TEntry *entry_p, int flag); void inlined_wrong (TEntry *entry_p, int flag) { unsigned char index; entry_p->num = 0; if (flag == 0) abort(); for (index = 0; index < 1; index++) entry_p->num++; if (!entry_p->num) { abort(); } } void expect_func (int a, unsigned char *b) { if (abs ((a == 0))) abort (); if (abs ((b == 0))) abort (); } int main () { unsigned char index = 0; TTable *table_p = init(); TEntry work; inlined_wrong (&(table_p->data[1]), 1); expect_func (1, &index); inlined_wrong (&work, 1); free (table_p); return 1; } --- Built with the command line: --- gcc -O2 -finline-functions-called-once fail.c --- When built with GCC 4.4.1 the resultant executable incorrectly calls abort. When built with GCC 3.4.6 the resultant executable doesn't abort.
*** Bug 42620 has been marked as a duplicate of this bug. ***
Somehow patched openSUSE GCC 4.3 is also affected. <bb 2>: # index_18 = VDEF <index_17(D)> index = 0; # index_21 = VDEF <index_18> # SMT.57_22 = VDEF <SMT.57_19(D)> # SMT.58_23 = VDEF <SMT.58_20(D)> table_p_1 = init (); D.1295_3 = &table_p_1->data[1]; # SMT.57_24 = VDEF <SMT.57_22> table_p_1->data[1].num = 0; goto <bb 4>; <bb 3>: # VUSE <SMT.58_10> D.1355_5 = D.1295_3->num; D.1356_6 = D.1355_5 + 1; # SMT.58_25 = VDEF <SMT.58_10> D.1295_3->num = D.1356_6; index_8 = index_7 + 1; <bb 4>: # index_7 = PHI <0(2), index_8(3)> # SMT.58_10 = PHI <SMT.58_23(2), SMT.58_25(3)> if (index_7 == 0) goto <bb 3>; else goto <bb 5>; <bb 5>: # VUSE <SMT.57_24> D.1355_9 = table_p_1->data[1].num; FRE CSEs the load from table_p_1->data[1].num with the store in BB2 not seeing the must-alias in BB3. And it's obvious why when you look at the vops. SMT.58 needs to have SMT.57 in its may-aliases but doesn't have it for some reason.
Hm, I guess I'm the only one that is likely going to fix it.
I can try and look into it if you give me some pointers. I can guarantee I won't be able to fix it anywhere near as quickly as you though :)
Shorter testcase, fails at -O2 -fno-early-inlining: extern void abort(void); typedef struct SEntry { int num; } TEntry; typedef struct STable { TEntry data[2]; } TTable; TTable * __attribute__((noinline)) init () { static TTable t; return &t; } static inline void inlined_wrong (TEntry *entry_p) { unsigned char index; entry_p->num = 0; for (index = 0; index < 1; index++) entry_p->num++; if (!entry_p->num) abort(); } void __attribute__((noinline)) expect_func (unsigned char *b) { if (!b) abort (); } int main () { unsigned char index = 0; TTable *table_p; expect_func (&index); table_p = init(); inlined_wrong (&(table_p->data[1])); return 0; }
Well, I'm sure it goes wrong in compute_flow_insensitive_aliasing - let me have a quick look there.
Ok, it's simple. We add false aliases to &index to both SMTs of SEntry and STable because may_alias_p (SEntry, char) returns true as every alias-set is a subset of alias-set zero. But then when coming along to adding SMT aliases we do not add STable as an alias of SEntry because they already have (false) aliases in common. These false aliases are pruned by access_can_touch_variable later and boom - there we go. I have a fix.
Created attachment 19472 [details] patch Testing on the application that originally failed appreciated.
Thanks for your time and the extra quick fix! I'll do some testing with that patch now.
Your patch fixes our original application. Thanks again for your help.
Subject: Bug 42614 Author: rguenth Date: Tue Jan 5 13:41:41 2010 New Revision: 155646 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=155646 Log: 2010-01-05 Richard Guenther <rguenther@suse.de> PR tree-optimization/42614 * tree-ssa-alias.c (compute_flow_insensitive_aliasing): Compute SMT aliases before symbol aliases. * gcc.c-torture/execute/pr42614.c: New testcase. Added: branches/gcc-4_4-branch/gcc/testsuite/gcc.c-torture/execute/pr42614.c Modified: branches/gcc-4_4-branch/gcc/ChangeLog branches/gcc-4_4-branch/gcc/testsuite/ChangeLog branches/gcc-4_4-branch/gcc/tree-ssa-alias.c
Subject: Bug 42614 Author: rguenth Date: Tue Jan 5 13:42:40 2010 New Revision: 155647 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=155647 Log: 2010-01-05 Richard Guenther <rguenther@suse.de> PR tree-optimization/42614 * gcc.c-torture/execute/pr42614.c: New testcase. Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr42614.c Modified: trunk/gcc/testsuite/ChangeLog
Fixed.
*** Bug 42794 has been marked as a duplicate of this bug. ***