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