On Tue, Jan 24, 2017 at 11:49 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> 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.
Just chiming in to say (again) this folding is bad and/or premature. A way
to avoid it for C++ would be to move it to gimplification? It won't
ever trigger
on GIMPLE. Not that I like gimplification specific "simplifications" too much.
Richard.
>
> Jakub
