PING 2 [PATCH] correct handling of variable offset minus constant in -Warray-bounds (PR 100137)
Marek Polacek
polacek@redhat.com
Thu Jul 8 01:48:47 GMT 2021
On Wed, Jul 07, 2021 at 02:38:11PM -0600, Martin Sebor via Gcc-patches wrote:
> On 7/7/21 1:38 AM, Richard Biener wrote:
> > On Tue, Jul 6, 2021 at 5:47 PM Martin Sebor via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> > >
> > > Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-June/573349.html
> >
> > + if (TREE_CODE (axstype) != UNION_TYPE)
> >
> > what about QUAL_UNION_TYPE? (why constrain union type accesses
> > here - note you don't seem to constrain accesses of union members here)
>
> I didn't know a QUAL_UNION_TYPE was a thing. Removing the test
> doesn't seem to cause any regressions so let me do that in a followup.
>
> >
> > + if (tree access_size = TYPE_SIZE_UNIT (axstype))
> >
> > + /* The byte size of the array has already been determined above
> > + based on a pointer ARG. Set ELTSIZE to the size of the type
> > + it points to and REFTYPE to the array with the size, rounded
> > + down as necessary. */
> > + if (POINTER_TYPE_P (reftype))
> > + reftype = TREE_TYPE (reftype);
> > + if (TREE_CODE (reftype) == ARRAY_TYPE)
> > + reftype = TREE_TYPE (reftype);
> > + if (tree refsize = TYPE_SIZE_UNIT (reftype))
> > + if (TREE_CODE (refsize) == INTEGER_CST)
> > + eltsize = wi::to_offset (refsize);
> >
> > probably pre-existing but the pointer indirection is definitely confusing
> > me again and again given the variable is named 'reftype' - obviously
> > an access to a pointer does not have any element size. Possibly the
> > paths arriving here ensure somehow that the only case is when
> > reftype is not the access type but a pointer to the accessed memory.
> > "jump-threading" the source might help me avoiding to trip over this
> > again and again ...
>
> I agree (it is confusing). There's more to simplify here. It's on
> my to do list so let me see about this piece of code then.
>
> >
> > The patch removes a lot of odd code, I like that. You know this code best
> > and it's hard to spot errors.
> >
> > So OK, you'll deal with the fallout.
>
> I certainly will. Pushed in r12-2132.
I think this patch breaks bootstrap on x86_64:
In member function ‘availability varpool_node::get_availability(symtab_node*)’,
inlined from ‘availability symtab_node::get_availability(symtab_node*)’ at /opt/notnfs/polacek/gcc/gcc/cgraph.h:3360:63,
inlined from ‘availability symtab_node::get_availability(symtab_node*)’ at /opt/notnfs/polacek/gcc/gcc/cgraph.h:3355:1,
inlined from ‘symtab_node* symtab_node::ultimate_alias_target(availability*, symtab_node*)’ at /opt/notnfs/polacek/gcc/gcc/cgraph.h:3199:35,
inlined from ‘symtab_node* symtab_node::ultimate_alias_target(availability*, symtab_node*)’ at /opt/notnfs/polacek/gcc/gcc/cgraph.h:3193:1,
inlined from ‘varpool_node* varpool_node::ultimate_alias_target(availability*, symtab_node*)’ at /opt/notnfs/polacek/gcc/gcc/cgraph.h:3234:5,
inlined from ‘availability varpool_node::_ZN12varpool_node16get_availabilityEP11symtab_node.part.0(symtab_node*)’ at /opt/notnfs/polacek/gcc/gcc/varpool.c:501:29:
/opt/notnfs/polacek/gcc/gcc/varpool.c:490:19: error: array subscript ‘varpool_node[0]’ is partly outside array bounds of ‘varpool_node [0]’ [-Werror=array-bounds]
490 | if (!definition && !in_other_partition)
| ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
In file included from /opt/notnfs/polacek/gcc/gcc/varpool.c:29:
/opt/notnfs/polacek/gcc/gcc/cgraph.h: In member function ‘availability varpool_node::_ZN12varpool_node16get_availabilityEP11symtab_node.part.0(symtab_node*)’:
/opt/notnfs/polacek/gcc/gcc/cgraph.h:1969:39: note: object ‘varpool_node::<anonymous>’ of size 120
1969 | struct GTY((tag ("SYMTAB_VARIABLE"))) varpool_node : public symtab_node
| ^~~~~~~~~~~~
cc1plus: all warnings being treated as errors
Marek
More information about the Gcc-patches
mailing list