[PATCH] issue consistent warning for past-the-end array stores (PR 91457)

[Richard Biener]

On Sat, Aug 17, 2019 at 12:43 AM Martin Sebor <msebor@gmail.com> wrote:
>
> With the recent enhancement to the strlen handling of multibyte
> stores the g++.dg/warn/Warray-bounds-4.C for zero-length arrays
> started failing on hppa (and probably elsewhere as well).  This
> is partly the result of the added detection of past-the-end
> writes into the strlen pass which detects more instances of
> the problem than -Warray-bounds.  Since the IL each warning
> works with varies between targets, the same invalid code can
> be diagnosed by one warning one target and different warning
> on another.
>
> The attached patch does three things:
>
> 1) It enhances compute_objsize to also determine the size of
> a flexible array member (and its various variants), including
> from its initializer if necessary.  (This resolves 91457 but
> introduces another warning where was previously just one.)
> 2) It guards the new instance of -Wstringop-overflow with
> the no-warning bit on the assignment to avoid warning on code
> that's already been diagnosed.
> 3) It arranges for -Warray-bounds to set the no-warning bit on
> the enclosing expression to keep -Wstringop-overflow from issuing
> another warning for the same problem.
>
> Testing the compute_objsize enhancement to bring it up to par
> with -Warray-bounds in turn exposed a weakness in the latter
> warning for flexible array members.  Rather than snowballing
> additional improvements into this one I decided to put that
> off until later, so the new -Warray-bounds test has a bunch
> of XFAILs.  I'll see if I can find the time to deal with those
> either still in stage 1 or in stage 3 (one of them is actually
> an ancient regression).

+static tree
+get_initializer_for (tree init, tree decl)
+{

can't you use fold_ctor_reference here?

+/* Determine the size of the flexible array FLD from the initializer
+   expression for the struct object DECL in which the meber is declared
+   (possibly recursively).  Return the size or zero constant if it isn't
+   initialized.  */
+
+static tree
+get_flexarray_size (tree decl, tree fld)
+{
+  if (tree init = DECL_INITIAL (decl))
+    {
+      init = get_initializer_for (init, fld);
+      if (init)
+       return TYPE_SIZE_UNIT (TREE_TYPE (init));
+    }
+
+  return integer_zero_node;

so you're hoping that the (sub-)CONSTRUCTOR get_initializer_for
returns has a complete type but the initialized object didn't get it
completed.  Isnt that wishful thinking?  And why return integer_zero_node
rather than NULL_TREE here?

+  if (TREE_CODE (dest) == COMPONENT_REF)
+    {
+      *pdecl = TREE_OPERAND (dest, 1);
+
+      /* If the member has a size return it.  Otherwise it's a flexible
+        array member.  */
+      if (tree size = DECL_SIZE_UNIT (*pdecl))
+       return size;

because here you do.

Also once you have an underlying VAR_DECL you can compute
the flexarray size by DECL_SIZE (var) - offset-of flexarray member.
Isn't that way cheaper than walking the initializer (possibly many
times?)

Richard.

>
> Martin
>
> PS I imagine the new get_flexarray_size function will probably
> need to move somewhere more appropriate so that other warnings
> (like -Warray-bounds to remove the XFAILs) and optimizations
> can make use of it.