On Wed, Oct 18, 2017 at 5:34 AM, Martin Sebor <mse...@gmail.com> wrote:
> While testing my latest -Wrestrict changes I noticed a number of
> opportunities to improve the -Warray-bounds warning.  Attached
> is a patch that implements a solution for the following subset
> of these:
>
> PR tree-optimization/82596 - missing -Warray-bounds on an out-of
>   bounds index into string literal
> PR tree-optimization/82588 - missing -Warray-bounds on an excessively
>   large index
> PR tree-optimization/82583 - missing -Warray-bounds on out-of-bounds
>   inner indices
>
> The patch also adds more detail to the -Warray-bounds diagnostics
> to make it easier to see the cause of the problem.
>
> Richard, since the changes touch tree-vrp.c, I look in particular
> for your comments.

+      /* Accesses to trailing arrays via pointers may access storage
+        beyond the types array bounds.  For such arrays, or for flexible
+        array members as well as for other arrays of an unknown size,
+        replace the upper bound with a more permissive one that assumes
+        the size of the largest object is SSIZE_MAX.  */

I believe handling those cases are somewhat academic, but ...

+      tree eltype = TREE_TYPE (ref);
+      tree eltsize = TYPE_SIZE_UNIT (eltype);

needs to use array_ref_element_size.  Note that this size can be
non-constant in which case you now ICE so you have to check
this is an INTEGER_CST.

+      tree maxbound = TYPE_MAX_VALUE (ssizetype);

please don't use ssizetype - sizetype can be of different precision
than pointers (and thus objects).  size_type_node would come
close (maps to size_t), eventually ptrdiff_type_node is also
defined by all frontends.

+      up_bound_p1 = fold_build2 (TRUNC_DIV_EXPR, ssizetype, maxbound, eltsize);
+

int_const_binop if you insist on using trees...

+      tree arg = TREE_OPERAND (ref, 0);
+      tree_code code = TREE_CODE (arg);
+      if (code == COMPONENT_REF)
+       {
+         HOST_WIDE_INT off;
+         if (tree base = get_addr_base_and_unit_offset (ref, &off))
+           up_bound_p1 = fold_build2 (MINUS_EXPR, ssizetype, up_bound_p1,
+                                      TYPE_SIZE_UNIT (TREE_TYPE (base)));
+         else
+           return;

so this gives up on a.b[i].c.d[k] (ok, array_at_struct_end_p will be false).
simply not subtracting anyhing instead of returning would be conservatively
correct, no?  Likewise subtracting the offset of the array for all "previous"
variably indexed components with assuming the lowest value for the index.
But as above I think compensating for the offset of the array within the object
is academic ... ;)

+      else if (code == STRING_CST)
+       up_bound_p1 = build_int_cst (ssizetype, TREE_STRING_LENGTH (arg));

that one is more interesting -- why's the TYPE_DOMAIN of the STRING_CST lacking
a max value?  Iff we use build_string_literal it should have the proper type.

Thanks,
Richard.

>
> Thanks
> Martin

Reply via email to