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