-Warray-bounds is an important option to enable linux kernal to keep
the array out-of-bound errors out of the source tree.
However, due to the false positive warnings reported in PR109071
(-Warray-bounds false positive warnings due to code duplication from
jump threading), -Warray-bounds=1 cannot be added on by default.
Although it's impossible to elinimate all the false positive warnings
from -Warray-bounds=1 (See PR104355 Misleading -Warray-bounds
documentation says "always out of bounds"), we should minimize the
false positive warnings in -Warray-bounds=1.
The root reason for the false positive warnings reported in PR109071 is:
When the thread jump optimization tries to reduce the # of branches
inside the routine, sometimes it needs to duplicate the code and
split into two conditional pathes. for example:
The original code:
void sparx5_set (int * ptr, struct nums * sg, int index)
{
if (index >= 4)
warn ();
*ptr = 0;
*val = sg->vals[index];
if (index >= 4)
warn ();
*ptr = *val;
return;
}
With the thread jump, the above becomes:
void sparx5_set (int * ptr, struct nums * sg, int index)
{
if (index >= 4)
{
warn ();
*ptr = 0; // Code duplications since "warn" does return;
*val = sg->vals[index]; // same this line.
// In this path, since it's under the condition
// "index >= 4", the compiler knows the value
// of "index" is larger then 4, therefore the
// out-of-bound warning.
warn ();
}
else
{
*ptr = 0;
*val = sg->vals[index];
}
*ptr = *val;
return;
}
We can see, after the thread jump optimization, the # of branches inside
the routine "sparx5_set" is reduced from 2 to 1, however, due to the
code duplication (which is needed for the correctness of the code), we
got a false positive out-of-bound warning.
In order to eliminate such false positive out-of-bound warning,
A. Add one more flag for GIMPLE: is_splitted.
B. During the thread jump optimization, when the basic blocks are
duplicated, mark all the STMTs inside the original and duplicated
basic blocks as "is_splitted";
C. Inside the array bound checker, add the following new heuristic:
If
1. the stmt is duplicated and splitted into two conditional paths;
+ 2. the warning level < 2;
+ 3. the current block is not dominating the exit block
Then not report the warning.
The false positive warnings are moved from -Warray-bounds=1 to
-Warray-bounds=2 now.
Bootstrapped and regression tested on both x86 and aarch64. adjusted
-Warray-bounds-61.c due to the false positive warnings.
Let me know if you have any comments and suggestions.
Thanks.
Qing
PR tree optimization/109071
gcc/ChangeLog:
* gimple-array-bounds.cc (check_out_of_bounds_and_warn): Add two new
arguments for the new heuristic to not issue warnings.
(array_bounds_checker::check_array_ref): Call the new prototype of the
routine check_out_of_bounds_and_warn.
(array_bounds_checker::check_mem_ref): Add one new argument for the
new heuristic to not issue warnings.
(array_bounds_checker::check_addr_expr): Call the new prototype of the
routine check_mem_ref, add new heuristic for not issue warnings.
(array_bounds_checker::check_array_bounds): Call the new prototype of
the routine check_mem_ref.
* gimple-array-bounds.h: New prototype of check_mem_ref.
* gimple.h (struct GTY): Add one new flag is_splitted for gimple.
(gimple_is_splitted_p): New function.
(gimple_set_is_splitted): New function.
* tree-ssa-threadupdate.cc (set_stmts_in_bb_is_splitted): New function.
(back_jt_path_registry::duplicate_thread_path): Mark all the stmts in
both original and copied blocks as IS_SPLITTED.
gcc/testsuite/ChangeLog:
* gcc.dg/Warray-bounds-61.c: Adjust testing case.
* gcc.dg/pr109071-1.c: New test.
* gcc.dg/pr109071.c: New test.
---
gcc/gimple-array-bounds.cc | 46 +++++++++++++++++++++----
gcc/gimple-array-bounds.h | 2 +-
gcc/gimple.h | 21 +++++++++--
gcc/testsuite/gcc.dg/Warray-bounds-61.c | 6 ++--
gcc/testsuite/gcc.dg/pr109071-1.c | 22 ++++++++++++
gcc/testsuite/gcc.dg/pr109071.c | 22 ++++++++++++
gcc/tree-ssa-threadupdate.cc | 15 ++++++++
7 files changed, 122 insertions(+), 12 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pr109071-1.c
create mode 100644 gcc/testsuite/gcc.dg/pr109071.c
diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
index 008071cd5464..4a2975623bc1 100644
--- a/gcc/gimple-array-bounds.cc
+++ b/gcc/gimple-array-bounds.cc
@@ -264,7 +264,9 @@ check_out_of_bounds_and_warn (location_t location, tree ref,
tree up_bound, tree up_bound_p1,
const value_range *vr,
bool ignore_off_by_one, bool for_array_bound,
- bool *out_of_bound)
+ bool *out_of_bound,
+ bool is_splitted,
+ bool is_dominate_exit)
{
tree min, max;
tree low_bound = array_ref_low_bound (ref);
@@ -273,6 +275,13 @@ check_out_of_bounds_and_warn (location_t location, tree
ref,
bool warned = false;
*out_of_bound = false;
+ /* If the stmt is duplicated and splitted, the warning level is not 2,
+ and the current block is not dominating the exit block, not report
+ the warning. */
+ if (is_splitted && warn_array_bounds < 2
+ && !is_dominate_exit)
+ return false;
+
/* Empty array. */
if (up_bound && tree_int_cst_equal (low_bound, up_bound_p1))
{
@@ -386,12 +395,17 @@ array_bounds_checker::check_array_ref (location_t
location, tree ref,
}
}
+ bool is_dominate_exit = dominated_by_p (CDI_DOMINATORS,
+ EXIT_BLOCK_PTR_FOR_FN (fun),
+ gimple_bb (stmt));
+
warned = check_out_of_bounds_and_warn (location, ref,
low_sub_org, low_sub, up_sub,
up_bound, up_bound_p1, &vr,
ignore_off_by_one, warn_array_bounds,
- &out_of_bound);
-
+ &out_of_bound,
+ gimple_is_splitted_p (stmt),
+ is_dominate_exit);
if (!warned && sam == special_array_member::int_0)
warned = warning_at (location, OPT_Wzero_length_bounds,
@@ -476,7 +490,7 @@ array_bounds_checker::check_array_ref (location_t location,
tree ref,
bool
array_bounds_checker::check_mem_ref (location_t location, tree ref,
- bool ignore_off_by_one)
+ gimple *stmt, bool ignore_off_by_one)
{
if (warning_suppressed_p (ref, OPT_Warray_bounds_))
return false;
@@ -574,6 +588,16 @@ array_bounds_checker::check_mem_ref (location_t location,
tree ref,
}
}
+ /* If the stmt is duplicated and splitted, the warning level is not 2,
+ and the current block is not dominating the exit block, not report
+ the warning. */
+ bool is_dominate_exit = dominated_by_p (CDI_DOMINATORS,
+ EXIT_BLOCK_PTR_FOR_FN (fun),
+ gimple_bb (stmt));
+ if (gimple_is_splitted_p (stmt) && warn_array_bounds < 2
+ && !is_dominate_exit)
+ return false;
+
bool warned = false;
if (lboob)
{
@@ -654,7 +678,7 @@ array_bounds_checker::check_addr_expr (location_t location,
tree t,
ignore_off_by_one = false;
}
else if (TREE_CODE (t) == MEM_REF)
- warned = check_mem_ref (location, t, ignore_off_by_one);
+ warned = check_mem_ref (location, t, stmt, ignore_off_by_one);
if (warned)
suppress_warning (t, OPT_Warray_bounds_);
@@ -690,6 +714,16 @@ array_bounds_checker::check_addr_expr (location_t
location, tree t,
if (!mem_ref_offset (t).is_constant (&idx))
return;
+ /* If the stmt is duplicated and splitted, the warning level is not 2,
+ and the current block is not dominating the exit block, not report
+ the warning. */
+ bool is_dominate_exit = dominated_by_p (CDI_DOMINATORS,
+ EXIT_BLOCK_PTR_FOR_FN (fun),
+ gimple_bb (stmt));
+ if (gimple_is_splitted_p (stmt) && warn_array_bounds < 2
+ && !is_dominate_exit)
+ return;
+
bool warned = false;
idx = wi::sdiv_trunc (idx, wi::to_offset (el_sz));
if (idx < 0)
@@ -809,7 +843,7 @@ array_bounds_checker::check_array_bounds (tree *tp, int
*walk_subtree,
warned = checker->check_array_ref (location, t, wi->stmt,
false/*ignore_off_by_one*/);
else if (TREE_CODE (t) == MEM_REF)
- warned = checker->check_mem_ref (location, t,
+ warned = checker->check_mem_ref (location, t, wi->stmt,
false /*ignore_off_by_one*/);
else if (TREE_CODE (t) == ADDR_EXPR)
{
diff --git a/gcc/gimple-array-bounds.h b/gcc/gimple-array-bounds.h
index 3e077d0178ff..7c98f02204c9 100644
--- a/gcc/gimple-array-bounds.h
+++ b/gcc/gimple-array-bounds.h
@@ -33,7 +33,7 @@ public:
private:
static tree check_array_bounds (tree *tp, int *walk_subtree, void *data);
bool check_array_ref (location_t, tree, gimple *, bool ignore_off_by_one);
- bool check_mem_ref (location_t, tree, bool ignore_off_by_one);
+ bool check_mem_ref (location_t, tree, gimple *, bool ignore_off_by_one);
void check_addr_expr (location_t, tree, gimple *);
void get_value_range (irange &r, const_tree op, gimple *);
diff --git a/gcc/gimple.h b/gcc/gimple.h
index bd315ffc2dd4..08f52d107084 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -254,8 +254,8 @@ struct GTY((desc ("gimple_statement_structure (&%h)"), tag
("GSS_BASE"),
/* Nonzero if this statement contains volatile operands. */
unsigned has_volatile_ops : 1;
- /* Padding to get subcode to 16 bit alignment. */
- unsigned pad : 1;
+ /* Nonzero if this statement is duplicated and splitted to two pathes. */
+ unsigned is_splitted : 1;
/* The SUBCODE field can be used for tuple-specific flags for tuples
that do not require subcodes. Note that SUBCODE should be at
@@ -2327,6 +2327,23 @@ gimple_set_has_volatile_ops (gimple *stmt, bool
volatilep)
stmt->has_volatile_ops = (unsigned) volatilep;
}
+/* Return true if statement G's is_splitted field has
+ been set. */
+
+inline bool
+gimple_is_splitted_p (const gimple *g)
+{
+ return (bool) g->is_splitted;
+}
+
+/* Set the IS_SPLITTED flag to IS_SPLITTEDP. */
+
+inline void
+gimple_set_is_splitted (gimple *s, bool is_splittedp)
+{
+ s->is_splitted = (unsigned) is_splittedp;
+}
+
/* Return true if STMT is in a transaction. */
inline bool
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-61.c
b/gcc/testsuite/gcc.dg/Warray-bounds-61.c
index 5b66cdc0aab1..cb3c64a813d7 100644
--- a/gcc/testsuite/gcc.dg/Warray-bounds-61.c
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-61.c
@@ -23,7 +23,7 @@ void test_ua3_ua0_a0 (int i)
if (i < __LINE__)
i = 5;
- ua3_a0.a0[i] = 0; // { dg-warning "\\\[-Warray-bounds" }
+ ua3_a0.a0[i] = 0; // { dg-bogus "\\\[-Warray-bounds" }
if (i > -1)
i = -1;
@@ -44,7 +44,7 @@ void test_ua3_ua0_a1 (int i)
if (i > -1)
i = -1;
- ua3_a0.a1[i] = 0; // { dg-warning "\\\[-Warray-bounds" }
+ ua3_a0.a1[i] = 0; // { dg-bogus "\\\[-Warray-bounds" }
if (i < 7)
i = 7;
@@ -60,7 +60,7 @@ void test_ua3_ua0_a2 (int i)
if (i < __LINE__)
i = __LINE__;
- ua3_a0.a2[i] = 0; // { dg-warning "\\\[-Warray-bounds" }
+ ua3_a0.a2[i] = 0; // { dg-bogus "\\\[-Warray-bounds" }
if (i > -1)
i = -1;
diff --git a/gcc/testsuite/gcc.dg/pr109071-1.c
b/gcc/testsuite/gcc.dg/pr109071-1.c
new file mode 100644
index 000000000000..a405c80bd549
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr109071-1.c
@@ -0,0 +1,22 @@
+/* PR tree-optimization/109071 -Warray-bounds false positive warnings
+ due to code duplication from jump threading
+ { dg-do compile }
+ { dg-options "-O2 -Warray-bounds=2" }
+ */
+
+extern void warn(void);
+static inline void assign(int val, int *regs, int index)
+{
+ if (index >= 4)
+ warn();
+ *regs = val;
+}
+struct nums {int vals[4];};
+
+void sparx5_set (int *ptr, struct nums *sg, int index)
+{
+ int *val = &sg->vals[index]; /* { dg-warning "is above array bounds" } */
+
+ assign(0, ptr, index);
+ assign(*val, ptr, index);
+}
diff --git a/gcc/testsuite/gcc.dg/pr109071.c b/gcc/testsuite/gcc.dg/pr109071.c
new file mode 100644
index 000000000000..782dfad84ea2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr109071.c
@@ -0,0 +1,22 @@
+/* PR tree-optimization/109071 -Warray-bounds false positive warnings
+ due to code duplication from jump threading
+ { dg-do compile }
+ { dg-options "-O2 -Wall" }
+ */
+
+extern void warn(void);
+static inline void assign(int val, int *regs, int index)
+{
+ if (index >= 4)
+ warn();
+ *regs = val;
+}
+struct nums {int vals[4];};
+
+void sparx5_set (int *ptr, struct nums *sg, int index)
+{
+ int *val = &sg->vals[index]; /* { dg-bogus "is above array bounds" } */
+
+ assign(0, ptr, index);
+ assign(*val, ptr, index);
+}
diff --git a/gcc/tree-ssa-threadupdate.cc b/gcc/tree-ssa-threadupdate.cc
index fa61ba9512b7..9f338dd4d54d 100644
--- a/gcc/tree-ssa-threadupdate.cc
+++ b/gcc/tree-ssa-threadupdate.cc
@@ -2371,6 +2371,17 @@ back_jt_path_registry::adjust_paths_after_duplication
(unsigned curr_path_num)
}
}
+/* Set all the stmts in the basic block BB as IS_SPLITTED. */
+
+static void
+set_stmts_in_bb_is_splitted (basic_block bb)
+{
+ gimple_stmt_iterator gsi;
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ gimple_set_is_splitted (gsi_stmt (gsi), true);
+ return;
+}
+
/* Duplicates a jump-thread path of N_REGION basic blocks.
The ENTRY edge is redirected to the duplicate of the region.
@@ -2418,6 +2429,10 @@ back_jt_path_registry::duplicate_thread_path (edge entry,
basic_block *region_copy = XNEWVEC (basic_block, n_region);
copy_bbs (region, n_region, region_copy, &exit, 1, &exit_copy, loop,
split_edge_bb_loc (entry), false);
+ /* Mark all the stmts in both original and copied basic blocks
+ as IS_SPLITTED. */
+ set_stmts_in_bb_is_splitted (*region);
+ set_stmts_in_bb_is_splitted (*region_copy);
/* Fix up: copy_bbs redirects all edges pointing to copied blocks. The
following code ensures that all the edges exiting the jump-thread path are
--
2.31.1
