Repository navigation
build: link test runners against a ReleaseSafe Zig library - #108
Merged
Merged
Conversation
The test runners are built with ASan and UBSan, but the sanitizers do not instrument Zig code, and libcbm_zig.a was built ReleaseFast for tests too. Out-of-bounds indexing, integer overflow and failed @intcast in the Zig modules were therefore undefined behaviour that no test could catch. Test builds now compile the Zig library ReleaseSafe, which keeps those checks and turns a violation into a panic with a stack trace. Release builds stay ReleaseFast. - build.zig: addCbmZigLibrary takes the build kind and picks ReleaseSafe for .tests. - Makefile.cbm: translate-c gets its own cbm_c.zig rule, shared by the release library and a new libcbm_zig_test.a that test-runner and test-repro-runner link. With CBM_PREBUILT_ZIG=1 (release-only artifacts) the test runners fall back to the release library. Signed-off-by: ata-sesli <atasesli@protonmail.com>
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.
Summary
Test builds now compile the Zig library (
libcbm_zig.a) as ReleaseSafe. Before this change it was built ReleaseFast for tests as well as for releases. Release builds stay ReleaseFast.Why
The test runners are built with AddressSanitizer and UBSan, but those sanitizers only instrument C code. In a ReleaseFast Zig build, Zig's own runtime checks are also off. So in the ported modules (git, simhash, watcher, discover and the CLI module), these errors were undefined behaviour that no test could detect:
@intCastthat doesn't fit;unreachable.ReleaseSafe keeps those checks. A violation becomes a panic with a stack trace pointing at the Zig line.
This matters more for the next migration step:
cypher.cis about 6,000 lines of string and buffer code that the sanitizers check today. Once it's ported, these checks are the only thing that can catch the same class of bug in tests.Changes
build.zig:addCbmZigLibrarynow takes the build kind. It uses ReleaseSafe for.tests(the test-support library and every test runner) and ReleaseFast for.prod. CI's test runner is built through this path (scripts/zova-build-test-runner.sh).Makefile.cbm:cbm_c.zigrule, shared by two library rules: the existing ReleaseFastlibcbm_zig.aand a new ReleaseSafelibcbm_zig_test.a;test-runnerandtest-repro-runnerlink the test library;CBM_PREBUILT_ZIG=1, the test runners fall back to the release library. Prebuilt artifacts are used only by the Windows release build, which ships only the release library.Verification
All on macOS arm64.
CBM_TEST_TIERS=all(all language tiers): 6,447 passed, the same asmainat ea95a61. No module panicked under the new checks.cbm_minhash_jaccard. Thesimhashsuite then aborted withpanic: index out of bounds: index 1, len 1at that line, with a trace intotest_minhash_renamed_vars_same_fingerprint. The probe was removed and is not in this PR.nmshows 31 Zig panic-handler symbols inlibcbm_zig_test.aand none inlibcbm_zig.a.tests/test_github_workflows.sh,git diff --checkandzig fmt --check build.zigall pass.Not checked locally
make testlink through the Makefile. Only the two library targets were built that way.