From 6cd51207f5732b5258e883e9030b94c987b6d696 Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Thu, 6 Jan 2022 16:13:04 -0500 Subject: [PATCH] c++: check delete access with trivial init [PR20040] Apparently we need to check the accessibility of the deallocation function even if there is no initialization. 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 | 128 +++++++++++++++++++----------------- gcc/testsuite/g++.dg/init/delete4.C | 14 ++++ 3 files changed, 82 insertions(+), 61 deletions(-) create mode 100644 gcc/testsuite/g++.dg/init/delete4.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index e204182da..f8225c1 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 c932699..6226812 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,76 +3706,76 @@ build_new_1 (vec **placement, tree type, tree nelts, if (init_expr == error_mark_node) return error_mark_node; + } + 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) + /* 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." */ { - 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; + tree end, sentry, begin; - begin = get_target_expr (boolean_true_node); - CLEANUP_EH_ONLY (begin) = 1; + begin = get_target_expr (boolean_true_node); + CLEANUP_EH_ONLY (begin) = 1; - sentry = TARGET_EXPR_SLOT (begin); + sentry = TARGET_EXPR_SLOT (begin); - /* CLEANUP is compiler-generated, so no diagnostics. */ - suppress_warning (cleanup); + /* CLEANUP is compiler-generated, so no diagnostics. */ + suppress_warning (cleanup); - TARGET_EXPR_CLEANUP (begin) - = build3 (COND_EXPR, void_type_node, sentry, - cleanup, void_node); + 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); + 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); - } + 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; /* Now build up the return value in reverse order. */ diff --git a/gcc/testsuite/g++.dg/init/delete4.C b/gcc/testsuite/g++.dg/init/delete4.C new file mode 100644 index 0000000..94932b4 --- /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; +} -- 2.7.4