[Bug c/97125] New: new problem with -Wduplicated-branches

[Bug c/97125] New: new problem with -Wduplicated-branches

[dcb314 at hotmail dot com]

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

            Bug ID: 97125
           Summary: new problem with -Wduplicated-branches
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dcb314 at hotmail dot com
  Target Milestone: ---

Created attachment 49243
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49243&action=edit
C source code

The attached C code goes wrong for the first time sometime
between 20200918 and 20200920.

I'll have a go at reducing it.

[Bug c/97125] new problem with -Wduplicated-branches

[dcb314 at hotmail dot com]

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

--- Comment #1 from David Binderman <dcb314 at hotmail dot com> ---

Reduced code is

void a() {
  if (0)
    a;
  else
    switch (0)
      ;
}

[Bug c/97125] [11 Regression] new problem with -Wduplicated-branches

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mpolacek at gcc dot gnu.org
   Target Milestone|---                         |11.0
           Keywords|                            |ice-on-valid-code
            Summary|new problem with            |[11 Regression] new problem
                   |-Wduplicated-branches       |with -Wduplicated-branches

--- Comment #2 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Started with r11-3302.

$ ./cc1 -quiet 97125.C -Wduplicated-branches
97125.C: In function ‘a’:
97125.C:11:1: internal compiler error: in hash_operand, at fold-const.c:3768
   11 | }
      | ^
0xd67ede operand_compare::hash_operand(tree_node const*, inchash::hash&,
unsigned int)
        /home/mpolacek/src/gcc/gcc/fold-const.c:3768
0xd6853c operand_compare::hash_operand(tree_node const*, inchash::hash&,
unsigned int)
        /home/mpolacek/src/gcc/gcc/fold-const.c:3858
0xd686fa inchash::add_expr(tree_node const*, inchash::hash&, unsigned int)
        /home/mpolacek/src/gcc/gcc/fold-const.c:3919
0xac5f4c do_warn_duplicated_branches
        /home/mpolacek/src/gcc/gcc/c-family/c-warn.c:2768
0xac6068 do_warn_duplicated_branches_r(tree_node**, int*, void*)
        /home/mpolacek/src/gcc/gcc/c-family/c-warn.c:2790
0x1725206 walk_tree_1(tree_node**, tree_node* (*)(tree_node**, int*, void*),
void*, hash_set<tree_node*, false, default_hash_traits<tree_node*> >*,
tree_node* (*)(tree_node**, int*, tree_node* (*)(tree_node**, int*, void*),
void*, hash_set<tree_node*, false, default_hash_traits<tree_node*> >*))
        /home/mpolacek/src/gcc/gcc/tree.c:12001
0x17269ee walk_tree_without_duplicates_1(tree_node**, tree_node*
(*)(tree_node**, int*, void*), void*, tree_node* (*)(tree_node**, int*,
tree_node* (*)(tree_node**, int*, void*), void*, hash_set<tree_node*, false,
default_hash_traits<tree_node*> >*))
        /home/mpolacek/src/gcc/gcc/tree.c:12362
0xa67f30 c_genericize(tree_node*)
        /home/mpolacek/src/gcc/gcc/c-family/c-gimplify.c:537
0x951b29 finish_function(unsigned int)
        /home/mpolacek/src/gcc/gcc/c/c-decl.c:10219
0x9b66b7 c_parser_declaration_or_fndef
        /home/mpolacek/src/gcc/gcc/c/c-parser.c:2562
0x9b4a36 c_parser_external_declaration
        /home/mpolacek/src/gcc/gcc/c/c-parser.c:1777
0x9b4598 c_parser_translation_unit
        /home/mpolacek/src/gcc/gcc/c/c-parser.c:1650
0x9f28ef c_parse_file()
        /home/mpolacek/src/gcc/gcc/c/c-parser.c:21821
0xa8237d c_common_parse_file()
        /home/mpolacek/src/gcc/gcc/c-family/c-opts.c:1188

[Bug c/97125] [11 Regression] new problem with -Wduplicated-branches

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-09-20

[Bug c/97125] [11 Regression] new problem with -Wduplicated-branches

[mpolacek at gcc dot gnu.org]

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

--- Comment #3 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
We crash because now the C FE uses a SWITCH_STMT in the else branch:

2766   /* Compute the hash of the else branch.  */
2767   inchash::hash hstate1 (0);
2768   inchash::add_expr (elseb, hstate1);
2769   hashval_t h1 = hstate1.end ();

but the fold-const.c routines don't know how to handle such a code.  This is
not a problem in the C++ FE 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.

[Bug c/97125] [11 Regression] new problem with -Wduplicated-branches

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |mpolacek at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

[Bug c/97125] [11 Regression] new problem with -Wduplicated-branches

[sandra at gcc dot gnu.org]

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

sandra at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sandra at gcc dot gnu.org

--- Comment #4 from sandra at gcc dot gnu.org ---
Hmmm, maybe it needs to know about the other structured control flow tree nodes
I moved from cp/ to c-family/, as well as SWITCH_STMT?

[Bug c/97125] [11 Regression] new problem with -Wduplicated-branches

[mpolacek at gcc dot gnu.org]

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

--- Comment #5 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
I think we just want to call do_warn_duplicated_branches_r after we've lowered
control statements:

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

[Bug c/97125] [11 Regression] new problem with -Wduplicated-branches

[sandra at gcc dot gnu.org]

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

--- Comment #6 from sandra at gcc dot gnu.org ---
Yeah, that looks like an easy fix.  Thanks for tracking it down.

[Bug c/97125] [11 Regression] new problem with -Wduplicated-branches

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jim at meyering dot net

--- Comment #7 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
*** Bug 97157 has been marked as a duplicate of this bug. ***

[Bug c/97125] [11 Regression] new problem with -Wduplicated-branches

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>:

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

commit r11-3397-gc48ffe21f8f3478cf08f9442e3f973df358caf2a
Author: Marek Polacek <polacek@redhat.com>
Date:   Sun Sep 20 16:11:00 2020 -0400

    c: Fix -Wduplicated-branches ICE [PR97125]

    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.

    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.

[Bug c/97125] [11 Regression] new problem with -Wduplicated-branches

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

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

--- Comment #9 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Fixed.