diff --git a/.github/workflows/hipobject-build.yml b/.github/workflows/hipobject-build.yml index ed75b1c..be0b267 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 a41c12d..2492ecf 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