Hi, Maxim, thanks for the patch! See comments below: On 12.09.2024 13:21, mandesero--- via Tarantool-patches wrote: > From: Maksim Tiushev > > This patch adds CI testing with Valgrind in three scenarios: Patch adds a *workflow* with testing under Valgrind... > - Full checks enabled. > - No leak checks, with memory fill set to `--malloc-fill=0x00` > and `--free-fill=0x00`. > - No leak checks, with memory fill set to `--malloc-fill=0xFF` > and `--free-fill=0xFF`. > --- > .github/actions/setup-valgrind/README.md | 12 +++ > .github/actions/setup-valgrind/action.yml | 19 +++++ > .github/workflows/valgrind-testing.yaml | 91 +++++++++++++++++++++++ > 3 files changed, 122 insertions(+) > create mode 100644 .github/actions/setup-valgrind/README.md > create mode 100644 .github/actions/setup-valgrind/action.yml > create mode 100644 .github/workflows/valgrind-testing.yaml > > diff --git a/.github/actions/setup-valgrind/README.md b/.github/actions/setup-valgrind/README.md > new file mode 100644 > index 00000000..fabd5af1 > --- /dev/null > +++ b/.github/actions/setup-valgrind/README.md > @@ -0,0 +1,12 @@ > +# Setup environment for Valgrind on Linux > + > +Action setups the environment on Linux runners (install requirements, setup the > +workflow environment, etc) for testing with Valgrind. > + > +## How to use Github Action from Github workflow > + > +Add the following code to the running steps before LuaJIT configuration: > +``` > +- uses: ./.github/actions/setup-linux s/setup-linux/setup-valgrind/ > + if: ${{ matrix.OS == 'Linux' }} > +``` > diff --git a/.github/actions/setup-valgrind/action.yml b/.github/actions/setup-valgrind/action.yml > new file mode 100644 > index 00000000..4f6cfba4 > --- /dev/null > +++ b/.github/actions/setup-valgrind/action.yml > @@ -0,0 +1,19 @@ > +name: Setup CI environment for Valgrind on Linux > +description: Common part to tweak Linux CI runner environment copy-pasted? > +runs: > + using: composite > + steps: > + - name: Setup CI environment > + uses: ./.github/actions/setup > + - name: Set CMAKE_BUILD_PARALLEL_LEVEL > + run: | > + # Set CMAKE_BUILD_PARALLEL_LEVEL environment variable to > + # limit the number of parallel jobs for build/test step. > + NPROC=$(nproc) > + echo CMAKE_BUILD_PARALLEL_LEVEL=$(($NPROC + 1)) | tee -a $GITHUB_ENV Do you really need this for setup Valgrind? > + shell: bash > + - name: Install build and test dependencies > + run: | > + apt -y update > + apt -y install cmake gcc make ninja-build perl valgrind Why do you need gcc/cmake/make etc if an action about Valgrind setup? > + shell: bash > diff --git a/.github/workflows/valgrind-testing.yaml b/.github/workflows/valgrind-testing.yaml > new file mode 100644 > index 00000000..693799ea > --- /dev/null > +++ b/.github/workflows/valgrind-testing.yaml > @@ -0,0 +1,91 @@ > +name: Valgrind testing > + > +on: > + push: > + branches-ignore: > + - '**-notest' > + - 'upstream-**' > + tags-ignore: > + - '**' > + > +concurrency: > + # An update of a developer branch cancels the previously > + # scheduled workflow run for this branch. However, the default > + # branch, and long-term branch (tarantool/release/2.11, > + # tarantool/release/2.10, etc) workflow runs are never canceled. > + # > + # We use a trick here: define the concurrency group as 'workflow > + # run ID' + # 'workflow run attempt' because it is a unique > + # combination for any run. So it effectively discards grouping. > + # > + # XXX: we cannot use `github.sha` as a unique identifier because > + # pushing a tag may cancel a run that works on a branch push > + # event. > + group: ${{ startsWith(github.ref, 'refs/heads/tarantool/') > + && format('{0}-{1}', github.run_id, github.run_attempt) > + || format('{0}-{1}', github.workflow, github.ref) }} > + cancel-in-progress: true > + > +jobs: > + test-valgrind: > + strategy: > + fail-fast: false > + matrix: > + # XXX: Let's start with only Linux/x86_64 > + BUILDTYPE: [Debug, Release] > + VALGRIND_SCENARIO: [full, malloc-free-fill-0x00, malloc-free-fill-0xff] > + include: > + - BUILDTYPE: Debug > + CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON > + - BUILDTYPE: Release > + CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo > + - VALGRIND_SCENARIO: full > + VALGRIND_OPTIONS: --leak-check=full --show-leak-kinds=all --track-origins=yes --verbose > + JOB_POSTFIX: "leak-check: full" > + - VALGRIND_SCENARIO: malloc-free-fill-0x00 > + VALGRIND_OPTIONS: --leak-check=no --malloc-fill=0x00 --free-fill=0x00 > + JOB_POSTFIX: "malloc/free-fill: 0x00" > + - VALGRIND_SCENARIO: malloc-free-fill-0xff > + VALGRIND_OPTIONS: --leak-check=no --malloc-fill=0xff --free-fill=0xff > + JOB_POSTFIX: "malloc/free-fill: 0xff" > + runs-on: [self-hosted, regular, Linux, x86_64] > + name: > > + LuaJIT with Valgrind (Linux/x86_64) > + ${{ matrix.BUILDTYPE }} > + CC: gcc > + GC64:ON SYSMALLOC:ON > + ${{ matrix.JOB_POSTFIX }} > + steps: > + - uses: actions/checkout@v4 > + with: > + fetch-depth: 0 > + submodules: recursive > + - name: setup Linux for Valgrind > + uses: ./.github/actions/setup-valgrind > + - name: configure > + # XXX: LuaJIT configuration requires a couple of tweaks: > + # LUAJIT_USE_SYSMALLOC=ON: Unfortunately, internal LuaJIT > + # memory allocator is not instrumented yet, so to find > + # any memory errors it's better to build LuaJIT with > + # system provided memory allocator (i.e. run CMake > + # configuration phase with -DLUAJIT_USE_SYSMALLOC=ON). > + # For more info, see root CMakeLists.txt. > + # LUAJIT_ENABLE_GC64=ON: LUAJIT_USE_SYSMALLOC cannot be > + # enabled on x64 without GC64, since realloc usually > + # doesn't return addresses in the right address range. > + # For more info, see root CMakeLists.txt. > + env: > + VALGRIND_OPTIONS: ${{ matrix.VALGRIND_OPTIONS }} > + run: > > + cmake -S . -B ${{ env.BUILDDIR }} > + -G Ninja > + ${{ matrix.CMAKEFLAGS }} > + -DLUAJIT_USE_VALGRIND=ON > + -DLUAJIT_ENABLE_GC64=ON > + -DLUAJIT_USE_SYSMALLOC=ON > + - name: build > + run: cmake --build . --parallel > + working-directory: ${{ env.BUILDDIR }} > + - name: test > + run: cmake --build . --parallel --target LuaJIT-test > + working-directory: ${{ env.BUILDDIR }}