Repository navigation
Run the tests under valgrind in CI's GCC 15 job - #257
Merged
Merged
Conversation
The sanitizers don't look for reads of uninitialized memory, and MemorySanitizer, which does, needs every library in the process built with it, including the HIP runtime. Valgrind's memcheck finds them in any build, so the Build workflow's GCC 15 job now runs the tests under it, after the normal test run. - The new valgrind matrix key turns the step on. The GCC 15 job sets it and installs the valgrind package. - The step runs ctest -T memcheck with --leak-check=full --track-origins=yes --num-callers=40 --error-exitcode=1, so a test fails on any memory error, and on a leak that's definitely or possibly lost. It skips the five tests that run CMake scripts. - Valgrind writes its reports to a log for each test, not to ctest's output, so the step prints the logs of the tests with a report. - Memory that's still reachable at exit, such as the HIP runtime's, isn't reported, so the step doesn't need test/valgrind.supp. Valgrind can't run in the Clang jobs. With -ftrivial-auto-var-init=pattern, optimized Clang code copies std::format()'s arguments with x87 instructions, which valgrind emulates with too few bits. It can't run in the sanitizer jobs either, since valgrind can't run a program built with ASan or TSan. AGENTS.md adds a Valgrind-clean check, with the command CI runs, how to check still-reachable memory with test/valgrind.supp, and why an optimized Clang build can't be used. Testing: ran the step's script on a local copy of the GCC 15 job's configuration (GCC 15.2, RelWithDebInfo, HIPOBJ_WERROR, the MinIO bridge, and HIPOBJ_TEST_REQUIRE_LIBIBVERBS=1). All 28 test programs passed with no errors, in about 17 s. With one test replaced by a program that reads uninitialized memory and leaks, the step failed and printed that test's valgrind log. actionlint, shellcheck, and codespell are clean. Co-Authored-By: Claude <noreply@anthropic.com>
derobins
requested review from
akolliasAMD,
gaoikawa,
jordan-turbofish,
kurtmcmillan,
riley-dixon,
sbates130272 and
zbyrne
as code owners
October 18, 2026 04:10
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused CI and documentation changes are consistent, with no blocking issues identified.
No open issues 🎉
What changed in this PR
Adds Valgrind checks to hipObject’s GCC 15 CI job to detect memory errors that existing sanitizers miss.
Changes:
- Installs Valgrind and runs eligible tests under memcheck, printing error logs.
- Documents local Valgrind checks and compiler limitations.
| File | Description |
|---|---|
| AGENTS.md | Documents Valgrind requirements and usage. |
| .github/workflows/hipobject-build.yml | Enables memcheck in the non-sanitized GCC 15 job. |
Review effort: Balanced
Contributor
Security scan found no critical or high findings on lines changed by this pull requestView the security scan run for complete reports. “New” means the finding is located in a changed file on an added or modified line; it is not a base-versus-head findings delta. |
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.
The sanitizers don't look for reads of uninitialized memory, and MemorySanitizer, which does, needs every library in the process built with it, including the HIP runtime. Valgrind's memcheck finds them in any build, so the Build workflow's GCC 15 job now runs the tests under it, after the normal test run.
Valgrind can't run in the Clang jobs. With
-ftrivial-auto-var-init=pattern, optimized Clang code copies std::format()'s arguments with x87 instructions, which valgrind emulates with too few bits. It can't run in the sanitizer jobs either, since valgrind can't run a program built with ASan or TSan.
AGENTS.md adds a Valgrind-clean check, with the command CI runs, how to check still-reachable memory with test/valgrind.supp, and why an optimized Clang build can't be used.
Testing: ran the step's script on a local copy of the GCC 15 job's configuration (GCC 15.2, RelWithDebInfo, HIPOBJ_WERROR, the MinIO bridge, and HIPOBJ_TEST_REQUIRE_LIBIBVERBS=1). All 28 test programs passed with no errors, in about 17 s. With one test replaced by a program that reads uninitialized memory and leaks, the step failed and printed that test's valgrind log. actionlint, shellcheck, and codespell are clean.