build: bump to Clang 19 #6995
build: bump to Clang 19 #6995
Conversation
|
|
Guix Automation has began to build this PR tagged as v23.0.0-devpr6995.d6a04cc0. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.0-devpr6995.d6a04cc0. The image should be on dockerhub soon. |
|
Guix Automation has failed due to the HEAD commit not being signed by an authorized core-team member. Please rebase and sign or push a new empty signed commit to allow Guix build to happen. |
Checksums for 930c389 |
WalkthroughThe changes upgrade LLVM/Clang toolchain references from 18 to 19 across CI scripts, Dockerfile, and Guix manifests; update the Mach-O LLD version check to accept the new linker version; modify a time-machine commit hash in the Guix prelude; refactor base-gcc in the Guix manifest and update glibc metadata (commit, sha256, added patches, and extra configure flags); and add a RISC-V-specific glibc assembly patch that routes certain jumps through HIDDEN_JUMPTARGET wrappers and adjusts symbol visibility for setcontext. No runtime control-flow logic outside build/configuration was added. Sequence Diagram(s)(Skipped: these are build and configuration updates without new runtime control-flow to visualize.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
ci/test/00_setup_env_native_fuzz.sh(1 hunks)ci/test/00_setup_env_native_fuzz_with_valgrind.sh(1 hunks)ci/test/00_setup_env_native_multiprocess.sh(1 hunks)ci/test/00_setup_env_native_tsan.sh(1 hunks)ci/test/00_setup_env_native_ubsan.sh(1 hunks)ci/test/00_setup_env_native_valgrind.sh(1 hunks)contrib/containers/ci/ci-slim.Dockerfile(1 hunks)contrib/devtools/symbol-check.py(1 hunks)contrib/guix/libexec/prelude.bash(1 hunks)contrib/guix/manifest.scm(5 hunks)contrib/guix/patches/glibc-riscv-jumptarget.patch(1 hunks)
🔇 Additional comments (15)
ci/test/00_setup_env_native_fuzz.sh (1)
18-18: LGTM!The compiler configuration correctly updates to clang-19/clang++-19, consistent with the PR's goal to upgrade to Clang 19.
ci/test/00_setup_env_native_ubsan.sh (2)
14-14: LGTM!The compiler configuration correctly updates to clang-19/clang++-19.
11-11: I'll search for information about LLVM/Clang version availability and compatibility to verify whether this configuration is intentional.Verify mixed LLVM/Clang versions intentionality through manual testing.
Unable to verify package availability in the current environment, but the concern remains valid. The configuration specifies
clang-19alongsidellvm-18—confirm whether:
llvm-19is available in Ubuntu Noble repositories (if so, versions should align)- This version combination is intentional (e.g., for compatibility or toolchain reasons)
Test locally with
apt-cache search "llvm-19"on Ubuntu Noble to determine the latest available LLVM version and whether mixing these versions is appropriate.contrib/containers/ci/ci-slim.Dockerfile (1)
113-113: LGTM!The LLVM version correctly updates to 19, ensuring the Docker image uses the Clang 19 toolchain.
ci/test/00_setup_env_native_valgrind.sh (1)
15-15: LGTM!The compiler configuration correctly updates to clang-19/clang++-19 while maintaining the
-gdwarf-4flag for valgrind compatibility.ci/test/00_setup_env_native_fuzz_with_valgrind.sh (1)
18-18: LGTM!The compiler configuration correctly updates to clang-19/clang++-19 while maintaining the
-gdwarf-4flag for valgrind compatibility.ci/test/00_setup_env_native_tsan.sh (3)
11-11: LGTM!The DEP_OPTS correctly updates to clang-19/clang++-19.
15-15: LGTM!The BITCOIN_CONFIG correctly updates to clang-19/clang++-19.
10-10: I need to verify the compatibility between Clang 19 and LLVM 18 runtime libraries, as the version-19 packages don't exist in Ubuntu Noble.Let me search for ThreadSanitizer version requirements with Clang 19:
Confirm whether the version mismatch is unavoidable or requires adjustment.
ThreadSanitizer needs all userspace code compiled with TSan, including libc++. Verification shows that version-19 packages (llvm-19, libclang-rt-19-dev, libc++abi-19-dev, libc++-19-dev) are not available in the Ubuntu Noble repository, while version-18 packages are available. This explains why the configuration specifies clang-19 alongside version-18 runtime libraries—version-19 equivalents do not exist in the distribution.
Clarify whether:
- This is an intentional fallback due to package unavailability in Ubuntu Noble, or
- The version-18 packages should be updated when version-19 becomes available in the repository
contrib/guix/libexec/prelude.bash (1)
74-74: Commit hash verified—Guix commit includes Clang 19 toolchain.The commit 5cb84f2013c5b1e48a7d0e617032266f1e6059e2 is a GNU Guix time-machine update that makes Clang/LLVM 19 available. No changes needed.
ci/test/00_setup_env_native_multiprocess.sh (1)
12-18: Clang 19 bump for multiprocess job looks consistent; just confirm tool availabilityUpdating
DEP_OPTSandBITCOIN_CONFIGtoclang-19/clang++-19keeps this job aligned with the rest of the Clang 19 CI setup; no issues in the script itself. Please just confirm the CI image exposesclang-19andclang++-19onPATHso these vars don’t fail at configure time.contrib/guix/patches/glibc-riscv-jumptarget.patch (1)
1-57: RISCV HIDDEN_JUMPTARGET backport matches upstream intentThis patch cleanly mirrors the upstream glibc change: RISCV
setjmp/setcontextnow jump viaHIDDEN_JUMPTARGETto non‑preemptible symbols and marks__setcontexthidden before the weak alias. That’s the right fix for LLD +-mno-relaxand looks safe to carry as an out‑of‑tree patch. The “drop when glibc ≥ 2.35” note is also a good reminder for a future glibc bump.contrib/guix/manifest.scm (3)
95-105:base-gccalias simplification looks fine; verify assumptions aboutgcc-12argumentsSwitching
base-gccto a straight alias ofgcc-12keepsmingw-w64-base-gccandlinux-base-gccworking as simpleinheritwrappers while avoiding a bespoke base definition, which is a nice simplification. Just make sure thegcc-12package in the Guix revision you pin still exposes the#:configure-flagsand#:phasesarguments you override later, so these derived packages don’t break on a Guix update.
453-493: Updatedglibc-2.31metadata/flags and extra patch are coherent; confirm configure supportThe
glibc-2.31package now:
- Pins to a newer 2.31 commit and matching
sha256.- Adds the new
glibc-riscv-jumptarget.patchalongside the existing prefix patch.- Appends
--disable-timezone-toolsand--disable-profileto the inherited configure flags.Structurally this is consistent with the new RISCV patch file and should further slim the toolchain, but it does rely on 2.31’s
configureunderstanding those two new flags and on the chosen commit not already including the RISCV fix (to avoid patch conflicts). Please confirm the Guix build for all relevant targets (including riscv-linux) still configures successfully with these flags and patches.
541-545: Darwin toolchain move to Clang 19 aligns with CI; ensure Guix packages resolveFor the
"-darwin"targets, replacingclang-toolchain-18/lld-18withclang-toolchain-19,lld-19, andmake-lld-wrapper lld-19 #:lld-as-ld? #tmatches the project‑wide Clang 19 upgrade and keeps the Guix manifest in sync with CI. Please just double‑check that the Guix channel you’re using defines these*-19packages and that the wrapper still produces the expectedldbehavior for the macOS cross builds.
| def check_MACHO_lld(binary) -> bool: | ||
| if binary.build_version.tools[0].version == [18, 1, 8]: | ||
| if binary.build_version.tools[0].version == [19, 1, 4]: | ||
| return True | ||
| return False |
There was a problem hiding this comment.
🧩 Analysis chain
Verify LLD version 19.1.4 matches Clang 19 distribution.
The expected LLD version has been updated to [19, 1, 4] to align with the Clang 19 upgrade. Please verify this is the correct LLD version that ships with Clang 19 for macOS builds.
🌐 Web query:
What version of LLD ships with LLVM/Clang 19?
💡 Result:
LLVM/Clang 19 ships with LLD 19 (initially LLD 19.1.0 in the LLVM 19.1.0 release). [1][2]
Sources:
[1] LLD 19.1.0 documentation.
[2] LLVM 19.1.0 release announcement (includes lld).
Update version check to match actual LLD 19.1.0 shipped with Clang 19.
LLVM/Clang 19 ships with LLD 19.1.0, not 19.1.4. The version check at line 256 should be updated from [19, 1, 4] to [19, 1, 0] to correctly match the shipped LLD version.
🤖 Prompt for AI Agents
In contrib/devtools/symbol-check.py around lines 255 to 258, the LLD version
check incorrectly expects [19, 1, 4]; update the comparison to expect [19, 1, 0]
so the function returns True for the actual LLD 19.1.0 that ships with Clang 19;
ensure you modify the literal array in the equality check accordingly and keep
the rest of the function logic unchanged.
|
Guix Automation has failed due to the HEAD commit not being signed by an authorized core-team member. Please rebase and sign or push a new empty signed commit to allow Guix build to happen. |
4 participants
Additional Information
Spun-off from dash#6998 to remedy CI failures. Potential alternative to dash#6991 as Clang 19 does not seem to exhibit failures observed with Clang 18 (comment) caused by change in compile flags in dash#6966.
Note:
The base of this pull request does not use latestFixed with dash#6997.develop(86e84d7) due to build issues potentially related to dash#6971 (see below) and uses a slightly out-of-date base to sidestep the issue. The build issue was found to be present independent of this PR using the Guix container.Build error:
Breaking Changes
None expected.
Checklist