* [pushed] c++: computed goto warning [PR37722]
@ 2023-12-21 2:06 Jason Merrill
2023-12-22 3:17 ` [pushed] c++: computed goto from catch block [PR81438] Jason Merrill
0 siblings, 1 reply; 2+ messages in thread
From: Jason Merrill @ 2023-12-21 2:06 UTC (permalink / raw)
To: gcc-patches
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
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;
}
base-commit: af3fc0306948f5b6abecad8453938c5bedb976fc
--
2.39.3
^ permalink raw reply [flat|nested] 2+ messages in thread
* [pushed] c++: computed goto from catch block [PR81438]
2023-12-21 2:06 [pushed] c++: computed goto warning [PR37722] Jason Merrill
@ 2023-12-22 3:17 ` Jason Merrill
0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2023-12-22 3:17 UTC (permalink / raw)
To: gcc-patches
Tested x86_64-pc-linux-gnu, applying to trunk.
-- 8< --
As with 37722, we don't clean up the exception object if a computed goto
leaves a catch block, but we can warn about that.
PR c++/81438
gcc/cp/ChangeLog:
* decl.cc (poplevel_named_label_1): Handle leaving catch.
(check_previous_goto_1): Likewise.
(check_goto_1): Likewise.
gcc/testsuite/ChangeLog:
* g++.dg/ext/label15.C: Require indirect_jumps.
* g++.dg/ext/label16.C: New test.
---
gcc/cp/decl.cc | 42 ++++++++++++++++++++++++------
gcc/testsuite/g++.dg/ext/label15.C | 1 +
gcc/testsuite/g++.dg/ext/label16.C | 34 ++++++++++++++++++++++++
3 files changed, 69 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ext/label16.C
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index e044bfa6701..6b4d89e7115 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -571,10 +571,14 @@ poplevel_named_label_1 (named_label_entry **slot, cp_binding_level *bl)
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);
+ {
+ if (bl->kind == sk_catch)
+ vec_safe_push (cg, get_identifier ("catch"));
+ 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;
@@ -3820,7 +3824,12 @@ check_previous_goto_1 (tree decl, cp_binding_level* level, tree names,
identified = 2;
if (complained)
for (tree d : computed)
- inform (DECL_SOURCE_LOCATION (d), " does not destroy %qD", d);
+ {
+ if (DECL_P (d))
+ inform (DECL_SOURCE_LOCATION (d), " does not destroy %qD", d);
+ else if (d == get_identifier ("catch"))
+ inform (*locus, " does not clean up handled exception");
+ }
}
return !identified;
@@ -3963,15 +3972,32 @@ check_goto_1 (named_label_entry *ent, bool computed)
auto names = ent->names_in_scope;
for (auto b = current_binding_level; ; b = b->level_chain)
{
+ if (b->kind == sk_catch)
+ {
+ if (!identified)
+ {
+ complained
+ = identify_goto (decl, DECL_SOURCE_LOCATION (decl),
+ &input_location, DK_ERROR, computed);
+ identified = 2;
+ }
+ if (complained)
+ inform (input_location,
+ " does not clean up handled exception");
+ }
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 (!identified)
+ {
+ complained
+ = identify_goto (decl, DECL_SOURCE_LOCATION (decl),
+ &input_location, DK_ERROR, computed);
+ identified = 2;
+ }
if (complained)
inform (DECL_SOURCE_LOCATION (d),
" does not destroy %qD", d);
diff --git a/gcc/testsuite/g++.dg/ext/label15.C b/gcc/testsuite/g++.dg/ext/label15.C
index f9d6a0dd626..5a23895d52d 100644
--- a/gcc/testsuite/g++.dg/ext/label15.C
+++ b/gcc/testsuite/g++.dg/ext/label15.C
@@ -1,4 +1,5 @@
// PR c++/37722
+// { dg-do compile { target indirect_jumps } }
// { dg-options "" }
extern "C" int printf (const char *, ...);
diff --git a/gcc/testsuite/g++.dg/ext/label16.C b/gcc/testsuite/g++.dg/ext/label16.C
new file mode 100644
index 00000000000..ea79b6ef1fc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/label16.C
@@ -0,0 +1,34 @@
+// PR c++/81438
+// { dg-do compile { target indirect_jumps } }
+// { dg-options "" }
+
+bool b;
+int main()
+{
+ try
+ {
+ try { throw 3; }
+ catch(...) {
+ h:; // { dg-warning "jump to label" }
+ try { throw 7; }
+ catch(...) {
+ if (b)
+ goto *&&h; // { dg-message "computed goto" }
+ // { dg-message "handled exception" "" { target *-*-* } .-1 }
+ else
+ goto *&&g; // { dg-message "computed goto" }
+ // { dg-message "handled exception" "" { target *-*-* } .-1 }
+ }
+ g:; // { dg-warning "jump to label" }
+ throw;
+ }
+ }
+ catch(int v)
+ {
+ __builtin_printf("%d\n", v);
+ if(v != 3) // 7 because we don't clean up the catch on
+ __builtin_abort(); // computed goto
+ }
+
+ return 0;
+}
base-commit: d26f589e61a178e898d8b247042b487287ffe121
--
2.39.3
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-12-22 3:18 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-21 2:06 [pushed] c++: computed goto warning [PR37722] Jason Merrill
2023-12-22 3:17 ` [pushed] c++: computed goto from catch block [PR81438] Jason Merrill
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).