The following plugs a hole in the tree_could_trap_p check that
cselim performs when making a store unconditional. The hole is
that tree_could_trap_p does not consider accesses to readonly
memory possibly trapping even though the accesses could be stores.
The fix is to add a lhs_p parameter (defaulted to false in this RFC),
do the appropriate check (modeled after what tree-if-conv.cc does)
and makes sure cselim sets it when querying the lhs.
There are not many calls to tree_could_trap_p and I'll adjust them
all if folks are happy with this adjustment of the API.
OK in principle? As said, I'll remove the defaulting and make
the obviously correct changes (hoping there's not too much
fallout).
Richard.
PR tree-optimization/120043
* tree-eh.h (tree_could_trap_p): Add lhs_p, defaulted to false.
* tree-eh.cc (tree_could_trap_p): When lhs_p, trap when
a VAR_DECL might be readonly. Handle CONST_DECL
and STRING_CST.
* tree-ssa-phiopt.cc (cond_store_replacement): Pass true
as lhs_p to tree_could_trap_p.
* gcc.dg/torture/pr120043.c: New testcase.
---
gcc/testsuite/gcc.dg/torture/pr120043.c | 11 +++++++++++
gcc/tree-eh.cc | 24 ++++++++++++++++++------
gcc/tree-eh.h | 2 +-
gcc/tree-ssa-phiopt.cc | 2 +-
4 files changed, 31 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr120043.c
diff --git a/gcc/testsuite/gcc.dg/torture/pr120043.c
b/gcc/testsuite/gcc.dg/torture/pr120043.c
new file mode 100644
index 00000000000..5d09c1a731e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr120043.c
@@ -0,0 +1,11 @@
+/* { dg-do run } */
+/* { dg-additional-options "-fallow-store-data-races" } */
+
+const int a;
+int *b;
+int main()
+{
+ &a != b || (*b = 1);
+ return 0;
+}
+
diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc
index a4d59954c05..4169238f282 100644
--- a/gcc/tree-eh.cc
+++ b/gcc/tree-eh.cc
@@ -47,6 +47,7 @@ along with GCC; see the file COPYING3. If not see
#include "attribs.h"
#include "asan.h"
#include "gimplify.h"
+#include "varasm.h"
/* In some instances a tree and a gimple need to be stored in a same table,
i.e. in hash tables. This is a structure to do this. */
@@ -2669,10 +2670,11 @@ access_in_bounds_of_type_p (tree type, poly_uint64
size, poly_uint64 offset)
/* Return true if EXPR can trap, as in dereferencing an invalid pointer
location or floating point arithmetic. C.f. the rtl version, may_trap_p.
- This routine expects only GIMPLE lhs or rhs input. */
+ This routine expects only GIMPLE lhs or rhs input. LHS_P should be
+ set to true when EXPR can be a memory store. */
bool
-tree_could_trap_p (tree expr)
+tree_could_trap_p (tree expr, bool lhs_p)
{
enum tree_code code;
bool fp_operation = false;
@@ -2728,7 +2730,7 @@ tree_could_trap_p (tree expr)
case ARRAY_RANGE_REF:
base = TREE_OPERAND (expr, 0);
- if (tree_could_trap_p (base))
+ if (tree_could_trap_p (base, lhs_p))
return true;
if (TREE_THIS_NOTRAP (expr))
return false;
@@ -2736,7 +2738,7 @@ tree_could_trap_p (tree expr)
case ARRAY_REF:
base = TREE_OPERAND (expr, 0);
- if (tree_could_trap_p (base))
+ if (tree_could_trap_p (base, lhs_p))
return true;
if (TREE_THIS_NOTRAP (expr))
return false;
@@ -2745,7 +2747,8 @@ tree_could_trap_p (tree expr)
case TARGET_MEM_REF:
case MEM_REF:
if (TREE_CODE (TREE_OPERAND (expr, 0)) == ADDR_EXPR
- && tree_could_trap_p (TREE_OPERAND (TREE_OPERAND (expr, 0), 0)))
+ && tree_could_trap_p (TREE_OPERAND (TREE_OPERAND (expr, 0), 0),
+ lhs_p))
return true;
if (TREE_THIS_NOTRAP (expr))
return false;
@@ -2793,7 +2796,7 @@ tree_could_trap_p (tree expr)
if (!t || !DECL_P (t))
return true;
if (DECL_WEAK (t))
- return tree_could_trap_p (t);
+ return tree_could_trap_p (t, false);
return false;
case FUNCTION_DECL:
@@ -2820,8 +2823,17 @@ tree_could_trap_p (tree expr)
node = node->ultimate_alias_target ();
return !(node && node->in_other_partition);
}
+ /* When the variable might be readonly, storing to it may traps. */
+ if (lhs_p
+ && (TREE_READONLY (expr)
+ || !decl_binds_to_current_def_p (expr)))
+ return true;
return false;
+ case STRING_CST:
+ case CONST_DECL:
+ return lhs_p;
+
default:
return false;
}
diff --git a/gcc/tree-eh.h b/gcc/tree-eh.h
index 1fe6de80c42..975ad2dcd8a 100644
--- a/gcc/tree-eh.h
+++ b/gcc/tree-eh.h
@@ -37,7 +37,7 @@ extern bool operation_could_trap_helper_p (enum tree_code,
bool, bool, bool,
bool, tree, bool *);
extern bool operation_could_trap_p (enum tree_code, bool, bool, tree);
extern bool access_in_bounds_of_type_p (tree, poly_uint64, poly_uint64);
-extern bool tree_could_trap_p (tree);
+extern bool tree_could_trap_p (tree, bool = false);
extern tree rewrite_to_non_trapping_overflow (tree);
extern bool stmt_could_throw_p (function *, gimple *);
extern bool stmt_unremovable_because_of_non_call_eh_p (function *, gimple *);
diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index 54ecd93495a..23b5362a968 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -3547,7 +3547,7 @@ cond_store_replacement (basic_block middle_bb,
basic_block join_bb,
(or when we allow data races) and known not to trap, we could
always safely move down the store. */
if (ref_can_have_store_data_races (lhs)
- || tree_could_trap_p (lhs))
+ || tree_could_trap_p (lhs, true))
return false;
}
--
2.43.0
