[Bug ipa/103267] New: Wrong code with ipa-sra

[Bug ipa/103267] New: Wrong code with ipa-sra

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

            Bug ID: 103267
           Summary: Wrong code with ipa-sra
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: ipa
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hubicka at gcc dot gnu.org
                CC: marxin at gcc dot gnu.org
  Target Milestone: ---

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);
}

[Bug ipa/103267] Wrong code with ipa-sra

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-11-16
             Status|UNCONFIRMED                 |WAITING
     Ever confirmed|0                           |1

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
How do we ICE, I can't reproduce it.

[Bug ipa/103267] Wrong code with ipa-sra

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #2 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
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)

[Bug ipa/103267] Wrong code with ipa-sra

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
Ah, ok, so no ICE, but wrong code.

[Bug ipa/103267] Wrong code with ipa-sra

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
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()
      | ^~~~

Re: [Bug ipa/103267] Wrong code with ipa-sra

[Jan Hubicka]

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

[Bug ipa/103267] Wrong code with ipa-sra

[hubicka at kam dot mff.cuni.cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #5 from hubicka at kam dot mff.cuni.cz ---
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

Re: [Bug ipa/103267] Wrong code with ipa-sra

[Jan Hubicka]

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);
}

[Bug ipa/103267] Wrong code with ipa-sra

[hubicka at kam dot mff.cuni.cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #6 from hubicka at kam dot mff.cuni.cz ---
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);
}

[Bug ipa/103267] Wrong code with ipa-sra

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #7 from Martin Liška <marxin at gcc dot gnu.org> ---
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)
 {

Re: [Bug ipa/103267] Wrong code with ipa-sra

[Jan Hubicka]

> @@ -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.

[Bug ipa/103267] Wrong code with ipa-sra

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW

[Bug ipa/103267] Wrong code with ipa-sra

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #8 from Martin Liška <marxin at gcc dot gnu.org> ---
(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.

[Bug ipa/103267] Wrong code with ipa-sra

[hubicka at kam dot mff.cuni.cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #9 from hubicka at kam dot mff.cuni.cz ---
> @@ -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.

[Bug ipa/103267] Wrong code with ipa-sra

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #10 from Martin Jambor <jamborm at gcc dot gnu.org> ---
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.)

[Bug ipa/103267] Wrong code with ipa-sra

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug ipa/103267] Wrong code with ipa-sra

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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)

[Bug ipa/103267] Wrong code with ipa-sra

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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)

[Bug ipa/103267] Wrong code with ipa-sra

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug ipa/103267] Wrong code with ipa-sra

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103267

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #15 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Fixed (with the caveat explained in comment #10) on master and all opened
release branches, thus closing.