From patchwork Fri Jan 7 22:12:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 1576966 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=LJi6o9LQ; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from 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 RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JVyD25HYNz9s1l for ; Sat, 8 Jan 2022 09:13:37 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 680A43857C77 for ; Fri, 7 Jan 2022 22:13:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 680A43857C77 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1641593614; bh=uYWjoa9p/9DlL4WoFMCdXIwDn+LzMIzxhYrI11u6l7Q=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=LJi6o9LQMjoTVi9oInk7EoYRUKt2AsvmyGdxAWG68uFrwv0DWMjUoRzWQ87y4zXT4 Ns8UWD9wsjQKS7XDpIV9bt3lhnI9Xc/MZBHCECXw7kV/tZ9Tj4x83PQFwgDiqQbZp6 zQWlymdJrGKUM14PZaLvdIzpxXjUWYgN8Y5PZk2A= 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 4738C3858D28 for ; Fri, 7 Jan 2022 22:12:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4738C3858D28 Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-316-69vjYc8-ObKMrEvtZMfNxA-1; Fri, 07 Jan 2022 17:12:50 -0500 X-MC-Unique: 69vjYc8-ObKMrEvtZMfNxA-1 Received: by mail-qk1-f199.google.com with SMTP id q5-20020a05620a0d8500b004738c1b48beso5203184qkl.7 for ; Fri, 07 Jan 2022 14:12:49 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=uYWjoa9p/9DlL4WoFMCdXIwDn+LzMIzxhYrI11u6l7Q=; b=7rvv8ko9NmjUbQ0kUsQYx43IYwRUhcP9cIM6tPuHXAtN7R7N3g1+nk1dBoluOvzM03 OHfhmcYxLCsoeW1E+daTW9L8DZyrX/h9WGrbfxvCOVtk61WZVOlcxD3pad2ZXTZDQysr xyRQakTxnLy8ULR5RQe4cbbb2M93uhR9BcJiWPCtyMDSf1jDoRuwYrf4u7HCOtJNNOwN q0iBopPqCLstbyoN6dzgIZ3iQ2TJXYtslozSV/gD28bmFj/WxHS0G6WqOGMINzkO6/3t sgnktSqesE1pqGFaap6L5anllXzpmDfRpWSi9+yqwW97Vm8IA5MD7TXbnIy6WbFBrCui TIQQ== X-Gm-Message-State: AOAM531qEkLQBEEuGB4I01LT7+bJ0EFXAOgqyqoAPi7xqFh82aUXNZrY yFTjr2ujE5BzrQkwPKQXJHRBJ16dODAW5pf69aRQY8RvNu97RUxYEe216HVfDrufwHY6NJMYgCJ KwinQQDSz+yqjYxaNqUWhfqbHUbdkV2DULrbRz+9p6fDY+lkQdH9lckUjbMmsm2vHbw== X-Received: by 2002:a37:315:: with SMTP id 21mr45464450qkd.52.1641593569074; Fri, 07 Jan 2022 14:12:49 -0800 (PST) X-Google-Smtp-Source: ABdhPJydhiH6GreJeSWCv0mL0MFJoj6IpNfMK3iMSKYkJW3XbjcaAGh94w6BhD5KSgYDRkw5mMIKeQ== X-Received: by 2002:a37:315:: with SMTP id 21mr45464433qkd.52.1641593568469; Fri, 07 Jan 2022 14:12:48 -0800 (PST) Received: from barrymore.redhat.com (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id v10sm4629364qtk.13.2022.01.07.14.12.47 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 07 Jan 2022 14:12:47 -0800 (PST) To: gcc-patches@gcc.gnu.org Subject: [pushed] c++: check delete access with trivial init [PR20040] Date: Fri, 7 Jan 2022 17:12:45 -0500 Message-Id: <20220107221245.3074079-1-jason@redhat.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.2 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_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jason Merrill via Gcc-patches From: Jason Merrill Reply-To: Jason Merrill Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Apparently we need to check the accessibility of the deallocation function even if there is no initialization. Tested x86_64-pc-linux-gnu, applying to trunk. PR c++/20040 gcc/cp/ChangeLog: * init.c (build_new_1): Also build pointer cleanup if TYPE_GETS_DELETE. * cp-tree.h (TYPE_GETS_VEC_DELETE): New. gcc/testsuite/ChangeLog: * g++.dg/init/delete4.C: New test. --- gcc/cp/cp-tree.h | 1 + gcc/cp/init.c | 142 +++++++++++++++------------- gcc/testsuite/g++.dg/init/delete4.C | 14 +++ 3 files changed, 89 insertions(+), 68 deletions(-) create mode 100644 gcc/testsuite/g++.dg/init/delete4.C base-commit: 997130f778c56466a825627644e510960585521b diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index e204182da97..f8225c18a48 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -2395,6 +2395,7 @@ struct GTY(()) lang_type { /* Nonzero for _CLASSTYPE means that operator delete is defined. */ #define TYPE_GETS_DELETE(NODE) (LANG_TYPE_CLASS_CHECK (NODE)->gets_delete) #define TYPE_GETS_REG_DELETE(NODE) (TYPE_GETS_DELETE (NODE) & 1) +#define TYPE_GETS_VEC_DELETE(NODE) (TYPE_GETS_DELETE (NODE) & 2) /* Nonzero if `new NODE[x]' should cause the allocation of extra storage to indicate how many array elements are in use. */ diff --git a/gcc/cp/init.c b/gcc/cp/init.c index c932699ffa6..6226812b470 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -3316,6 +3316,12 @@ build_new_1 (vec **placement, tree type, tree nelts, ? TYPE_HAS_ARRAY_NEW_OPERATOR (elt_type) : TYPE_HAS_NEW_OPERATOR (elt_type)); + bool member_delete_p = (!globally_qualified_p + && CLASS_TYPE_P (elt_type) + && (array_p + ? TYPE_GETS_VEC_DELETE (elt_type) + : TYPE_GETS_REG_DELETE (elt_type))); + if (member_new_p) { /* Use a class-specific operator new. */ @@ -3473,7 +3479,7 @@ build_new_1 (vec **placement, tree type, tree nelts, /* In the simple case, we can stop now. */ pointer_type = build_pointer_type (type); - if (!cookie_size && !is_initialized) + if (!cookie_size && !is_initialized && !member_delete_p) return build_nop (pointer_type, alloc_call); /* Store the result of the allocation call in a variable so that we can @@ -3700,77 +3706,77 @@ build_new_1 (vec **placement, tree type, tree nelts, if (init_expr == error_mark_node) return error_mark_node; - - /* If any part of the object initialization terminates by throwing an - exception and a suitable deallocation function can be found, the - deallocation function is called to free the memory in which the - object was being constructed, after which the exception continues - to propagate in the context of the new-expression. If no - unambiguous matching deallocation function can be found, - propagating the exception does not cause the object's memory to be - freed. */ - if (flag_exceptions) - { - enum tree_code dcode = array_p ? VEC_DELETE_EXPR : DELETE_EXPR; - tree cleanup; - - /* The Standard is unclear here, but the right thing to do - is to use the same method for finding deallocation - functions that we use for finding allocation functions. */ - cleanup = (build_op_delete_call - (dcode, - alloc_node, - size, - globally_qualified_p, - placement_allocation_fn_p ? alloc_call : NULL_TREE, - alloc_fn, - complain)); - - if (cleanup && !processing_template_decl) - /* Ack! First we allocate the memory. Then we set our sentry - variable to true, and expand a cleanup that deletes the - memory if sentry is true. Then we run the constructor, and - finally clear the sentry. - - We need to do this because we allocate the space first, so - if there are any temporaries with cleanups in the - constructor args, we need this EH region to extend until - end of full-expression to preserve nesting. - - We used to try to evaluate the args first to avoid this, but - since C++17 [expr.new] says that "The invocation of the - allocation function is sequenced before the evaluations of - expressions in the new-initializer." */ - { - tree end, sentry, begin; - - begin = get_target_expr (boolean_true_node); - CLEANUP_EH_ONLY (begin) = 1; - - sentry = TARGET_EXPR_SLOT (begin); - - /* CLEANUP is compiler-generated, so no diagnostics. */ - suppress_warning (cleanup); - - TARGET_EXPR_CLEANUP (begin) - = build3 (COND_EXPR, void_type_node, sentry, - cleanup, void_node); - - end = build2 (MODIFY_EXPR, TREE_TYPE (sentry), - sentry, boolean_false_node); - - init_expr - = build2 (COMPOUND_EXPR, void_type_node, begin, - build2 (COMPOUND_EXPR, void_type_node, init_expr, - end)); - /* Likewise, this is compiler-generated. */ - suppress_warning (init_expr); - } - } } else init_expr = NULL_TREE; + /* If any part of the object initialization terminates by throwing an + exception and a suitable deallocation function can be found, the + deallocation function is called to free the memory in which the + object was being constructed, after which the exception continues + to propagate in the context of the new-expression. If no + unambiguous matching deallocation function can be found, + propagating the exception does not cause the object's memory to be + freed. */ + if (flag_exceptions && (init_expr || member_delete_p)) + { + enum tree_code dcode = array_p ? VEC_DELETE_EXPR : DELETE_EXPR; + tree cleanup; + + /* The Standard is unclear here, but the right thing to do + is to use the same method for finding deallocation + functions that we use for finding allocation functions. */ + cleanup = (build_op_delete_call + (dcode, + alloc_node, + size, + globally_qualified_p, + placement_allocation_fn_p ? alloc_call : NULL_TREE, + alloc_fn, + complain)); + + if (cleanup && init_expr && !processing_template_decl) + /* Ack! First we allocate the memory. Then we set our sentry + variable to true, and expand a cleanup that deletes the + memory if sentry is true. Then we run the constructor, and + finally clear the sentry. + + We need to do this because we allocate the space first, so + if there are any temporaries with cleanups in the + constructor args, we need this EH region to extend until + end of full-expression to preserve nesting. + + We used to try to evaluate the args first to avoid this, but + since C++17 [expr.new] says that "The invocation of the + allocation function is sequenced before the evaluations of + expressions in the new-initializer." */ + { + tree end, sentry, begin; + + begin = get_target_expr (boolean_true_node); + CLEANUP_EH_ONLY (begin) = 1; + + sentry = TARGET_EXPR_SLOT (begin); + + /* CLEANUP is compiler-generated, so no diagnostics. */ + suppress_warning (cleanup); + + TARGET_EXPR_CLEANUP (begin) + = build3 (COND_EXPR, void_type_node, sentry, + cleanup, void_node); + + end = build2 (MODIFY_EXPR, TREE_TYPE (sentry), + sentry, boolean_false_node); + + init_expr + = build2 (COMPOUND_EXPR, void_type_node, begin, + build2 (COMPOUND_EXPR, void_type_node, init_expr, + end)); + /* Likewise, this is compiler-generated. */ + suppress_warning (init_expr); + } + } + /* Now build up the return value in reverse order. */ rval = data_addr; diff --git a/gcc/testsuite/g++.dg/init/delete4.C b/gcc/testsuite/g++.dg/init/delete4.C new file mode 100644 index 00000000000..94932b4b82c --- /dev/null +++ b/gcc/testsuite/g++.dg/init/delete4.C @@ -0,0 +1,14 @@ +// PR c++/20040 + +class X +{ + void operator delete(void *p) throw () {} // { dg-message "declared private" } +}; + +X xa; + +int mymain() +{ + X *p = new X; // { dg-error "is private" } + return 0; +}