Force full memory barriers for atomics on MSVC/ARM64 (aarch64) - #49
Open
gburd wants to merge 1 commit into
Open
Conversation
PostgreSQL's MSVC atomics in src/include/port/atomics/generic-msvc.h
rely on _ReadWriteBarrier() (a compiler barrier only, emitting no
instruction) for the compare-exchange primitives, and let read_u64 /
write_u64 fall through to generic.h's plain volatile access. On x86/x64
that is correct because the ISA is strongly ordered (TSO).
On ARM64 (aarch64), which is weakly ordered, it is not:
* a plain volatile 64-bit load/store is not an acquire/release; and
* MSVC may satisfy an interlocked operation with an out-of-line helper
that, on ARMv8.1 targets, uses an LSE "casal" instruction. "casal"
gives acquire/release ordering for that one location but is NOT a
full fence -- it is weaker than the "dmb ish" that PostgreSQL's
barrier macros (MemoryBarrier() -> dmb) are expected to provide.
Code that assumes full-fence semantics can then misbehave on Windows
ARM64. This was reported to Microsoft as an MSVC code-generation issue
(Developer Community feedback 10982137, "MSVC compiler optimization in
PostgreSQL's XLogInsertRecord() function leads to incorrect runtime
behaviour on Windows ARM64"); it is not yet fixed in MSVC. It is
separately visible on an assert-enabled MSVC/aarch64 buildfarm animal as
a BufferLockAcquire() lockmode assertion under the recovery TAP tests.
Rather than depend on the intrinsics' implicit (and, per the above,
insufficient) ordering, force an explicit full data memory barrier,
__dmb(_ARM64_BARRIER_ISH), on the atomic read, write and compare-exchange
paths on ARM64. This matches the manual-__dmb workaround demonstrated in
the reduced test case shared with Microsoft. The directly-defined
exchange/fetch_add helpers are left undefined on ARM64 so generic.h
derives them from the fenced compare-exchange. All x86/x64 code paths
are unchanged (everything is behind PG_MSVC_ARM64_FENCE).
|
pg-history: Bedrock call failed: An error occurred (ServiceUnavailableException) when calling the Converse operation (reached max retries: 4): Bedrock is unable to process your request. |
|
gburd
force-pushed
the
master
branch
4 times, most recently
from
July 29, 2026 06:34
fcdfa2a to
70421e4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On ARM64, MSVC may satisfy interlocked/barrier intrinsics with an out-of-line helper that uses LSE
casal— which is acquire/release for one location, NOT the full fence (dmb ish) PostgreSQL's barrier macros are expected to provide (per Microsoft's own analysis in MSVC Developer Community feedback 10982137)._ReadWriteBarrier()is compiler-only, and a plain volatile 64-bit load is not an acquire on this weakly ordered ISA.Observed as
Assert(entry->data.lockmode == BUFFER_LOCK_UNLOCK)failures in BufferLockAcquire() on an assert-enabled MSVC/aarch64 buildfarm animal, and separately as incorrect XLogInsertRecord() behavior (the MSVC feedback case).Fix: on
_M_ARM64only, force explicit__dmb(_ARM64_BARRIER_ISH)on the atomic read/write/compare-exchange paths; leave exchange/fetch_add undefined there so generic.h derives them from the fenced CAS. x86/x64 provably unchanged (all behindPG_MSVC_ARM64_FENCE).Stress-tested on aarch64 Windows 11 (MSVC cl 14.44, cassert): full build (2111/2111), concurrent buffer-lock + VACUUM stress with no assertion/crash/dumps.
Intended for pgsql-hackers submission (cc MSVC team); PR opened to run CI.