fix: avoid mixing '__sync_*' and '__atomic_*' built-ins on ref-counted pointers
When operating on reference counted pointers that take up a double-word, `refcounted_ptr_peek` was using a single-word read as an optimisation because it only needs the first word of the data. Meanwhile the other operations on reference counted pointers were using double-word compare-and-swap. On x86-64 with `-mcx16`, this results in near-optimal code: `CMPXCHG16B` for the double-word operations and `MOV` for the single-word read.¹ Similar for x86. Unfortunately on other platforms, this design can result in non-atomic operations. To understand why, note that the compiler has a number of options for lowering both the `__sync_*` built-ins and the `__atomic_*` built-ins. Two of these options are (1) an inline instruction sequence and (2) a call to a libatomic function. A constraint is that its choices must interoperate correctly. For example, lowering a 16-bit `__atomic_load` to a `MOV` and a 16-bit `__atomic_store` to a libatomic call (which is typically implemented with a per-instance mutex) would be incorrect. The following interleaving could occur, assuming X begins with the value 0x0: 1. Thread A calls `__atomic_store` on X 2. Thread A’s libatomic call takes a lock on X 3. Thread A writes 0xad into the first byte of X 4. Thread B loads both bytes of X in a single instruction 5. Thread A writes 0xde into the second byte of X 6. Thread A releases the lock on X It would have been valid for thread B to read either 0x0 (seeing the value before A’s store) or 0xdead (seeing the value after A’s store), but it instead saw a torn read of 0x00ad. Because the load and store do not agree on the protocol for synchronisation, atomicity can be violated. Crucially the compiler is only required to maintain this compatibility between the _same_ built-ins on the _same_ data type. The `__sync_*` built-ins and the `__atomic_*` built-ins are not required to use compatible protocols (and indeed they do not on x86-64 with `-mcx16`). And operations on a double-word type are not required to use a compatible protocol with operations on a single-word type (and indeed they do not on x86-64 _without_ `-mcx16`). The code in `refcounted_ptr_peek` was violating _both_ of these assumptions. On x86-64 without `-mcx16` it resulted in racy code, as it also did on ARM64. We could try to detect the narrow scenario wherein it is safe to mix built-ins because we know exactly which single instructions they will be lowered to (the ideal case on x86-64 described in the first paragraph), but this change conservatively switches to a double-word read which we know meets the compiler’s assumptions. ¹ `MOV` is atomic on naturally aligned 8-/16-/32-/64-bit data on x86-64.
parent
a9ec3b8d
Please register or sign in to comment