[Bug middle-end/101062] [10/11/12 Regression] wrong code with "-O2 -fno-toplevel-reorder -frename-registers"

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062

--- Comment #9 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 15 Jun 2021, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101062
> 
> --- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #7)
> > Now, it looks to me this is rather an issue that the access is larger than
> > the object and thus a general bug - at least I don't see how it should only
> > manifest with bitfields in unions?
> > 
> > Note we do
> > 
> >       if (TREE_CODE (to) == COMPONENT_REF
> >           && DECL_BIT_FIELD_TYPE (TREE_OPERAND (to, 1)))
> >         get_bit_range (&bitregion_start, &bitregion_end, to, &bitpos,
> > &offset);
> >       /* The C++ memory model naturally applies to byte-aligned fields.
> >          However, if we do not have a DECL_BIT_FIELD_TYPE but BITPOS or
> >          BITSIZE are not byte-aligned, there is no need to limit the range
> >          we can access.  This can occur with packed structures in Ada.  */
> >       else if (maybe_gt (bitsize, 0)
> >                && multiple_p (bitsize, BITS_PER_UNIT)
> >                && multiple_p (bitpos, BITS_PER_UNIT))
> >         {
> >           bitregion_start = bitpos;
> >           bitregion_end = bitpos + bitsize - 1;
> >         }
> > 
> > but if we assume that for DECL_BIT_FIELD_TYPE there's a representative
> > then we miss the else if, so - maybe get_bit_range should return whether
> > it handled things or the else if part should be done unconditionally
> > in case bitregion_start/end is not {0,0}?
> 
> This wouldn't help us, bitsize is > 0, but not a multiple of BITS_PER_UNIT in
> this case.  Furthermore, even if we add there bitregion_start/end for the base
> variable if any as further fallthrough, I think most C/C++ programmers will
> expect that with
> union U { int a; int b : 5; } u[64];
> u[4].b = 1; can be done safely in one thread and
> u[5].a = 2; in another one.
> 
> My patch fixes that (or another possibility would be to compute the
> representative even in UNION_TYPE (no idea about QUAL_UNION_TYPE) - could be as
> simple as removing the early out and instead of doing prev = field; in the loop
> do if (TREE_CODE (t) != RECORD_TYPE) { finish_bitfield_representative (repr,
> field); repr = NULL_TREE; } else prev = field; and in
> finish_bitfield_representative override nextf to NULL_TREE).

Yes, as said - the caller of get_bit_range seems to expect it to always
handle cases that do not run into the byte-align case to make sure to
not cross to next objects.  I don't remember why I didn't handle
UNION_TYPE (I guess because it was about bitfield groups, not single
bitfields), so maybe we should indeed handle them.

There are also other callers of get_bit_range it seems, even on
!DECL_BIT_FIELD_TYPE fields.

So let's try handling [QUAL_]UNION_TYPE in finish_bitfield_layout?