From ceb93ba0d043eeacfe5bbe01eabae2a9ecb7fa38 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Sat, 17 Oct 2026 21:10:07 -0700 Subject: [PATCH] Run the tests under valgrind in CI's GCC 15 job 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 --- .github/workflows/hipobject-build.yml | 43 +++++++++++++++++++++++++++ AGENTS.md | 31 ++++++++++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/.github/workflows/hipobject-build.yml b/.github/workflows/hipobject-build.yml index ed75b1c6..be0b267f 100644 --- a/.github/workflows/hipobject-build.yml +++ b/.github/workflows/hipobject-build.yml @@ -75,6 +75,8 @@ jobs: # backends ON, which turns on both HIPOBJ_BNXT and # HIPOBJ_IONIC # packages None. Extra apt packages, such as a compiler. + # valgrind OFF. ON runs the tests again under valgrind's + # memcheck, which needs the valgrind package. # # Every job uses ROCm 10.1, the latest ROCm (see Install ROCm below). matrix: @@ -131,11 +133,23 @@ jobs: # GCC is a second set of eyes: its warnings and sanitizers find # problems that Clang's don't. The sanitizer job builds # RelWithDebInfo, since the amdclang one builds Debug. + # + # The plain GCC job also runs the tests under valgrind, which + # finds reads of uninitialized memory that the sanitizers don't + # look for. It can't run in a Clang job: with + # -ftrivial-auto-var-init=pattern, which hipObject's Clang + # options add, optimized Clang code copies std::format()'s + # arguments with x87 instructions, which valgrind emulates with + # too few bits (see test/valgrind.supp). It can't run in a + # sanitizer job either, since valgrind can't run a program built + # with ASan or TSan. - label: GCC 15 ubuntu: '26.04' cc: gcc-15 cxx: g++-15 build_type: RelWithDebInfo + packages: valgrind + valgrind: ON - label: GCC 15, ASan and UBSan ubuntu: '26.04' cc: gcc-15 @@ -248,6 +262,35 @@ jobs: run: | cd build && ctest --output-on-failure -j"$(nproc)" + # Runs the test programs under valgrind's memcheck. Valgrind makes a + # test fail if it finds an error, such as a read of uninitialized + # memory or of memory that isn't allocated, or a leak that's + # definitely or possibly lost. It doesn't report memory that's still + # reachable at exit, such as the HIP runtime's, so this doesn't need + # test/valgrind.supp. + # + # The excluded tests run CMake scripts, so valgrind would only check + # CMake. Valgrind writes its reports to a log for each test, not to + # ctest's output, so the logs of the tests with a report are printed + # here. + - name: Test with valgrind + if: matrix.valgrind == 'ON' + run: | + status=0 + ctest --test-dir build -T memcheck --output-on-failure --no-tests=error -j"$(nproc)" \ + --overwrite "MemoryCheckCommand=$(command -v valgrind)" \ + --overwrite "MemoryCheckCommandOptions=--leak-check=full --track-origins=yes --num-callers=40 --error-exitcode=1" \ + -E '^hipobj-(exports|subproject|install|package-deb|package-rpm)$' \ + || status=$? + shopt -s nullglob + for log in build/Testing/Temporary/MemoryChecker.*.log; do + if ! grep -q 'ERROR SUMMARY: 0 errors' "${log}"; then + echo "::error::valgrind reported errors in ${log}" + cat "${log}" + fi + done + exit "${status}" + # hipObject has to load libibverbs with only its runtime package # (libibverbs1) installed, not the development package that the # build needs, whose libibverbs.so link a user may not have. Remove diff --git a/AGENTS.md b/AGENTS.md index a41c12de..2492ecf2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -93,7 +93,8 @@ Notes: `-DHIPOBJ_USE_CODE_COVERAGE=ON`. Single jobs smoke-test the rest: with amdclang, a static `Release` build (`-DBUILD_SHARED_LIBS=OFF`), one with both backends off, and C++23 and C++26 (`HIPOBJ_CXX_STANDARD`); upstream - Clang 21; GCC 15, with and without `-DHIPOBJ_USE_SANITIZERS=ON`; and GCC 13 + Clang 21; GCC 15, with and without `-DHIPOBJ_USE_SANITIZERS=ON` (the job + without it also runs the tests under [valgrind](#c-and-c)); and GCC 13 and Clang 18 on Ubuntu 24.04, which also has an older CMake (3.28). Coverage is reported on the run's summary page, and the job fails below the [coverage target](#tests); on `develop`, a job then publishes the result to @@ -607,6 +608,34 @@ configuration files. `HIPOBJSanitizers.cmake`: it instruments every target itself, at the end of the configuration. CI runs both builds with amdclang, and `HIPOBJ_USE_SANITIZERS` with GCC too. +- **Valgrind-clean**: the tests must also run without a report from + valgrind's memcheck, which finds reads of uninitialized memory that the + sanitizers don't look for. Run it in the GCC build you make for the + compiler warnings, without the sanitizers (valgrind can't run a program + built with them): + + ```bash + ctest --test-dir build -T memcheck --output-on-failure \ + --overwrite "MemoryCheckCommand=$(command -v valgrind)" \ + --overwrite "MemoryCheckCommandOptions=--leak-check=full --track-origins=yes --num-callers=40 --error-exitcode=1" \ + -E '^hipobj-(exports|subproject|install|package-deb|package-rpm)$' + ``` + + The excluded tests run CMake scripts, so valgrind would only check CMake. + Valgrind writes its reports to `build/Testing/Temporary/MemoryChecker.*.log`, + one log for each test, not to ctest's output. It doesn't report memory + that's still reachable when the program exits, such as the HIP runtime's, + unless it's asked to with `--show-leak-kinds`. To look at that memory too, + add `--show-leak-kinds=all --errors-for-leak-kinds=all` to the options, + and `--overwrite "MemoryCheckSuppressionFile=$PWD/test/valgrind.supp"`, + which suppresses what ROCm, OpenSSL, and `systemPlatform()` keep until the + program exits; the path has to be absolute. Don't run valgrind on + an optimized Clang build: with `-ftrivial-auto-var-init=pattern`, which + hipObject's Clang options add, Clang at `-O1` and above copies + `std::format()`'s arguments with x87 instructions, which valgrind emulates + with too few bits, so formatted numbers come out wrong and formatting a + string can crash. A Clang Debug build works. CI runs this in its GCC 15 + job. - **Lifetime safety, when a change could leave something dangling.** Clang 23's `-Wlifetime-safety` looks for use-after-free, use-after-scope, and dangling references, but it reports too many false positives to leave