Date: Fri, 2 Dec 2022 16:59:58 -0500
On Fri, Dec 2, 2022 at 4:03 PM Lénárd Szolnoki via Std-Proposals <
std-proposals_at_[hidden]> wrote:
> Hi,
>
> On 2 December 2022 20:04:08 GMT, "Johnston, Daniel via Std-Proposals" <
> std-proposals_at_[hidden]> wrote:
> >Reference: https://github.com/llvm/llvm-project/issues/59306
> >
> >Short version, this code seems to be undefined behavior:
> >
> >//==============================================================
> >struct Foo
> >{
> > uint64_t Stuff : 6;
> > uint64_t Value: 27;
> > uint64_t OtherStuff: 31;
> >};
> >
> >Foo f;
> >f.Value = (uint64_t(1) << 27) - 1;
> >
> >// Caution UB here due to integral promotion of bitifields!
> >uint64_t shiftedValue = f.Value << 6;
> >uint64_t maskedValue = reinterpret_cast<uint64_t&>(f) & (((uint64_t(1) <<
> 27) - 1) << 6);
>
> This is tangential, but reading through this reinterpret_cast is undefined
> behavior. You should use bit_cast or memcpy.
>
> >assert(shiftedValue == maskedValue); // this may fail depending on
> compiler
> >//==============================================================
> >
> >In the above scenario, a bitwidth for Value of 32 or higher guarantees
> correct operation. A bitwidth of less than (32 - shift amount) works as
> well. A bitwidth that exactly matches (32 - shift amount) could *sign
> extend* the result even though it should be unsigned.
> >
> >This seems like a defect in the standard and has caused bugs for us when
> simulating HW that requires structures with these bit layouts.
> >
> >Can we fix the standard to disallow integral promotion of bitfields
> completely? I *always* want the type of the bitfield value to match what
> was specified.
> >
> >Thank you,
> >
> >-dan
>
> I agree that this integral promotion rule is pretty weird. Integral
> promotion is quite broken in general. But I suspect that changing promotion
> rules would just break too much code. Also this would need to be
> coordinated with the C standard as well, which probably has the same rule.
>
Well, the funny thing is that the C standard only specifies promotions of
bit-fields of type _Bool, int, signed int, and unsigned int. The
implementation may allow additional bit-field types, but they are not
supposed to be subject to promotion.
But in practice, Clang and GCC appear to use C++'s rules for bit-field
promotion even when compiling C code, in order to avoid incompatibilities.
In any case I agree that if we change the behaviour in C++, then the C
committee also needs to be involved.
>
> I suppose the workaround is
>
> uint64_t shiftedValue = (uint64_t)f.Value << 6;
>
> I wonder if this weird promotion rule also affects function overloading
> with bitfield arguments.
>
> Cheers,
> Lénárd
> --
> Std-Proposals mailing list
> Std-Proposals_at_[hidden]
> https://lists.isocpp.org/mailman/listinfo.cgi/std-proposals
>
std-proposals_at_[hidden]> wrote:
> Hi,
>
> On 2 December 2022 20:04:08 GMT, "Johnston, Daniel via Std-Proposals" <
> std-proposals_at_[hidden]> wrote:
> >Reference: https://github.com/llvm/llvm-project/issues/59306
> >
> >Short version, this code seems to be undefined behavior:
> >
> >//==============================================================
> >struct Foo
> >{
> > uint64_t Stuff : 6;
> > uint64_t Value: 27;
> > uint64_t OtherStuff: 31;
> >};
> >
> >Foo f;
> >f.Value = (uint64_t(1) << 27) - 1;
> >
> >// Caution UB here due to integral promotion of bitifields!
> >uint64_t shiftedValue = f.Value << 6;
> >uint64_t maskedValue = reinterpret_cast<uint64_t&>(f) & (((uint64_t(1) <<
> 27) - 1) << 6);
>
> This is tangential, but reading through this reinterpret_cast is undefined
> behavior. You should use bit_cast or memcpy.
>
> >assert(shiftedValue == maskedValue); // this may fail depending on
> compiler
> >//==============================================================
> >
> >In the above scenario, a bitwidth for Value of 32 or higher guarantees
> correct operation. A bitwidth of less than (32 - shift amount) works as
> well. A bitwidth that exactly matches (32 - shift amount) could *sign
> extend* the result even though it should be unsigned.
> >
> >This seems like a defect in the standard and has caused bugs for us when
> simulating HW that requires structures with these bit layouts.
> >
> >Can we fix the standard to disallow integral promotion of bitfields
> completely? I *always* want the type of the bitfield value to match what
> was specified.
> >
> >Thank you,
> >
> >-dan
>
> I agree that this integral promotion rule is pretty weird. Integral
> promotion is quite broken in general. But I suspect that changing promotion
> rules would just break too much code. Also this would need to be
> coordinated with the C standard as well, which probably has the same rule.
>
Well, the funny thing is that the C standard only specifies promotions of
bit-fields of type _Bool, int, signed int, and unsigned int. The
implementation may allow additional bit-field types, but they are not
supposed to be subject to promotion.
But in practice, Clang and GCC appear to use C++'s rules for bit-field
promotion even when compiling C code, in order to avoid incompatibilities.
In any case I agree that if we change the behaviour in C++, then the C
committee also needs to be involved.
>
> I suppose the workaround is
>
> uint64_t shiftedValue = (uint64_t)f.Value << 6;
>
> I wonder if this weird promotion rule also affects function overloading
> with bitfield arguments.
>
> Cheers,
> Lénárd
> --
> Std-Proposals mailing list
> Std-Proposals_at_[hidden]
> https://lists.isocpp.org/mailman/listinfo.cgi/std-proposals
>
-- *Brian Bi*
Received on 2022-12-02 22:00:12