| Message ID | 20200920230809.2415770-1-polacek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c: Fix -Wduplicated-branches ICE [PR97125] | expand |
On 9/20/20 5:08 PM, Marek Polacek via Gcc-patches wrote: > We crash here because since r11-3302 the C FE uses codes like SWITCH_STMT > in the else branches in the attached test, and inchash::add_expr in > do_warn_duplicated_branches doesn't handle these front-end codes. In > the C++ FE this works because by the time we get to do_warn_duplicated_branches > we've already cp_genericize'd the SWITCH_STMT tree into a SWITCH_EXPR. > > The fix is to call do_warn_duplicated_branches_r only after loops and other > structured control constructs have been lowered. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > gcc/c-family/ChangeLog: > > PR c/97125 > * c-gimplify.c (c_genericize): Only call do_warn_duplicated_branches_r > after loops and other structured control constructs have been lowered. > > gcc/testsuite/ChangeLog: > > PR c/97125 > * c-c++-common/Wduplicated-branches-15.c: New test. As I noted in the issue, this was my bad, and the fix looks good to me (although I have no authority to approve it). The same problem was independently reported as PR c/97157. -Sandra
On Tue, Sep 22, 2020 at 08:18:09PM -0600, Sandra Loosemore wrote: > On 9/20/20 5:08 PM, Marek Polacek via Gcc-patches wrote: > > We crash here because since r11-3302 the C FE uses codes like SWITCH_STMT > > in the else branches in the attached test, and inchash::add_expr in > > do_warn_duplicated_branches doesn't handle these front-end codes. In > > the C++ FE this works because by the time we get to do_warn_duplicated_branches > > we've already cp_genericize'd the SWITCH_STMT tree into a SWITCH_EXPR. > > > > The fix is to call do_warn_duplicated_branches_r only after loops and other > > structured control constructs have been lowered. > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > gcc/c-family/ChangeLog: > > > > PR c/97125 > > * c-gimplify.c (c_genericize): Only call do_warn_duplicated_branches_r > > after loops and other structured control constructs have been lowered. > > > > gcc/testsuite/ChangeLog: > > > > PR c/97125 > > * c-c++-common/Wduplicated-branches-15.c: New test. > > As I noted in the issue, this was my bad, and the fix looks good to me > (although I have no authority to approve it). No worries, I can wait. > The same problem was independently reported as PR c/97157. Thanks, closed that one. Marek
On Sun, Sep 20, 2020 at 07:08:09PM -0400, Marek Polacek via Gcc-patches wrote: > We crash here because since r11-3302 the C FE uses codes like SWITCH_STMT > in the else branches in the attached test, and inchash::add_expr in > do_warn_duplicated_branches doesn't handle these front-end codes. In > the C++ FE this works because by the time we get to do_warn_duplicated_branches > we've already cp_genericize'd the SWITCH_STMT tree into a SWITCH_EXPR. > > The fix is to call do_warn_duplicated_branches_r only after loops and other > structured control constructs have been lowered. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > gcc/c-family/ChangeLog: > > PR c/97125 > * c-gimplify.c (c_genericize): Only call do_warn_duplicated_branches_r > after loops and other structured control constructs have been lowered. > > gcc/testsuite/ChangeLog: > > PR c/97125 > * c-c++-common/Wduplicated-branches-15.c: New test. LGTM, thanks. Jakub
diff --git a/gcc/c-family/c-gimplify.c b/gcc/c-family/c-gimplify.c index 8b326c99d48..d1e391590dd 100644 --- a/gcc/c-family/c-gimplify.c +++ b/gcc/c-family/c-gimplify.c @@ -533,10 +533,6 @@ c_genericize (tree fndecl) &pset); } - if (warn_duplicated_branches) - walk_tree_without_duplicates (&DECL_SAVED_TREE (fndecl), - do_warn_duplicated_branches_r, NULL); - /* Genericize loops and other structured control constructs. The C++ front end has already done this in lang-specific code. */ if (!c_dialect_cxx ()) @@ -550,6 +546,10 @@ c_genericize (tree fndecl) pop_cfun (); } + if (warn_duplicated_branches) + walk_tree_without_duplicates (&DECL_SAVED_TREE (fndecl), + do_warn_duplicated_branches_r, NULL); + /* Dump the C-specific tree IR. */ dump_orig = get_dump_info (TDI_original, &local_dump_flags); if (dump_orig) diff --git a/gcc/testsuite/c-c++-common/Wduplicated-branches-15.c b/gcc/testsuite/c-c++-common/Wduplicated-branches-15.c new file mode 100644 index 00000000000..d4943607086 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wduplicated-branches-15.c @@ -0,0 +1,32 @@ +/* PR c/97125 */ +/* { dg-do compile } */ +/* { dg-options "-Wduplicated-branches" } */ + +void foo (void); + +void +fn1 (void) +{ + if (0) + foo (); + else + switch (0); +} + +void +fn2 (void) +{ + if (0) + foo (); + else + while (0); +} + +void +fn3 (void) +{ + if (0) + foo (); + else + for (;;); +}