On Mon, Jan 23, 2017 at 08:49:43AM -0500, Nathan Sidwell wrote:
> This patch addresses 79118, where we ICE on a constexpr involving bitfields
> in an unnamed struct (unnamed struct members are a g++ extension).
>
> This is really a band-aid, because our internal representation BITFIELD_REF
> and the (premature) optimizations it encourages is incompatible with
> constexpr requirements. There's already signs of that with Jakub's code to
> deal with fold turning things like:
> int foo: 5;
> int baz: 3;
> ...
> ptr->baz == cst
> turns into the moral equivalent of (little endian example)
> ((*(unsigned char *)ptr & 0xe0) == (cst << 5)
>
> In this particular case we've also made the base object the containing
> class, not the unnamed struct member. That means we're looking in the wrong
> CONSTRUCTOR and see CONSTRUCTOR_NO_IMPLICIT_ZERO is true. Whereas for the
> subobj's CONSTRUCTOR it is false. With this patch we end up thinking this
> is an OK constexpr of value zero, whereas it's actually an uninitialized
> bitfield read.
>
> But I think we could already make that mistake given the above example &
> fold's behaviour. If 'ptr->foo' has been initialized, we'll construct a
> value using just that field and think we have a valid initialization of
> ptr->baz too.
>
> The equivalent testcase using non-bitfields has a base object of the unnamed
> struct member, and behaves correctly (given this is an extension anyway).
>
> The right solution is to fix the IR. In the C++ FE have BITFIELD_REF (or a
> new node) look much more like COMPONENT_REF (or even be COMPONENT_REF, but I
> suspect lots of places think ADDR (COMPONENT_REF (...)) is legit). And
> lower it to the middle-end generic representation in cp_genericize. I don't
> think this is the right time for a change of that magnitude though.
>
> Perhaps for now we should just always err on the side of safety, and behave
> as-if uninitialized if we fall of the end of looking for a bitfield?
Please note that say for:
struct S { int : 1; int a : 3; int : 1; int b : 2; };
int
foo (S a, S b)
{
return a.a == b.a && a.b == b.b;
}
(haven't tried to turn that into a constexpr testcase, but shouldn't be
hard), the folding creates
((BIT_FIELD_REF <a, 8, 0> ^ BIT_FIELD_REF <b, 8, 0>) & 110) == 0
out of that. So unless we DTRT (i.e. save constexpr bodies before
cp_fold for constexpr evaluation purposes), the workaround would need
to handle this properly (basically pattern recognize whatever the
folding may create for the bitfield tests and undo it or track bitwise which
bits are known to be constexpr and which bits are undefined and during
the BIT_AND_EXPR testing verify it isn't asking for any bits that aren't
constexpr). And that there can be multiple bitfields covered by the same
BIT_FIELD_REF.
Jakub
