On Wed, 2 Feb 2022, Michael Matz wrote:
> Hello,
>
> On Wed, 2 Feb 2022, Richard Biener via Gcc-patches wrote:
>
> > This adds a flag to CONSTRUCTOR nodes indicating that for clobbers this
> > marks the end-of-life of storage as opposed to just ending the lifetime
> > of the object that occupied it. The dangling pointer diagnostics uses
> > CLOBBERs but is confused by those emitted by the C++ frontend for
> > example which emits them for the second purpose at the start of CTORs.
> > The issue is also appearant for aarch64 in PR104092.
> >
> > Distinguishing the two cases is also necessary for the PR90348 fix.
>
> (Just FWIW: I agree with the plan to have different types of CLOBBERs, in
> particular those for marking births)
>
> A style nit:
>
> > tree clobber = build_clobber (TREE_TYPE (t));
> > + CLOBBER_MARKS_EOL (clobber) = 1;
>
> I think it really were better if build_clobber() gained an additional
> argument (non-optional!) that sets the type of it.
It indeed did occur to me as well, so as we're two now I tried to see
how it looks like. It does like the following (didn't bother to
replace all build_clobber calls but defaulted the parameter - there
are too many). With CLOBBER_BIRTH added for the fix for PR90348
the enum would be constructed like
#define CLOBBER_KIND(NODE) \
((clobber_kind) (CONSTRUCTOR_CHECK (NODE)->base.private_flag << 1
| CONSTRUCTOR_CHECK (NODE)->base.protected_flag))
or so (maybe different dependent on bitfield endianess to make
extracting the two adjacent bits cheap). That would leave us space
for a fourth state.
So do you think that makes it nicer? What do others think?
Thanks,
Richard.
--
>From a06a7b8f8ffbd6b3b0f77f9dfbbea1822f39b714 Mon Sep 17 00:00:00 2001
From: Richard Biener <rguent...@suse.de>
Date: Wed, 2 Feb 2022 14:24:39 +0100
Subject: [PATCH] Add CLOBBER_EOL to mark storage end-of-life clobbers
To: gcc-patches@gcc.gnu.org
This adds a flag to CONSTRUCTOR nodes indicating that for
clobbers this marks the end-of-life of storage as opposed to
just ending the lifetime of the object that occupied it.
The dangling pointer diagnostics uses CLOBBERs but is confused
by those emitted by the C++ frontend for example which emits
them for the second purpose at the start of CTORs. The issue
is also appearant for aarch64 in PR104092.
Distinguishing the two cases is also necessary for the PR90348 fix.
Since I'm going to add another flag I added an enum clobber_flags
and a defaulted argument to build_clobber plus a convenient way to
query the enum from the CTOR tree and specify it for gimple_clobber_p.
Since 'CLOBBER' is already taken and I needed a name for the unspecified
clobber we have now I used 'CLOBBER_UNDEF'.
2022-02-03 Richard Biener <rguent...@suse.de>
PR middle-end/90348
PR middle-end/104092
* tree-core.h: Document protected_flag use on CONSTRUCTOR nodes.
* tree.h (clobber_kind): New enum.
(CLOBBER_KIND): Add.
(build_clobber): Add clobber kind argument, defaulted to
CLOBBER_UNDEF.
* gimple.h (gimple_clobber_p): New overload with specified kind.
* tree-pretty-print.cc (dump_generic_node): Mark EOL CLOBBERs.
* gimplify.c (gimplify_bind_expr): Build storage end-of-life clobbers
with CLOBBER_EOL.
(gimplify_target_expr): Likewise.
* tree-inline.cc (expand_call_inline): Likewise.
* tree-ssa-ccp.cc (insert_clobber_before_stack_restore): Likewise.
* gimple-ssa-warn-access.cc (pass_waccess::check_stmt): Only treat
CLOBBER_EOL clobbers as ending lifetime of storage.
* gcc.dg/pr87052.c: Adjust.
---
gcc/gimple-ssa-warn-access.cc | 3 ++-
gcc/gimple.h | 9 +++++++++
gcc/gimplify.cc | 4 ++--
gcc/testsuite/gcc.dg/pr87052.c | 2 +-
gcc/tree-core.h | 3 +++
gcc/tree-inline.cc | 4 ++--
gcc/tree-pretty-print.cc | 6 +++++-
gcc/tree-ssa-ccp.cc | 2 +-
gcc/tree.cc | 9 ++++++++-
gcc/tree.h | 9 ++++++++-
10 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index ad5e2f4458e..2559ad1506a 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -4330,7 +4330,8 @@ is_auto_decl (tree x)
void
pass_waccess::check_stmt (gimple *stmt)
{
- if (m_check_dangling_p && gimple_clobber_p (stmt))
+ if (m_check_dangling_p
+ && gimple_clobber_p (stmt, CLOBBER_EOL))
{
/* Ignore clobber statemts in blocks with exceptional edges. */
basic_block bb = gimple_bb (stmt);
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 441d29a109a..77a5a07e9b5 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -2939,6 +2939,15 @@ gimple_clobber_p (const gimple *s)
&& TREE_CLOBBER_P (gimple_assign_rhs1 (s));
}
+/* Return true if S is a clobber statement. */
+
+static inline bool
+gimple_clobber_p (const gimple *s, enum clobber_kind kind)
+{
+ return gimple_clobber_p (s)
+ && CLOBBER_KIND (gimple_assign_rhs1 (s)) == kind;
+}
+
/* Return true if GS is a GIMPLE_CALL. */
static inline bool
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index cd4b61362b4..875b115d02d 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -1475,7 +1475,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
&& !is_gimple_reg (t)
&& flag_stack_reuse != SR_NONE)
{
- tree clobber = build_clobber (TREE_TYPE (t));
+ tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_EOL);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (t, clobber);
gimple_set_location (clobber_stmt, end_locus);
@@ -6981,7 +6981,7 @@ gimplify_target_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p)
{
if (flag_stack_reuse == SR_ALL)
{
- tree clobber = build_clobber (TREE_TYPE (temp));
+ tree clobber = build_clobber (TREE_TYPE (temp), CLOBBER_EOL);
clobber = build2 (MODIFY_EXPR, TREE_TYPE (temp), temp, clobber);
gimple_push_cleanup (temp, clobber, false, pre_p, true);
}
diff --git a/gcc/testsuite/gcc.dg/pr87052.c b/gcc/testsuite/gcc.dg/pr87052.c
index 2affc480f4e..18e092c4674 100644
--- a/gcc/testsuite/gcc.dg/pr87052.c
+++ b/gcc/testsuite/gcc.dg/pr87052.c
@@ -38,4 +38,4 @@ void test (void)
{ dg-final { scan-tree-dump-times "c = \"\";" 1 "gimple" } }
{ dg-final { scan-tree-dump-times "d = { *};" 1 "gimple" } }
{ dg-final { scan-tree-dump-times "e = " 1 "gimple" } }
- { dg-final { scan-tree-dump-times "e = {CLOBBER}" 1 "gimple" } } */
+ { dg-final { scan-tree-dump-times "e = {CLOBBER\\(eol\\)}" 1 "gimple" } }
*/
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index e83669f20d8..924a6a2b2cf 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1281,6 +1281,9 @@ struct GTY(()) tree_base {
ASM_INLINE_P in
ASM_EXPR
+ CLOBBER_MARKS_EOL in
+ CONSTRUCTOR
+
side_effects_flag:
TREE_SIDE_EFFECTS in
diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
index 497aa667eb9..ca66a8266b1 100644
--- a/gcc/tree-inline.cc
+++ b/gcc/tree-inline.cc
@@ -5138,7 +5138,7 @@ expand_call_inline (basic_block bb, gimple *stmt,
copy_body_data *id,
&& !is_gimple_reg (*varp)
&& !(id->debug_map && id->debug_map->get (p)))
{
- tree clobber = build_clobber (TREE_TYPE (*varp));
+ tree clobber = build_clobber (TREE_TYPE (*varp), CLOBBER_EOL);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (*varp, clobber);
gimple_set_location (clobber_stmt, gimple_location (stmt));
@@ -5207,7 +5207,7 @@ expand_call_inline (basic_block bb, gimple *stmt,
copy_body_data *id,
&& !is_gimple_reg (id->retvar)
&& !stmt_ends_bb_p (stmt))
{
- tree clobber = build_clobber (TREE_TYPE (id->retvar));
+ tree clobber = build_clobber (TREE_TYPE (id->retvar), CLOBBER_EOL);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (id->retvar, clobber);
gimple_set_location (clobber_stmt, gimple_location (old_stmt));
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 6130c307e9a..666b7a70ea2 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -2500,7 +2500,11 @@ dump_generic_node (pretty_printer *pp, tree node, int
spc, dump_flags_t flags,
}
pp_left_brace (pp);
if (TREE_CLOBBER_P (node))
- pp_string (pp, "CLOBBER");
+ {
+ pp_string (pp, "CLOBBER");
+ if (CLOBBER_KIND (node) == CLOBBER_EOL)
+ pp_string (pp, "(eol)");
+ }
else if (TREE_CODE (TREE_TYPE (node)) == RECORD_TYPE
|| TREE_CODE (TREE_TYPE (node)) == UNION_TYPE)
is_struct_init = true;
diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc
index 48683f04d49..9164efe3037 100644
--- a/gcc/tree-ssa-ccp.cc
+++ b/gcc/tree-ssa-ccp.cc
@@ -2505,7 +2505,7 @@ insert_clobber_before_stack_restore (tree saved_val, tree
var,
FOR_EACH_IMM_USE_STMT (stmt, iter, saved_val)
if (gimple_call_builtin_p (stmt, BUILT_IN_STACK_RESTORE))
{
- clobber = build_clobber (TREE_TYPE (var));
+ clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOL);
clobber_stmt = gimple_build_assign (var, clobber);
i = gsi_for_stmt (stmt);
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 7ce4f242751..035e1cb3453 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -2311,10 +2311,17 @@ build_constructor_va (tree type, int nelts, ...)
/* Return a node of type TYPE for which TREE_CLOBBER_P is true. */
tree
-build_clobber (tree type)
+build_clobber (tree type, enum clobber_kind kind)
{
tree clobber = build_constructor (type, NULL);
TREE_THIS_VOLATILE (clobber) = true;
+ switch (kind)
+ {
+ case CLOBBER_EOL:
+ TREE_PROTECTED (clobber) = 1;
+ break;
+ case CLOBBER_UNDEF:;
+ }
return clobber;
}
diff --git a/gcc/tree.h b/gcc/tree.h
index e2157d66d6c..239100b12b9 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -1155,6 +1155,13 @@ extern void omp_clause_range_check_failed (const_tree,
const char *, int,
#define TREE_CLOBBER_P(NODE) \
(TREE_CODE (NODE) == CONSTRUCTOR && TREE_THIS_VOLATILE (NODE))
+/* CLOBBER_EOL clobbers mark the end-of-life of storage. */
+enum clobber_kind { CLOBBER_UNDEF = 0, CLOBBER_EOL = 1};
+
+/* Return the clobber_kind of a CLOBBER CONSTRUCTOR. */
+#define CLOBBER_KIND(NODE) \
+ ((clobber_kind) CONSTRUCTOR_CHECK (NODE)->base.protected_flag)
+
/* Define fields and accessors for some nodes that represent expressions. */
/* Nonzero if NODE is an empty statement (NOP_EXPR <0>). */
@@ -4559,7 +4566,7 @@ extern tree build_constructor_single (tree, tree, tree);
extern tree build_constructor_from_list (tree, tree);
extern tree build_constructor_from_vec (tree, const vec<tree, va_gc> *);
extern tree build_constructor_va (tree, int, ...);
-extern tree build_clobber (tree);
+extern tree build_clobber (tree, enum clobber_kind = CLOBBER_UNDEF);
extern tree build_real_from_int_cst (tree, const_tree);
extern tree build_real_from_wide (tree, const wide_int_ref &, signop);
extern tree build_complex (tree, tree, tree);
--
2.34.1
