From: Jakub Jelinek Date: Tue, 28 Nov 2017 21:24:32 +0000 (+0100) Subject: re PR sanitizer/81275 (-fsanitize=thread produce incorrect -Wreturn-type warning) X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=1a2e970832f6076e76adc06b42c106bdb568a86c;p=gcc.git re PR sanitizer/81275 (-fsanitize=thread produce incorrect -Wreturn-type warning) PR sanitizer/81275 * cp-tree.h (SWITCH_STMT_ALL_CASES_P): Define. (SWITCH_STMT_NO_BREAK_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. * cp-gimplify.c (genericize_switch_stmt): Copy SWITCH_STMT_ALL_CASES_P bit to SWITCH_ALL_CASES_P. Assert that if SWITCH_STMT_NO_BREAK_P then the break label is not TREE_USED. * g++.dg/warn/pr81275-1.C: New test. * g++.dg/warn/pr81275-2.C: New test. * g++.dg/warn/pr81275-3.C: New test. * c-c++-common/tsan/pr81275.c: Skip for C++ and -O2. From-SVN: r255218 --- diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index a93a9012958..2cb90b82bfc 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,32 @@ +2017-11-28 Jakub Jelinek + + PR sanitizer/81275 + * cp-tree.h (SWITCH_STMT_ALL_CASES_P): Define. + (SWITCH_STMT_NO_BREAK_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. + * cp-gimplify.c (genericize_switch_stmt): Copy SWITCH_STMT_ALL_CASES_P + bit to SWITCH_ALL_CASES_P. Assert that if SWITCH_STMT_NO_BREAK_P then + the break label is not TREE_USED. + 2017-11-28 Julia Koval Sebastian Peryt diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c index 49fdd05ee24..3187a64a725 100644 --- a/gcc/cp/cp-gimplify.c +++ b/gcc/cp/cp-gimplify.c @@ -331,6 +331,9 @@ genericize_switch_stmt (tree *stmt_p, int *walk_subtrees, void *data) *walk_subtrees = 0; *stmt_p = build2_loc (stmt_locus, SWITCH_EXPR, type, cond, body); + SWITCH_ALL_CASES_P (*stmt_p) = SWITCH_STMT_ALL_CASES_P (stmt); + gcc_checking_assert (!SWITCH_STMT_NO_BREAK_P (stmt) + || !TREE_USED (break_block)); finish_bc_block (stmt_p, bc_break, break_block); } diff --git a/gcc/cp/cp-objcp-common.c b/gcc/cp/cp-objcp-common.c index 59087df64c9..dc558eed1a5 100644 --- a/gcc/cp/cp-objcp-common.c +++ b/gcc/cp/cp-objcp-common.c @@ -349,6 +349,11 @@ cxx_block_may_fallthru (const_tree stmt) case THROW_EXPR: return false; + case SWITCH_STMT: + return (!SWITCH_STMT_ALL_CASES_P (stmt) + || !SWITCH_STMT_NO_BREAK_P (stmt) + || block_may_fallthru (SWITCH_STMT_BODY (stmt))); + default: return true; } diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index bdfe3fdc497..4780df4dbf6 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -364,6 +364,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX]; IF_STMT_CONSTEXPR_P (IF_STMT) TEMPLATE_TYPE_PARM_FOR_CLASS (TEMPLATE_TYPE_PARM) DECL_NAMESPACE_INLINE_P (in NAMESPACE_DECL) + SWITCH_STMT_ALL_CASES_P (in SWITCH_STMT) 1: IDENTIFIER_KIND_BIT_1 (in IDENTIFIER_NODE) TI_PENDING_TEMPLATE_FLAG. TEMPLATE_PARMS_FOR_INLINE. @@ -395,6 +396,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX]; AGGR_INIT_ZERO_FIRST (in AGGR_INIT_EXPR) CONSTRUCTOR_MUTABLE_POISON (in CONSTRUCTOR) OVL_HIDDEN_P (in OVERLOAD) + SWITCH_STMT_NO_BREAK_P (in SWITCH_STMT) 3: (TREE_REFERENCE_EXPR) (in NON_LVALUE_EXPR) (commented-out). ICS_BAD_FLAG (in _CONV) FN_TRY_BLOCK_P (in TRY_BLOCK) @@ -4840,6 +4842,14 @@ more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter) #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) +/* True if there are case labels for all possible values of switch cond, either + because there is a default: case label or because the case label ranges cover + all values. */ +#define SWITCH_STMT_ALL_CASES_P(NODE) \ + TREE_LANG_FLAG_0 (SWITCH_STMT_CHECK (NODE)) +/* True if the body of a switch stmt contains no BREAK_STMTs. */ +#define SWITCH_STMT_NO_BREAK_P(NODE) \ + TREE_LANG_FLAG_2 (SWITCH_STMT_CHECK (NODE)) /* STMT_EXPR accessor. */ #define STMT_EXPR_STMT(NODE) TREE_OPERAND (STMT_EXPR_CHECK (NODE), 0) @@ -6102,6 +6112,9 @@ enum cp_tree_node_structure_enum cp_tree_node_structure 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); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 9f557b8d00f..7085d5a3976 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -3426,6 +3426,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 @@ -3448,6 +3455,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; } @@ -3468,11 +3478,55 @@ 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 + case labels cover all possible values and if there are break; stmts. */ + if (cs->has_default_p + || (!processing_template_decl + && c_switch_covers_all_cases_p (cs->cases, + SWITCH_STMT_TYPE (cs->switch_stmt)))) + SWITCH_STMT_ALL_CASES_P (cs->switch_stmt) = 1; + if (!cs->break_stmt_seen_p) + SWITCH_STMT_NO_BREAK_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 @@ -3507,6 +3561,9 @@ finish_case_label (location_t loc, tree low_value, tree high_value) 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; diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index b2de4400727..43fc1be299f 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -12043,7 +12043,9 @@ cp_parser_iteration_statement (cp_parser* parser, bool *if_p, bool ivdep) 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); @@ -12058,7 +12060,9 @@ cp_parser_iteration_statement (cp_parser* parser, bool *if_p, bool ivdep) 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. */ @@ -12090,7 +12094,9 @@ cp_parser_iteration_statement (cp_parser* parser, bool *if_p, bool ivdep) /* 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. */ diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 706e3a972f6..7e2f7740106 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -16124,7 +16124,11 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl, 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; @@ -16148,7 +16152,9 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl, 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; @@ -16157,13 +16163,21 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl, 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); diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 141921dcc61..e2daab4339e 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -1102,6 +1102,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)); } diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 87332af7a29..c2488b6412d 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,5 +1,11 @@ 2017-11-28 Jakub Jelinek + PR sanitizer/81275 + * g++.dg/warn/pr81275-1.C: New test. + * g++.dg/warn/pr81275-2.C: New test. + * g++.dg/warn/pr81275-3.C: New test. + * c-c++-common/tsan/pr81275.c: Skip for C++ and -O2. + PR sanitizer/81275 * c-c++-common/tsan/pr81275.c: New test. diff --git a/gcc/testsuite/c-c++-common/tsan/pr81275.c b/gcc/testsuite/c-c++-common/tsan/pr81275.c index 024b0c72f2c..14158506406 100644 --- a/gcc/testsuite/c-c++-common/tsan/pr81275.c +++ b/gcc/testsuite/c-c++-common/tsan/pr81275.c @@ -1,6 +1,7 @@ /* PR sanitizer/81275 */ /* { dg-do compile } */ /* { dg-options "-Wreturn-type -fsanitize=thread" } */ +/* { dg-skip-if "" { c++ } { "*" } { "-O0" } } */ int f1 (int a, int b) diff --git a/gcc/testsuite/g++.dg/warn/pr81275-1.C b/gcc/testsuite/g++.dg/warn/pr81275-1.C new file mode 100644 index 00000000000..339b58c4d29 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/pr81275-1.C @@ -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); +} diff --git a/gcc/testsuite/g++.dg/warn/pr81275-2.C b/gcc/testsuite/g++.dg/warn/pr81275-2.C new file mode 100644 index 00000000000..fcfe262461b --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/pr81275-2.C @@ -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); +} diff --git a/gcc/testsuite/g++.dg/warn/pr81275-3.C b/gcc/testsuite/g++.dg/warn/pr81275-3.C new file mode 100644 index 00000000000..12e1320bb93 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/pr81275-3.C @@ -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); +}