From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18100 invoked by alias); 26 Nov 2017 00:23:08 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 18090 invoked by uid 89); 26 Nov 2017 00:23:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.7 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 26 Nov 2017 00:23:04 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 46F6481DF4; Sun, 26 Nov 2017 00:23:03 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-116-247.ams2.redhat.com [10.36.116.247]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D47285D6B2; Sun, 26 Nov 2017 00:23:01 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id vAQ0MwfS012267; Sun, 26 Nov 2017 01:22:58 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id vAQ0MuPg012266; Sun, 26 Nov 2017 01:22:56 +0100 Date: Sun, 26 Nov 2017 09:05:00 -0000 From: Jakub Jelinek To: Jason Merrill , Nathan Sidwell Cc: gcc-patches@gcc.gnu.org Subject: Re: [C++ PATCH] Avoid -Wreturn-type warnings if a switch has default label, no breaks inside of it, but is followed by a break (PR sanitizer/81275) Message-ID: <20171126002256.GJ14653@tucnak> Reply-To: Jakub Jelinek References: <20171124215953.GE14653@tucnak> <20171125090122.GH14653@tucnak> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171125090122.GH14653@tucnak> User-Agent: Mutt/1.7.1 (2016-10-04) X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg02246.txt.bz2 On Sat, Nov 25, 2017 at 10:01:22AM +0100, Jakub Jelinek wrote: > Actually, thinking about it some more, maybe it would be more efficient > to gather this information during construction of the SWITCH_STMT in some > new flag on the tree, so cxx_block_may_fallthru would just: Here it is implemented, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2017-11-25 Jakub Jelinek PR sanitizer/81275 * cp-tree.h (SWITCH_STMT_CANNOT_FALLTHRU_P): Define. (note_break_stmt, note_iteration_stmt_body_start, note_iteration_stmt_body_end): Declare. * decl.c (struct cp_switch): Add has_default_p, break_stmt_seen_p and in_loop_body_p fields. (push_switch): Clear them. (pop_switch): Set SWITCH_STMT_CANNOT_FALLTHRU_P if has_default_p and !break_stmt_seen_p. Assert in_loop_body_p is false. (note_break_stmt, note_iteration_stmt_body_start, note_iteration_stmt_body_end): New functions. (finish_case_label): Set has_default_p when both low and high are NULL_TREE. * parser.c (cp_parser_iteration_statement): Use note_iteration_stmt_body_start and note_iteration_stmt_body_end around parsing iteration body. * pt.c (tsubst_expr): Likewise. * cp-objcp-common.c (cxx_block_may_fallthru): Return false for SWITCH_STMT which contains no BREAK_STMTs, contains a default: CASE_LABEL_EXPR and where SWITCH_STMT_BODY isn't empty and can't fallthru. * semantics.c (finish_break_stmt): Call note_break_stmt. * g++.dg/warn/pr81275-1.C: New test. * g++.dg/warn/pr81275-2.C: New test. * g++.dg/warn/pr81275-3.C: New test. --- gcc/cp/cp-tree.h.jj 2017-11-17 08:40:32.000000000 +0100 +++ gcc/cp/cp-tree.h 2017-11-25 21:25:48.277897180 +0100 @@ -364,6 +364,7 @@ extern GTY(()) tree cp_global_trees[CPTI IF_STMT_CONSTEXPR_P (IF_STMT) TEMPLATE_TYPE_PARM_FOR_CLASS (TEMPLATE_TYPE_PARM) DECL_NAMESPACE_INLINE_P (in NAMESPACE_DECL) + SWITCH_STMT_CANNOT_FALLTHRU_P (in SWITCH_STMT) 1: IDENTIFIER_KIND_BIT_1 (in IDENTIFIER_NODE) TI_PENDING_TEMPLATE_FLAG. TEMPLATE_PARMS_FOR_INLINE. @@ -4840,6 +4841,12 @@ more_aggr_init_expr_args_p (const aggr_i #define SWITCH_STMT_BODY(NODE) TREE_OPERAND (SWITCH_STMT_CHECK (NODE), 1) #define SWITCH_STMT_TYPE(NODE) TREE_OPERAND (SWITCH_STMT_CHECK (NODE), 2) #define SWITCH_STMT_SCOPE(NODE) TREE_OPERAND (SWITCH_STMT_CHECK (NODE), 3) +/* Set if the body of a switch stmt contains a default: case label + and does not contain any break; stmts, thus if SWITCH_STMT_BODY + is not empty and doesn't fallthru, then the whole SWITCH_STMT + can't fallthru either. */ +#define SWITCH_STMT_CANNOT_FALLTHRU_P(NODE) \ + TREE_LANG_FLAG_0 (SWITCH_STMT_CHECK (NODE)) /* STMT_EXPR accessor. */ #define STMT_EXPR_STMT(NODE) TREE_OPERAND (STMT_EXPR_CHECK (NODE), 0) @@ -6102,6 +6109,9 @@ enum cp_tree_node_structure_enum cp_tree extern void finish_scope (void); extern void push_switch (tree); extern void pop_switch (void); +extern void note_break_stmt (void); +extern bool note_iteration_stmt_body_start (void); +extern void note_iteration_stmt_body_end (bool); extern tree make_lambda_name (void); extern int decls_match (tree, tree); extern bool maybe_version_functions (tree, tree); --- gcc/cp/decl.c.jj 2017-11-22 21:37:46.000000000 +0100 +++ gcc/cp/decl.c 2017-11-25 21:25:26.306162437 +0100 @@ -3427,6 +3427,13 @@ struct cp_switch /* Remember whether there was a case value that is outside the range of the original type of the controlling expression. */ bool outside_range_p; + /* Remember whether a default: case label has been seen. */ + bool has_default_p; + /* Remember whether a BREAK_STMT has been seen in this SWITCH_STMT. */ + bool break_stmt_seen_p; + /* Set if inside of {FOR,DO,WHILE}_BODY nested inside of a switch, + where BREAK_STMT doesn't belong to the SWITCH_STMT. */ + bool in_loop_body_p; }; /* A stack of the currently active switch statements. The innermost @@ -3449,6 +3456,9 @@ push_switch (tree switch_stmt) p->switch_stmt = switch_stmt; p->cases = splay_tree_new (case_compare, NULL, NULL); p->outside_range_p = false; + p->has_default_p = false; + p->break_stmt_seen_p = false; + p->in_loop_body_p = false; switch_stack = p; } @@ -3469,11 +3479,50 @@ pop_switch (void) SWITCH_STMT_COND (cs->switch_stmt), bool_cond_p, cs->outside_range_p); + /* For the benefit of block_may_fallthru remember if the switch body + contains a default: case label and no break; stmts. */ + if (cs->has_default_p && !cs->break_stmt_seen_p) + SWITCH_STMT_CANNOT_FALLTHRU_P (cs->switch_stmt) = 1; + gcc_assert (!cs->in_loop_body_p); splay_tree_delete (cs->cases); switch_stack = switch_stack->next; free (cs); } +/* Note that a BREAK_STMT is about to be added. If it is inside of + a SWITCH_STMT and not inside of a loop body inside of it, note + in switch_stack we've seen a BREAK_STMT. */ + +void +note_break_stmt (void) +{ + if (switch_stack && !switch_stack->in_loop_body_p) + switch_stack->break_stmt_seen_p = true; +} + +/* Note the start of processing of an iteration statement's body. + The note_break_stmt function will do nothing while processing it. + Return a flag that should be passed to note_iteration_stmt_body_end. */ + +bool +note_iteration_stmt_body_start (void) +{ + if (!switch_stack) + return false; + bool ret = switch_stack->in_loop_body_p; + switch_stack->in_loop_body_p = true; + return ret; +} + +/* Note the end of processing of an iteration statement's body. */ + +void +note_iteration_stmt_body_end (bool prev) +{ + if (switch_stack) + switch_stack->in_loop_body_p = prev; +} + /* Convert a case constant VALUE in a switch to the type TYPE of the switch condition. Note that if TYPE and VALUE are already integral we don't really do the conversion because the language-independent @@ -3508,6 +3557,9 @@ finish_case_label (location_t loc, tree cp_binding_level *p; tree type; + if (low_value == NULL_TREE && high_value == NULL_TREE) + switch_stack->has_default_p = true; + if (processing_template_decl) { tree label; --- gcc/cp/parser.c.jj 2017-11-23 21:17:52.000000000 +0100 +++ gcc/cp/parser.c 2017-11-25 21:29:52.644946109 +0100 @@ -12256,7 +12256,9 @@ cp_parser_iteration_statement (cp_parser parens.require_close (parser); /* Parse the dependent statement. */ parser->in_statement = IN_ITERATION_STMT; + bool prev = note_iteration_stmt_body_start (); cp_parser_already_scoped_statement (parser, if_p, guard_tinfo); + note_iteration_stmt_body_end (prev); parser->in_statement = in_statement; /* We're done with the while-statement. */ finish_while_stmt (statement); @@ -12271,7 +12273,9 @@ cp_parser_iteration_statement (cp_parser statement = begin_do_stmt (); /* Parse the body of the do-statement. */ parser->in_statement = IN_ITERATION_STMT; + bool prev = note_iteration_stmt_body_start (); cp_parser_implicitly_scoped_statement (parser, NULL, guard_tinfo); + note_iteration_stmt_body_end (prev); parser->in_statement = in_statement; finish_do_body (statement); /* Look for the `while' keyword. */ @@ -12303,7 +12307,9 @@ cp_parser_iteration_statement (cp_parser /* Parse the body of the for-statement. */ parser->in_statement = IN_ITERATION_STMT; + bool prev = note_iteration_stmt_body_start (); cp_parser_already_scoped_statement (parser, if_p, guard_tinfo); + note_iteration_stmt_body_end (prev); parser->in_statement = in_statement; /* We're done with the for-statement. */ --- gcc/cp/cp-objcp-common.c.jj 2017-11-24 22:38:51.214379987 +0100 +++ gcc/cp/cp-objcp-common.c 2017-11-25 20:43:09.221755658 +0100 @@ -349,6 +349,11 @@ cxx_block_may_fallthru (const_tree stmt) case THROW_EXPR: return false; + case SWITCH_STMT: + return (!SWITCH_STMT_CANNOT_FALLTHRU_P (stmt) + || SWITCH_STMT_BODY (stmt) == NULL_TREE + || block_may_fallthru (SWITCH_STMT_BODY (stmt))); + default: return true; } --- gcc/cp/semantics.c.jj 2017-11-23 21:13:28.000000000 +0100 +++ gcc/cp/semantics.c 2017-11-25 21:07:22.443247156 +0100 @@ -1118,6 +1118,7 @@ finish_break_stmt (void) understand. */ if (!block_may_fallthru (cur_stmt_list)) return void_node; + note_break_stmt (); return add_stmt (build_stmt (input_location, BREAK_STMT)); } --- gcc/cp/pt.c.jj 2017-11-23 21:13:28.000000000 +0100 +++ gcc/cp/pt.c 2017-11-25 21:32:26.956082032 +0100 @@ -16122,7 +16122,11 @@ tsubst_expr (tree t, tree args, tsubst_f finish_for_cond (tmp, stmt, false); tmp = RECUR (FOR_EXPR (t)); finish_for_expr (tmp, stmt); - RECUR (FOR_BODY (t)); + { + bool prev = note_iteration_stmt_body_start (); + RECUR (FOR_BODY (t)); + note_iteration_stmt_body_end (prev); + } finish_for_stmt (stmt); break; @@ -16146,7 +16150,9 @@ tsubst_expr (tree t, tree args, tsubst_f else stmt = cp_convert_range_for (stmt, decl, expr, NULL_TREE, 0, RANGE_FOR_IVDEP (t)); + bool prev = note_iteration_stmt_body_start (); RECUR (RANGE_FOR_BODY (t)); + note_iteration_stmt_body_end (prev); finish_for_stmt (stmt); } break; @@ -16155,13 +16161,21 @@ tsubst_expr (tree t, tree args, tsubst_f stmt = begin_while_stmt (); tmp = RECUR (WHILE_COND (t)); finish_while_stmt_cond (tmp, stmt, false); - RECUR (WHILE_BODY (t)); + { + bool prev = note_iteration_stmt_body_start (); + RECUR (WHILE_BODY (t)); + note_iteration_stmt_body_end (prev); + } finish_while_stmt (stmt); break; case DO_STMT: stmt = begin_do_stmt (); - RECUR (DO_BODY (t)); + { + bool prev = note_iteration_stmt_body_start (); + RECUR (DO_BODY (t)); + note_iteration_stmt_body_end (prev); + } finish_do_body (stmt); tmp = RECUR (DO_COND (t)); finish_do_stmt (tmp, stmt, false); --- gcc/testsuite/g++.dg/warn/pr81275-1.C.jj 2017-11-25 20:40:07.679937379 +0100 +++ gcc/testsuite/g++.dg/warn/pr81275-1.C 2017-11-25 21:44:13.303549363 +0100 @@ -0,0 +1,165 @@ +// PR sanitizer/81875 +// { dg-do compile } +// { dg-options "-Wreturn-type" } + +struct C { C (); ~C (); }; + +int +f1 (int a, int b) +{ + C f; + switch (a) + { + case 0: + switch (b) + { + case 13: + return 7; + case 24: + return 19; + default: + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-bogus "control reaches end of non-void function" } + +int +f2 (int a, int b, int c, int d) +{ + C f; + switch (a) + { + case 0: + switch (b) + { + case 13: + while (c >= 10) + { + if (c == d) + break; + c--; + } + return 7; + case 29: + switch (d) + { + case 35: + break; + default: + return 9; + } + return 8; + case 24: + do + { + if (c == d) + break; + c--; + } + while (c >= 10); + return 19; + default: + for (int e = 0; e < c; ++e) + if (e == d) + break; + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-bogus "control reaches end of non-void function" } + +template +int +f3 (int a, int b) +{ + C f; + switch (a) + { + case 0: + switch (b) + { + case 13: + return 7; + case 24: + return 19; + default: + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-bogus "control reaches end of non-void function" } + +template +int +f4 (int a, int b, int c, int d) +{ + C f; + switch (a) + { + case 0: + switch (b) + { + case 13: + while (c >= 10) + { + if (c == d) + break; + c--; + } + return 7; + case 29: + switch (d) + { + case 35: + break; + default: + return 9; + } + return 8; + case 24: + do + { + if (c == d) + break; + c--; + } + while (c >= 10); + return 19; + default: + for (int e = 0; e < c; ++e) + if (e == d) + break; + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-bogus "control reaches end of non-void function" } + +int +f5 (int a, int b) +{ + return f3 <0> (a, b); +} + +int +f6 (int a, int b, int c, int d) +{ + return f4 <2> (a, b, c, d); +} --- gcc/testsuite/g++.dg/warn/pr81275-2.C.jj 2017-11-25 21:37:40.291296947 +0100 +++ gcc/testsuite/g++.dg/warn/pr81275-2.C 2017-11-25 21:44:21.796446769 +0100 @@ -0,0 +1,165 @@ +// PR sanitizer/81875 +// { dg-do compile } +// { dg-options "-Wreturn-type" } + +struct C { C (); ~C (); }; + +int +f1 (int a, int b) +{ + C f; + switch (a) + { + case 0: + switch (b) + { + case 13: + return 7; + case 24: + return 19; + case 25: + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-warning "control reaches end of non-void function" } + +int +f2 (int a, int b, int c, int d) +{ + C f; + switch (a) + { + case 0: + switch (b) + { + case 13: + while (c >= 10) + { + if (c == d) + break; + c--; + } + return 7; + case 29: + switch (d) + { + case 35: + break; + default: + return 9; + } + return 8; + case 24: + do + { + if (c == d) + break; + c--; + } + while (c >= 10); + return 19; + case 25: + for (int e = 0; e < c; ++e) + if (e == d) + break; + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-warning "control reaches end of non-void function" } + +template +int +f3 (int a, int b) +{ + C f; + switch (a) + { + case 0: + switch (b) + { + case 13: + return 7; + case 24: + return 19; + case 25: + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-warning "control reaches end of non-void function" } + +template +int +f4 (int a, int b, int c, int d) +{ + C f; + switch (a) + { + case 0: + switch (b) + { + case 13: + while (c >= 10) + { + if (c == d) + break; + c--; + } + return 7; + case 29: + switch (d) + { + case 35: + break; + default: + return 9; + } + return 8; + case 24: + do + { + if (c == d) + break; + c--; + } + while (c >= 10); + return 19; + case 25: + for (int e = 0; e < c; ++e) + if (e == d) + break; + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-warning "control reaches end of non-void function" } + +int +f5 (int a, int b) +{ + return f3 <0> (a, b); +} + +int +f6 (int a, int b, int c, int d) +{ + return f4 <2> (a, b, c, d); +} --- gcc/testsuite/g++.dg/warn/pr81275-3.C.jj 2017-11-25 21:45:03.282945613 +0100 +++ gcc/testsuite/g++.dg/warn/pr81275-3.C 2017-11-25 21:47:08.797429400 +0100 @@ -0,0 +1,173 @@ +// PR sanitizer/81875 +// { dg-do compile } +// { dg-options "-Wreturn-type" } + +struct C { C (); ~C (); }; + +int +f1 (int a, int b, int c) +{ + C f; + switch (a) + { + case 0: + switch (b) + { + case 13: + return 7; + case 24: + if (c == 5) + break; + return 19; + default: + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-warning "control reaches end of non-void function" } + +int +f2 (int a, int b, int c, int d) +{ + C f; + switch (a) + { + case 0: + switch (b) + { + case 13: + while (c >= 10) + { + if (c == d) + break; + c--; + } + return 7; + case 29: + switch (d) + { + case 35: + break; + default: + return 9; + } + if (c == d + 20) + break; + return 8; + case 24: + do + { + if (c == d) + break; + c--; + } + while (c >= 10); + return 19; + default: + for (int e = 0; e < c; ++e) + if (e == d) + break; + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-warning "control reaches end of non-void function" } + +template +int +f3 (int a, int b, int c) +{ + C f; + switch (a) + { + case 0: + switch (b) + { + case 13: + return 7; + case 24: + if (c == 5) + break; + return 19; + default: + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-warning "control reaches end of non-void function" } + +template +int +f4 (int a, int b, int c, int d) +{ + C f; + switch (a) + { + case 0: + switch (b) + { + case 13: + while (c >= 10) + { + if (c == d) + break; + c--; + } + return 7; + case 29: + switch (d) + { + case 35: + break; + default: + return 9; + } + if (c == d + 20) + break; + return 8; + case 24: + do + { + if (c == d) + break; + c--; + } + while (c >= 10); + return 19; + default: + for (int e = 0; e < c; ++e) + if (e == d) + break; + return 0; + } + break; + default: + return 0; + case 9: + return 17; + } +} // { dg-warning "control reaches end of non-void function" } + +int +f5 (int a, int b, int c) +{ + return f3 <0> (a, b, c); +} + +int +f6 (int a, int b, int c, int d) +{ + return f4 <2> (a, b, c, d); +} Jakub