[PATCH] correct handling of indices into arrays with elements larger than 1 (PR c++/96511)

[Jason Merrill]

On 8/11/20 12:19 PM, Martin Sebor via Gcc-patches wrote:
> -Wplacement-new handles array indices and pointer offsets the same:
> by adjusting them by the size of the element.  That's correct for
> the latter but wrong for the former, causing false positives when
> the element size is greater than one.
> 
> In addition, the warning doesn't even attempt to handle arrays of
> arrays.  I'm not sure if I forgot or if I simply didn't think of
> it.
> 
> The attached patch corrects these oversights by replacing most
> of the -Wplacement-new code with a call to compute_objsize which
> handles all this correctly (plus more), and is also better tested.
> But even compute_objsize has bugs: it trips up while converting
> wide_int to offset_int for some pointer offset ranges.  Since
> handling the C++ IL required changes in this area the patch also
> fixes that.
> 
> For review purposes, the patch affects just the middle end.
> The C++ diff pretty much just removes code from the front end.

The C++ changes are OK.

> -compute_objsize (tree ptr, int ostype, access_ref *pref,
> -                bitmap *visited, const vr_values *rvals /* = NULL */)
> +compute_objsize (tree ptr, int ostype, access_ref *pref, bitmap *visited,
> +                const vr_values *rvals)

This reformatting seems unnecessary, and I prefer to keep the comment 
about the default argument.

> -      if (!size || TREE_CODE (size) != INTEGER_CST)
> -       return false;
 >...

You change some failure cases in compute_objsize to return success with 
a maximum range, while others continue to return failure.  This needs 
commentary about the design rationale.

> +  special_array_member sam{ };

sam is always set by component_ref_size, so I don't think it's necessary 
to initialize it at the declaration.

> @@ -187,7 +187,7 @@ decl_init_size (tree decl, bool min)
>    tree last_type = TREE_TYPE (last);
>    if (TREE_CODE (last_type) != ARRAY_TYPE
>        || TYPE_SIZE (last_type))
> -    return size;
> +    return size ? size : TYPE_SIZE_UNIT (type);

This change seems to violate the comment for the function.

Jason