From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>,
Maksim Tiushev <mandesero@gmail.com>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v5 luajit 3/3] ci: add Valgrind testing workflow
Date: Fri, 13 Dec 2024 16:23:42 +0300 [thread overview]
Message-ID: <cd58c406-42a4-4a1b-b26d-b20dc78ad2e4@tarantool.org> (raw)
In-Reply-To: <b8a4d956f9bbe196803fc76f55add6677444e06b.1733909411.git.skaplun@tarantool.org>
[-- Attachment #1: Type: text/plain, Size: 7101 bytes --]
Hi, Sergey,
thanks for the patch! LGTM
On 11.12.2024 16:21, Sergey Kaplun wrote:
> From: Maksim Tiushev<mandesero@gmail.com>
>
> This patch adds CI testing with Valgrind in three scenarios:
> - Full leak 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`.
>
> The use of `0x00` and `0xff` for memory fill helps to detect dirty
> reads. `0x00` mimics zero-initialized memory, which can mask some
> uninitialized memory usage. `0xff` fills memory with non-zero values to
> make such errors easier to spot.
>
> Closes tarantool/tarantool#3705
> ---
> .github/actions/setup-valgrind/README.md | 12 +++
> .github/actions/setup-valgrind/action.yml | 12 +++
> .github/workflows/valgrind-testing.yaml | 102 ++++++++++++++++++++++
> 3 files changed, 126 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..e7d66a3a
> --- /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-valgrind
> + if: ${{ matrix.OS == 'Linux' }}
> +```
> \ No newline at end of file
> diff --git a/.github/actions/setup-valgrind/action.yml b/.github/actions/setup-valgrind/action.yml
> new file mode 100644
> index 00000000..5c11fdaa
> --- /dev/null
> +++ b/.github/actions/setup-valgrind/action.yml
> @@ -0,0 +1,12 @@
> +name: Setup CI environment with Valgrind on Linux
> +description: Extend setup-linux with Valgrind installation
> +
> +runs:
> + using: composite
> + steps:
> + - name: Setup CI environment (Linux)
> + uses: ./.github/actions/setup-linux
> + - name: Install Valgrind
> + run: |
> + apt -y install valgrind
> + shell: bash
> diff --git a/.github/workflows/valgrind-testing.yaml b/.github/workflows/valgrind-testing.yaml
> new file mode 100644
> index 00000000..e6606478
> --- /dev/null
> +++ b/.github/workflows/valgrind-testing.yaml
> @@ -0,0 +1,102 @@
> +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.
> + # We don't test on arm64 because the address returned by
> + # the system allocator may exceed 47 bits. As a result, we
> + # are unable to allocate memory for `lua_State`.
> + # Therefore, testing on this platform is currently
> + # disabled.
> + 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_OPTS: --leak-check=full --show-leak-kinds=all --track-origins=yes
> + JOB_POSTFIX: "leak-check: full"
> + # The use of `0x00` and `0xFF` for memory fill helps
> + # to detect dirty reads. `0x00` mimics
> + # zero-initialized memory, which can mask some
> + # uninitialized memory usage. `0xFF` fills memory with
> + # a non-zero values to make such errors easier to
> + # spot.
> + - VALGRIND_SCENARIO: malloc-free-fill-0x00
> + VALGRIND_OPTS: --leak-check=no --malloc-fill=0x00 --free-fill=0x00
> + JOB_POSTFIX: "malloc/free-fill: 0x00"
> + - VALGRIND_SCENARIO: malloc-free-fill-0xff
> + VALGRIND_OPTS: --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, the internal
> + # LuaJIT memory allocator is not instrumented yet, so to
> + # find any memory errors, it's better to build LuaJIT with
> + # the 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.
> + 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
> + env:
> + VALGRIND_OPTS: ${{ matrix.VALGRIND_OPTS }}
> + run: cmake --build . --parallel --target LuaJIT-test
> + working-directory: ${{ env.BUILDDIR }}
[-- Attachment #2: Type: text/html, Size: 7342 bytes --]
prev parent reply other threads:[~2024-12-13 13:23 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-11 13:21 [Tarantool-patches] [PATCH v5 luajit 0/3] Valgrind testing Sergey Kaplun via Tarantool-patches
2024-12-11 13:21 ` [Tarantool-patches] [PATCH v5 luajit 1/3] Ensure full init of IR_NOP instructions Sergey Kaplun via Tarantool-patches
2024-12-13 12:54 ` Sergey Bronnikov via Tarantool-patches
2024-12-16 11:24 ` Sergey Kaplun via Tarantool-patches
2024-12-17 11:08 ` Sergey Bronnikov via Tarantool-patches
2024-12-11 13:21 ` [Tarantool-patches] [PATCH v5 luajit 2/3] cmake: run tests with Valgrind Sergey Kaplun via Tarantool-patches
2024-12-13 13:18 ` Sergey Bronnikov via Tarantool-patches
2024-12-16 16:40 ` Sergey Kaplun via Tarantool-patches
2024-12-17 11:42 ` Sergey Bronnikov via Tarantool-patches
2024-12-17 12:17 ` Sergey Kaplun via Tarantool-patches
2024-12-17 19:31 ` Sergey Bronnikov via Tarantool-patches
2024-12-11 13:21 ` [Tarantool-patches] [PATCH v5 luajit 3/3] ci: add Valgrind testing workflow Sergey Kaplun via Tarantool-patches
2024-12-13 13:23 ` Sergey Bronnikov via Tarantool-patches [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cd58c406-42a4-4a1b-b26d-b20dc78ad2e4@tarantool.org \
--to=tarantool-patches@dev.tarantool.org \
--cc=mandesero@gmail.com \
--cc=sergeyb@tarantool.org \
--cc=skaplun@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH v5 luajit 3/3] ci: add Valgrind testing workflow' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox