* [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)
@ 2017-11-25 0:37 Jakub Jelinek
2017-11-25 10:16 ` Jakub Jelinek
2017-11-27 12:44 ` Nathan Sidwell
0 siblings, 2 replies; 9+ messages in thread
From: Jakub Jelinek @ 2017-11-25 0:37 UTC (permalink / raw)
To: Jason Merrill, Nathan Sidwell; +Cc: gcc-patches
Hi!
The testcase below has a useless break; that causes a bogus -Wreturn-type
warning. The C++ FE already has code to avoid adding a BREAK_STMT
after a return or similar sequence that is known not to return.
The following patch extends block_may_fallthrough to also return false
for SWITCH_STMTs that can't fall through.
Those are ones with non-empty body where the whole body can't fallthrough,
additionally they need to have a default: case label (or cover the whole
range of values, but that is not what this patch can compute, that would
be too big duplication of the gimplification processing) and no BREAK_STMT.
For the default: case label we need to look in all SWITCH_BODY children
except for nested SWITCH_STMTs, for BREAK_STMTs also not in
{FOR,DO,WHILE}_BODY.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-11-24 Jakub Jelinek <jakub@redhat.com>
PR sanitizer/81275
* cp-objcp-common.c (struct find_default_and_break_s): New type.
(find_default_and_break): New function.
(cxx_block_may_fallthru): Return false for SWITCH_STMT which
contains no BREAK_STMTs and contains a default: CASE_LABEL_EXPR.
* g++.dg/warn/pr81275.C: New test.
--- gcc/cp/cp-objcp-common.c.jj 2017-11-15 09:38:25.000000000 +0100
+++ gcc/cp/cp-objcp-common.c 2017-11-24 20:08:35.047132170 +0100
@@ -335,6 +335,96 @@ init_shadowed_var_for_decl (void)
= hash_table<tree_decl_map_cache_hasher>::create_ggc (512);
}
+/* Data structure for find_default_and_break. */
+
+struct find_default_and_break_s
+{
+ hash_set<tree> *pset;
+ bool found_default;
+ bool look_for_break;
+};
+
+/* Helper function for cxx_block_may_fallthru SWITCH_STMT processing.
+ Determine if SWITCH_STMT body contains a BREAK_STMT and also record
+ if it contains default: CASE_LABEL_EXPR. */
+
+static tree
+find_default_and_break (tree *stmt_p, int *walk_subtrees, void *d)
+{
+ find_default_and_break_s *data = (find_default_and_break_s *) d;
+ tree stmt = *stmt_p;
+ switch (TREE_CODE (stmt))
+ {
+ case FOR_STMT:
+ if (cp_walk_tree (&FOR_INIT_STMT (stmt), find_default_and_break,
+ d, data->pset)
+ || cp_walk_tree (&FOR_COND (stmt), find_default_and_break,
+ d, data->pset)
+ || cp_walk_tree (&FOR_EXPR (stmt), find_default_and_break,
+ d, data->pset))
+ return stmt;
+ stmt = FOR_BODY (stmt);
+ goto loop_body;
+ case WHILE_STMT:
+ if (cp_walk_tree (&WHILE_COND (stmt), find_default_and_break,
+ d, data->pset))
+ return stmt;
+ stmt = WHILE_BODY (stmt);
+ goto loop_body;
+ case DO_STMT:
+ if (cp_walk_tree (&DO_COND (stmt), find_default_and_break,
+ d, data->pset))
+ return stmt;
+ stmt = DO_BODY (stmt);
+ loop_body:
+ *walk_subtrees = 0;
+ /* Inside of {FOR,WHILE,DO}_BODY we can ignore BREAK_STMTs that
+ are related to the corresponding loop, not the outer SWITCH_STMT.
+ If we haven't still found a default:, we need to look for it
+ though. */
+ if (!data->found_default)
+ {
+ if (data->look_for_break)
+ {
+ if (cp_walk_tree (&stmt, find_default_and_break, d, data->pset))
+ return *stmt_p;
+ }
+ else
+ {
+ data->look_for_break = true;
+ if (cp_walk_tree (&stmt, find_default_and_break, d, data->pset))
+ {
+ data->look_for_break = false;
+ return *stmt_p;
+ }
+ data->look_for_break = false;
+ }
+ }
+ return NULL_TREE;
+ case CASE_LABEL_EXPR:
+ if (CASE_LOW (stmt) == NULL_TREE && CASE_HIGH (stmt) == NULL_TREE)
+ {
+ /* Found default:. */
+ data->found_default = true;
+ if (!data->look_for_break)
+ return stmt;
+ }
+ return NULL_TREE;
+ case BREAK_STMT:
+ /* Found a break that can fall through out of the SWITCH_STMT. */
+ return data->look_for_break ? stmt : NULL_TREE;
+ case SWITCH_STMT:
+ /* Nested switch can't contain a break; nor default: for the outer
+ switch. */
+ *walk_subtrees = 0;
+ return NULL_TREE;
+ default:
+ if (TYPE_P (stmt))
+ *walk_subtrees = 0;
+ return NULL_TREE;
+ }
+}
+
/* Return true if stmt can fall through. Used by block_may_fallthru
default case. */
@@ -349,6 +439,25 @@ cxx_block_may_fallthru (const_tree stmt)
case THROW_EXPR:
return false;
+ case SWITCH_STMT:
+ if (SWITCH_STMT_BODY (stmt) == NULL_TREE
+ || block_may_fallthru (SWITCH_STMT_BODY (stmt)))
+ return true;
+ else
+ {
+ /* If the switch stmt body can't fall through, it can still
+ fall through if there is no default: label (checking whether
+ without default: case labels cover the whole range might be
+ too expensive) or if there are any break; stmts. */
+ hash_set<tree> pset;
+ find_default_and_break_s data = { &pset, false, true };
+ if (cp_walk_tree (&SWITCH_STMT_BODY (stmt), find_default_and_break,
+ &data, &pset)
+ || !data.found_default)
+ return true;
+ return false;
+ }
+
default:
return true;
}
--- gcc/testsuite/g++.dg/warn/pr81275.C.jj 2017-11-24 20:24:08.232869743 +0100
+++ gcc/testsuite/g++.dg/warn/pr81275.C 2017-11-24 20:23:38.000000000 +0100
@@ -0,0 +1,29 @@
+// PR sanitizer/81875
+// { dg-do compile }
+// { dg-options "-Wreturn-type" }
+
+struct C { C (); ~C (); };
+
+int
+foo (int a, int b)
+{
+ C c;
+ 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" }
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* 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)
2017-11-25 0:37 [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) Jakub Jelinek
@ 2017-11-25 10:16 ` Jakub Jelinek
2017-11-26 9:05 ` Jakub Jelinek
2017-11-27 12:49 ` [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) Nathan Sidwell
2017-11-27 12:44 ` Nathan Sidwell
1 sibling, 2 replies; 9+ messages in thread
From: Jakub Jelinek @ 2017-11-25 10:16 UTC (permalink / raw)
To: Jason Merrill, Nathan Sidwell; +Cc: gcc-patches
On Fri, Nov 24, 2017 at 10:59:53PM +0100, Jakub Jelinek wrote:
> The testcase below has a useless break; that causes a bogus -Wreturn-type
> warning. The C++ FE already has code to avoid adding a BREAK_STMT
> after a return or similar sequence that is known not to return.
> The following patch extends block_may_fallthrough to also return false
> for SWITCH_STMTs that can't fall through.
>
> Those are ones with non-empty body where the whole body can't fallthrough,
> additionally they need to have a default: case label (or cover the whole
> range of values, but that is not what this patch can compute, that would
> be too big duplication of the gimplification processing) and no BREAK_STMT.
>
> For the default: case label we need to look in all SWITCH_BODY children
> except for nested SWITCH_STMTs, for BREAK_STMTs also not in
> {FOR,DO,WHILE}_BODY.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
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:
case SWITCH_STMT:
if (SWITCH_STMT_CANT_FALLTHRU_P (stmt)
&& SWITCH_STMT_BODY (stmt)
&& !block_may_fallthru (SWITCH_STMT_BODY (stmt)))
return false;
return true;
and
/* 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. */
#define SWITCH_STMT_CANT_FALLTHRU_P(NODE) \
TREE_LANG_FLAG_0 (SWITCH_STMT_CHECK (NODE))
Seems the C++ FE already has switch_stack, so we could just add there
a has_default_p, has_break_stmt_p and inside_loop_p flags and both
inside of templates and outside in finish_case_label, in
finish_break_stmt if actually adding BREAK_STMT and when entering
a body of a FOR/DO/WHILE loop tweak those flags. Seems switch_stack
is also maintained during pt.c, but we should compute it both during
parsing and during pt.c (start with the bit clear on a new SWITCH_STMT).
Thoughts on this?
Guess for the C FE we should similarly handle SWITCH_EXPR, though a break;
in there is a goto outside of the switch, so all we need to note is a
default: case label.
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* 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)
2017-11-25 10:16 ` Jakub Jelinek
@ 2017-11-26 9:05 ` Jakub Jelinek
2017-11-27 13:39 ` Nathan Sidwell
2017-11-27 12:49 ` [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) Nathan Sidwell
1 sibling, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2017-11-26 9:05 UTC (permalink / raw)
To: Jason Merrill, Nathan Sidwell; +Cc: gcc-patches
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 <jakub@redhat.com>
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 N>
+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 N>
+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 N>
+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 N>
+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 N>
+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 N>
+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
^ permalink raw reply [flat|nested] 9+ messages in thread
* 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)
2017-11-25 0:37 [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) Jakub Jelinek
2017-11-25 10:16 ` Jakub Jelinek
@ 2017-11-27 12:44 ` Nathan Sidwell
1 sibling, 0 replies; 9+ messages in thread
From: Nathan Sidwell @ 2017-11-27 12:44 UTC (permalink / raw)
To: Jakub Jelinek, Jason Merrill; +Cc: gcc-patches
On 11/24/2017 04:59 PM, Jakub Jelinek wrote:
> Hi!
>
> The testcase below has a useless break; that causes a bogus -Wreturn-type
> warning. The C++ FE already has code to avoid adding a BREAK_STMT
> after a return or similar sequence that is known not to return.
> The following patch extends block_may_fallthrough to also return false
> for SWITCH_STMTs that can't fall through.
>
> Those are ones with non-empty body where the whole body can't fallthrough,
> additionally they need to have a default: case label (or cover the whole
> range of values, but that is not what this patch can compute, that would
> be too big duplication of the gimplification processing) and no BREAK_STMT.
>
> For the default: case label we need to look in all SWITCH_BODY children
> except for nested SWITCH_STMTs, for BREAK_STMTs also not in
> {FOR,DO,WHILE}_BODY.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2017-11-24 Jakub Jelinek <jakub@redhat.com>
>
> PR sanitizer/81275
> * cp-objcp-common.c (struct find_default_and_break_s): New type.
> (find_default_and_break): New function.
> (cxx_block_may_fallthru): Return false for SWITCH_STMT which
> contains no BREAK_STMTs and contains a default: CASE_LABEL_EXPR.
>
> * g++.dg/warn/pr81275.C: New test.
ok
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 9+ messages in thread
* 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)
2017-11-25 10:16 ` Jakub Jelinek
2017-11-26 9:05 ` Jakub Jelinek
@ 2017-11-27 12:49 ` Nathan Sidwell
1 sibling, 0 replies; 9+ messages in thread
From: Nathan Sidwell @ 2017-11-27 12:49 UTC (permalink / raw)
To: Jakub Jelinek, Jason Merrill; +Cc: gcc-patches
On 11/25/2017 04:01 AM, Jakub Jelinek wrote:
> /* 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. */
> #define SWITCH_STMT_CANT_FALLTHRU_P(NODE) \
> TREE_LANG_FLAG_0 (SWITCH_STMT_CHECK (NODE))
>
> Seems the C++ FE already has switch_stack, so we could just add there
> a has_default_p, has_break_stmt_p and inside_loop_p flags and both
> inside of templates and outside in finish_case_label, in
> finish_break_stmt if actually adding BREAK_STMT and when entering
> a body of a FOR/DO/WHILE loop tweak those flags. Seems switch_stack
> is also maintained during pt.c, but we should compute it both during
> parsing and during pt.c (start with the bit clear on a new SWITCH_STMT).
>
> Thoughts on this?
Sigh, should have read further. Yes, I'm all for setting such a flag
during construction. (I presume doing so is essentially free)
nathan
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 9+ messages in thread
* 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)
2017-11-26 9:05 ` Jakub Jelinek
@ 2017-11-27 13:39 ` Nathan Sidwell
2017-11-27 14:10 ` Jakub Jelinek
0 siblings, 1 reply; 9+ messages in thread
From: Nathan Sidwell @ 2017-11-27 13:39 UTC (permalink / raw)
To: Jakub Jelinek, Jason Merrill; +Cc: gcc-patches
On 11/25/2017 07:22 PM, Jakub Jelinek wrote:
> 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?
nice.
> --- 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
> +/* 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))
The macro name isn't quite right. As the comment says, it's not
sufficient that this flag is set for the switch to not fall through --
the switch body must be non-empty (which I presume it cannot be as there
must be a default label), and it cannot fall through in its own right.
The semantics of this flag are more like SWITCH_STMT_COVERS_ALL_CASES,
perhaps something of that ilk would be a clearer name?
nathan
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 9+ messages in thread
* 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)
2017-11-27 13:39 ` Nathan Sidwell
@ 2017-11-27 14:10 ` Jakub Jelinek
2017-11-28 9:14 ` [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, take 2) Jakub Jelinek
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2017-11-27 14:10 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: Jason Merrill, gcc-patches
On Mon, Nov 27, 2017 at 07:49:41AM -0500, Nathan Sidwell wrote:
> On 11/25/2017 07:22 PM, Jakub Jelinek wrote:
> > 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?
>
> nice.
>
> > --- 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
>
> > +/* 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))
>
> The macro name isn't quite right. As the comment says, it's not sufficient
> that this flag is set for the switch to not fall through -- the switch body
> must be non-empty (which I presume it cannot be as there must be a default
> label), and it cannot fall through in its own right.
You are right that I can remove the || SWITCH_STMT_BODY (stmt) == NULL_TREE,
part, because then there wouldn't be any case labels in it either.
> The semantics of this flag are more like SWITCH_STMT_COVERS_ALL_CASES,
> perhaps something of that ilk would be a clearer name?
Well, that is only part of it. Right now in the patch it does
SWITCH_STMT_WITH_DEFAULT_WITHOUT_BREAK_P(NODE)
When not processing_template_decl, we could perhaps do better and have it
SWITCH_STMT_COVERS_ALL_CASES_NO_BREAK_P(NODE),
because in that case we have the splay tree of all the case labels and we
could compute whether even without default: they cover all values. Could
add that as a follow-up.
Any preference on the macro name then?
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* [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, take 2)
2017-11-27 14:10 ` Jakub Jelinek
@ 2017-11-28 9:14 ` Jakub Jelinek
2017-11-28 11:58 ` Nathan Sidwell
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2017-11-28 9:14 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: Jason Merrill, gcc-patches
On Mon, Nov 27, 2017 at 02:01:05PM +0100, Jakub Jelinek wrote:
> You are right that I can remove the || SWITCH_STMT_BODY (stmt) == NULL_TREE,
> part, because then there wouldn't be any case labels in it either.
...
Here is an updated patch, on top of the C patch I've just posted:
http://gcc.gnu.org/ml/gcc-patches/2017-11/msg02372.html
(though that dependency could be easily removed if needed by dropping the
c_switch_covers_all_cases_p call and SWITCH_ALL_CASES_P setting from
SWITCH_STMT_ALL_CASES_P).
Note, looking for default is still needed, because in templates we do not
build the cases splay tree and therefore would never set
SWITCH_STMT_ALL_CASES_P. Computing the cases splay tree is probably too
expensive, but default tracking is cheap.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-11-28 Jakub Jelinek <jakub@redhat.com>
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.
--- gcc/cp/cp-tree.h.jj 2017-11-27 09:30:51.742077295 +0100
+++ gcc/cp/cp-tree.h 2017-11-27 16:29:44.806456638 +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_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
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_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)
+/* True if there all 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
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-27 09:30:51.744077271 +0100
+++ gcc/cp/decl.c 2017-11-27 16:32:01.308812566 +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,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
@@ -3508,6 +3562,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-27 09:30:51.749077208 +0100
+++ gcc/cp/parser.c 2017-11-27 16:26:15.918972537 +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-27 09:30:51.741077308 +0100
+++ gcc/cp/cp-objcp-common.c 2017-11-27 16:33:00.513099493 +0100
@@ -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;
}
--- gcc/cp/semantics.c.jj 2017-11-27 09:30:51.746077246 +0100
+++ gcc/cp/semantics.c 2017-11-27 16:26:15.921972501 +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-27 09:30:51.745077258 +0100
+++ gcc/cp/pt.c 2017-11-27 16:26:15.926972441 +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/cp/cp-gimplify.c.jj 2017-11-27 14:28:15.000000000 +0100
+++ gcc/cp/cp-gimplify.c 2017-11-27 16:37:29.240862862 +0100
@@ -333,6 +333,9 @@ genericize_switch_stmt (tree *stmt_p, in
*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);
}
--- gcc/testsuite/g++.dg/warn/pr81275-1.C.jj 2017-11-27 16:26:15.926972441 +0100
+++ gcc/testsuite/g++.dg/warn/pr81275-1.C 2017-11-27 16:26:15.926972441 +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 N>
+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 N>
+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-27 16:26:15.926972441 +0100
+++ gcc/testsuite/g++.dg/warn/pr81275-2.C 2017-11-27 16:26:15.926972441 +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 N>
+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 N>
+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-27 16:26:15.927972428 +0100
+++ gcc/testsuite/g++.dg/warn/pr81275-3.C 2017-11-27 16:26:15.927972428 +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 N>
+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 N>
+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);
+}
--- gcc/testsuite/c-c++-common/tsan/pr81275.c.jj 2017-11-27 16:40:05.000000000 +0100
+++ gcc/testsuite/c-c++-common/tsan/pr81275.c 2017-11-27 16:47:33.532570778 +0100
@@ -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)
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* 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, take 2)
2017-11-28 9:14 ` [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, take 2) Jakub Jelinek
@ 2017-11-28 11:58 ` Nathan Sidwell
0 siblings, 0 replies; 9+ messages in thread
From: Nathan Sidwell @ 2017-11-28 11:58 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches
On 11/28/2017 03:53 AM, Jakub Jelinek wrote:
> On Mon, Nov 27, 2017 at 02:01:05PM +0100, Jakub Jelinek wrote:
>> You are right that I can remove the || SWITCH_STMT_BODY (stmt) == NULL_TREE,
>> part, because then there wouldn't be any case labels in it either.
>
> ...
>
> Here is an updated patch, on top of the C patch I've just posted:
> http://gcc.gnu.org/ml/gcc-patches/2017-11/msg02372.html
> (though that dependency could be easily removed if needed by dropping the
> c_switch_covers_all_cases_p call and SWITCH_ALL_CASES_P setting from
> SWITCH_STMT_ALL_CASES_P).
> Note, looking for default is still needed, because in templates we do not
> build the cases splay tree and therefore would never set
> SWITCH_STMT_ALL_CASES_P. Computing the cases splay tree is probably too
> expensive, but default tracking is cheap.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2017-11-28 Jakub Jelinek <jakub@redhat.com>
>
> 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.
Ok. one nit.
> #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 all case labels for all possible values of switch cond, either
s/all/are/ (first one, no trailing 'g' modifier :)
> + because there is a default: case label or because the case label ranges cover
> + all values. */
nathan
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-11-28 11:55 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-25 0:37 [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) Jakub Jelinek
2017-11-25 10:16 ` Jakub Jelinek
2017-11-26 9:05 ` Jakub Jelinek
2017-11-27 13:39 ` Nathan Sidwell
2017-11-27 14:10 ` Jakub Jelinek
2017-11-28 9:14 ` [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, take 2) Jakub Jelinek
2017-11-28 11:58 ` Nathan Sidwell
2017-11-27 12:49 ` [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) Nathan Sidwell
2017-11-27 12:44 ` Nathan Sidwell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).