Re: [PATCH] bring -Warray-bounds closer to -Wstringop-overflow (PR91647, 91463, 91679)

[Martin Sebor <msebor@gmail.com>]

Just a heads up that I tested the patch with Glibc and the kernel.
It exposes some of the same "abuses" of (near) zero-length arrays
as the most recent improvement in this area.  In glibc, it
complains about code in fileops.c, iofwide.c, libc-tls.c, and
rtld.c.  The ones I looked at all look like the last one we saw.
I'll look into how to deal with them next.  In the kernel it
issues a variety of warnings that I need to investigate after
I get back from Cauldron.

On 9/6/19 1:27 PM, Martin Sebor wrote:
> Recent enhancements to -Wstringop-overflow improved the warning
> to the point that it detects a superset of the problems -Warray-
> bounds is intended detect in character accesses.  Because both
> warnings detect overlapping sets of problems, and because the IL
> they work with tends to change in subtle ways from target to
> targer, tests designed to verify one or the other sometimes fail
> with a target where the warning isn't robust enough to detect
> the problem given the IL representation.
> 
> To reduce these test suite failures the attached patch extends
> -Warray-bounds to handle some of the same problems -Wstringop-
> overflow does, pecifically, out-of-bounds accesses to array
> members of structs, including zero-length arrays and flexible
> array members of defined objects.
> 
> In the process of testing the enhancement I realized that
> the recently added component_size() function doesn't work as
> intended for non-character array members (see below).  The patch
> corrects this by reverting back to the original implementation
> of the function until the better/simpler solution can be put in
> place as mentioned below.
> 
> Tested on x86_64-linux.
> 
> Martin
> 
> 
> [*] component_size() happens to work for char arrays because those
> are transformed to STRING_CSTs, but not for arrays that are not.
> E.g., in
> 
>    struct S { int64_t i; int16_t j; int16_t a[]; }
>      s = { 0, 0, { 1, 0 } };
> 
> unless called with type set to int16_t[2], fold_ctor_reference
> will return s.a[0] rather than all of s.a.  But set type to
> int16_t[2] we would need to know that s.a's initializer has two
> elements, and that's just what we're using fold_ctor_reference
> to find out.
> 
> I think this could probably be made to work somehow by extending
> useless_type_conversion_p to handle this case as special somehow,
> but it doesn't seem worth the effort given that there should be
> an easier way to do it as you noted below.
> 
> Given the above, the long term solution should be to rely on
> DECL_SIZE_UNIT(decl) - TYPE_SIZE_UNIT(decl_type) as Richard
> suggested in the review of its initial implementation.
> Unfortunately, because of bugs in both the C and C++ front ends
> (I just opened PR 65403 with the details) the simple formula
> doesn't give the right answers either.  So until the bugs are
> fixed, the patch reverts back to the original loopy solution.
> It's no more costly than the current fold_ctor_reference
> approach.