Skip to content

Commit bbd491a

Browse files
authored
atomics: correctly implement padding write of 11 byte atomics with 4 byte pointer (#59395)
On 32 bit machines, for an atomic of size 9 to 11 bytes, the result fits in the 16 byte pool, but only with a maximum write of 12 bytes (there is 1 byte reserved for the `success` plus 4 for the type tag, leaving 11 bytes for the data). This was accidentally doing a 16 byte write instead, which could smash the type tag field (usually will NULL) of the next object. Not sure how to test, since just noticed this while reading the code.
1 parent 547f858 commit bbd491a

File tree

1 file changed

+15
-6
lines changed

1 file changed

+15
-6
lines changed

src/datatype.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,6 +1108,10 @@ static inline jl_uint128_t zext_read128(const jl_value_t *x, size_t nb) JL_NOTSA
11081108
memcpy(&y, x, nb);
11091109
return y;
11101110
}
1111+
static void assign_uint128(jl_value_t *v, jl_uint128_t x, size_t nb) JL_NOTSAFEPOINT
1112+
{
1113+
memcpy(v, &x, nb);
1114+
}
11111115
#endif
11121116

11131117
JL_DLLEXPORT jl_value_t *jl_new_bits(jl_value_t *dt, const void *data)
@@ -1173,6 +1177,8 @@ JL_DLLEXPORT jl_value_t *jl_atomic_new_bits(jl_value_t *dt, const char *data)
11731177
*(uint64_t*)v = jl_atomic_load((_Atomic(uint64_t)*)data);
11741178
#endif
11751179
#if MAX_POINTERATOMIC_SIZE >= 16
1180+
else if (nb <= 12)
1181+
assign_uint128(v, jl_atomic_load((_Atomic(jl_uint128_t)*)data), 12);
11761182
else if (nb <= 16)
11771183
*(jl_uint128_t*)v = jl_atomic_load((_Atomic(jl_uint128_t)*)data);
11781184
#endif
@@ -1239,6 +1245,8 @@ JL_DLLEXPORT jl_value_t *jl_atomic_swap_bits(jl_value_t *dt, char *dst, const jl
12391245
*(uint64_t*)v = jl_atomic_exchange((_Atomic(uint64_t)*)dst, zext_read64(src, nb));
12401246
#endif
12411247
#if MAX_POINTERATOMIC_SIZE >= 16
1248+
else if (nb <= 12)
1249+
assign_uint128(v, jl_atomic_exchange((_Atomic(jl_uint128_t)*)dst, zext_read128(src, nb)), 12);
12421250
else if (nb <= 16)
12431251
*(jl_uint128_t*)v = jl_atomic_exchange((_Atomic(jl_uint128_t)*)dst, zext_read128(src, nb));
12441252
#endif
@@ -1288,7 +1296,7 @@ JL_DLLEXPORT int jl_atomic_bool_cmpswap_bits(char *dst, const jl_value_t *expect
12881296
return success;
12891297
}
12901298

1291-
JL_DLLEXPORT int jl_atomic_cmpswap_bits(jl_datatype_t *dt, jl_value_t *y /* pre-allocated output */, char *dst, const jl_value_t *expected, const jl_value_t *src, int nb)
1299+
JL_DLLEXPORT int jl_atomic_cmpswap_bits(jl_datatype_t *dt, jl_value_t *y /* NEW pre-allocated output */, char *dst, const jl_value_t *expected, const jl_value_t *src, int nb)
12921300
{
12931301
// dst must have the required alignment for an atomic of the given size
12941302
// n.b.: this does not spuriously fail if there are padding bits
@@ -1359,18 +1367,19 @@ JL_DLLEXPORT int jl_atomic_cmpswap_bits(jl_datatype_t *dt, jl_value_t *y /* pre-
13591367
#endif
13601368
#if MAX_POINTERATOMIC_SIZE >= 16
13611369
else if (nb <= 16) {
1362-
jl_uint128_t *y128 = (jl_uint128_t*)y;
13631370
if (dt == et) {
1364-
*y128 = zext_read128(expected, nb);
1371+
jl_uint128_t y128 = zext_read128(expected, nb);
13651372
jl_uint128_t z128 = zext_read128(src, nb);
13661373
while (1) {
1367-
success = jl_atomic_cmpswap((_Atomic(jl_uint128_t)*)dst, y128, z128);
1368-
if (success || (dt->layout->flags.isbitsegal && !dt->layout->flags.haspadding) || !jl_egal__bits(y, expected, dt))
1374+
success = jl_atomic_cmpswap((_Atomic(jl_uint128_t)*)dst, &y128, z128);
1375+
assign_uint128(y, y128, nb);
1376+
if (success || (dt->layout->flags.isbitsegal && !dt->layout->flags.haspadding) || !jl_egal__bits(y, expected, dt)) {
13691377
break;
1378+
}
13701379
}
13711380
}
13721381
else {
1373-
*y128 = jl_atomic_load((_Atomic(jl_uint128_t)*)dst);
1382+
assign_uint128(y, jl_atomic_load((_Atomic(jl_uint128_t)*)dst), nb);
13741383
success = 0;
13751384
}
13761385
}

0 commit comments

Comments
 (0)