From e59582b6f8f1be3e675866f6a5d661eb4c8ed448 Mon Sep 17 00:00:00 2001 From: Schrodinger ZHU Yifan Date: Mon, 18 Nov 2024 16:04:41 -0500 Subject: [PATCH] [libc] avoid type-punning with inactive union member (#116685) --- libc/fuzzing/__support/hashtable_fuzz.cpp | 16 +++++++-------- .../HashTable/generic/bitmask_impl.inc | 11 +++++----- libc/src/__support/hash.h | 20 +++++++++---------- .../src/__support/HashTable/group_test.cpp | 15 +++++++------- .../src/__support/HashTable/table_test.cpp | 2 +- 5 files changed, 30 insertions(+), 34 deletions(-) diff --git a/libc/fuzzing/__support/hashtable_fuzz.cpp b/libc/fuzzing/__support/hashtable_fuzz.cpp index 7d61e106c9c4..8ab5e3b55cfd 100644 --- a/libc/fuzzing/__support/hashtable_fuzz.cpp +++ b/libc/fuzzing/__support/hashtable_fuzz.cpp @@ -10,6 +10,7 @@ /// //===----------------------------------------------------------------------===// #include "include/llvm-libc-types/ENTRY.h" +#include "src/__support/CPP/bit.h" #include "src/__support/CPP/string_view.h" #include "src/__support/HashTable/table.h" #include "src/__support/macros/config.h" @@ -81,15 +82,14 @@ static struct { template T next() { static_assert(cpp::is_integral::value, "T must be an integral type"); - union { - T result; - char data[sizeof(T)]; - }; - for (size_t i = 0; i < sizeof(result); i++) + + char data[sizeof(T)]; + + for (size_t i = 0; i < sizeof(T); i++) data[i] = buffer[i]; - buffer += sizeof(result); - remaining -= sizeof(result); - return result; + buffer += sizeof(T); + remaining -= sizeof(T); + return cpp::bit_cast(data); } cpp::string_view next_string() { diff --git a/libc/src/__support/HashTable/generic/bitmask_impl.inc b/libc/src/__support/HashTable/generic/bitmask_impl.inc index 469ddeeed8a8..d526dc1ece29 100644 --- a/libc/src/__support/HashTable/generic/bitmask_impl.inc +++ b/libc/src/__support/HashTable/generic/bitmask_impl.inc @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "src/__support/CPP/bit.h" #include "src/__support/common.h" #include "src/__support/endian_internal.h" #include "src/__support/macros/config.h" @@ -44,13 +45,11 @@ struct Group { // Load a group of control words from an arbitary address. LIBC_INLINE static Group load(const void *addr) { - union { - bitmask_t value; - char bytes[sizeof(bitmask_t)]; - } data; + char bytes[sizeof(bitmask_t)]; + for (size_t i = 0; i < sizeof(bitmask_t); ++i) - data.bytes[i] = static_cast(addr)[i]; - return {data.value}; + bytes[i] = static_cast(addr)[i]; + return Group{cpp::bit_cast(bytes)}; } // Load a group of control words from an aligned address. diff --git a/libc/src/__support/hash.h b/libc/src/__support/hash.h index 527c83993fd5..49138b1f43b9 100644 --- a/libc/src/__support/hash.h +++ b/libc/src/__support/hash.h @@ -13,8 +13,8 @@ #include "src/__support/CPP/limits.h" // numeric_limits #include "src/__support/macros/attributes.h" // LIBC_INLINE #include "src/__support/macros/config.h" -#include "src/__support/uint128.h" // UInt128 -#include // For uint64_t +#include "src/__support/uint128.h" // UInt128 +#include // For uint64_t namespace LIBC_NAMESPACE_DECL { namespace internal { @@ -34,25 +34,23 @@ LIBC_INLINE uint64_t folded_multiply(uint64_t x, uint64_t y) { // Therefore, we use a union to read the value. template LIBC_INLINE T read_little_endian(const void *ptr) { const uint8_t *bytes = static_cast(ptr); - union { - T value; - uint8_t buffer[sizeof(T)]; - } data; + uint8_t buffer[sizeof(T)]; #if __BYTE_ORDER__ != __ORDER_LITTLE_ENDIAN__ - // Compiler should able to optimize this as a load followed by a byte swap. - // On aarch64 (-mbig-endian), this compiles to the following for int: + // Compiler should able to optimize this as a load followed by a byte + // swap. On aarch64 (-mbig-endian), this compiles to the following for + // int: // ldr w0, [x0] // rev w0, w0 // ret for (size_t i = 0; i < sizeof(T); ++i) { - data.buffer[i] = bytes[sizeof(T) - i - 1]; + buffer[i] = bytes[sizeof(T) - i - 1]; } #else for (size_t i = 0; i < sizeof(T); ++i) { - data.buffer[i] = bytes[i]; + buffer[i] = bytes[i]; } #endif - return data.value; + return cpp::bit_cast(buffer); } // Specialized read functions for small values. size must be <= 8. diff --git a/libc/test/src/__support/HashTable/group_test.cpp b/libc/test/src/__support/HashTable/group_test.cpp index 25b15312ad66..acdc58e20585 100644 --- a/libc/test/src/__support/HashTable/group_test.cpp +++ b/libc/test/src/__support/HashTable/group_test.cpp @@ -8,6 +8,7 @@ #include "src/__support/HashTable/bitmask.h" +#include "src/__support/CPP/bit.h" #include "src/__support/macros/config.h" #include "src/stdlib/rand.h" #include "test/UnitTest/Test.h" @@ -28,14 +29,13 @@ TEST(LlvmLibcHashTableBitMaskTest, Match) { size_t appearance[4][sizeof(Group)]; ByteArray array{}; - union { - uintptr_t random; - int data[sizeof(uintptr_t) / sizeof(int)]; - }; + int data[sizeof(uintptr_t) / sizeof(int)]; for (int &i : data) i = rand(); + uintptr_t random = cpp::bit_cast(data); + for (size_t i = 0; i < sizeof(Group); ++i) { size_t choice = random % 4; random /= 4; @@ -62,14 +62,13 @@ TEST(LlvmLibcHashTableBitMaskTest, MaskAvailable) { for (size_t i = 0; i < sizeof(Group); ++i) { ByteArray array{}; - union { - uintptr_t random; - int data[sizeof(uintptr_t) / sizeof(int)]; - }; + int data[sizeof(uintptr_t) / sizeof(int)]; for (int &j : data) j = rand(); + uintptr_t random = cpp::bit_cast(data); + ASSERT_FALSE(Group::load(array.data).mask_available().any_bit_set()); array.data[i] = 0x80; diff --git a/libc/test/src/__support/HashTable/table_test.cpp b/libc/test/src/__support/HashTable/table_test.cpp index f8ffa4d4123d..c3b8697f2087 100644 --- a/libc/test/src/__support/HashTable/table_test.cpp +++ b/libc/test/src/__support/HashTable/table_test.cpp @@ -82,7 +82,7 @@ TEST(LlvmLibcTableTest, GrowthSequence) { } TEST(LlvmLibcTableTest, Insertion) { - union key { + struct key { char bytes[2]; } keys[256]; for (size_t k = 0; k < 256; ++k) {