C++ Logo

std-proposals

Advanced search

Re: [std-proposals] Problematic integral promotions on bitfields

From: Johnston, Daniel <daniel.johnston_at_[hidden]>
Date: Fri, 2 Dec 2022 21:39:42 +0000
Thanks for the feedback.

> This is tangential, but reading through this reinterpret_cast is undefined behavior. You should use bit_cast or memcpy.

Thanks. We don't actually use that code, I was just trying to demonstrate what the actual value should be.

> 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.

FWIW, integer promotion isn't a "rule" for bitfields, it's an allowance. MSVC does not exhibit any of this behavior and works as expected (including the function overloading discussed below). It should be possible to remove this allowance for bitfields (only) and not break anything. GCC *used* to function as I would expect, and only started showing this behavior in version 10.

Or perhaps restrict the promotion to guarantee that the type is never *smaller* than the bitfield type, and never changes sign.

> I suppose the workaround is
>
> uint64_t shiftedValue = (uint64_t)f.Value << 6;

Yes, we found that this works. However, it seems extremely unintuitive and is only required in specific cases. It boggles my mind that these promotion rules could change the sign and cause sign-extension! In the end, we will probably move away from using bitfields all-together and roll our own bitfield classes.

> I wonder if this weird promotion rule also affects function overloading with bitfield arguments.

If I pass the bitfield in directly, function overloading selects the uint64_t version. If I pass "bitfield << 1" (or "bitfield + 1", etc.), it will resolve to the int version. :(

This seems to only apply to operators that support integer promotion (which is all of them?). This is my first foray into that part of the spec, and as-is to be expected, I'm left wondering how anything works. :)

-dan

-----Original Message-----
From: Std-Proposals <std-proposals-bounces_at_lists.isocpp.org> On Behalf Of Lénárd Szolnoki via Std-Proposals
Sent: Friday, December 2, 2022 1:03 PM
To: std-proposals_at_lists.isocpp.org
Cc: Lénárd Szolnoki <cpp_at_lenardszolnoki.com>
Subject: Re: [std-proposals] Problematic integral promotions on bitfields

Hi,

On 2 December 2022 20:04:08 GMT, "Johnston, Daniel via Std-Proposals" <std-proposals_at_lists.isocpp.org> 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.

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

Received on 2022-12-02 21:40:09