The following testcase in C should be valid expectd to loop infinitely while with IPA SRA we ICE. static int __attribute__ ((noinline,const)) infinite (int p) { if (p) while (1); return p; } __attribute__ ((noinline)) static void test(int p, int *a) { int v = infinite (p); if (*a && v) __builtin_abort (); } test2(int *a) { test(0,a); } main() { test (1,0); }
How do we ICE, I can't reproduce it.
jan@localhost:~> gcc t.c t.c:2:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 2 | infinite (int p) | ^~~~~~~~ t.c:16:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 16 | test2(int *a) | ^~~~~ t.c:20:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 20 | main() | ^~~~ jan@localhost:~> ./a.out ...loops infinitely.... jan@localhost:~> gcc t.c -O2 t.c:2:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 2 | infinite (int p) | ^~~~~~~~ t.c:16:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 16 | test2(int *a) | ^~~~~ t.c:20:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 20 | main() | ^~~~ jan@localhost:~> ./a.out Segmentation fault (core dumped)
Ah, ok, so no ICE, but wrong code.
Still, I can't reproduce with the current master. Apparently, the code snippet from #c0 produces only 2 warnings: $ gcc pr103267.c -c -O2 && ./a.out pr103267.c:17:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 17 | test2(int *a) | ^~~~~ pr103267.c:21:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 21 | main() | ^~~~
Works for me even with the 3 warnings. hubicka@lomikamen:/aux/hubicka/trunk/build-lto2/gcc$ cat >tt.c __attribute__ ((noinline,const)) infinite (int p) { if (p) while (1); return p; } __attribute__ ((noinline)) static void test(int p, int *a) { int v = infinite (p); if (*a && v) __builtin_abort (); } test2(int *a) { test(0,a); } main() { test (1,0); } hubicka@lomikamen:/aux/hubicka/trunk/build-lto2/gcc$ ./xgcc --version xgcc (GCC) 12.0.0 20211114 (experimental) Copyright (C) 2021 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. hubicka@lomikamen:/aux/hubicka/trunk/build-lto2/gcc$ ./xgcc -B ./ -O2 tt.c tt.c:2:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 2 | infinite (int p) | ^~~~~~~~ tt.c:16:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 16 | test2(int *a) | ^~~~~ tt.c:20:1: warning: return type defaults to ‘int’ [-Wimplicit-int] 20 | main() | ^~~~ hubicka@lomikamen:/aux/hubicka/trunk/build-lto2/gcc$ ./a.out Segmentation fault
Aha, but here is better example (reproduces same way). In the former one I forgot const attribute which makes it invalid. The testcase tests that ipa-sra is missing ECF_LOOPING_CONST_OR_PURE check static int __attribute__ ((noinline)) infinite (int p) { if (p) while (1); return p; } __attribute__ ((noinline)) static void test(int p, int *a) { int v = infinite (p); if (*a && v) __builtin_abort (); } test2(int *a) { test(0,a); } main() { test (1,0); }
Now, I can reproduce it, the original code snippet was different: diff -u pr103267-o.c pr103267.c --- pr103267-o.c 2021-11-16 09:47:41.463349286 +0100 +++ pr103267.c 2021-11-16 09:47:11.811550854 +0100 @@ -1,4 +1,3 @@ -static int __attribute__ ((noinline,const)) infinite (int p) {
(In reply to hubicka from comment #6) > Aha, but here is better example (reproduces same way). > In the former one I forgot const attribute which makes it invalid. > The testcase tests that ipa-sra is missing ECF_LOOPING_CONST_OR_PURE > check > > static int > __attribute__ ((noinline)) > infinite (int p) > { > if (p) > while (1); > return p; > } > __attribute__ ((noinline)) > static void > test(int p, int *a) > { > int v = infinite (p); > if (*a && v) > __builtin_abort (); > } > test2(int *a) > { > test(0,a); > } > main() > { > test (1,0); > } This one is very old :) Also 4.8.0 crashes.
> @@ -1,4 +1,3 @@ > -static int > __attribute__ ((noinline,const)) > infinite (int p) > { Just for a record, it crahes with or without static int here for me :) I run across it because the code tracking must access in ipa-sra is IMO conceptually wrong. I noticed that because ipa-modref solves similar problem for kills (both need to verify that given access will always happen). The post-dominance check is not enough to verify that because earlier function calls can do things like EH. I failed to construct an actual testcase because on interesting stuff like EH we punt for other reasons (missed fnspec annotations on EH builtins). I will play with it more today.
I have proposed a patch to address this issue in: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585756.html Well, it prevents the infinite loop testcase from segfaulting when the function infinite() is not declared with const attribute explicitely but it is only internally auto-discovered to be ECF_CONST. I think that is OK because explicit attribute const should guarantee no side-effects and infinite loop is IMHO one (though I have read how we document it in our manual and I am not entirely sure that is how we handle it, but IMHO the manual disallows infinite looping too). (We also discussed the post-dominance check with Honza in person and at least my impression is that the situation is not as dire as comment #9 might suggest.)
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>: https://gcc.gnu.org/g:e5440bc08e07fd491dcccd47e1b86a5985ee117c commit r12-5633-ge5440bc08e07fd491dcccd47e1b86a5985ee117c Author: Martin Jambor <mjambor@suse.cz> Date: Tue Nov 30 18:45:11 2021 +0100 ipa-sra: Check also ECF_LOOPING_CONST_OR_PURE when evaluating calls in PR 103267 Honza found out that IPA-SRA does not look at ECF_LOOPING_CONST_OR_PURE when evaluating if a call can have side effects. Fixed with this patch. The testcase infinitely loops in a const function, so it would not make a good addition to the testsuite. gcc/ChangeLog: 2021-11-29 Martin Jambor <mjambor@suse.cz> PR ipa/103267 * ipa-sra.c (scan_function): Also check ECF_LOOPING_CONST_OR_PURE flag.
The releases/gcc-11 branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>: https://gcc.gnu.org/g:5e2e6cc84c46ff7ceb6395c0309a2c6b71d83cb1 commit r11-9347-g5e2e6cc84c46ff7ceb6395c0309a2c6b71d83cb1 Author: Martin Jambor <mjambor@suse.cz> Date: Wed Dec 1 14:13:35 2021 +0100 ipa-sra: Check also ECF_LOOPING_CONST_OR_PURE when evaluating calls in PR 103267 Honza found out that IPA-SRA does not look at ECF_LOOPING_CONST_OR_PURE when evaluating if a call can have side effects. Fixed with this patch. The testcase infinitely loops in a const function, so it would not make a good addition to the testsuite. gcc/ChangeLog: 2021-11-29 Martin Jambor <mjambor@suse.cz> PR ipa/103267 * ipa-sra.c (scan_function): Also check ECF_LOOPING_CONST_OR_PURE flag. (cherry picked from commit e5440bc08e07fd491dcccd47e1b86a5985ee117c)
The releases/gcc-10 branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>: https://gcc.gnu.org/g:d2ecc42195e8af1992d12e678e761c73557eaf56 commit r10-10318-gd2ecc42195e8af1992d12e678e761c73557eaf56 Author: Martin Jambor <mjambor@suse.cz> Date: Wed Dec 1 14:25:16 2021 +0100 ipa-sra: Check also ECF_LOOPING_CONST_OR_PURE when evaluating calls in PR 103267 Honza found out that IPA-SRA does not look at ECF_LOOPING_CONST_OR_PURE when evaluating if a call can have side effects. Fixed with this patch. The testcase infinitely loops in a const function, so it would not make a good addition to the testsuite. gcc/ChangeLog: 2021-11-29 Martin Jambor <mjambor@suse.cz> PR ipa/103267 * ipa-sra.c (scan_function): Also check ECF_LOOPING_CONST_OR_PURE flag. (cherry picked from commit e5440bc08e07fd491dcccd47e1b86a5985ee117c)
The releases/gcc-9 branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>: https://gcc.gnu.org/g:7dd5b92bbe0944dc27e6175b0df72ed0a7188016 commit r9-9852-g7dd5b92bbe0944dc27e6175b0df72ed0a7188016 Author: Martin Jambor <mjambor@suse.cz> Date: Wed Dec 1 18:29:50 2021 +0100 ipa-sra: Check also ECF_LOOPING_CONST_OR_PURE when evaluating calls in PR 103267 Honza found out that IPA-SRA does not look at ECF_LOOPING_CONST_OR_PURE when evaluating if a call can have side effects. Fixed with this patch. The testcase infinitely loops in a const function, so it would not make a good addition to the testsuite. This patch is a manual backport of commit e5440bc08e07fd491dcccd47e1b86a5985ee117c to the old "early" IPA-SRA. gcc/ChangeLog: 2021-12-01 Martin Jambor <mjambor@suse.cz> PR ipa/103267 * tree-sra.c (scan_function): Also check ECF_LOOPING_CONST_OR_PURE flag.
Fixed (with the caveat explained in comment #10) on master and all opened release branches, thus closing.