From 7976e1852ce38b7d66b5255b4523036fe611a964 Mon Sep 17 00:00:00 2001 From: Ali Mohammad Pur Date: Wed, 1 Nov 2023 01:59:31 +0330 Subject: [PATCH] AK: Ensure unions with bitfield structs actually have correct sizes The fun pattern of `union { struct { u32 a : 1; u64 b : 7; }; u8 x; }` produces complete garbage on windows, this commit fixes the two instances of those that exist in AK. This commit also makes sure that the generated unions have the correct size (whereas FloatExtractor previously did not!) by adding some nice static_asserts. --- AK/FPControl.h | 2 +- AK/FloatingPoint.h | 36 +++++++++++++++++++++++------------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/AK/FPControl.h b/AK/FPControl.h index 6639338abac..b5cf575b1d2 100644 --- a/AK/FPControl.h +++ b/AK/FPControl.h @@ -13,7 +13,7 @@ VALIDATE_IS_X86(); namespace AK { -enum class RoundingMode : u8 { +enum class RoundingMode : u16 { NEAREST = 0b00, DOWN = 0b01, UP = 0b10, diff --git a/AK/FloatingPoint.h b/AK/FloatingPoint.h index 77ee165b2e7..1f3346dcfab 100644 --- a/AK/FloatingPoint.h +++ b/AK/FloatingPoint.h @@ -22,16 +22,16 @@ union FloatExtractor { static constexpr int exponent_bias = 16383; static constexpr int exponent_bits = 15; static constexpr unsigned exponent_max = 32767; - struct { + struct [[gnu::packed]] { unsigned __int128 mantissa : 112; - unsigned exponent : 15; - unsigned sign : 1; + unsigned __int128 exponent : 15; + unsigned __int128 sign : 1; }; f128 d; }; // Validate that f128 and the FloatExtractor union are 128 bits. -static_assert(sizeof(f128) == 16); -static_assert(sizeof(FloatExtractor) == 16); +static_assert(AssertSize()); +static_assert(AssertSize, sizeof(f128)>()); #endif #ifdef AK_HAS_FLOAT_80 @@ -42,13 +42,14 @@ union FloatExtractor { static constexpr int exponent_bias = 16383; static constexpr int exponent_bits = 15; static constexpr unsigned exponent_max = 32767; - struct { + struct [[gnu::packed]] { unsigned long long mantissa; - unsigned exponent : 15; - unsigned sign : 1; + unsigned long long exponent : 15; + unsigned long long sign : 1; }; f80 d; }; +static_assert(AssertSize, sizeof(f80)>()); #endif template<> @@ -58,13 +59,21 @@ union FloatExtractor { static constexpr int exponent_bias = 1023; static constexpr int exponent_bits = 11; static constexpr unsigned exponent_max = 2047; - struct { + struct [[gnu::packed]] { + // FIXME: These types have to all be the same, otherwise this struct + // goes from being a bitfield describing the layout of an f64 + // into being a multibyte mess on windows. + // Technically, '-mno-ms-bitfields' is supposed to disable this + // very intuitive and portable behaviour on windows, but it doesn't + // work with the msvc ABI. + // See unsigned long long mantissa : 52; - unsigned exponent : 11; - unsigned sign : 1; + unsigned long long exponent : 11; + unsigned long long sign : 1; }; f64 d; }; +static_assert(AssertSize, sizeof(f64)>()); template<> union FloatExtractor { @@ -73,13 +82,14 @@ union FloatExtractor { static constexpr int exponent_bias = 127; static constexpr int exponent_bits = 8; static constexpr unsigned exponent_max = 255; - struct { - unsigned long long mantissa : 23; + struct [[gnu::packed]] { + unsigned mantissa : 23; unsigned exponent : 8; unsigned sign : 1; }; f32 d; }; +static_assert(AssertSize, sizeof(f32)>()); template requires(S <= 1 && E >= 1 && M >= 1 && (S + E + M) <= 64) class FloatingPointBits final {