This is a small patch to address
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92386
When a variable is used before being shadowed in the same scope, GCC
outputs incorrect/misleading debug information. The Bugzilla report has
a minimal example and explains why a direct fix was not desirable (debug
info size would blow up).
In the Bugzilla discussion, Richard Biener suggests "this probably
deserves a warning and should be considered bad style?"
This patch implements such a "-Wuse-before-shadow" warning for these
scenarios. When gimplifying a named var, we throw a warning if it isn't
the most-recently bound declaration for that name. I wasn't able to find
a way to get the current stack of bindings for a given name, so I added
a new hash map (binding_stacks) to keep track of this (only when
-Wuse-before-shadow).
I tested on x86-64-linux-gnu (Debian 12) with make -k check and saw no
testsuite regressions using ./contrib/compare_tests. I also built Git
and Linux with the flag: each triggered one 'true positive' warning,
neither of which was an actual bug.
I have not tested on major projects in other languages (C++, etc.) so
there might be more false positives when -Wuse-before-shadow is enabled
for those languages (could restrict the warning to just C if this is a
concern?).
Any comments would be much appreciated; I'm new to the codebase.
(Also, sorry for any duplicate sends --- I had some DMARC issues
originally.)
PR debug/92386 - gdb issue with variable-shadowing
PR debug/92386
gcc/ChangeLog:
* common.opt: Added Wuse-before-shadow option; defaults to off.
* gimplify.cc (gimplify_bind_expr): when gimplifying a bind,
keep track of the stack of variables bound to each identifier.
(gimplify_var_or_parm_decl): when gimplifying a named var,
check if it is the most-recently bound variable for that
identifier.
gcc/testsuite/ChangeLog:
* gcc.dg/warn-use-before-shadow.c: New test.
---
gcc/common.opt | 5 ++
gcc/gimplify.cc | 70 +++++++++++++++++++
gcc/testsuite/gcc.dg/warn-use-before-shadow.c | 28 ++++++++
3 files changed, 103 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/warn-use-before-shadow.c
diff --git a/gcc/common.opt b/gcc/common.opt
index e3fa0dacec4..cb4a2754912 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -880,6 +880,11 @@ Wunused-variable
Common Var(warn_unused_variable) Warning EnabledBy(Wunused)
Warn when a variable is unused.
+Wuse-before-shadow
+Common Var(warn_use_before_shadow) Warning Init(0)
+Warn if a variable from an outer scope is used before it is shadowed in the
+current scope.
+
Wcoverage-mismatch
Common Var(warn_coverage_mismatch) Init(1) Warning
Warn in case profiles in -fprofile-use do not match.
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 4f385b1b779..6854ced7b06 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -120,6 +120,17 @@ tree_associate_condition_with_expr (tree stmt, unsigned
uid)
/* Hash set of poisoned variables in a bind expr. */
static hash_set<tree> *asan_poisoned_variables = NULL;
+/* Maps a var name to a stack of variables currently bound to that name. This
+ global is only used if -Wuse-before-shadow is passed, in which case every
+ time a variable is gimplified we check this stack and throw a warning if
+ that variable is not the innermost binding to its name. */
+static hash_map <tree, auto_vec <tree>> binding_stacks;
+
+/* A list of all of the currently bound named variables. Basically a flattened
+ version of @binding_stacks. Used to pop bindings off @binding_stacks when
+ gimplify_bind_expr returns. */
+static auto_vec <tree> all_bindings;
+
/* Hash set of already-resolved calls to OpenMP "declare variant"
functions. A call can resolve to the original function and
we don't want to repeat the resolution multiple times. */
@@ -1417,6 +1428,25 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
tree temp = voidify_wrapper_expr (bind_expr, NULL);
+ /* Update the stack of variables bound to a given name. Used for
+ -Wuse-before-shadow to detect when a variable is used in a scope where it is
+ not the innermost bound instance of that name. Note that gimplification will
+ sometimes change the DECL_NAME associated with a tree (e.g., promoting an
+ initialized const array to a static) so we store the names in a separate
+ vector @all_bindings to pop from after exiting the bind expr. */
+ size_t all_bindings_reset_idx = all_bindings.length ();
+ if (warn_use_before_shadow)
+ {
+ for (tree t = BIND_EXPR_VARS (bind_expr); t ; t = DECL_CHAIN (t))
+ {
+ if (VAR_P (t) && DECL_NAME (t) != NULL_TREE)
+ {
+ binding_stacks.get_or_insert (DECL_NAME (t)).safe_push (t);
+ all_bindings.safe_push (DECL_NAME (t));
+ }
+ }
+ }
+
/* Mark variables seen in this bind expr. */
for (t = BIND_EXPR_VARS (bind_expr); t ; t = DECL_CHAIN (t))
{
@@ -1828,6 +1858,17 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
gimplify_seq_add_stmt (pre_p, bind_stmt);
+ /* Pop off bindings used for -Wuse-before-shadow now that we're
+ exiting the bind. */
+ while (all_bindings.length () > all_bindings_reset_idx)
+ {
+ tree name = all_bindings.pop ();
+ binding_stacks.get (name)->pop ();
+ if (!binding_stacks.get (name)->length ())
+ binding_stacks.remove (name);
+ }
+
+
if (temp)
{
*expr_p = temp;
@@ -3330,6 +3371,35 @@ gimplify_var_or_parm_decl (tree *expr_p)
{
tree decl = *expr_p;
+ /* Check that any named variable we use is the one actually bound to that
+ name in this scope. */
+ if (warn_use_before_shadow && VAR_P (decl) && DECL_NAME (decl) != NULL_TREE
+ && !warning_suppressed_p (decl, OPT_Wuse_before_shadow))
+ {
+ vec <tree> *binding_stack = binding_stacks.get (DECL_NAME (decl));
+ if (binding_stack && binding_stack->length ())
+ {
+ tree scope_decl = binding_stack->last ();
+ if ((!DECL_EXTERNAL (scope_decl) && scope_decl != decl)
+ || (DECL_EXTERNAL (scope_decl) && !is_global_var (decl)))
+ {
+ if (warning_at (input_location, OPT_Wuse_before_shadow,
+ "variable %qD is used before being shadowed",
+ decl))
+ {
+ inform (DECL_SOURCE_LOCATION (scope_decl),
+ "the shadowing declaration of %qD in this scope",
+ scope_decl);
+ inform (DECL_SOURCE_LOCATION (decl),
+ "outer scope declaration of %qD being "
+ "used instead",
+ decl);
+ }
+ suppress_warning (decl, OPT_Wuse_before_shadow);
+ }
+ }
+ }
+
/* ??? If this is a local variable, and it has not been seen in any
outer BIND_EXPR, then it's probably the result of a duplicate
declaration, for which we've already issued an error. It would
diff --git a/gcc/testsuite/gcc.dg/warn-use-before-shadow.c
b/gcc/testsuite/gcc.dg/warn-use-before-shadow.c
new file mode 100644
index 00000000000..5d381e4c30f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/warn-use-before-shadow.c
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-options "-Wuse-before-shadow" } */
+int A, B, C, D;
+
+int main(void) {
+ A++; /* { dg-warning "used before being shadowed" } */
+ B++;
+ int A = 7, C = 8;
+ {
+ C++; /* { dg-warning "used before being shadowed" } */
+ extern int C, D;
+ C++;
+ D++;
+ }
+
+ D++;
+ extern int D;
+ D++;
+
+ int x = 5, x2 = 6;
+ {
+ x++; /* { dg-warning "used before being shadowed" } */
+ int x = 7, x2 = 7;
+ x--;
+ x2--;
+ }
+ x++;
+}
--
2.39.5
