Instead of adding it to -Wextra, here is my attempt to improve this
warning as discussed in the PR and make it suitable for -Wall.
There were only two tests I had to add -Wno-jump-misses-init.
Bootstrapped and regression tested for x86_64.
c: Revise -Wjump-misses-init to better support idiomatic C code [PR87038]
This change revises -Wjump-misses-init to emit a diagnostic only when the
variable is read somewhere after the label until the end of the scope, e.g.
no warning is emitted anymore in the following example. With this
change warning is suitable for -Wall.
void f(void)
{
goto err;
int x = 1; // missed initialization
f(&x);
err:
return; // no use of 'x'
}
This is implemented by deferring all warnings until the end of the scope by
recording potential warnings in a data structure, resetting DECL_READ
(while recording the current value), and emitting warnings at the end
of the scope only for declarations that were read. We still emit
diagnostics directly for variably modified types, and for omp allocations,
and for all cases with -Wc++-compat.
There is overlap with -Wmaybe-uninitialized, but -Wjump-misses-init
captures some new situations, e.g. when the address of the variable
escapes as in the example above. The overlap could be reduced, but the
two warnings provide complementary information, so it seems more useful
to simply emit both.
Finally, we now do emit labels for switch cases even when there is an
error to prevent incorrect warnings about unreachable switch statements.
PR c/87038
gcc/c-family/ChangeLog:
* c.opts: Add -Wjump-misses-init to -Wall.
gcc/c/ChangeLog:
* c-tree.h (c_check_switch_jump_warnings): Change return type to
void.
* c-decl.cc (decl_jump_unsafe): Fix comment.
(emit_deferred_jump_warnings): New function.
(warn_about_goto): Forward to warn_about_jump.
(warn_about_jump): Emit or record warnings for goto or switch when
missing an initialization.
(pop_scope): Call emit_deferred_jump_warnings.
(c_check_switch_jump_warnings): Refactor to use warn_about_jump.
* c-typeck.cc (do_case): Adapt call to c_check_switch_jump_warnings.
gcc/testsuite/ChangeLog:
* c-c++-common/gomp/allocate-11.c: Remove incorrect warning.
* gcc.dg/Wjump-misses-init-1.c: Use -Wc++-compat.
* gcc.dg/Wjump-misses-init-4.c: New test.
* gcc.dg/c23-labels-3.c: Use -Wno-jump-misses-init.
* gccdg./uninit-pr102403-c2.c: Use -Wno-jump-misses-init.
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 50ba856fedb..023fc186b03 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -938,7 +938,7 @@ C ObjC C++ ObjC++ CPP(cpp_warn_invalid_utf8)
CppReason(CPP_W_INVALID_UTF8) Var(w
Warn about invalid UTF-8 characters.
Wjump-misses-init
-C ObjC Var(warn_jump_misses_init) Warning LangEnabledBy(C ObjC,Wc++-compat)
+C ObjC Var(warn_jump_misses_init) Warning LangEnabledBy(C ObjC, Wc++-compat ||
Wall)
Warn when a jump misses a variable initialization.
Enum
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 8bbd6ebc66a..5e6c510802f 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -749,7 +749,7 @@ decl_jump_unsafe (tree decl)
&& c_type_variably_modified_p (TREE_TYPE (decl)))
return true;
- /* Otherwise, only warn if -Wgoto-misses-init and this is an
+ /* Otherwise, only warn if -Wjump-misses-init and this is an
initialized automatic decl. */
if (warn_jump_misses_init
&& VAR_P (decl)
@@ -1227,6 +1227,87 @@ update_label_decls (struct c_scope *scope)
}
}
+
+enum jump_type { JUMP_GOTO, JUMP_SWITCH };
+
+
+/* Issue a warning about a missed initialization for a goto or switch
+ statement. */
+
+static void
+emit_warn_about_jump (enum jump_type jtype, location_t jump_loc, tree label,
+ tree decl, location_t label_loc)
+{
+ auto_diagnostic_group d;
+ switch (jtype)
+ {
+ case JUMP_GOTO:
+ if (c_type_variably_modified_p (TREE_TYPE (decl)))
+ error_at (jump_loc, "jump into scope of identifier with "
+ "variably modified type");
+ else if (flag_openmp
+ && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl)))
+ error_at (jump_loc, "jump skips OpenMP %<allocate%> allocation");
+ else if (!warning_at (jump_loc, OPT_Wjump_misses_init,
+ "jump skips variable initialization"))
+ return;
+ inform (DECL_SOURCE_LOCATION (label), "label %qD defined here", label);
+ inform (DECL_SOURCE_LOCATION (decl), "%qD declared here", decl);
+ break;
+ case JUMP_SWITCH:
+ if (c_type_variably_modified_p (TREE_TYPE (decl)))
+ error_at (label_loc, "switch jumps into scope of identifier with "
+ "variably modified type");
+ else if (flag_openmp
+ && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl)))
+ error_at (label_loc, "switch jumps over OpenMP %<allocate%>
allocation");
+ else if (!warning_at (label_loc, OPT_Wjump_misses_init,
+ "switch jumps over variable initialization"))
+ return;
+ inform (jump_loc, "switch starts here");
+ inform (DECL_SOURCE_LOCATION (decl), "%qD declared here", decl);
+ break;
+ }
+}
+
+
+
+struct deferred_jump_warning {
+ enum jump_type jump_type;
+ bool decl_read;
+ location_t goto_loc;
+ location_t label_loc;
+ tree decl;
+ tree label;
+};
+
+static
+vec<deferred_jump_warning, va_gc> *deferred_jump_warnings;
+
+
+
+/* Emit deferred warnings for variables for which there was a jump that
+ skips over the initialization, but only when the variable was used
+ since then. */
+static void
+emit_deferred_jump_warnings (void)
+{
+ if (!deferred_jump_warnings)
+ return;
+ while (!deferred_jump_warnings->is_empty ())
+ {
+ struct deferred_jump_warning p = deferred_jump_warnings->pop();
+ gcc_assert (VAR_OR_FUNCTION_DECL_P (p.decl));
+ if (DECL_READ_P (p.decl))
+ emit_warn_about_jump (p.jump_type, p.goto_loc, p.label, p.decl,
+ p.label_loc);
+ /* Merge back the information from before the label was reached. */
+ DECL_READ_P (p.decl) = DECL_READ_P (p.decl) || p.decl_read;
+ }
+ gcc_assert (deferred_jump_warnings->is_empty ());
+}
+
+
/* Exit a scope. Restore the state of the identifier-decl mappings
that were in effect when this scope was entered. Return a BLOCK
node containing all the DECLs in this scope that are of interest
@@ -1243,6 +1324,7 @@ pop_scope (void)
bool keep = functionbody || scope->keep || scope->bindings;
update_label_decls (scope);
+ emit_deferred_jump_warnings ();
/* If appropriate, create a BLOCK to record the decls for the life
of this function. */
@@ -4080,27 +4162,55 @@ lookup_label (tree name)
return label;
}
+
+/* Record a forbidden or possibly unsafe jump of type JTYPE from
+ GOTO_LOC to LABEL at LABEL_LOC skipping initialization of DECL. */
+
+static void warn_about_jump(enum jump_type jtype, location_t goto_loc,
+ location_t label_loc, tree decl, tree label)
+{
+ /* We emit the warning now if we screen for C++ compatibility, or have
+ to emit it for variable modified types (including typedefs), or for
+ openmp allocations. */
+ if (warn_cxx_compat
+ || c_type_variably_modified_p (TREE_TYPE (decl))
+ || !VAR_OR_FUNCTION_DECL_P (decl)
+ || (flag_openmp
+ && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl))))
+ emit_warn_about_jump (jtype, goto_loc, label, decl, label_loc);
+ else
+ {
+ /* Record the required information. We will emit a warning at the end
+ of the scope, but only if the declaration was read between the label
+ and the end of the scop. */
+ struct deferred_jump_warning d;
+ d.jump_type = jtype;
+ d.goto_loc = goto_loc;
+ d.label_loc = label_loc;
+ d.decl = decl;
+ d.label = label;
+ d.decl_read = DECL_READ_P (decl);
+ if (!deferred_jump_warnings)
+ vec_alloc (deferred_jump_warnings, 1);
+ vec_safe_push (deferred_jump_warnings, d);
+
+ /* Temporarily clear this flag. We want to detect when the decl is used
+ (again). The information is merged again at the end of the scope. */
+ DECL_READ_P (decl) = 0;
+ }
+}
+
/* Issue a warning about DECL for a goto statement at GOTO_LOC going
to LABEL. */
static void
warn_about_goto (location_t goto_loc, tree label, tree decl)
{
- auto_diagnostic_group d;
- if (c_type_variably_modified_p (TREE_TYPE (decl)))
- error_at (goto_loc,
- "jump into scope of identifier with variably modified type");
- else if (flag_openmp
- && lookup_attribute ("omp allocate", DECL_ATTRIBUTES (decl)))
- error_at (goto_loc, "jump skips OpenMP %<allocate%> allocation");
- else
- if (!warning_at (goto_loc, OPT_Wjump_misses_init,
- "jump skips variable initialization"))
- return;
- inform (DECL_SOURCE_LOCATION (label), "label %qD defined here", label);
- inform (DECL_SOURCE_LOCATION (decl), "%qD declared here", decl);
+ warn_about_jump (JUMP_GOTO, goto_loc, DECL_SOURCE_LOCATION (label),
+ decl, label);
}
+
/* Look up a label because of a goto statement. This is like
lookup_label, but also issues any appropriate warnings. */
@@ -4336,17 +4446,14 @@ c_release_switch_bindings (struct c_spot_bindings
*bindings)
}
/* This is called at the point of a case or default label to issue
- warnings about decls as needed. It returns true if it found an
- error, not just a warning. */
+ warnings about decls as needed. */
-bool
+void
c_check_switch_jump_warnings (struct c_spot_bindings *switch_bindings,
location_t switch_loc, location_t case_loc)
{
- bool saw_error;
struct c_scope *scope;
- saw_error = false;
for (scope = current_scope;
scope != switch_bindings->scope;
scope = scope->outer)
@@ -4359,51 +4466,16 @@ c_check_switch_jump_warnings (struct c_spot_bindings
*switch_bindings,
continue;
for (b = scope->bindings; b != NULL; b = b->prev)
- {
- if (decl_jump_unsafe (b->decl))
- {
- auto_diagnostic_group d;
- bool emitted;
- if (c_type_variably_modified_p (TREE_TYPE (b->decl)))
- {
- saw_error = true;
- error_at (case_loc,
- ("switch jumps into scope of identifier with "
- "variably modified type"));
- emitted = true;
- }
- else if (flag_openmp
- && lookup_attribute ("omp allocate",
- DECL_ATTRIBUTES (b->decl)))
- {
- saw_error = true;
- error_at (case_loc,
- "switch jumps over OpenMP %<allocate%> allocation");
- emitted = true;
- }
- else
- emitted
- = warning_at (case_loc, OPT_Wjump_misses_init,
- "switch jumps over variable initialization");
- if (emitted)
- {
- inform (switch_loc, "switch starts here");
- inform (DECL_SOURCE_LOCATION (b->decl), "%qD declared here",
- b->decl);
- }
- }
- }
+ if (decl_jump_unsafe (b->decl))
+ warn_about_jump (JUMP_SWITCH, switch_loc, case_loc, b->decl,
NULL_TREE);
}
if (switch_bindings->stmt_exprs > 0)
{
- saw_error = true;
auto_diagnostic_group d;
error_at (case_loc, "switch jumps into statement expression");
inform (switch_loc, "switch starts here");
}
-
- return saw_error;
}
/* Given NAME, an IDENTIFIER_NODE,
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 364f51df58c..eceab8b40f6 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -676,7 +676,7 @@ extern tree declare_label (tree);
extern tree define_label (location_t, tree);
extern struct c_spot_bindings *c_get_switch_bindings (void);
extern void c_release_switch_bindings (struct c_spot_bindings *);
-extern bool c_check_switch_jump_warnings (struct c_spot_bindings *,
+extern void c_check_switch_jump_warnings (struct c_spot_bindings *,
location_t, location_t);
extern void finish_decl (tree, location_t, tree, tree, tree);
extern tree finish_enum (tree, tree, tree);
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index e24629be918..127330d8b03 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -13132,10 +13132,9 @@ do_case (location_t loc, tree low_value, tree
high_value, tree attrs)
return NULL_TREE;
}
- if (c_check_switch_jump_warnings (c_switch_stack->bindings,
- EXPR_LOCATION (c_switch_stack->switch_stmt),
- loc))
- return NULL_TREE;
+ c_check_switch_jump_warnings (c_switch_stack->bindings,
+ EXPR_LOCATION (c_switch_stack->switch_stmt),
+ loc);
label = c_add_case_label (loc, c_switch_stack->cases,
SWITCH_STMT_COND (c_switch_stack->switch_stmt),
diff --git a/gcc/testsuite/c-c++-common/gomp/allocate-11.c
b/gcc/testsuite/c-c++-common/gomp/allocate-11.c
index dceb97f8c5f..26f895cf8d8 100644
--- a/gcc/testsuite/c-c++-common/gomp/allocate-11.c
+++ b/gcc/testsuite/c-c++-common/gomp/allocate-11.c
@@ -13,7 +13,6 @@ f (int i)
#pragma omp allocate(j)
case 42: /* { dg-error "switch jumps over OpenMP 'allocate' allocation" }
*/
bar ();
- /* { dg-warning "statement will never be executed
\\\[-Wswitch-unreachable\\\]" "" { target *-*-* } .-1 } */
break;
case 51: /* { dg-error "switch jumps over OpenMP 'allocate' allocation" }
*/
use (&j);
diff --git a/gcc/testsuite/gcc.dg/Wjump-misses-init-1.c
b/gcc/testsuite/gcc.dg/Wjump-misses-init-1.c
index 71809be5933..98ddc728b42 100644
--- a/gcc/testsuite/gcc.dg/Wjump-misses-init-1.c
+++ b/gcc/testsuite/gcc.dg/Wjump-misses-init-1.c
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-Wjump-misses-init -Wno-switch-unreachable" } */
+/* { dg-options "-Wc++-compat -Wno-switch-unreachable" } */
int
f1 (int a)
{
diff --git a/gcc/testsuite/gcc.dg/Wjump-misses-init-4.c
b/gcc/testsuite/gcc.dg/Wjump-misses-init-4.c
new file mode 100644
index 00000000000..4083585cc3c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wjump-misses-init-4.c
@@ -0,0 +1,100 @@
+/* { dg-do compile } */
+/* { dg-options "-Wjump-misses-init" } */
+
+int f1()
+{
+ int b = 1;
+ goto a;
+a:
+ return b;
+}
+
+int f2()
+{
+ goto a; /* { dg-warning "jump" } */
+ int b = 1; /* { dg-message "here" } */
+a: /* { dg-message "here" } */
+ return b;
+}
+
+int f2b()
+{
+ goto a;
+ int b = 1;
+a:
+}
+
+int f2c()
+{
+ {
+ goto a;
+ }
+ int b = 1;
+a:
+}
+
+int f3()
+{
+ goto a;
+a:
+ int b = 1;
+ return b;
+}
+
+int f4()
+{
+ int b = 1;
+a:
+ goto a;
+ return b;
+}
+
+int f5()
+{
+a:
+ int b = 1;
+ goto a;
+ return b;
+}
+
+int f6()
+{
+a:
+ goto a;
+ int b = 1;
+ return b;
+}
+
+int f7(int n)
+{
+ goto a; /* { dg-error "jump" } */
+ char x[n]; /* { dg-message "here" } */
+a: ; /* { dg-message "here" } */
+}
+
+int f8(int n)
+{
+ goto a; /* { dg-error "jump" } */
+ typedef char x[n]; /* { dg-message "here" } */
+a: ; /* { dg-message "here" } */
+}
+
+int f9a()
+{
+ goto foo; /* { dg-warning "jump" } */
+ goto bar;
+ int x = 1; /* { dg-message "here" } */
+foo: /* { dg-message "here" } */
+ return x;
+bar:
+}
+
+int f9b()
+{
+ goto foo;
+ goto bar; /* { dg-warning "jump" } */
+ int x = 1; /* { dg-message "here" } */
+bar: /* { dg-message "here" } */
+ return x;
+foo:
+}
diff --git a/gcc/testsuite/gcc.dg/c23-labels-3.c
b/gcc/testsuite/gcc.dg/c23-labels-3.c
index 2d75e34ab1b..dfd21ed16af 100644
--- a/gcc/testsuite/gcc.dg/c23-labels-3.c
+++ b/gcc/testsuite/gcc.dg/c23-labels-3.c
@@ -1,7 +1,7 @@
/* Tests for labels before declarations and at ends of compound statements
* in combination with attributes. */
/* { dg-do compile } */
-/* { dg-options "-std=c23 -Wall" } */
+/* { dg-options "-std=c23 -Wall -Wno-jump-misses-init" } */
int f(void)
{
diff --git a/gcc/testsuite/gcc.dg/uninit-pr102403-c2.c
b/gcc/testsuite/gcc.dg/uninit-pr102403-c2.c
index 81811432da5..af0bd7106fd 100644
--- a/gcc/testsuite/gcc.dg/uninit-pr102403-c2.c
+++ b/gcc/testsuite/gcc.dg/uninit-pr102403-c2.c
@@ -1,7 +1,7 @@
/* PR middle-end/102403 - ICE in init_from_control_deps, at
gimple-predicate-analysis.cc:2364
{ dg-do compile }
- { dg-options "-O2 -Wall" } */
+ { dg-options "-O2 -Wall -Wno-jump-misses-init" } */
extern int a[], b, c, d, e, f, g, h;
