Skip to content

Run the tests under valgrind in CI's GCC 15 job - #257

Merged
derobins merged 1 commit into
developfrom
derobins/valgrind_ci
Oct 18, 2026
Merged

derobins merged 1 commit into
developfrom
derobins/valgrind_ci

Conversation

@derobins

Copy link
Copy Markdown
Collaborator

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.

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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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

@github-actions

Copy link
Copy Markdown
Contributor
Security scan found no critical or high findings on lines changed by this pull request

View 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.

@derobins
derobins merged commit 9c7dcc1 into develop Oct 18, 2026
29 checks passed
@derobins
derobins deleted the derobins/valgrind_ci branch October 18, 2026 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants