On Jan 21, 2025, Richard Biener <richard.guent...@gmail.com> wrote:
> So - your fix looks almost good, I'd adjust it to
>> + case BIT_FIELD_REF:
>> + if (DECL_P (TREE_OPERAND (expr, 0))
>> + && !bit_field_ref_in_bounds_p (expr))
>> + return true;
>> + /* Fall through. */
> OK if that works.
It does. But my intuition isn't happy with this. I don't get why only
DECLs need to be checked here, and why we should check the type size
rather than the decl size if we're only checking decls.
I have another patch coming up that doesn't raise concerns for me, so
I'll hold off from installing the above, in case you also prefer the
other one.
[ifcombine] out-of-bounds bitfield refs can trap [PR118514]
Check that BIT_FIELD_REFs of DECLs are in range before deciding they
don't trap.
Check that a replacement bitfield load is as trapping as the replaced
load.
for gcc/ChangeLog
PR tree-optimization/118514
* tree-eh.cc (bit_field_ref_in_bounds_p): New.
(tree_could_trap_p) <BIT_FIELD_REF>: Call it.
* gimple-fold.cc (make_bit_field_load): Check trapping status
of replacement load against original load.
for gcc/testsuite/ChangeLog
PR tree-optimization/118514
* gcc.dg/field-merge-23.c: New.
---
gcc/gimple-fold.cc | 5 +++++
gcc/testsuite/gcc.dg/field-merge-23.c | 19 +++++++++++++++++++
gcc/tree-eh.cc | 29 ++++++++++++++++++++++++++++-
3 files changed, 52 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/field-merge-23.c
diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
index 3c971a29ef045..01c4d076af26c 100644
--- a/gcc/gimple-fold.cc
+++ b/gcc/gimple-fold.cc
@@ -7859,6 +7859,11 @@ make_bit_field_load (location_t loc, tree inner, tree
orig_inner, tree type,
gimple *new_stmt = gsi_stmt (i);
if (gimple_has_mem_ops (new_stmt))
gimple_set_vuse (new_stmt, reaching_vuse);
+ gcc_checking_assert (! (gimple_assign_load_p (point)
+ && gimple_assign_load_p (new_stmt))
+ || (tree_could_trap_p (gimple_assign_rhs1 (point))
+ == tree_could_trap_p (gimple_assign_rhs1
+ (new_stmt))));
}
gimple_stmt_iterator gsi = gsi_for_stmt (point);
diff --git a/gcc/testsuite/gcc.dg/field-merge-23.c
b/gcc/testsuite/gcc.dg/field-merge-23.c
new file mode 100644
index 0000000000000..d60f76206ebea
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/field-merge-23.c
@@ -0,0 +1,19 @@
+/* { dg-do run } */
+/* { dg-options "-O" } */
+
+/* PR tree-optimization/118514 */
+
+/* Check that we don't pull optimized references that could trap out of
+ loops. */
+
+int a, c = 1;
+unsigned char b[1], d;
+int main() {
+ while (a || !c) {
+ signed char e = b[1000000000];
+ d = e < 0 || b[1000000000] > 1;
+ if (d)
+ __builtin_abort ();
+ }
+ return 0;
+}
diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc
index 1033b124e4df3..27a4b2b5b1665 100644
--- a/gcc/tree-eh.cc
+++ b/gcc/tree-eh.cc
@@ -2646,6 +2646,29 @@ range_in_array_bounds_p (tree ref)
return true;
}
+/* Return true iff EXPR, a BIT_FIELD_REF, accesses a bit range that is known to
+ be in bounds for the referred operand type. */
+
+static bool
+bit_field_ref_in_bounds_p (tree expr)
+{
+ tree size_tree;
+ poly_uint64 size_max, min, wid, max;
+
+ size_tree = TYPE_SIZE (TREE_TYPE (TREE_OPERAND (expr, 0)));
+ if (!size_tree || !poly_int_tree_p (size_tree, &size_max))
+ return false;
+
+ min = bit_field_offset (expr);
+ wid = bit_field_size (expr);
+ max = min + wid;
+ if (maybe_lt (max, min)
+ || maybe_lt (size_max, max))
+ return false;
+
+ return true;
+}
+
/* 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. */
@@ -2688,10 +2711,14 @@ tree_could_trap_p (tree expr)
restart:
switch (code)
{
+ case BIT_FIELD_REF:
+ if (DECL_P (TREE_OPERAND (expr, 0)) && !bit_field_ref_in_bounds_p (expr))
+ return true;
+ /* Fall through. */
+
case COMPONENT_REF:
case REALPART_EXPR:
case IMAGPART_EXPR:
- case BIT_FIELD_REF:
case VIEW_CONVERT_EXPR:
case WITH_SIZE_EXPR:
expr = TREE_OPERAND (expr, 0);
--
Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive
