Skip to content

Commit e5d47ed

Browse files
committed
Merge #18167: Fix a violation of C++ standard rules where unions are used for type-punning
0653939 Add static_asserts to ser_X_to_Y() methods (Samer Afach) be94096 Fix a violation of C++ standard rules that unions cannot be switched. (Samer Afach) Pull request description: Type punning in C++ is not like C. As per the C++ standard, one cannot use unions to convert the bit type. A discussion about this can be found [here](https://stackoverflow.com/questions/25664848/unions-and-type-punning). In C++, a union is supposed to only hold one type at a time. It's intended to be used only as `std::variant`. Switching types is undefined behavior. In fact, C++20 has a special casting function, called [`bit_cast`](https://en.cppreference.com/w/cpp/numeric/bit_cast) that solved this problem. Why has it been working so far? Because some compilers tolerate using unions and switching types, like gcc. More information [here](https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#Type-punning). One important thing to mention is that performance is generally not affected by that memcpy. Compilers are smart enough to convert that to a memory cast when possible. But we have to do it the right way, otherwise, it's jut undefined behavior that depends on the compiler. ACKs for top commit: practicalswift: ACK 0653939 elichai: ACK 0653939 laanwj: Code review ACK 0653939 kristapsk: ACK 0653939 Tree-SHA512: f6e89de39fc964750429139bab6b5a1346f7060334b7afa020e315bdad8f8c195bce2b8a9e343f06e7fff175e2dfb1cdabfcb6fe405bea0febe4962f0cc62557
2 parents 89a97a7 + 0653939 commit e5d47ed

File tree

1 file changed

+17
-12
lines changed

1 file changed

+17
-12
lines changed

src/serialize.h

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <compat/endian.h>
1010

1111
#include <algorithm>
12+
#include <cstring>
1213
#include <ios>
1314
#include <limits>
1415
#include <map>
@@ -139,27 +140,31 @@ template<typename Stream> inline uint64_t ser_readdata64(Stream &s)
139140
}
140141
inline uint64_t ser_double_to_uint64(double x)
141142
{
142-
union { double x; uint64_t y; } tmp;
143-
tmp.x = x;
144-
return tmp.y;
143+
uint64_t tmp;
144+
std::memcpy(&tmp, &x, sizeof(x));
145+
static_assert(sizeof(tmp) == sizeof(x), "double and uint64_t assumed to have the same size");
146+
return tmp;
145147
}
146148
inline uint32_t ser_float_to_uint32(float x)
147149
{
148-
union { float x; uint32_t y; } tmp;
149-
tmp.x = x;
150-
return tmp.y;
150+
uint32_t tmp;
151+
std::memcpy(&tmp, &x, sizeof(x));
152+
static_assert(sizeof(tmp) == sizeof(x), "float and uint32_t assumed to have the same size");
153+
return tmp;
151154
}
152155
inline double ser_uint64_to_double(uint64_t y)
153156
{
154-
union { double x; uint64_t y; } tmp;
155-
tmp.y = y;
156-
return tmp.x;
157+
double tmp;
158+
std::memcpy(&tmp, &y, sizeof(y));
159+
static_assert(sizeof(tmp) == sizeof(y), "double and uint64_t assumed to have the same size");
160+
return tmp;
157161
}
158162
inline float ser_uint32_to_float(uint32_t y)
159163
{
160-
union { float x; uint32_t y; } tmp;
161-
tmp.y = y;
162-
return tmp.x;
164+
float tmp;
165+
std::memcpy(&tmp, &y, sizeof(y));
166+
static_assert(sizeof(tmp) == sizeof(y), "float and uint32_t assumed to have the same size");
167+
return tmp;
163168
}
164169

165170

0 commit comments

Comments
 (0)