Use ASAN when running unit tests in CI#56
Open
JoukoVirtanen wants to merge 3 commits into
Open
Conversation
JoukoVirtanen
commented
Mar 26, 2026
JoukoVirtanen
commented
Mar 26, 2026
JoukoVirtanen
force-pushed
the
jv-use-asan
branch
from
5734b31 to
6572533
Compare
erthalion
reviewed
Mar 27, 2026
| source "${HOME}"/.cargo/env && | ||
| rustup component add rust-src --toolchain nightly && | ||
| TARGET=$(rustc --version --verbose | grep host | cut -d" " -f2) && | ||
| RUSTFLAGS="-Z sanitizer=address" cargo +nightly test -Z build-std --target "$TARGET" |
Collaborator
There was a problem hiding this comment.
Tests are invoked from the image itself nowadays:
https://github.com/stackrox/berserker/blob/main/Dockerfile.build#L32
Contributor
Author
There was a problem hiding this comment.
I now invoke the ASAN unit tests from the image itself.
erthalion
force-pushed
the
feature/new-script-worker
branch
from
881dc23 to
d030a12
Compare
erthalion
force-pushed
the
feature/new-script-worker
branch
from
d030a12 to
c135bad
Compare
erthalion
force-pushed
the
jv-use-asan
branch
2 times, most recently
from
ebc47a6 to
bcef02e
Compare
Merged
Co-authored-by: Dmitrii Dolgov <9erthalion6@gmail.com>
To prevent memory leaks from allocations in helper functions, remember references to allocated objects. For now we assume only CString is reference.
JoukoVirtanen
force-pushed
the
jv-use-asan
branch
from
7ef2bab to
2fab6d0
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a Cargo ChangesNightly toolchain and script cleanup
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Containerfile`:
- Line 1: Fix the Dockerfile lint issues by updating the stage alias in the
top-level FROM instruction to use matching uppercase AS, and change the
ENTRYPOINT in the Containerfile to JSON/exec form instead of shell form. Use the
FROM and ENTRYPOINT instructions in this file as the targets for the cleanup.
- Around line 45-50: The nightly ASAN test branch in the Containerfile is
missing the crate’s `nightly` Cargo feature, so the `sanitize` feature gate in
`src/lib.rs` never turns on. Update the `cargo +nightly test` invocation in this
RUN block to include the crate’s `nightly` feature alongside the existing `-Z
build-std` and `RUSTFLAGS="-Z sanitizer=address"` settings, so the ASAN path
actually exercises the intended nightly-only code.
In `@Makefile`:
- Around line 9-10: The Makefile’s two docker build steps currently reuse the
same berserker tag, so the nightly build overwrites the stable image. Update the
second docker build invocation to use a distinct tag for the nightly variant
while keeping the first build as the default berserker image, so later steps
consume the intended stable image; make this change in the build commands near
the two docker build lines.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: af1e2ced-f111-4070-bda9-21101f6c0352
📒 Files selected for processing (7)
Cargo.tomlContainerfileMakefilesrc/lib.rssrc/main.rssrc/worker/script.rssrc/worker/syscalls/ioctl.rs
| @@ -1,10 +1,8 @@ | |||
| FROM registry.fedoraproject.org/fedora:43 as builder | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Minor Dockerfile lint fixes flagged by pipeline.
as should match FROM casing (AS) on line 1, and ENTRYPOINT should use JSON/exec form on line 62.
🧹 Proposed fix
-FROM registry.fedoraproject.org/fedora:43 as builder
+FROM registry.fedoraproject.org/fedora:43 AS builder-ENTRYPOINT berserker
+ENTRYPOINT ["berserker"]Also applies to: 62-62
🧰 Tools
🪛 GitHub Actions: Main berserker CI / 0_build-and-push.txt
[warning] 1-1: Dockerfile lint warning: FromAsCasing - 'as' and 'FROM' keywords' casing do not match.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Containerfile` at line 1, Fix the Dockerfile lint issues by updating the
stage alias in the top-level FROM instruction to use matching uppercase AS, and
change the ENTRYPOINT in the Containerfile to JSON/exec form instead of shell
form. Use the FROM and ENTRYPOINT instructions in this file as the targets for
the cleanup.
Source: Pipeline failures
Comment on lines
+45
to
+50
| RUN if [ "${RUST_VERSION}" == "nightly" ]; then \ | ||
| TARGET=$(rustc --version --verbose | grep host | cut -d" " -f2) && \ | ||
| RUSTFLAGS="-Z sanitizer=address" cargo +nightly test -Z build-std --target "$TARGET"; \ | ||
| else \ | ||
| cargo test; \ | ||
| fi |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Nightly ASAN run never enables the crate's nightly Cargo feature.
This branch runs cargo +nightly test -Z build-std ... but doesn't pass --features nightly, so #![cfg_attr(feature = "nightly", feature(sanitize))] in src/lib.rs stays disabled during the ASAN run, defeating the purpose of the new feature gate.
🔧 Proposed fix
RUN if [ "${RUST_VERSION}" == "nightly" ]; then \
TARGET=$(rustc --version --verbose | grep host | cut -d" " -f2) && \
- RUSTFLAGS="-Z sanitizer=address" cargo +nightly test -Z build-std --target "$TARGET"; \
+ RUSTFLAGS="-Z sanitizer=address" cargo +nightly test --features nightly -Z build-std --target "$TARGET"; \
else \
cargo test; \
fi📝 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
| RUN if [ "${RUST_VERSION}" == "nightly" ]; then \ | |
| TARGET=$(rustc --version --verbose | grep host | cut -d" " -f2) && \ | |
| RUSTFLAGS="-Z sanitizer=address" cargo +nightly test -Z build-std --target "$TARGET"; \ | |
| else \ | |
| cargo test; \ | |
| fi | |
| RUN if [ "${RUST_VERSION}" == "nightly" ]; then \ | |
| TARGET=$(rustc --version --verbose | grep host | cut -d" " -f2) && \ | |
| RUSTFLAGS="-Z sanitizer=address" cargo +nightly test --features nightly -Z build-std --target "$TARGET"; \ | |
| else \ | |
| cargo test; \ | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Containerfile` around lines 45 - 50, The nightly ASAN test branch in the
Containerfile is missing the crate’s `nightly` Cargo feature, so the `sanitize`
feature gate in `src/lib.rs` never turns on. Update the `cargo +nightly test`
invocation in this RUN block to include the crate’s `nightly` feature alongside
the existing `-Z build-std` and `RUSTFLAGS="-Z sanitizer=address"` settings, so
the ASAN path actually exercises the intended nightly-only code.
Comment on lines
9
to
+10
| docker build -t berserker -f Containerfile . | ||
| docker build -t berserker -f Containerfile --build-arg=RUST_VERSION=nightly . |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files | rg '(^|/)(Makefile|Containerfile|Dockerfile|README.md)$'
printf '\n== Makefile (mapped) ==\n'
ast-grep outline Makefile --view expanded || true
printf '\n== Makefile with line numbers ==\n'
cat -n Makefile | sed -n '1,120p'
printf '\n== Containerfile with line numbers ==\n'
cat -n Containerfile | sed -n '1,220p'
printf '\n== Search for berserker tag usage ==\n'
rg -n 'berserker|RUST_VERSION=nightly|build-and-push|docker build' -S .Repository: stackrox/berserker
Length of output: 8595
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== README build / image references ==\n'
rg -n 'make |docker build|docker run|latest|BERSERKER_TAG|tag:' README.md Makefile Containerfile Containerfile.test scripts -S
printf '\n== README excerpt around usage ==\n'
cat -n README.md | sed -n '1,140p'Repository: stackrox/berserker
Length of output: 5722
Makefile:9-10 — Use a separate tag for the nightly build. The second docker build -t berserker ... --build-arg=RUST_VERSION=nightly overwrites the first image, so berserker:latest ends up pointing at the nightly build. That means the later test/network images consume the nightly toolchain image, and the stable build is never used.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Makefile` around lines 9 - 10, The Makefile’s two docker build steps
currently reuse the same berserker tag, so the nightly build overwrites the
stable image. Update the second docker build invocation to use a distinct tag
for the nightly variant while keeping the first build as the default berserker
image, so later steps consume the intended stable image; make this change in the
build commands near the two docker build lines.
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.
Adds CI unit test jobs with and without ASAN.
The ASAN tests found a failure https://github.com/stackrox/berserker/actions/runs/24287041420/job/70918030561?pr=56 when the target branch was #55