DMTCP requires __atomic_compare_exchange_16 ; clang++ wasn't finding it - #1242
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds Autoconf checks to detect 128-bit atomic support by probing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@configure`:
- Around line 12325-12416: The configure probe that tries -latomic should add
the flag to LIBS (so it appears after object files) instead of putting it into
LDFLAGS: save and restore the original LDFLAGS (e.g. save into
ac_func_search_save_LDFLAGS), set LIBS="-latomic $ac_func_search_save_LIBS" for
the probe (remove any assignment to LDFLAGS), run the existing ac_fn_c_try_link
probe that sets ac_cv_search___atomic_compare_exchange_16, then restore both
LIBS and LDFLAGS from the saved variables; this keeps the later logic that
inspects gccAtomicBuiltins and ac_cv_search___atomic_compare_exchange_16
correct.
In `@configure.ac`:
- Around line 939-958: The libatomic probe (AC_SEARCH_LIBS for
__atomic_compare_exchange_16) must be moved into or reconciled with the C++
probe so the C++ decision uses its result: either run AC_SEARCH_LIBS inside the
same C++ link test that sets gccAtomicBuiltins (or immediately rerun the C++
AC_LINK_IFELSE/AC_LINK_IFELSE-equivalent after AC_SEARCH_LIBS) and update the
variable used to indicate 128-bit atomic support (gccAtomicBuiltins or a new
flag) so that AC_DEFINE([HAS_128_ATOMIC],[1]) is emitted when either the gcc
builtins or libatomic check succeeds; also change the clang-specific failure
check (the if that tests gccAtomicBuiltins and is_clang_cpp) to test the
updated/combined result so the "Missing Linux packages" error only fires if
neither probe found 128-bit atomic support.
Comment on lines
+939
to
+958
| dnl __atomic_compare_exchange_16 is often in libatomic | ||
| AC_SEARCH_LIBS([__atomic_compare_exchange_16], [atomic], [], [ | ||
| AC_MSG_WARN([Could not find 128-bit atomic support in libatomic.]) | ||
| ]) | ||
|
|
||
| if test "$gccAtomicBuiltins" = "yes"; then | ||
| AC_DEFINE([HAS_128_ATOMIC],[1],[Define to 1 if 'gcc test.c -latomic' works for __atomic_compare_exchange]) | ||
| fi | ||
| dnl $CXX might be "clang++ -std=c++14". Further, we need "-x c++" | ||
| dnl or else clang++ defaults to assuming a C file. | ||
| is_clang_cpp=no | ||
| $CXX -x c++ -dM -E - < /dev/null | grep -q "__clang__" && is_clang_cpp=yes | ||
| if test "$gccAtomicBuiltins" = "no" -a "$is_clang_cpp" = "yes"; then | ||
| AC_MSG_FAILURE([Using C++14 with clang++ to support atomic built-ins. | ||
| Missing Linux packages. If Ubuntu/Debian, consider: | ||
| sudo apt install clang libclang-rt-dev libc++abi-dev | ||
| libstdc++-14-dev (or later version of libstdc++-14-dev). | ||
| If Red Hat/Fedora/CentOS/Rocky Linux, consider: | ||
| sudo dnf install clang-devel libstdc++-devel libatomic]) | ||
| fi |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Atomic capability block:"
nl -ba configure.ac | sed -n '903,958p'
echo
echo "Assignments/references for gccAtomicBuiltins and the new libatomic search:"
rg -n -C2 'gccAtomicBuiltins|AC_SEARCH_LIBS\(\[__atomic_compare_exchange_16\]' configure.acRepository: dmtcp/dmtcp
Length of output: 139
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Atomic capability block (lines 903-958) ==="
sed -n '903,958p' configure.ac
echo
echo "=== All gccAtomicBuiltins assignments ==="
rg -n 'gccAtomicBuiltins=' configure.ac
echo
echo "=== AC_SEARCH_LIBS for atomic ==="
rg -n -A3 'AC_SEARCH_LIBS.*atomic' configure.ac
echo
echo "=== Lines 940-958 in detail ==="
sed -n '940,958p' configure.acRepository: dmtcp/dmtcp
Length of output: 3844
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check rwlock.cpp for atomic usage ==="
sed -n '1,62p' src/rwlock.cpp | head -40
echo
echo "=== Search for __atomic_compare_exchange usage across codebase ==="
rg -n '__atomic_compare_exchange' --type cpp src/
echo
echo "=== Check if HAS_128_ATOMIC guards exist in source files ==="
rg -n 'HAS_128_ATOMIC' src/Repository: dmtcp/dmtcp
Length of output: 1609
Move the libatomic probe into the actual C++ decision path.
Lines 944 and 951 still branch only on gccAtomicBuiltins, but that variable is assigned by the earlier C++ AC_LINK_IFELSE and never updated by the new AC_SEARCH_LIBS on Line 940. So this added probe cannot recover a failed atomic check: HAS_128_ATOMIC stays unset and the clang-specific "Missing Linux packages" failure still fires even if the later search finds the library. That configure verdict is load-bearing because src/rwlock.cpp uses __atomic_compare_exchange unconditionally. Please run the libatomic search inside the same C++ probe, or rerun the C++ link test after the search and base the later failure path on that result instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@configure.ac` around lines 939 - 958, The libatomic probe (AC_SEARCH_LIBS for
__atomic_compare_exchange_16) must be moved into or reconciled with the C++
probe so the C++ decision uses its result: either run AC_SEARCH_LIBS inside the
same C++ link test that sets gccAtomicBuiltins (or immediately rerun the C++
AC_LINK_IFELSE/AC_LINK_IFELSE-equivalent after AC_SEARCH_LIBS) and update the
variable used to indicate 128-bit atomic support (gccAtomicBuiltins or a new
flag) so that AC_DEFINE([HAS_128_ATOMIC],[1]) is emitted when either the gcc
builtins or libatomic check succeeds; also change the clang-specific failure
check (the if that tests gccAtomicBuiltins and is_clang_cpp) to test the
updated/combined result so the "Missing Linux packages" error only fires if
neither probe found 128-bit atomic support.
karya0
approved these changes
Mar 15, 2026
gc00
force-pushed
the
fix-clang-configure
branch
from
March 26, 2026 23:52
901885e to
0013902
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
configure.ac (1)
937-941:⚠️ Potential issue | 🟠 Major
AC_SEARCH_LIBSresult is not used to update the decision variable.The
AC_SEARCH_LIBSprobe cannot recover a failed atomic check:gccAtomicBuiltinsis set by the earlier C++AC_LINK_IFELSE(lines 911-934) and is never updated by this search. Even ifAC_SEARCH_LIBSfindslibatomicand adds it toLIBS,gccAtomicBuiltinsremainsno, so:
HAS_128_ATOMICis not defined (line 946)- The clang-specific failure (lines 952-959) still fires
src/rwlock.cppandjalib/jalloc.cppuse__atomic_compare_exchangeunconditionally whenHAS_128_ATOMICis definedConsider either:
- Running
AC_SEARCH_LIBSbefore the C++ link test and letting that test pick up-latomicfromLIBS, or- Re-running the C++ link test after
AC_SEARCH_LIBSsucceeds and updatinggccAtomicBuiltinsbased on the combined result
,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@configure.ac` around lines 937 - 941, The AC_SEARCH_LIBS probe currently runs after the C++ link test and its success is not used to update the decision variable gccAtomicBuiltins, so HAS_128_ATOMIC may remain unset even when -latomic is available; fix by either moving the AC_SEARCH_LIBS([__atomic_compare_exchange_16], [atomic]) call to run before the C++ AC_LINK_IFELSE so the link test can pick up -latomic from LIBS, or if you keep the current order, re-run the C++ link test (the same check that sets gccAtomicBuiltins) after AC_SEARCH_LIBS succeeds and explicitly set gccAtomicBuiltins to yes when the combined link succeeds so that HAS_128_ATOMIC and the clang-specific checks reflect the actual available libraries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@configure.ac`:
- Around line 948-959: Update the failure message emitted when test
"$gccAtomicBuiltins" = "no" and "$is_clang_cpp" = "yes": include Debian/Ubuntu's
libatomic package name (libatomic1) in the apt install suggestions alongside
clang, libclang-rt-dev, libc++abi-dev and libstdc++-14-dev; also clarify that
this path is triggered by the gccAtomicBuiltins predicate (and that
AC_SEARCH_LIBS may not have updated it), so either adjust the message to mention
checking that libatomic1 is installed or modify the surrounding logic that sets
gccAtomicBuiltins to ensure it reflects the result of the prior
AC_SEARCH_LIBS/link test before printing the AC_MSG_FAILURE (referencing
is_clang_cpp, gccAtomicBuiltins and the AC_MSG_FAILURE block).
---
Duplicate comments:
In `@configure.ac`:
- Around line 937-941: The AC_SEARCH_LIBS probe currently runs after the C++
link test and its success is not used to update the decision variable
gccAtomicBuiltins, so HAS_128_ATOMIC may remain unset even when -latomic is
available; fix by either moving the
AC_SEARCH_LIBS([__atomic_compare_exchange_16], [atomic]) call to run before the
C++ AC_LINK_IFELSE so the link test can pick up -latomic from LIBS, or if you
keep the current order, re-run the C++ link test (the same check that sets
gccAtomicBuiltins) after AC_SEARCH_LIBS succeeds and explicitly set
gccAtomicBuiltins to yes when the combined link succeeds so that HAS_128_ATOMIC
and the clang-specific checks reflect the actual available libraries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff610a15-1a01-4811-9c63-6b54fcc5fb21
📒 Files selected for processing (3)
configureconfigure.acinclude/config.h.in
✅ Files skipped from review due to trivial changes (1)
- include/config.h.in
🚧 Files skipped from review as they are similar to previous changes (1)
- configure
Comment on lines
+948
to
+959
| dnl $CXX might be "clang++ -std=c++14". Further, we need "-x c++" | ||
| dnl or else clang++ defaults to assuming a C file. | ||
| is_clang_cpp=no | ||
| $CXX -x c++ -dM -E - < /dev/null | grep -q "__clang__" && is_clang_cpp=yes | ||
| if test "$gccAtomicBuiltins" = "no" -a "$is_clang_cpp" = "yes"; then | ||
| AC_MSG_FAILURE([Using C++14 with clang++ to support atomic built-ins. | ||
| Missing Linux packages. If Ubuntu/Debian, consider: | ||
| sudo apt install clang libclang-rt-dev libc++abi-dev | ||
| libstdc++-14-dev (or later version of libstdc++-14-dev). | ||
| If Red Hat/Fedora/CentOS/Rocky Linux, consider: | ||
| sudo dnf install clang-devel libstdc++-devel libatomic]) | ||
| fi |
Contributor
There was a problem hiding this comment.
Debian/Ubuntu package list missing libatomic1.
The error message recommends libatomic for Red Hat/Fedora but omits it for Debian/Ubuntu. On Debian/Ubuntu, the package providing libatomic.so is libatomic1. Since this PR addresses clang++ not finding libatomic, consider adding it:
🛠️ Suggested fix
if test "$gccAtomicBuiltins" = "no" -a "$is_clang_cpp" = "yes"; then
AC_MSG_FAILURE([Using C++14 with clang++ to support atomic built-ins.
Missing Linux packages. If Ubuntu/Debian, consider:
- sudo apt install clang libclang-rt-dev libc++abi-dev
+ sudo apt install clang libclang-rt-dev libc++abi-dev libatomic1
libstdc++-14-dev (or later version of libstdc++-14-dev).
If Red Hat/Fedora/CentOS/Rocky Linux, consider:
sudo dnf install clang-devel libstdc++-devel libatomic])
fiAdditionally, note that this failure path depends on gccAtomicBuiltins, which is not updated by the preceding AC_SEARCH_LIBS check—so users may see this error even if libatomic is installed but the C++ link test failed for other reasons.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| dnl $CXX might be "clang++ -std=c++14". Further, we need "-x c++" | |
| dnl or else clang++ defaults to assuming a C file. | |
| is_clang_cpp=no | |
| $CXX -x c++ -dM -E - < /dev/null | grep -q "__clang__" && is_clang_cpp=yes | |
| if test "$gccAtomicBuiltins" = "no" -a "$is_clang_cpp" = "yes"; then | |
| AC_MSG_FAILURE([Using C++14 with clang++ to support atomic built-ins. | |
| Missing Linux packages. If Ubuntu/Debian, consider: | |
| sudo apt install clang libclang-rt-dev libc++abi-dev | |
| libstdc++-14-dev (or later version of libstdc++-14-dev). | |
| If Red Hat/Fedora/CentOS/Rocky Linux, consider: | |
| sudo dnf install clang-devel libstdc++-devel libatomic]) | |
| fi | |
| dnl $CXX might be "clang++ -std=c++14". Further, we need "-x c++" | |
| dnl or else clang++ defaults to assuming a C file. | |
| is_clang_cpp=no | |
| $CXX -x c++ -dM -E - < /dev/null | grep -q "__clang__" && is_clang_cpp=yes | |
| if test "$gccAtomicBuiltins" = "no" -a "$is_clang_cpp" = "yes"; then | |
| AC_MSG_FAILURE([Using C++14 with clang++ to support atomic built-ins. | |
| Missing Linux packages. If Ubuntu/Debian, consider: | |
| sudo apt install clang libclang-rt-dev libc++abi-dev libatomic1 | |
| libstdc++-14-dev (or later version of libstdc++-14-dev). | |
| If Red Hat/Fedora/CentOS/Rocky Linux, consider: | |
| sudo dnf install clang-devel libstdc++-devel libatomic]) | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@configure.ac` around lines 948 - 959, Update the failure message emitted when
test "$gccAtomicBuiltins" = "no" and "$is_clang_cpp" = "yes": include
Debian/Ubuntu's libatomic package name (libatomic1) in the apt install
suggestions alongside clang, libclang-rt-dev, libc++abi-dev and
libstdc++-14-dev; also clarify that this path is triggered by the
gccAtomicBuiltins predicate (and that AC_SEARCH_LIBS may not have updated it),
so either adjust the message to mention checking that libatomic1 is installed or
modify the surrounding logic that sets gccAtomicBuiltins to ensure it reflects
the result of the prior AC_SEARCH_LIBS/link test before printing the
AC_MSG_FAILURE (referencing is_clang_cpp, gccAtomicBuiltins and the
AC_MSG_FAILURE block).
* In clang++, it wasn't finding the libatomic to support that.
This commit provides recommendations for the user about
which Linux distro packages to load, to support libatomic in clang++.
gc00
force-pushed
the
fix-clang-configure
branch
from
March 27, 2026 00:00
0013902 to
dbbbf2f
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
configure (1)
12325-12416:⚠️ Potential issue | 🔴 CriticalStale
gccAtomicBuiltinsgate can still trigger a false configure failure.Line 12388 can successfully add
-latomictoLIBS, butHAS_128_ATOMICand the clang hard-fail at Line 12406 still rely ongccAtomicBuiltinscomputed earlier with-latomicinLDFLAGS. Because link tests place$LDFLAGSbefore objects, this can remain a false negative and abort even when the later library search succeeds.🔧 Proposed fix (apply around the earlier C++ builtin probe near Line 12277)
-# It seems to be undocumented that this needs '-latomic' as of 2024. -LDFLAGS_orig="$LDFLAGS" -LDFLAGS="$LDFLAGS -latomic" +# Keep test libs in LIBS so they appear after objects in link commands. +LIBS_orig="$LIBS" +LIBS="-latomic $LIBS" @@ -LDFLAGS="$LDFLAGS_orig" +LIBS="$LIBS_orig"#!/bin/bash set -euo pipefail # Verify link-command ordering and stale gating flow. rg -n -C3 'ac_link=.*\$LDFLAGS conftest\.\$ac_ext \$LIBS|LDFLAGS="\$LDFLAGS -latomic"|gccAtomicBuiltins=|ac_cv_search___atomic_compare_exchange_16|if test "\$gccAtomicBuiltins" = "no" -a "\$is_clang_cpp" = "yes"' configureExpected result: confirms
gccAtomicBuiltinsis set before the new search block and still gates the clang error path, with the first probe usingLDFLAGSfor-latomic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@configure` around lines 12325 - 12416, The configure script can false-fail because the earlier gccAtomicBuiltins probe is evaluated before the libatomic search; update the gating so the clang hard-fail uses the result of the later library probe (ac_cv_search___atomic_compare_exchange_16 or the generated HAS_128_ATOMIC) rather than the stale gccAtomicBuiltins, or recompute gccAtomicBuiltins after trying to add -latomic to LIBS/LDFLAGS; specifically, adjust the logic around the symbols gccAtomicBuiltins, ac_cv_search___atomic_compare_exchange_16, HAS_128_ATOMIC, is_clang_cpp, LIBS and LDFLAGS so that the clang error path checks the post-library-search state (e.g., test ac_cv_search___atomic_compare_exchange_16 or HAS_128_ATOMIC before erroring) or rerun the C++ builtin probe after modifying LDFLAGS.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@configure`:
- Around line 12325-12416: The configure script can false-fail because the
earlier gccAtomicBuiltins probe is evaluated before the libatomic search; update
the gating so the clang hard-fail uses the result of the later library probe
(ac_cv_search___atomic_compare_exchange_16 or the generated HAS_128_ATOMIC)
rather than the stale gccAtomicBuiltins, or recompute gccAtomicBuiltins after
trying to add -latomic to LIBS/LDFLAGS; specifically, adjust the logic around
the symbols gccAtomicBuiltins, ac_cv_search___atomic_compare_exchange_16,
HAS_128_ATOMIC, is_clang_cpp, LIBS and LDFLAGS so that the clang error path
checks the post-library-search state (e.g., test
ac_cv_search___atomic_compare_exchange_16 or HAS_128_ATOMIC before erroring) or
rerun the C++ builtin probe after modifying LDFLAGS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3b024b34-5b86-4b75-953f-9cfb82c14ce0
📒 Files selected for processing (3)
configureconfigure.acinclude/config.h.in
✅ Files skipped from review due to trivial changes (1)
- include/config.h.in
🚧 Files skipped from review as they are similar to previous changes (1)
- configure.ac
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
2 participants
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.
In clang++, it wasn't finding the libatomic to support that. This commit provides recommendations for the user about which Linux distro packages to load, to support libatomic in clang++.
Summary by CodeRabbit
Chores
Documentation