Skip to content

Commit dccfdee

Browse files
vtjnashKristofferC
authored andcommitted
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. (cherry picked from commit bbd491a)
1 parent 915075f commit dccfdee

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)