Skip to content

Commit e7a8d21

Browse files
htpivfacebook-github-bot
authored andcommitted
Rewrite CompactValue to avoid undefined behavior from the use of a union for type-punning (#1154)
Summary: C++ does not, pedantically, allow the use of unions for type-punning in the way that C does. Most compilers, in practice, do support it; however, recent versions of MSVC appear to have a bug that cause bad code to be generated due to this U.B. (see: https://developercommunity.visualstudio.com/t/Bad-code-generated-for-std::isnan-compil/10082631). This led to a series of issues in the react-native-windows project, see: * microsoft/react-native-windows#4122 * microsoft/react-native-windows#8675 In C++20, the `<bit>` header and `bit_cast` function provide a pleasant API for type-punning. Since C++20 is not universally available, if the feature-test macro for `bit_cast` is not defined, memcpy is used instead. X-link: facebook/yoga#1154 Reviewed By: Andrey-Mishanin Differential Revision: D38082048 Pulled By: rozele fbshipit-source-id: a5da08cfb7d4296c725fb44871c55dbb12dc71e5
1 parent 246c747 commit e7a8d21

File tree

1 file changed

+49
-29
lines changed

1 file changed

+49
-29
lines changed

ReactCommon/yoga/yoga/CompactValue.h

Lines changed: 49 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@
99

1010
#ifdef __cplusplus
1111

12+
#ifdef __cpp_lib_bit_cast
13+
#include <bit>
14+
#else
15+
#include <cstring>
16+
#endif
1217
#include "YGValue.h"
1318
#include "YGMacros.h"
1419
#include <cmath>
@@ -55,7 +60,7 @@ class YOGA_EXPORT CompactValue {
5560
if (value == 0.0f || (value < LOWER_BOUND && value > -LOWER_BOUND)) {
5661
constexpr auto zero =
5762
Unit == YGUnitPercent ? ZERO_BITS_PERCENT : ZERO_BITS_POINT;
58-
return {Payload{zero}};
63+
return {zero};
5964
}
6065

6166
constexpr auto upperBound =
@@ -65,9 +70,9 @@ class YOGA_EXPORT CompactValue {
6570
}
6671

6772
uint32_t unitBit = Unit == YGUnitPercent ? PERCENT_BIT : 0;
68-
auto data = Payload{value};
69-
data.repr -= BIAS;
70-
data.repr |= unitBit;
73+
auto data = asU32(value);
74+
data -= BIAS;
75+
data |= unitBit;
7176
return {data};
7277
}
7378

@@ -78,21 +83,20 @@ class YOGA_EXPORT CompactValue {
7883
}
7984

8085
static constexpr CompactValue ofZero() noexcept {
81-
return CompactValue{Payload{ZERO_BITS_POINT}};
86+
return CompactValue{ZERO_BITS_POINT};
8287
}
8388

8489
static constexpr CompactValue ofUndefined() noexcept {
8590
return CompactValue{};
8691
}
8792

8893
static constexpr CompactValue ofAuto() noexcept {
89-
return CompactValue{Payload{AUTO_BITS}};
94+
return CompactValue{AUTO_BITS};
9095
}
9196

92-
constexpr CompactValue() noexcept
93-
: payload_(std::numeric_limits<float>::quiet_NaN()) {}
97+
constexpr CompactValue() noexcept : repr_(0x7FC00000) {}
9498

95-
CompactValue(const YGValue& x) noexcept : payload_(uint32_t{0}) {
99+
CompactValue(const YGValue& x) noexcept : repr_(uint32_t{0}) {
96100
switch (x.unit) {
97101
case YGUnitUndefined:
98102
*this = ofUndefined();
@@ -110,7 +114,7 @@ class YOGA_EXPORT CompactValue {
110114
}
111115

112116
operator YGValue() const noexcept {
113-
switch (payload_.repr) {
117+
switch (repr_) {
114118
case AUTO_BITS:
115119
return YGValueAuto;
116120
case ZERO_BITS_POINT:
@@ -119,34 +123,28 @@ class YOGA_EXPORT CompactValue {
119123
return YGValue{0.0f, YGUnitPercent};
120124
}
121125

122-
if (std::isnan(payload_.value)) {
126+
if (std::isnan(asFloat(repr_))) {
123127
return YGValueUndefined;
124128
}
125129

126-
auto data = payload_;
127-
data.repr &= ~PERCENT_BIT;
128-
data.repr += BIAS;
130+
auto data = repr_;
131+
data &= ~PERCENT_BIT;
132+
data += BIAS;
129133

130134
return YGValue{
131-
data.value, payload_.repr & 0x40000000 ? YGUnitPercent : YGUnitPoint};
135+
asFloat(data), repr_ & 0x40000000 ? YGUnitPercent : YGUnitPoint};
132136
}
133137

134138
bool isUndefined() const noexcept {
135139
return (
136-
payload_.repr != AUTO_BITS && payload_.repr != ZERO_BITS_POINT &&
137-
payload_.repr != ZERO_BITS_PERCENT && std::isnan(payload_.value));
140+
repr_ != AUTO_BITS && repr_ != ZERO_BITS_POINT &&
141+
repr_ != ZERO_BITS_PERCENT && std::isnan(asFloat(repr_)));
138142
}
139143

140-
bool isAuto() const noexcept { return payload_.repr == AUTO_BITS; }
144+
bool isAuto() const noexcept { return repr_ == AUTO_BITS; }
141145

142146
private:
143-
union Payload {
144-
float value;
145-
uint32_t repr;
146-
Payload() = delete;
147-
constexpr Payload(uint32_t r) : repr(r) {}
148-
constexpr Payload(float v) : value(v) {}
149-
};
147+
uint32_t repr_;
150148

151149
static constexpr uint32_t BIAS = 0x20000000;
152150
static constexpr uint32_t PERCENT_BIT = 0x40000000;
@@ -157,11 +155,33 @@ class YOGA_EXPORT CompactValue {
157155
static constexpr uint32_t ZERO_BITS_POINT = 0x7f8f0f0f;
158156
static constexpr uint32_t ZERO_BITS_PERCENT = 0x7f80f0f0;
159157

160-
constexpr CompactValue(Payload data) noexcept : payload_(data) {}
158+
constexpr CompactValue(uint32_t data) noexcept : repr_(data) {}
161159

162-
Payload payload_;
160+
VISIBLE_FOR_TESTING uint32_t repr() { return repr_; }
163161

164-
VISIBLE_FOR_TESTING uint32_t repr() { return payload_.repr; }
162+
static uint32_t asU32(float f) {
163+
#ifdef __cpp_lib_bit_cast
164+
return std::bit_cast<uint32_t>(f);
165+
#else
166+
uint32_t u;
167+
static_assert(
168+
sizeof(u) == sizeof(f), "uint32_t and float must have the same size");
169+
std::memcpy(&u, &f, sizeof(f));
170+
return u;
171+
#endif
172+
}
173+
174+
static float asFloat(uint32_t u) {
175+
#ifdef __cpp_lib_bit_cast
176+
return std::bit_cast<float>(u);
177+
#else
178+
float f;
179+
static_assert(
180+
sizeof(f) == sizeof(u), "uint32_t and float must have the same size");
181+
std::memcpy(&f, &u, sizeof(u));
182+
return f;
183+
#endif
184+
}
165185
};
166186

167187
template <>
@@ -174,7 +194,7 @@ template <>
174194
CompactValue CompactValue::ofMaybe<YGUnitAuto>(float) noexcept = delete;
175195

176196
constexpr bool operator==(CompactValue a, CompactValue b) noexcept {
177-
return a.payload_.repr == b.payload_.repr;
197+
return a.repr_ == b.repr_;
178198
}
179199

180200
constexpr bool operator!=(CompactValue a, CompactValue b) noexcept {

0 commit comments

Comments
 (0)