Bug 106099 - [13 Regression] ICE in execute_todo, at passes.cc:2134 since r13-1204-gd68d366425369649
Summary: [13 Regression] ICE in execute_todo, at passes.cc:2134 since r13-1204-gd68d36...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 13.0
: P3 normal
Target Milestone: 13.0
Assignee: Jakub Jelinek
URL:
Keywords: ice-on-valid-code
: 106134 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-06-27 06:52 UTC by Arseny Solokha
Modified: 2022-10-18 13:02 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-06-27 00:00:00


Attachments
gcc13-pr106099.patch (1.15 KB, patch)
2022-07-26 10:25 UTC, Jakub Jelinek
Details | Diff
gcc13-pr106099.patch (1.03 KB, patch)
2022-08-23 12:54 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Arseny Solokha 2022-06-27 06:52:59 UTC
gfortran 13.0.0 20220626 snapshot (g:ff35dbc02092fbcd3d814fcd9fe8e871c3f741fd) ICEs when compiling libgomp/testsuite/libgomp.oacc-fortran/routine-1.f90:

% gfortran-13.0.0 -O1 -fopenacc -funreachable-traps -fno-tree-ccp -c libgomp/testsuite/libgomp.oacc-fortran/routine-1.f90
during GIMPLE pass: ivcanon
libgomp/testsuite/libgomp.oacc-fortran/routine-1.f90:21:16:

   21 |   !$acc parallel
      |                ^
internal compiler error: in execute_todo, at passes.cc:2134
0x71c660 execute_todo
	/var/tmp/portage/sys-devel/gcc-13.0.0_p20220626/work/gcc-13-20220626/gcc/passes.cc:2134
Comment 1 Zdenek Sojka 2022-06-27 09:04:25 UTC
A simple C testcase:
$ cat testcase.c
void
foo (void)
{
  for (unsigned i = 0; i < sizeof (foo); i++)
    ;
}
$ x86_64-pc-linux-gnu-gcc -O -fsanitize=unreachable -fsanitize-undefined-trap-on-error -fno-tree-ccp -fno-tree-dominator-opts testcase.c
during GIMPLE pass: ivcanon
testcase.c: In function 'foo':
testcase.c:2:1: internal compiler error: in execute_todo, at passes.cc:2134
    2 | foo (void)
      | ^~~
0x755240 execute_todo
        /repo/gcc-trunk/gcc/passes.cc:2134
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
Comment 2 Martin Liška 2022-06-27 09:07:16 UTC
Started with r13-1204-gd68d366425369649.
Comment 3 Andrew Pinski 2022-06-27 09:18:19 UTC
(In reply to Martin Liška from comment #2)
> Started with r13-1204-gd68d366425369649.

Since -funreachable-traps is a new option, is this a regression then?
Comment 4 Martin Liška 2022-07-04 10:10:39 UTC
*** Bug 106134 has been marked as a duplicate of this bug. ***
Comment 5 Martin Liška 2022-07-04 10:12:38 UTC
(In reply to Andrew Pinski from comment #3)
> (In reply to Martin Liška from comment #2)
> > Started with r13-1204-gd68d366425369649.
> 
> Since -funreachable-traps is a new option, is this a regression then?

I have a new test-case that does not need the option:

$ cat new-alias.cpp
using size_t = decltype(sizeof(0));
extern "C" char *something(long long x) {}
void *operator new(size_t) __attribute__((alias("something")));
int *pr16715 = new int;

$ g++ new-alias.cpp -c -Og
new-alias.cpp: In function ‘char* something(long long int)’:
new-alias.cpp:2:42: warning: no return statement in function returning non-void [-Wreturn-type]
    2 | extern "C" char *something(long long x) {}
      |                                          ^
new-alias.cpp: At global scope:
new-alias.cpp:3:7: warning: ‘void* operator new(size_t)’ alias between functions of incompatible types ‘void*(size_t)’ {aka ‘void*(long unsigned int)’} and ‘char*(long long int)’ [-Wattribute-alias=]
    3 | void *operator new(size_t) __attribute__((alias("something")));
      |       ^~~~~~~~
new-alias.cpp:2:18: note: aliased declaration here
    2 | extern "C" char *something(long long x) {}
      |                  ^~~~~~~~~
during GIMPLE pass: local-pure-const
new-alias.cpp: In function ‘void __static_initialization_and_destruction_0()’:
new-alias.cpp:4:23: internal compiler error: in execute_todo, at passes.cc:2140
    4 | int *pr16715 = new int;
      |                       ^
0x17450d1 execute_todo
	/home/marxin/Programming/gcc/gcc/passes.cc:2140
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
Comment 6 Jakub Jelinek 2022-07-04 11:35:34 UTC
At least for #c1 (which can be simplified to:
void
foo (void)
{
  for (unsigned i = 0; i == 0; i++)
    ;
}
), the problem is that __builtin_unreachable that was emitted by ivcanon pass
is marked const and so is __ubsan_handle_builtin_unreachable but __builtin_trap is not.
Which means that __builtin_unreachable (looks ok) and __ubsan_handle_builtin_unreachable (admittedly surprising) doesn't need gimple_vdef nor gimple_vuse on it, but __builtin_trap needs it.
And ivcanon doesn't expect that and as it doesn't add the vdef manually, we remain with something that needs SSA update but we don't ask for it in todo flags.
At least for user calls of __builtin_trap, I think we need at least vuse on it, unlike __builtin_unreachable for traps we do care about say stores to memory before it so that one can investigate them, abort isn't const either.
For a noreturn function, it isn't clear if we need a vdef.
I'm afraid we need to at least investigate all the gimple_build_builtin_unreachable callers.
E.g. in gimple-fold.cc, it calls gimple_move_vops, so if the old method call wasn't const/pure, it will work properly, but if it was e.g. const, we won't add it and will ICE similarly.

Perhaps one way out of this would be to use 2 different __builtin_trap builtins, one as it is now that requires vops, and another one marked const for the __builtin_unreachable turned into __builtin_trap which would be marked const and wouldn't need vops (__builtin_unreachable_trap?) and expand both builtins the same into RTL.
Comment 7 Jakub Jelinek 2022-07-04 11:38:41 UTC
And there is another thing, we need to check all these gimple_build_builtin_unreachable and builtin_decl_unreachable and build_builtin_unreachable calls and check if any of them aren't done during IPA with the assumed new caller not being set as cfun.  Because those functions test
flag_* etc. vars and if there would be such calls, we'd need to pass struct function * and use opts_for_fn instead of flag_*.
Comment 8 Jakub Jelinek 2022-07-26 10:25:01 UTC
Created attachment 53357 [details]
gcc13-pr106099.patch

Untested fix.
Comment 9 CVS Commits 2022-07-28 10:43:21 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:f64eb636677d714781b4543f111b1c9239328db6

commit r13-1874-gf64eb636677d714781b4543f111b1c9239328db6
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jul 28 12:42:14 2022 +0200

    gimple, internal-fn: Add IFN_TRAP and use it for __builtin_unreachable [PR106099]
    
    __builtin_unreachable and __ubsan_handle_builtin_unreachable don't
    use vops, they are marked const/leaf/noreturn/nothrow/cold.
    But __builtin_trap uses vops, isn't const, just leaf/noreturn/nothrow/cold.
    This is I believe so that when users explicitly use __builtin_trap in their
    sources they get stores visible at the trap side.
    -fsanitize=unreachable -fsanitize-undefined-trap-on-error used to transform
    __builtin_unreachable to __builtin_trap even in the past, but the sanopt pass
    has TODO_update_ssa, so it worked fine.
    
    Now that gimple_build_builtin_unreachable can build a __builtin_trap call
    right away, we can run into problems that whenever we need it we would need
    to either manually or through TODO_update* ensure the vops being updated.
    
    Though, as it is originally __builtin_unreachable which is just implemented
    as trap, I think for this case it is fine to avoid vops.  For this the
    patch introduces IFN_TRAP, which has ECF_* flags like __builtin_unreachable
    and is expanded as __builtin_trap.
    
    2022-07-28  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/106099
            * internal-fn.def (TRAP): New internal fn.
            * internal-fn.h (expand_TRAP): Declare.
            * internal-fn.cc (expand_TRAP): Define.
            * gimple.cc (gimple_build_builtin_unreachable): For BUILT_IN_TRAP,
            use internal fn rather than builtin.
    
            * gcc.dg/ubsan/pr106099.c: New test.
Comment 10 Zdenek Sojka 2022-07-30 08:22:51 UTC
Thank you for the patch; I can still trigger the ICE with a degenerate testcase though:

$ cat testcase.c
void
foo (void)
{
  for (unsigned i = 0; i == 0; i++)
    __builtin_printf ("%d", i);
}
$ x86_64-pc-linux-gnu-gcc -O -fharden-compares -fno-tree-forwprop -fno-tree-ch -fno-tree-dominator-opts -fno-tree-ccp -funreachable-traps --param=scev-max-expr-size=1 testcase.c 
during GIMPLE pass: optimized
testcase.c: In function 'foo':
testcase.c:2:1: internal compiler error: in execute_todo, at passes.cc:2140
    2 | foo (void)
      | ^~~
0x75616a execute_todo
        /repo/gcc-trunk/gcc/passes.cc:2140
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
Comment 11 Zdenek Sojka 2022-08-23 05:23:42 UTC
And with another degenerate flags on the testcase above:
# x86_64-pc-linux-gnu-gcc -O -fno-tree-forwprop -fno-tree-dominator-opts -fharden-conditional-branches -funreachable-traps -fno-tree-ccp --param=max-loop-header-insns=0 --param=scev-max-expr-size=0 testcase.c
during GIMPLE pass: optimized
testcase.c: In function 'foo':
testcase.c:2:1: internal compiler error: in execute_todo, at passes.cc:2140
    2 | foo (void)
      | ^~~
0x75d426 execute_todo
        /repo/gcc-trunk/gcc/passes.cc:2140
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
Comment 12 Martin Liška 2022-08-23 07:04:33 UTC
@Jakub: Can you please take a look at the new testcase that still ICEs?
Comment 13 Martin Liška 2022-08-23 07:06:21 UTC
One another testcase:

$ gcc /home/marxin/Programming/gcc/gcc/testsuite/c-c++-common/ubsan/pr63839.c -Og
/home/marxin/Programming/gcc/gcc/testsuite/c-c++-common/ubsan/pr63839.c: In function ‘bar’:
/home/marxin/Programming/gcc/gcc/testsuite/c-c++-common/ubsan/pr63839.c:9:1: warning: ‘noreturn’ function does return
    9 | } /* { dg-warning "function does return" } */
      | ^
during GIMPLE pass: local-pure-const
/home/marxin/Programming/gcc/gcc/testsuite/c-c++-common/ubsan/pr63839.c:21:1: internal compiler error: in execute_todo, at passes.cc:2140
   21 | }
      | ^
0x76e8ac execute_todo
	/home/marxin/Programming/gcc/gcc/passes.cc:2140
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
Comment 14 Jakub Jelinek 2022-08-23 12:54:50 UTC
Created attachment 53496 [details]
gcc13-pr106099.patch

Untested fix.  There are 2 issues.  One was that tree-cfg.cc was emitting __builtin_trap instead of __builtin_unreachable which needed vops (fixed by emitting .TRAP instead) and another one was that CDDCE was actually throwing away .TRAP calls - since PR44485 flags_from_decl_or_type actually implicitly
adds ECF_LOOPING_CONST_OR_PURE to ECF_NORETURN|ECF_CONST normal calls, so
__builtin_unreachable wasn't DCEd even when it is const and lhs was NULL,
but for internal-fns that doesn't happen, so I had to add ECF_LOOPING_CONST_OR_PURE explicitly to avoid DCE of .TRAP.
Comment 15 CVS Commits 2022-08-26 07:29:51 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:530dc5aaaeb67c223fd0e3986d635408dcea4343

commit r13-2213-g530dc5aaaeb67c223fd0e3986d635408dcea4343
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Aug 26 09:28:48 2022 +0200

    internal-fn, tree-cfg: Fix .TRAP handling and another __builtin_trap vops issue [PR106099]
    
    This patch fixes 2 __builtin_unreachable/__builtin_trap related issues.
    One (first hunk) is that CDDCE happily removes calls to .TRAP ()
    internal-fn as useless.  The problem is that the internal-fn is
    ECF_CONST | ECF_NORETURN, doesn't have lhs and so DCE thinks it doesn't
    have side-effects and removes it.  __builtin_unreachable which has
    the same ECF_* flags works fine, as since PR44485 we implicitly add
    ECF_LOOPING_CONST_OR_PURE to ECF_CONST | ECF_NORETURN builtins, but
    do it in flags_from_decl_or_type which isn't called for internal-fns.
    As IFN_TRAP is the only ifn with such flags, it seems easier to
    add it explicitly.
    
    The other issue (which on the testcase can be seen only with the
    first bug unfixed) is that execute_fixup_cfg can add a __builtin_trap
    which needs vops, but nothing adds it and it can appear in many passes
    which don't have corresponding TODO_update_ssa_only_virtuals etc.
    Fixed similarly as last time but emitting ifn there instead.
    
    2022-08-26  Jakub Jelinek  <jakub@redhat.com>
    
            PR tree-optimization/106099
            * internal-fn.def (TRAP): Add ECF_LOOPING_CONST_OR_PURE flag.
            * tree-cfg.cc (execute_fixup_cfg): Add IFN_TRAP instead of
            __builtin_trap to avoid the need of vops.
    
            * gcc.dg/pr106099.c: New test.
Comment 16 Jakub Jelinek 2022-10-18 13:02:23 UTC
Should be fixed now.