https://gcc.gnu.org/bugzilla/show_bug.cgi?id=127667
Bug ID: 127667
Summary: GCC `__atomic_exchange_n` crashes (SIGSEGV) on
8-byte-aligned 16-byte `_BitInt` objects
Product: gcc
Version: 17.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: bic60176 at gmail dot com
Target Milestone: ---
Summary
On x86-64, `_BitInt(N)` for `64 < N <= 128` is a 16-byte integral type whose
ABI
alignment is only 8. `__atomic_exchange_n` on such an object is expanded to a
call to libatomic's `__atomic_exchange_16`, whose x86-64 IFUNC implementation
executes `lock cmpxchg16b` without checking the pointer's alignment.
`cmpxchg16b` raises `#GP` on an operand that is not 16-byte aligned, so any
exchange on a correctly-aligned-but-not-16-aligned object dies with SIGSEGV.
All the other 16-byte `__atomic` operations (`__atomic_load_n`,
`__atomic_store_n`, `__atomic_compare_exchange_n`, `__atomic_fetch_*`,
`__atomic_exchange`) crash the same way; `__atomic_exchange_n` is just the
entry point this was found through.
Reproducing the Bug
No pointer casts or attributes are needed — an ordinary struct is enough,
because `struct S` has alignment 8 and `val` sits at offset 8.
```c
/* gcc -O2 repro.c -o repro && ./repro => Segmentation fault */
struct S { long lock; _BitInt(128) val; };
struct S s;
int main(void)
{
return (int)__atomic_exchange_n(&s.val, (_BitInt(128))1, __ATOMIC_SEQ_CST);
}
```
An array of such structs shows it is address-dependent — element 0 succeeds,
element 1 crashes:
```c
#include <stdio.h>
struct S { _BitInt(128) v; long tag; };
struct S a[3];
int main(void)
{
for (int i = 0; i < 3; i++) {
printf("a[%d].v %% 16 = %d\n", i, (int)((unsigned long)&a[i].v % 16));
fflush(stdout);
__atomic_exchange_n(&a[i].v, (_BitInt(128))(i + 1), __ATOMIC_SEQ_CST);
}
return 0;
}
```
```
a[0].v % 16 = 0
a[1].v % 16 = 8
Segmentation fault
```
That the fault is inside libatomic is confirmed by interposing the symbol —
with a local `__atomic_exchange_16` the same program runs to completion:
```c
#include <stdio.h>
__int128 __atomic_exchange_16(volatile void *mem, __int128 val, int model) {
puts("our __atomic_exchange_16 was called");
__int128 old = *(volatile __int128 *)mem;
*(volatile __int128 *)mem = val;
return old;
}
struct S { long lock; _BitInt(128) val; };
struct S s = { 0, 42 };
int main(void) {
_BitInt(128) old = __atomic_exchange_n(&s.val, (_BitInt(128))7,
__ATOMIC_SEQ_CST);
printf("old=%d val=%d\n", (int)old, (int)s.val); /* old=42 val=7 */
return 0;
}
```
Fix
Make the x86-64 16-byte fast path conditional on the pointer being 16-byte
aligned, and let unaligned addresses fall through to libatomic's existing
lock-based implementation. Because the branch is a pure function of the
address, all accesses to a given object consistently take the same path, so the
locking and lock-free variants never mix on one object.
The sketch below does that for `libat_exchange_16`; the same guard is needed in
`cas_n.c`, `load_n.c`, `store_n.c` and `fop_n.c` for `N == 16`, so in practice
it is better introduced as a shared `ALIGNED_16(ptr)` helper used by every
`#if N == 16 && IFUNC_ALT != 0` block.
```diff
--- a/libatomic/exch_n.c
+++ b/libatomic/exch_n.c
@@
#if !DONE && defined(atomic_compare_exchange_n)
UTYPE
SIZE(libat_exchange) (UTYPE *mptr, UTYPE newval, int smodel)
{
UTYPE oldval;
+ /* The inline compare-and-swap used here (e.g. cmpxchg16b on x86-64) faults
+ on an operand that is not naturally aligned, but a 16-byte type may be
+ only 8-byte aligned -- _BitInt(65..128) on x86-64 is. Use the protected
+ path for those. */
+ if (!libat_aligned_p (mptr, N))
+ return SIZE(libat_exchange_locked) (mptr, newval, smodel);
+
pre_barrier (smodel);
oldval = *mptr;
while (!atomic_compare_exchange_n (mptr, &oldval, newval, true,
__ATOMIC_RELAXED, __ATOMIC_RELAXED))
continue;
post_barrier (smodel);
return oldval;
}
#define DONE 1
#endif /* atomic_compare_exchange_n */
```
where `libat_aligned_p` is
```diff
--- a/libatomic/libatomic_i.h
+++ b/libatomic/libatomic_i.h
@@
+static inline bool
+libat_aligned_p (void *ptr, size_t n)
+{
+ return ((uintptr_t) ptr & (n - 1)) == 0;
+}
```
