[libunwind] Remove MSVC C11 stdalign.h and stdatomic.h fake headers#129246
Open
mdh1418 wants to merge 2 commits into
Open
[libunwind] Remove MSVC C11 stdalign.h and stdatomic.h fake headers#129246mdh1418 wants to merge 2 commits into
mdh1418 wants to merge 2 commits into
Conversation
The fake C11 headers under src/native/external/libunwind/include/remote/win/ were added in 2020 (dotnet#37521 "Add libunwind to cross DAC") as a workaround for MSVC's missing C11 stdalign.h and stdatomic.h support. Both have since become either redundant or replaceable with a compile flag: - stdalign.h: shipping in MSVC for years. check_include_files(stdalign.h ...) returns TRUE on every supported MSVC, so the fake-generation branch was already dead code that never ran. - stdatomic.h: still gated behind /experimental:c11atomics, but the flag has been stable since VS 2022 17.5 (2023). Enabling it on the libunwind MSVC compile lets the real <stdatomic.h> from MSVC be used directly. The atomic_compare_exchange_strong call in Gaddress_validator.c compiles against the real header. Changes: - Delete the fake-header configure blocks in libunwind_extras/configure.cmake - Delete fakestdalign.h.in and fakestdatomic.h.in templates - Add /experimental:c11atomics to MSVC compile options in libunwind_extras/CMakeLists.txt (gated on MSVC, not HOST_WIN32, so clang-cl is excluded since it does not accept the flag). The _Thread_local workaround block is intentionally left alone: its check_c_source_compiles probe is missing the C-standard-required `static` qualifier, so it would still fail on modern MSVC and the -D_Thread_local= fallback continues to be applied (libunwind on Windows is UNW_REMOTE_ONLY, so no actual thread-local storage is needed). Side benefit: this also structurally eliminates the cross-build stale-fake shadowing bug (dotnet#127814 territory) because no fake exists to shadow real NDK/Bionic headers in a mixed CrossDac + Android build environment. Validation (Windows host, MSVC 19.44.35211 / VS 18): - build.cmd -s linuxdac -c Release: SUCCEEDS (1:25, 0 errors, 0 warnings). mscordaccore.dll built; Gaddress_validator.c.obj compiled (192720 bytes). - build.cmd clr.runtime -os android -arch x64 -c checked -lc release: SUCCEEDS (6:49, 0 errors, 0 warnings). libcoreclr.so for Android built. - artifacts/obj/external/libunwind/include/ contains only the real generated headers (config.h, libunwind-common.h, libunwind.h, tdep/libunwind_i.h); no stdatomic.h, no stdalign.h, no msvc-shim/ subdir. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the dotnet/runtime vendored libunwind build to stop generating MSVC “fake” C11 headers (stdalign.h / stdatomic.h) and instead rely on real toolchain headers (with MSVC C11 atomics enabled via a compiler flag).
Changes:
- Remove the libunwind_extras CMake configure-time generation of fake
stdalign.h/stdatomic.h. - Delete the fake-header template files from
include/remote/win/. - Add MSVC’s
/experimental:c11atomicsoption to allow compiling against MSVC’s real<stdatomic.h>.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/native/external/libunwind/include/remote/win/fakestdatomic.h.in | Deletes the fake stdatomic.h template used by older MSVC workarounds. |
| src/native/external/libunwind/include/remote/win/fakestdalign.h.in | Deletes the fake stdalign.h template used by older MSVC workarounds. |
| src/native/external/libunwind_extras/configure.cmake | Removes fake-header generation logic from the libunwind_extras configure step. |
| src/native/external/libunwind_extras/CMakeLists.txt | Adds MSVC compile option to enable C11 atomics support. |
Comment on lines
+129
to
+135
| # Enable C11 atomics so libunwind's <stdatomic.h> usage | ||
| # (e.g. atomic_compare_exchange_strong in Gaddress_validator.c) compiles | ||
| # against MSVC's real header instead of a generated fake. | ||
| # /std:c11 is already implied by CMAKE_C_STANDARD=11 set in libunwind/CMakeLists.txt. | ||
| if(MSVC) | ||
| add_compile_options(/experimental:c11atomics) | ||
| endif() |
Comment on lines
5
to
9
| if(CLR_CMAKE_HOST_WIN32) | ||
| # Our posix abstraction layer will provide these headers | ||
| set(HAVE_ELF_H 1) | ||
| set(HAVE_ENDIAN_H 1) | ||
|
|
Two fixes for PR dotnet#129246 review (Copilot reviewer): 1. libunwind_extras/CMakeLists.txt: gate /experimental:c11atomics more precisely. CMake's MSVC variable means "MSVC-like command-line syntax" and is also TRUE for clang-cl, which does not accept the flag. Use `CMAKE_C_COMPILER_ID STREQUAL "MSVC"` so only real cl.exe gets it. Also scope the flag to C sources via `$<$<COMPILE_LANGUAGE:C>:...>` so it is not passed to C++ compilation (e.g. remote-unwind.cpp). 2. libunwind/CMakeLists.txt: remove the `if (NOT HAVE_STDALIGN_H)` and `if (NOT HAVE_STDATOMIC_H)` blocks that still referenced the deleted fakestdalign.h.in / fakestdatomic.h.in templates. The CoreCLR build enters libunwind via libunwind_extras/CMakeLists.txt and never reads libunwind/CMakeLists.txt, so the dangling references survived local CrossDac/Android/Windows validation; they would break anyone configuring libunwind directly from the repo. Validated: `build.cmd -s linuxdac -c Release` succeeds (1:19, 0 errors), Gaddress_validator.c.obj = 192720 bytes (byte-identical to prior successful builds, proves the c11atomics flag is in effect), no fake headers generated, no remaining fakestd* references anywhere in the repo. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
|
Please upstream libunwind changes to https://github.com/libunwind/libunwind/ and update libunwind-version.txt file with that PR's URL. |
jkotas
reviewed
Jun 10, 2026
| # (e.g. atomic_compare_exchange_strong in Gaddress_validator.c) compiles | ||
| # against MSVC's real header instead of a generated fake. | ||
| # /std:c11 is already implied by CMAKE_C_STANDARD=11 set in libunwind/CMakeLists.txt. | ||
| # CMAKE_C_COMPILER_ID is used instead of MSVC so that clang-cl (which sets |
Member
There was a problem hiding this comment.
We do not support clang-cl and I expect we would need number of changes to actually support it. This can be simplified to just use /experimental:c11atomics unconditionally.
Member
|
@mdh1418 This is goodness that would be great to get in. Let me know what you need to address the outstanding comments. |
Contributor
IIRC while the compiler handles C11 atomics, the IDE IntelliSense still breaks on them. |
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
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.
From #127814 (comment), this aims to address the root of
src/native/external/libunwind/src/mi/Gaddress_validator.c:251:11: error: call to undeclared function 'atomic_compare_exchange_strong'from building Android CoreCLR on Windows host.The fake C11 headers under src/native/external/libunwind/include/remote/win/ were added in 2020 (#37521 "Add libunwind to cross DAC") as a workaround for MSVC's missing C11 stdalign.h and stdatomic.h support. Both have since become either redundant or replaceable with a compile flag.
stdalign.h: shipping in MSVC for years. check_include_files(stdalign.h ...) returns TRUE on every supported MSVC, so the fake-generation branch was already dead code that never ran.
stdatomic.h: still gated behind /experimental:c11atomics, but the flag has been stable since VS 2022 17.5 (2023). Enabling it on the libunwind MSVC compile lets the real <stdatomic.h> from MSVC be used directly. The atomic_compare_exchange_strong call in Gaddress_validator.c compiles against the real header.
Changes:
Validation (Windows host, MSVC 19.44.35211 / VS 18):