From patchwork Thu Dec 21 02:06:27 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 1878792 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=IMOHn806; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4SwYhZ3Hgqz1ydg for ; Thu, 21 Dec 2023 13:06:54 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CD725386187F for ; Thu, 21 Dec 2023 02:06:51 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 7B9663858286 for ; Thu, 21 Dec 2023 02:06:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7B9663858286 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 7B9663858286 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703124396; cv=none; b=cXfVaG2vTbtc/lrUbbHMqWZ2GRcIjJT9H1xGFmDIV1YJ7MWkd1sNPrqNDYjddpJvM+t2ozjxQoy4JEp36/NmXUBvXXSTtHTh4AjngZkoQIByh7Jk+7NPpTQXfxxB9E7aF8ESGueI8g1vdgXgHaL8wDQ5RkIWAeQMnvRYa8rYXA0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703124396; c=relaxed/simple; bh=xgO19nrrrSLATw8c/lsyybepdvZncREQu1UVSaJ5HOo=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=kDwES4NWFRLg1edpd54KCROIS90xjDn6Wu0aTBdafvHHPw0OTGZSAQAhIPsJq+ZXFpp7Oet2y5NroKximH35Kn4vf6AfSf+xJhmemLwZncz+cBN0Xf18L0H92Szip4wJ+yG/Rxeu2h32+QCYBR3aokq10g6J7fc3wac3vPjNCi0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1703124394; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Tm07Wd8h0va81zS/KX4JDIuiSQLFBG5gPosrJ1D0Rjc=; b=IMOHn806YLKj//u3v4lACNiiH3EowDapVRRkqUDnbrn2I87Q7Z3m+TWpj7yyPH9XWEjjXC uAJe4HUTV8JAGxa2MiO2sdaDKzLxbvJdHA0gxxgNB+W2NLk0FMBPmbpGKkS34OjRwWvgKB 4pZhmlKFqLrqUHGNFd+wFsD7rQcpWWA= Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-452-zXxgAmuiN0GhZbavjlBO2A-1; Wed, 20 Dec 2023 21:06:32 -0500 X-MC-Unique: zXxgAmuiN0GhZbavjlBO2A-1 Received: by mail-qt1-f198.google.com with SMTP id d75a77b69052e-427884c2eccso5487271cf.1 for ; Wed, 20 Dec 2023 18:06:31 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703124391; x=1703729191; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Tm07Wd8h0va81zS/KX4JDIuiSQLFBG5gPosrJ1D0Rjc=; b=CceSOVFIWr/IiLWIfbGZdt/0zLabgIdL1gfZN3r7A/+/zCyBIiaeB3YnmsE8SKdeNE 62QhEJG2KYDiFJ/e6WEIvsDPle6+u1EIBLpkwhb6MqorZMyZ2AbDGmZJQVM6BA8RHqYs fd4ox4txGafmTCyRjna3/lxpomkT9gI0X00bTzjw9HhqExg5qugoCRuq0MV06zr9UXzM 2rByFoC3D6W3cL/QzNzpVypFhNBnvpIhJ1cvcOw/2DUxFF6BV6/cJNBL170d55Np2jqk i9tfdOw8dKdpzolF3+/cLFQiRqDaZyddPcVnu29u8ykwlrLt3nZ3ivFK9eblHPOriF2B c7PA== X-Gm-Message-State: AOJu0YzeFEGm1/rwmvglPYPBRVLT6D/MxiHX0th1lIAH+an+BjLS9mLK ffxe93yYNyQy5nhRT+tDOLxmy77wkWQIIxRG3T9kLvVyW/6MClsYA0YLxG9Ga3NVIMiwagFyhCO IF77gcxX4oIch+no6uNY8MAlZfX0RT4EeNvJBNZFIlxk+80sRsNznGd6XwvE5Atm6td3f+PoYbA == X-Received: by 2002:a05:622a:201:b0:423:7060:3d5e with SMTP id b1-20020a05622a020100b0042370603d5emr5831565qtx.11.1703124390848; Wed, 20 Dec 2023 18:06:30 -0800 (PST) X-Google-Smtp-Source: AGHT+IHF//gsq0PiQ4zf24jKRzsM5V8n2QAD2bPTPo3GhWeXvtXiHgHO+yG7sOunlVwrd2J8shOw4A== X-Received: by 2002:a05:622a:201:b0:423:7060:3d5e with SMTP id b1-20020a05622a020100b0042370603d5emr5831547qtx.11.1703124390329; Wed, 20 Dec 2023 18:06:30 -0800 (PST) Received: from jason.com (130-44-146-16.s12558.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.146.16]) by smtp.gmail.com with ESMTPSA id k15-20020ac8478f000000b004240481cee0sm394596qtq.58.2023.12.20.18.06.29 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Dec 2023 18:06:29 -0800 (PST) From: Jason Merrill To: gcc-patches@gcc.gnu.org Subject: [pushed] c++: computed goto warning [PR37722] Date: Wed, 20 Dec 2023 21:06:27 -0500 Message-Id: <20231221020627.3266898-1-jason@redhat.com> X-Mailer: git-send-email 2.39.3 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org 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 --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 *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 *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 +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; }