On 9/28/20 3:09 PM, Jason Merrill wrote:
On 9/28/20 3:56 AM, Richard Biener wrote:
On Fri, 25 Sep 2020, Jason Merrill wrote:
On 9/25/20 2:30 AM, Richard Biener wrote:
On Thu, 24 Sep 2020, Jason Merrill wrote:
On 9/24/20 3:43 AM, Richard Biener wrote:
On Wed, 23 Sep 2020, Jason Merrill wrote:
On 9/23/20 2:42 PM, Richard Biener wrote:
On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill
<ja...@redhat.com>
wrote:
On 9/23/20 4:14 AM, Richard Biener wrote:
C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
does not cause the deleted object to be escaped. It also has no
other interesting side-effects for PTA so skip it like we do
for BUILT_IN_FREE.
Hmm, this is true of the default implementation, but since the
function
is replaceable, we don't know what a user definition might do
with the
pointer.
But can the object still be 'used' after delete? Can delete fail /
throw?
What guarantee does the predicate give us?
The deallocation function is called as part of a delete
expression in
order
to
release the storage for an object, ending its lifetime (if it was
not
ended
by
a destructor), so no, the object can't be used afterward.
OK, but the delete operator can access the object contents if there
wasn't a destructor ...
A deallocation function that throws has undefined behavior.
OK, so it seems the 'replaceable' operators are the global ones
(for user-defined/class-specific placement variants I see arbitrary
extra arguments that we'd possibly need to handle).
I'm happy to revert but I'd like to have a testcase that FAILs
with the patch ;)
Now, the following aborts:
struct X {
static struct X saved;
int *p;
X() { __builtin_memcpy (this, &saved, sizeof (X)); }
};
void operator delete (void *p)
{
__builtin_memcpy (&X::saved, p, sizeof (X));
}
int main()
{
int y = 1;
X *p = new X;
p->p = &y;
delete p;
X *q = new X;
*(q->p) = 2;
if (y != 2)
__builtin_abort ();
}
and I could fix this by not making *p but what *p points to escape.
The testcase is of course maximally awkward, but hey ... ;)
Now this would all be moot if operator delete may not access
the object (or if the object contents are undefined at that point).
Oh, and the testcase segfaults when compiled with GCC 10 because
there we elide the new X / delete p pair ... which is invalid then?
Hmm, we emit
MEM[(struct X *)_8] ={v} {CLOBBER};
operator delete (_8, 8);
so the object contents are undefined _before_ calling delete
even when I do not have a DTOR? That is, the above,
w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase.
Yes, all classes have a destructor, even if it's trivial, so the
object's
lifetime definitely ends before the call to operator delete. This
is less
clear for scalar objects, but treating them similarly would be
consistent
with
other recent changes, so I think it's fine for us to assume that
scalar
objects are also invalidated before the call to operator delete.
But of
course this doesn't apply to explicit calls to operator delete
outside of a
delete expression.
OK, so change the testcase main slightly to
int main()
{
int y = 1;
X *p = new X;
p->p = &y;
::operator delete(p);
X *q = new X;
*(q->p) = 2;
if (y != 2)
__builtin_abort ();
}
in this case the lifetime of *p does not end before calling
::operator delete() and delete can stash the object contents
somewhere before ending its lifetime. For the very same reason
we may not elide a new/delete pair like in
int main()
{
int *p = new int;
*p = 1;
::operator delete (p);
}
Correct; the permission to elide new/delete pairs are for the
expressions, not
the functions.
which we before the change did not do only because calling
operator delete made p escape. Unfortunately points-to analysis
cannot really reconstruct whether delete was called as part of
a delete expression or directly (and thus whether object lifetime
ended already), neither can DCE. So I guess we need to mark
the operator delete call in some way to make those transforms
safe. At least currently any operator delete call makes the
alias guarantee of a operator new call moot by forcing the object
to be aliased with all global and escaped memory ...
Looks like there are some unallocated flags for CALL_EXPR we could
pick but I wonder if we can recycle protected_flag which is
CALL_FROM_THUNK_P and
CALL_ALLOCA_FOR_VAR_P in
CALL_EXPR
for calls to DECL_IS_OPERATOR_{NEW,DELETE}_P, thus whether
we have CALL_FROM_THUNK_P for those operators. Guess picking
a new flag is safer.
We won't ever call those operators from a thunk, so it should be OK
to reuse
it.
But, does it seem correct that we need to distinguish
delete expressions from plain calls to operator delete?
A reason for that distinction came up in the context of omitting
new/delete
pairs: we want to consider the operator first called by the new or
delete
expression, not a call from that first operator to another operator
new/delete
and exposed by inlining.
https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543404.html
In this context I also wonder about non-replaceable operator delete,
specifically operator delete in classes - are there any semantic
differences between those or why did we choose to only mark
the replaceable ones?
The standard says that for omitting a 'new' allocation, the operator
new has
to be a replaceable one, but does not say the same about 'delete'; it
just
says that if the allocation was omitted, the delete-expression does
not call a
deallocation function. It may not be necessary to make this
distinction for
delete. And this distinction could be local to the front end.
In the front end, we currently have cxx_replaceable_global_alloc_fn that
already ignores the replaceability of operator delete. And we have
CALL_FROM_NEW_OR_DELETE_P, that would just need to move into the
middle end.
And perhaps get renamed to CALL_OMITTABLE_NEW_OR_DELETE_P, and not
get set for
calls to non-replaceable operator new.
CALL_FROM_NEW_OR_DELETE_P indeed looks like the best fit - it's
only evaluated when cxx_replaceable_global_alloc_fn matches in the C++
FE so could be made to cover only replaceable variants.
CALL_REPLACEABLE_NEW_OR_DELETE_P () maybe, since we already use
REPLACEABLE for the fndecl flags? OMITTABLE is too specific
for the PTA case where it really matters whether the object
lifetime ends before the delete call, not whether it can be
omitted (hmm, guess that's not 100% overlap then either...).
That seems like good overlap to me, if we agree that object lifetime
ends before any delete call from a delete-expression, whether or not the
operator delete is replaceable.
Mind doing the C++ side of things recycling protected_flag as suggested?
OK.
commit d4a8ce72c51e32c7577ca93a488028995fd45ebe
Author: Jason Merrill <ja...@redhat.com>
Date: Mon Sep 28 15:33:54 2020 -0400
c++: Move CALL_FROM_NEW_OR_DELETE_P to tree.h.
As discussed with richi, we should be able to use TREE_PROTECTED for this
flag, since CALL_FROM_THUNK_P will never be set on a call to an operator new
or delete.
gcc/cp/ChangeLog:
* lambda.c (call_from_lambda_thunk_p): New.
* cp-gimplify.c (cp_genericize_r): Use it.
* pt.c (tsubst_copy_and_build): Use it.
* typeck.c (check_return_expr): Use it.
* cp-tree.h: Declare it.
(CALL_FROM_NEW_OR_DELETE_P): Move to gcc/tree.h.
gcc/ChangeLog:
* tree.h (CALL_FROM_NEW_OR_DELETE_P): Move from cp-tree.h.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 42d0d76bf21..272ea88960b 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -464,7 +464,6 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
SWITCH_STMT_NO_BREAK_P (in SWITCH_STMT)
LAMBDA_EXPR_CAPTURE_OPTIMIZED (in LAMBDA_EXPR)
IMPLICIT_CONV_EXPR_BRACED_INIT (in IMPLICIT_CONV_EXPR)
- CALL_FROM_NEW_OR_DELETE_P (in CALL_EXPR)
3: IMPLICIT_RVALUE_P (in NON_LVALUE_EXPR or STATIC_CAST_EXPR)
ICS_BAD_FLAG (in _CONV)
FN_TRY_BLOCK_P (in TRY_BLOCK)
@@ -3840,11 +3839,6 @@ struct GTY(()) lang_decl {
should be performed at instantiation time. */
#define KOENIG_LOOKUP_P(NODE) TREE_LANG_FLAG_0 (CALL_EXPR_CHECK (NODE))
-/* In a CALL_EXPR, true for allocator calls from new or delete
- expressions. */
-#define CALL_FROM_NEW_OR_DELETE_P(NODE) \
- TREE_LANG_FLAG_2 (CALL_EXPR_CHECK (NODE))
-
/* True if the arguments to NODE should be evaluated in left-to-right
order regardless of PUSH_ARGS_REVERSED. */
#define CALL_EXPR_ORDERED_ARGS(NODE) \
@@ -7286,6 +7280,7 @@ extern bool lambda_fn_in_template_p (tree);
extern void maybe_add_lambda_conv_op (tree);
extern bool is_lambda_ignored_entity (tree);
extern bool lambda_static_thunk_p (tree);
+extern bool call_from_lambda_thunk_p (tree);
extern tree finish_builtin_launder (location_t, tree,
tsubst_flags_t);
extern tree cp_build_vec_convert (tree, location_t, tree,
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 0e158784d0e..752bec31c3f 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1220,7 +1220,8 @@ struct GTY(()) tree_base {
all decls
CALL_FROM_THUNK_P and
- CALL_ALLOCA_FOR_VAR_P in
+ CALL_ALLOCA_FOR_VAR_P and
+ CALL_FROM_NEW_OR_DELETE_P in
CALL_EXPR
OMP_CLAUSE_LINEAR_VARIABLE_STRIDE in
diff --git a/gcc/tree.h b/gcc/tree.h
index 5bb6e7bc000..f27a7399a37 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -921,7 +921,8 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
(TREE_CHECK (NODE, PARM_DECL)->decl_common.decl_nonshareable_flag)
/* In a CALL_EXPR, means that the call is the jump from a thunk to the
- thunked-to function. */
+ thunked-to function. Be careful to avoid using this macro when one of the
+ next two applies instead. */
#define CALL_FROM_THUNK_P(NODE) (CALL_EXPR_CHECK (NODE)->base.protected_flag)
/* In a CALL_EXPR, if the function being called is BUILT_IN_ALLOCA, means that
@@ -931,6 +932,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
#define CALL_ALLOCA_FOR_VAR_P(NODE) \
(CALL_EXPR_CHECK (NODE)->base.protected_flag)
+/* In a CALL_EXPR, if the function being called is DECL_IS_OPERATOR_NEW_P or
+ DECL_IS_OPERATOR_DELETE_P, true for allocator calls from C++ new or delete
+ expressions. */
+#define CALL_FROM_NEW_OR_DELETE_P(NODE) \
+ (CALL_EXPR_CHECK (NODE)->base.protected_flag)
+
/* Used in classes in C++. */
#define TREE_PRIVATE(NODE) ((NODE)->base.private_flag)
/* Used in classes in C++. */
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index bc8a03c7b41..07549828dc9 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -962,7 +962,7 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void *data)
omp_cxx_notice_variable (wtd->omp_ctx, stmt);
/* Don't dereference parms in a thunk, pass the references through. */
- if ((TREE_CODE (stmt) == CALL_EXPR && CALL_FROM_THUNK_P (stmt))
+ if ((TREE_CODE (stmt) == CALL_EXPR && call_from_lambda_thunk_p (stmt))
|| (TREE_CODE (stmt) == AGGR_INIT_EXPR && AGGR_INIT_FROM_THUNK_P (stmt)))
{
*walk_subtrees = 0;
diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
index 07a5401c97b..1a1647f465e 100644
--- a/gcc/cp/lambda.c
+++ b/gcc/cp/lambda.c
@@ -1325,6 +1325,13 @@ lambda_static_thunk_p (tree fn)
&& LAMBDA_TYPE_P (CP_DECL_CONTEXT (fn)));
}
+bool
+call_from_lambda_thunk_p (tree call)
+{
+ return (CALL_FROM_THUNK_P (call)
+ && lambda_static_thunk_p (current_function_decl));
+}
+
/* Returns true iff VAL is a lambda-related declaration which should
be ignored by unqualified lookup. */
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index a09633751ca..b0c61cf31c6 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -19951,7 +19951,7 @@ tsubst_copy_and_build (tree t,
/* Stripped-down processing for a call in a thunk. Specifically, in
the thunk template for a generic lambda. */
- if (CALL_FROM_THUNK_P (t))
+ if (call_from_lambda_thunk_p (t))
{
/* Now that we've expanded any packs, the number of call args
might be different. */
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 9166156a5d5..95b36a92491 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -10171,7 +10171,7 @@ check_return_expr (tree retval, bool *no_warning)
/* The call in a (lambda) thunk needs no conversions. */
if (TREE_CODE (retval) == CALL_EXPR
- && CALL_FROM_THUNK_P (retval))
+ && call_from_lambda_thunk_p (retval))
converted = true;
/* First convert the value to the function's return type, then
