diff mbox series

[pushed] c++: computed goto warning [PR37722]

Message ID 20231221020627.3266898-1-jason@redhat.com
State New
Headers show
Series [pushed] c++: computed goto warning [PR37722] | expand

Commit Message

Jason Merrill Dec. 21, 2023, 2:06 a.m. UTC
This is one of those patches where I first did the basic thing and then thought
"well, or I could do a bit better..." several times...

Tested x86_64-pc-linux-gnu, applying to trunk.

-- 8< --

PR37722 complains that a computed goto doesn't destroy objects that go out
of scope.  In the PR we agreed that it never will, but we can warn about
it.

	PR c++/37722

gcc/ChangeLog:

	* doc/extend.texi: Document that computed goto does not
	call destructors.

gcc/cp/ChangeLog:

	* decl.cc (identify_goto): Adjust for computed goto.
	(struct named_label_use_entry): Add computed_goto field.
	(poplevel_named_label_1): Add to computed_goto vec.
	(check_previous_goto_1): Dump computed_goto vec.
	(check_goto_1): Split out from check_goto.
	(check_goto): Check all addressable labels for computed goto.
	(struct named_label_entry): Add addressed field.
	(mark_label_addressed): New.
	* parser.cc (cp_parser_unary_expression): Call it.
	* cp-tree.h (mark_label_addressed): Declare it.

gcc/testsuite/ChangeLog:

	* g++.dg/ext/label15.C: New test.
	* g++.dg/torture/pr42739.C: Expect warning.
---
 gcc/doc/extend.texi                    |   3 +
 gcc/cp/cp-tree.h                       |   1 +
 gcc/cp/decl.cc                         | 150 ++++++++++++++++++++-----
 gcc/cp/parser.cc                       |   2 +
 gcc/testsuite/g++.dg/ext/label15.C     |  36 ++++++
 gcc/testsuite/g++.dg/torture/pr42739.C |   6 +-
 6 files changed, 170 insertions(+), 28 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ext/label15.C


base-commit: af3fc0306948f5b6abecad8453938c5bedb976fc
diff mbox series

Patch

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 4cc3b61b2f4..eb93de5da2a 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -419,6 +419,9 @@  relies on them being always the same,
 prevent inlining and cloning.  If @code{&&foo} is used in a static
 variable initializer, inlining and cloning is forbidden.
 
+Unlike a normal goto, in GNU C++ a computed goto will not call
+destructors for objects that go out of scope.
+
 @node Nested Functions
 @section Nested Functions
 @cindex nested functions
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 1979572c365..32ae0e3dbeb 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6952,6 +6952,7 @@  extern bool merge_default_template_args		(tree, tree, bool);
 extern tree duplicate_decls			(tree, tree,
 						 bool hiding = false,
 						 bool was_hidden = false);
+extern void mark_label_addressed		(tree);
 extern tree declare_local_label			(tree);
 extern tree define_label			(location_t, tree);
 extern void check_goto				(tree);
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 27f17808934..409d8f15448 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -105,6 +105,8 @@  static void store_parm_decls (tree);
 static void initialize_local_var (tree, tree);
 static void expand_static_init (tree, tree);
 static location_t smallest_type_location (const cp_decl_specifier_seq*);
+static bool identify_goto (tree, location_t, const location_t *,
+			   diagnostic_t, bool);
 
 /* The following symbols are subsumed in the cp_global_trees array, and
    listed here individually for documentation purposes.
@@ -179,6 +181,9 @@  struct GTY((chain_next ("%h.next"))) named_label_use_entry {
      or the inner scope popped.  These are the decls that will *not* be
      skipped when jumping to the label.  */
   tree names_in_scope;
+  /* If the use is a possible destination of a computed goto, a vec of decls
+     that aren't destroyed, filled in by poplevel_named_label_1.  */
+  vec<tree,va_gc> *computed_goto;
   /* The location of the goto, for error reporting.  */
   location_t o_goto_locus;
   /* True if an OpenMP structured block scope has been closed since
@@ -216,6 +221,10 @@  struct GTY((for_user)) named_label_entry {
   /* A list of uses of the label, before the label is defined.  */
   named_label_use_entry *uses;
 
+  /* True if we've seen &&label.  Appalently we can't use TREE_ADDRESSABLE for
+     this, it has a more specific meaning for LABEL_DECL.  */
+  bool addressed;
+
   /* The following bits are set after the label is defined, and are
      updated as scopes are popped.  They indicate that a jump to the
      label will illegally enter a scope of the given flavor.  */
@@ -561,6 +570,12 @@  poplevel_named_label_1 (named_label_entry **slot, cp_binding_level *bl)
       for (use = ent->uses; use ; use = use->next)
 	if (use->binding_level == bl)
 	  {
+	    if (auto &cg = use->computed_goto)
+	      for (tree d = use->names_in_scope; d; d = DECL_CHAIN (d))
+		if (TREE_CODE (d) == VAR_DECL && !TREE_STATIC (d)
+		    && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (d)))
+		  vec_safe_push (cg, d);
+
 	    use->binding_level = obl;
 	    use->names_in_scope = obl->names;
 	    if (bl->kind == sk_omp)
@@ -3617,6 +3632,15 @@  lookup_label (tree id)
   return ent ? ent->label_decl : NULL_TREE;
 }
 
+/* Remember that we've seen &&ID.  */
+
+void
+mark_label_addressed (tree id)
+{
+  named_label_entry *ent = lookup_label_1 (id, false);
+  ent->addressed = true;
+}
+
 tree
 declare_local_label (tree id)
 {
@@ -3649,26 +3673,35 @@  decl_jump_unsafe (tree decl)
 
 static bool
 identify_goto (tree decl, location_t loc, const location_t *locus,
-	       diagnostic_t diag_kind)
+	       diagnostic_t diag_kind, bool computed)
 {
+  if (computed)
+    diag_kind = DK_WARNING;
   bool complained
     = emit_diagnostic (diag_kind, loc, 0,
 		       decl ? G_("jump to label %qD")
 		       : G_("jump to case label"), decl);
   if (complained && locus)
-    inform (*locus, "  from here");
+    {
+      if (computed)
+	inform (*locus, "  as a possible target of computed goto");
+      else
+	inform (*locus, "  from here");
+    }
   return complained;
 }
 
 /* Check that a single previously seen jump to a newly defined label
    is OK.  DECL is the LABEL_DECL or 0; LEVEL is the binding_level for
    the jump context; NAMES are the names in scope in LEVEL at the jump
-   context; LOCUS is the source position of the jump or 0.  Returns
+   context; LOCUS is the source position of the jump or 0.  COMPUTED
+   is a vec of decls if the jump is a computed goto.  Returns
    true if all is well.  */
 
 static bool
 check_previous_goto_1 (tree decl, cp_binding_level* level, tree names,
-		       bool exited_omp, const location_t *locus)
+		       bool exited_omp, const location_t *locus,
+		       vec<tree,va_gc> *computed)
 {
   cp_binding_level *b;
   bool complained = false;
@@ -3678,7 +3711,8 @@  check_previous_goto_1 (tree decl, cp_binding_level* level, tree names,
 
   if (exited_omp)
     {
-      complained = identify_goto (decl, input_location, locus, DK_ERROR);
+      complained = identify_goto (decl, input_location, locus, DK_ERROR,
+				  computed);
       if (complained)
 	inform (input_location, "  exits OpenMP structured block");
       saw_omp = true;
@@ -3699,8 +3733,9 @@  check_previous_goto_1 (tree decl, cp_binding_level* level, tree names,
 
 	  if (!identified)
 	    {
-	      complained = identify_goto (decl, input_location, locus, DK_ERROR);
-	      identified = 1;
+	      complained = identify_goto (decl, input_location, locus, DK_ERROR,
+					  computed);
+	      identified = 2;
 	    }
 	  if (complained)
 	    inform (DECL_SOURCE_LOCATION (new_decls),
@@ -3766,13 +3801,25 @@  check_previous_goto_1 (tree decl, cp_binding_level* level, tree names,
       if (inf)
 	{
 	  if (identified < 2)
-	    complained = identify_goto (decl, input_location, locus, DK_ERROR);
+	    complained = identify_goto (decl, input_location, locus, DK_ERROR,
+					computed);
 	  identified = 2;
 	  if (complained)
 	    inform (loc, inf);
 	}
     }
 
+  if (!vec_safe_is_empty (computed))
+    {
+      if (!identified)
+	complained = identify_goto (decl, input_location, locus, DK_ERROR,
+				    computed);
+      identified = 2;
+      if (complained)
+	for (tree d : computed)
+	  inform (DECL_SOURCE_LOCATION (d), "  does not destroy %qD", d);
+    }
+
   return !identified;
 }
 
@@ -3781,30 +3828,23 @@  check_previous_goto (tree decl, struct named_label_use_entry *use)
 {
   check_previous_goto_1 (decl, use->binding_level,
 			 use->names_in_scope, use->in_omp_scope,
-			 &use->o_goto_locus);
+			 &use->o_goto_locus, use->computed_goto);
 }
 
 static bool
 check_switch_goto (cp_binding_level* level)
 {
-  return check_previous_goto_1 (NULL_TREE, level, level->names, false, NULL);
+  return check_previous_goto_1 (NULL_TREE, level, level->names,
+				false, NULL, nullptr);
 }
 
-/* Check that a new jump to a label DECL is OK.  Called by
-   finish_goto_stmt.  */
+/* Check that a new jump to a label ENT is OK.  COMPUTED is true
+   if this is a possible target of a computed goto.  */
 
 void
-check_goto (tree decl)
+check_goto_1 (named_label_entry *ent, bool computed)
 {
-  /* We can't know where a computed goto is jumping.
-     So we assume that it's OK.  */
-  if (TREE_CODE (decl) != LABEL_DECL)
-    return;
-
-  hashval_t hash = IDENTIFIER_HASH_VALUE (DECL_NAME (decl));
-  named_label_entry **slot
-    = named_labels->find_slot_with_hash (DECL_NAME (decl), hash, NO_INSERT);
-  named_label_entry *ent = *slot;
+  tree decl = ent->label_decl;
 
   /* If the label hasn't been defined yet, defer checking.  */
   if (! DECL_INITIAL (decl))
@@ -3821,6 +3861,7 @@  check_goto (tree decl)
       new_use->names_in_scope = current_binding_level->names;
       new_use->o_goto_locus = input_location;
       new_use->in_omp_scope = false;
+      new_use->computed_goto = computed ? make_tree_vector () : nullptr;
 
       new_use->next = ent->uses;
       ent->uses = new_use;
@@ -3843,7 +3884,7 @@  check_goto (tree decl)
 	  || ent->in_omp_scope || ent->in_stmt_expr)
 	diag_kind = DK_ERROR;
       complained = identify_goto (decl, DECL_SOURCE_LOCATION (decl),
-				  &input_location, diag_kind);
+				  &input_location, diag_kind, computed);
       identified = 1 + (diag_kind == DK_ERROR);
     }
 
@@ -3857,7 +3898,7 @@  check_goto (tree decl)
 	  if (identified == 1)
 	    {
 	      complained = identify_goto (decl, DECL_SOURCE_LOCATION (decl),
-					  &input_location, DK_ERROR);
+					  &input_location, DK_ERROR, computed);
 	      identified = 2;
 	    }
 	  if (complained)
@@ -3901,7 +3942,8 @@  check_goto (tree decl)
 	      {
 		complained = identify_goto (decl,
 					    DECL_SOURCE_LOCATION (decl),
-					    &input_location, DK_ERROR);
+					    &input_location, DK_ERROR,
+					    computed);
 		identified = 2;
 	      }
 	    if (complained)
@@ -3909,6 +3951,64 @@  check_goto (tree decl)
 	    break;
 	  }
       }
+
+  /* Warn if a computed goto might involve a local variable going out of scope
+     without being cleaned up.  */
+  if (computed)
+    {
+      auto level = ent->binding_level;
+      auto names = ent->names_in_scope;
+      for (auto b = current_binding_level; ; b = b->level_chain)
+	{
+	  tree end = b == level ? names : NULL_TREE;
+	  for (tree d = b->names; d != end; d = DECL_CHAIN (d))
+	    {
+	      if (TREE_CODE (d) == VAR_DECL && !TREE_STATIC (d)
+		  && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (d)))
+		{
+		  complained = identify_goto (decl, DECL_SOURCE_LOCATION (decl),
+					      &input_location, DK_ERROR,
+					      computed);
+		  if (complained)
+		    inform (DECL_SOURCE_LOCATION (d),
+			    "  does not destroy %qD", d);
+		}
+	    }
+	  if (b == level)
+	    break;
+	}
+    }
+}
+
+/* Check that a new jump to a label DECL is OK.  Called by
+   finish_goto_stmt.  */
+
+void
+check_goto (tree decl)
+{
+  if (!named_labels)
+    return;
+  if (TREE_CODE (decl) != LABEL_DECL)
+    {
+      /* We don't know where a computed goto is jumping,
+	 so check all addressable labels.  */
+      for (auto iter = named_labels->begin ();
+	   iter != named_labels->end ();
+	   ++iter)
+	{
+	  auto ent = *iter;
+	  if (ent->addressed)
+	    check_goto_1 (ent, true);
+	}
+    }
+  else
+    {
+      hashval_t hash = IDENTIFIER_HASH_VALUE (DECL_NAME (decl));
+      named_label_entry **slot
+	= named_labels->find_slot_with_hash (DECL_NAME (decl), hash, NO_INSERT);
+      named_label_entry *ent = *slot;
+      check_goto_1 (ent, false);
+    }
 }
 
 /* Check that a return is ok wrt OpenMP structured blocks.
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index cb1dcd8e402..379aeb56b15 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -9190,6 +9190,8 @@  cp_parser_unary_expression (cp_parser *parser, cp_id_kind * pidk,
 	    = make_location (start_loc, start_loc, parser->lexer);
 	  /* Create an expression representing the address.  */
 	  expression = finish_label_address_expr (identifier, combined_loc);
+	  if (TREE_CODE (expression) == ADDR_EXPR)
+	    mark_label_addressed (identifier);
 	  if (cp_parser_non_integral_constant_expression (parser,
 							  NIC_ADDR_LABEL))
 	    expression = error_mark_node;
diff --git a/gcc/testsuite/g++.dg/ext/label15.C b/gcc/testsuite/g++.dg/ext/label15.C
new file mode 100644
index 00000000000..f9d6a0dd626
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/label15.C
@@ -0,0 +1,36 @@ 
+// PR c++/37722
+// { dg-options "" }
+
+extern "C" int printf (const char *, ...);
+template<int i>
+struct foo {
+    foo() { printf("%s<%d>\n", __FUNCTION__, i); }
+    ~foo() { printf("%s<%d>\n", __FUNCTION__, i); }
+};
+enum {RETRY, INSIDE, OUTSIDE, EVIL};
+int bar(int idx) {
+    static void* const gotos[] = {&&RETRY, &&INSIDE, &&OUTSIDE, &&EVIL};
+    bool first = true;
+    {
+    RETRY:			// { dg-warning "jump" }
+        foo<1> f1;		// { dg-message "does not destroy" }
+        if(first) {
+            first = false;
+            goto *gotos[idx];	// { dg-message "computed goto" }
+        }
+    INSIDE:			// OK
+        return 1;
+    }
+    if(0) {
+        foo<2> f2;		// { dg-message "crosses initialization" }
+    EVIL:			// { dg-warning "jump" }
+        return 2;
+    }
+ OUTSIDE:			// { dg-warning "jump" }
+    return 0;
+}
+int main() {
+    for(int i=RETRY; i <= EVIL; i++)
+        printf("%d\n", bar(i));
+    return 0;
+}
diff --git a/gcc/testsuite/g++.dg/torture/pr42739.C b/gcc/testsuite/g++.dg/torture/pr42739.C
index 21206487542..ef6c67c88fa 100644
--- a/gcc/testsuite/g++.dg/torture/pr42739.C
+++ b/gcc/testsuite/g++.dg/torture/pr42739.C
@@ -5,13 +5,13 @@  struct s { ~s() { s(); } };
 
 int f()
 {
-  M:
-    s o = s();
+  M:				// { dg-warning "jump" }
+    s o = s();			// { dg-message "does not destroy" }
     f();
     f();
 
   L:
-    goto *(f() ? &&L : &&M);
+    goto *(f() ? &&L : &&M);	// { dg-message "computed goto" }
 
     return 0;
 }