On 10/11/19 10:34 AM, Martin Sebor wrote: > I've fixed the bug in the attached patch. The rest can be suppressed > by replacing the zero-length arrays with flexible array members but > that's just trading one misuse for another. If the code can't be > changed to avoid this (likely not an option since the arrays are in > visible in the public API) I think the best way to deal with them is > to suppress them by #pragma GCC diagnostic ignored. I opened BZ 25097 > in Glibc Bugzilla to track this. While it's trading one misuse for another, one could argue that a flexible array is less of a misuse :-)
> > The patch triggers 94 instances of -Warray-bounds (60 of which > are for distinct code) in 21 .c files. I haven't looked at all > of them but some of the patterns I noticed are: > > 1) Intentionally using an interior zero-length array to access > (e.g., memset) one or more subsequent members. E.g., > _dbgp_external_startup in drivers/usb/early/ehci-dbgp.c and > quite a few others. This is pretty pervasive but seems easily > avoidable. Yea, I saw something like this as well. I can't recall if it was in the kernel or a package that was reading structured data of some sort. THey'd use the zero length array to access the entire blob-o-data and subsequent fields when they wanted to access particular chunks of data that were at well known offsets from the start. > > 2) Overwriting a member array with more data (e.g., function > cxio_rdev_open in > drivers/infiniband/hw/cxgb3/cxio_hal.c or in function > pk_probe in drivers/hid/hid-prodikeys.c). At first glance > some of these look like bugs but with stuff obscured by macros > and no comments it's hard to tell. ACK. > > 3) Uses of the container_of() macro to access one member given > the address of another. This is undefined (and again breaks > the aliasing rules) but the macro is used all over the place > in the kernel. I count over 15,000 references to it. Ugh. > > 4) Uses of one-element arrays as members of other one-element > arrays (in include/scsi/fc/fc_ms.h). Was this ever meant > to be supported by GCC? (It isn't by _FORTIFY_SOURCE=2.) > > 5) Possible false positives due to the recent loop unrolling > change. I think I've worked around some #5 issues as well. It's got an aassociated BZ and hopefully will be addressed during stage3/stage4. > > It will be a quite a bit of work to clean this up. To make it > easier we would introduce a new option to control the warning > for some of the most common idioms, such as > -Wzero-length-array-bounds. I'm not too wild about this because > it would just paper over the problem. A better solution would > also involve avoiding the aliasing assumptions for overlapping > zero-length member arrays. > > Anyway, attached is the updated patch with just the one fix > I mentioned above, retested on x86_64-linux. I think the agreement in our meeting yesterday was to give some kind of knob, particularly for the kernel folks. > > Martin > > gcc-91647.diff > > PR middle-end/91679 - missing -Warray-bounds accessing a member array in a > local buffer > PR middle-end/91647 - new FAILs for Warray-bounds-8 and Wstringop-overflow-3.C > PR middle-end/91463 - missing -Warray-bounds accessing past the end of a > statically initialized flexible array member > > gcc/ChangeLog: > > PR middle-end/91679 > PR middle-end/91647 > PR middle-end/91463 > * tree-vrp.c (vrp_prop::check_array_ref): Handle trailing arrays with > initializers. > (vrp_prop::check_mem_ref): Handle declared struct objects. > * tree.c (last_field): New function. > (array_at_struct_end_p): Handle MEM_REF. > (get_initializer_for): New helper. > (component_ref_size): Rename locals. Call get_initializer_for instead > of fold_ctor_reference. Correct handling of flexible array members. > * wide-int.h (generic_wide_int <storage>::sign_mask): Assert invariant. > > gcc/testsuite/ChangeLog: > > PR middle-end/91679 > PR middle-end/91647 > PR middle-end/91463 > * c-c++-common/Warray-bounds-2.c: Disable VRP. Adjust expected > messages. > * gcc.dg/Warray-bounds-48.c: New test. > * gcc.dg/Warray-bounds-49.c: New test. > * gcc.dg/Wstringop-overflow-16.c: Adjust text of expected messages. > * gcc.dg/pr36902.c: Remove xfail. > * gcc.dg/strlenopt-57.c: Add an expected warning. So per our meeting yesterday, add a knob for the kernel folks to be able to control this and it's OK. jeff
