ci: link the MSan lane against an MSan-instrumented libc++ - #568
Open
Krilliac wants to merge 1 commit into
Open
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
🔍 CodeQL ReportLanguages:
|
| Rule | Languages | Location | Message |
|---|---|---|---|
cpp/integer\-multiplication\-cast\-to\-long |
c-cpp | ThirdParty/Utils/json/nlohmann\_json\.h:629 |
Multiplication result may overflow 'int' before it is converted to 'size_type'. |
cpp/integer\-multiplication\-cast\-to\-long |
c-cpp | ThirdParty/Utils/json/nlohmann\_json\.h:630 |
Multiplication result may overflow 'int' before it is converted to 'size_type'. |
Analyzed commit: 8a0cfc0b23302519d71a8d8f3987a6dce4c72451
Updated: 2026-09-07T00:37:48.032Z — this comment is updated in-place.
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Selected-subsystem coverage: 53.6% (55571/103601 lines) LCOV corpus coverage: 54.1% (69366/128180 lines); 24579 instrumented lines are outside the threshold table. |
Contributor
❌ CI Error ReportFailed jobs: linux-asan, linux-tsan, windows-vs2022, aggregate-test-stats Build ErrorsOther errors (4)Test Failures
Compiler Warnings (25)Updated: 2026-09-07T01:25:01Z — this comment is updated in-place, not duplicated. |
build-linux-msan compiled the engine with -fsanitize=memory but linked the distribution's libc++, which is not instrumented, so every std::string written inside libc++.so.1 read as uninitialized: 732 reports in the first 25 tests, a 16 MiB sanitizer log that hit the wrapper's file-size limit, SparkTests killed by SIGXFSZ with no completion evidence, and a lane red on Working for months at 18-26 runner minutes per push. The job now fetches the LLVM 18.1.3 source release matching the runner's clang-18 (size, SHA-256 and SHA-512 pinned), builds libc++/libc++abi with LLVM_USE_SANITIZER=MemoryWithOrigins from the checked-in recipe .github/msan/libcxx-runtime.cmake, caches the install prefix through split actions/cache restore/save keyed by recipe hash, LLVM tag and compiler package version, and verifies the prefix on every run before it is used or saved. SparkTests is compiled with -nostdinc++/-isystem and linked with -L/-rpath/-lc++abi against that prefix; libc++-dev is no longer installed so a lost flag fails the link. A verification step walks every ELF under build/bin and build/lib and checks which libc++ it resolves, its RUNPATH, its direct libc++abi dependency and that it is MSan-linked (executables define the runtime symbols, shared objects import them). Tests/TestMSanCanary.cpp proves at run time that this translation unit is instrumented and that libc++ stores update shadow memory. The shared sanitizer wrapper ignores SIGXFSZ so an over-cap write truncates the log instead of killing the suite (harness fixture added); the lane runs with halt_on_error=1 so each run yields one deduplicated signature; a new step labels pre-test infrastructure failures so they are never mistaken for findings; ENABLE_VULKAN=OFF removes the last uninstrumented library the tests could reach; the ignorelist loses dead patterns and the /usr/* entry. Parity expects the extra configure record, inventory regenerated last, readiness status, the reproducible recipe and both copies of the build/CI and validation skills updated. The lane stays continue-on-error and outside the required gate until a run classifies clean. Static and repository validators only (workflow shape, exact gate, privilege boundaries, sanitizer evidence and pipeline harness, parity, inventory, docs contract, wiki, site data, registration): all pass. The SIGXFSZ fixture and the canary's RED side need Linux; the first CI run of this branch is their proof. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Krilliac
force-pushed
the
claude/ci-msan-instrumented-libcxx
branch
from
September 6, 2026 23:22
633b0ee to
8a0cfc0
Compare
Contributor
Code Coverage (GCC + lcov)Per-Subsystem Coverage
Selected-subsystem coverage: 53.6% (55563/103601 lines) LCOV corpus coverage: 54.1% (69359/128180 lines); 24579 instrumented lines are outside the threshold table. |
This was referenced Sep 7, 2026
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.
Summary
Makes the
build-linux-msanlane capable of producing meaningful MemorySanitizer reports. Until now it compiled the engine with-fsanitize=memorybut linked the distribution's libc++, which is not instrumented; everystd::stringwritten insidelibc++.so.1then read as uninitialized. On the last run before this change that produced 732 reports within the first 25 tests, the 16 MiB sanitizer log hit the wrapper's file-size limit, SparkTests died of SIGXFSZ with no completion evidence, and the lane had been red onWorkingfor months while costing 18 to 26 runner minutes per push.What changes
clang-18(size, SHA-256 and SHA-512 pinned in the workflow), buildslibc++andlibc++abiwithLLVM_USE_SANITIZER=MemoryWithOriginsfrom the checked-in recipe.github/msan/libcxx-runtime.cmake, installs them underrunner.temp, and caches that prefix with the splitactions/cache/restoreandsaveactions keyed by the recipe hash, the LLVM tag and the exact compiler package version. The prefix is verified on every run, hit or miss, before it is used or saved.-nostdinc++ -isystem <prefix>/include/c++/v1and linked with-L<prefix>/lib -Wl,-rpath,<prefix>/lib -lc++abion both executable and shared-library link lines;libc++-devis no longer installed, so a lost flag fails the link instead of silently linking the distro library. A verification step walks every ELF underbuild/binandbuild/lib(SparkTests, probe executables, the dlopen'd fixtures and plugins, the engine and game-module shared libraries) and checks whichlibc++they resolve, that the RUNPATH names the prefix, thatlibc++abi.so.1is a direct dependency, and that each is MSan-linked.Tests/TestMSanCanary.cppadds two tests: one proves this translation unit is instrumented and not ignorelisted, the other proves that stores performed by out-of-line libc++ code update shadow memory. Against an uninstrumented runtime the second test is designed to fail; in non-MSan builds both are count-preserving stubs.halt_on_error=1, so a run yields one deduplicated signature at a time, and a new step labels pre-test infrastructure failures (fetch, runtime build, link verification) so they are never mistaken for findings.ENABLE_VULKAN=OFFin this lane (the loader and lavapipe are uninstrumented; the Vulkan test is already a stub without a device). SDL2 was already off. The ignorelist loses dead patterns and the/usr/*entry that would have blinded every inlinedstd::function once the headers moved; in-tree third-party entries stay for the proving phase.continue-on-errorand outside the required gate until a run classifies clean.Verification
Static and repository validators only. MemorySanitizer cannot run on the Windows machine this was authored on, and WSL is not installed there, so the two Linux-only RED proofs (the SIGXFSZ fixture and the canary against an uninstrumented runtime) are left to CI.
test-workflow-failure-propagation.py,test-verify-exact-required-gate.py,test_workflow_privilege_boundaries.py,test-verify-sanitizer-evidence.pybash .github/scripts/test-run-sanitizer-tests.shpython Tests/Tools/test_build_matrix_parity.pyTools/buildmatrix/inventory.py --check,check_parity.pyagainst the checked-in baselinetools/docs_contract.py validate,tools/publish-wiki.py --check,tools/site-data/validate.py,tools/check-test-registration.shTests/TestMSanCanary.cppDesigned by a four-reader reconnaissance (job and sanitizer tooling, repository contracts, upstream MSan/libc++ facts from the 18.1.3 sources, engine library boundaries), reviewed by three adversarial critics whose objections were all accepted, then implemented and re-verified by three reviewers. The second review caught a blocker inherited from the plan: executables link the MSan runtime whole-archive and define
__msan_*symbols rather than importing them, so the ELF check now distinguishes executables (by PT_INTERP) from shared objects. The plan's wrapper comment also contained an apostrophe inside a single-quoted script, which the harness itself exposed.What only the CI run can prove
cleanorsanitizer-findingrather thancrashorverification-failure.Do not merge until the first run's report has been read and the lane classifies clean.
🤖 Generated with Claude Code