On 2/17/21 3:12 AM, Jakub Jelinek via Gcc-patches wrote:
Hi!
check_mem_ref builds artificial arrays for variables that don't have
array type.
The C standard says:
"For the purposes of these operators, a pointer to an object that is not an
element of an
array behaves the same as a pointer to the first element of an array of length
one with the
type of the object as its element type."
so it isn't completely wrong and does simplify the function.
But, layout_type can fail if the size of the element type is not a multiple
of its alignment (i.e. overaligned types) and we then ICE because of that.
The following patch uses TYPE_MAIN_VARIANT in those cases instead, but only
for the types that need it, as for the diagnostics it is better to use the
typedef names etc. that were really used in the source if possible.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2021-02-17 Jakub Jelinek <ja...@redhat.com>
PR middle-end/99109
* gimple-array-bounds.cc (overaligned_type_p): New function.
(array_bounds_checker::check_mem_ref): For overaligned types use
TYPE_MAIN_VARIANT of reftype as element type of a new array.
* g++.dg/warn/Warray-bounds-17.C: New test.
--- gcc/gimple-array-bounds.cc.jj 2021-01-04 10:25:37.471249246 +0100
+++ gcc/gimple-array-bounds.cc 2021-02-16 17:00:41.961750114 +0100
@@ -386,6 +386,21 @@ build_zero_elt_array_type (tree eltype)
return arrtype;
}
+/* Return true if it is not possible to build an array with element type
+ ELTYPE, because either ELTYPE has alignment larger than its size
+ or its size is not a multiple of its alignment. */
+
+static bool
+overaligned_type_p (tree eltype)
+{
+ return (TYPE_SIZE_UNIT (eltype)
+ && TREE_CODE (TYPE_SIZE_UNIT (eltype)) == INTEGER_CST
+ && !integer_zerop (TYPE_SIZE_UNIT (eltype))
+ && TYPE_ALIGN_UNIT (eltype) > 1
+ && wi::zext (wi::to_wide (TYPE_SIZE_UNIT (eltype)),
+ ffs_hwi (TYPE_ALIGN_UNIT (eltype)) - 1) != 0);
+}
+
/* Checks one MEM_REF in REF, located at LOCATION, for out-of-bounds
references to string constants. If VRP can determine that the array
subscript is a constant, check if it is outside valid range.
@@ -553,6 +568,8 @@ array_bounds_checker::check_mem_ref (loc
reftype = TREE_TYPE (TREE_TYPE (arg));
if (TREE_CODE (reftype) == ARRAY_TYPE)
reftype = TREE_TYPE (reftype);
+ else if (overaligned_type_p (reftype))
+ reftype = TYPE_MAIN_VARIANT (reftype);
if (tree refsize = TYPE_SIZE_UNIT (reftype))
if (TREE_CODE (refsize) == INTEGER_CST)
eltsize = wi::to_offset (refsize);
@@ -675,7 +692,11 @@ array_bounds_checker::check_mem_ref (loc
/* Treat a reference to a non-array object as one to an array
of a single element. */
if (TREE_CODE (reftype) != ARRAY_TYPE)
- reftype = build_array_type_nelts (reftype, 1);
+ {
+ if (overaligned_type_p (reftype))
+ reftype = TYPE_MAIN_VARIANT (reftype);
+ reftype = build_array_type_nelts (reftype, 1);
+ }
Rather than complicating the logic in the caller (which is already
long and hard to follow) I'd suggest to consider changing
the build_zero_elt_array_type() helper to handle both kinds of arrays
(i.e., zero-length and otherwise), and strip the excess alignment
from reftype in it. That will simplify the function.
I recall considering this (using a single helper) but not why I didn't
do it to begin with.
Martin
/* Extract the element type out of MEM_REF and use its size
to compute the index to print in the diagnostic; arrays
--- gcc/testsuite/g++.dg/warn/Warray-bounds-17.C.jj 2021-02-16
17:24:14.178813304 +0100
+++ gcc/testsuite/g++.dg/warn/Warray-bounds-17.C 2021-02-16
17:23:35.305251062 +0100
@@ -0,0 +1,15 @@
+// PR middle-end/99109
+// { dg-do compile }
+// { dg-options "-O2 -Warray-bounds" }
+
+typedef int A __attribute__((aligned (64)));
+void foo (int *);
+
+void
+bar (void)
+{
+ A b; // { dg-message "while referencing" }
+ int *p = &b;
+ int *x = (p - 1); // { dg-warning "outside array bounds" }
+ foo (x);
+}
Jakub
