Tarantool development patches archive
 help / color / mirror / Atom feed
From: mandesero--- via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tarantool-patches@dev.tarantool.org
Cc: mandesero@gmail.com
Subject: [Tarantool-patches] [PATCH v3 luajit 2/2] ci: add Valgrind testing workflow
Date: Mon, 11 Nov 2024 13:04:50 +0000	[thread overview]
Message-ID: <20241111130450.8311-1-mandesero@gmail.com> (raw)
In-Reply-To: <8cf43811-f0bc-4acf-b4b2-d197cf28a87e@tarantool.org>

From: Maksim Tiushev <mandesero@gmail.com>

Hi, Sergey!

Thanks for the review!

I updated the commit message, see below.

> > +            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
> This config doesn't have VALGRIND_OPTIONS, is it intentional?

Yes, according to the previous patch, `VALGRIND_OPTIONS` is an
environment variable and must be set before building. It is not
a CMake option. It is setted in the `configure` step in this
workflow.

> > +          - BUILDTYPE: Release
> > +            CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> Why JOB_POSTFIX is empty here?

`JOB_POSTFIX` describes Valgrind options, not the build type
or CMake options.

---------------------------------------------------------------

This patch adds CI testing with Valgrind in three scenarios:
  - 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`.

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.

Closes #3705
---
 .github/actions/setup-valgrind/README.md  | 12 +++
 .github/actions/setup-valgrind/action.yml | 12 +++
 .github/workflows/valgrind-testing.yaml   | 99 +++++++++++++++++++++++
 3 files changed, 123 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..44056fcd
--- /dev/null
+++ b/.github/workflows/valgrind-testing.yaml
@@ -0,0 +1,99 @@
+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 Linux/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_OPTIONS: --leak-check=full --show-leak-kinds=all --track-origins=yes --verbose
+            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_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 }}
-- 
2.34.1


  reply	other threads:[~2024-11-11 13:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-18  8:50 [Tarantool-patches] [PATCH v3 luajit 0/2] Enable running tests with Valgrind, add CI " mandesero--- via Tarantool-patches
2024-09-18  8:50 ` [Tarantool-patches] [PATCH v3 luajit 1/2] cmake: run tests with Valgrind mandesero--- via Tarantool-patches
2024-11-01 14:05   ` Sergey Bronnikov via Tarantool-patches
2024-11-11 11:03     ` mandesero--- via Tarantool-patches
2024-09-18  8:50 ` [Tarantool-patches] [PATCH v3 luajit 2/2] ci: add Valgrind testing workflow mandesero--- via Tarantool-patches
2024-11-01 14:17   ` Sergey Bronnikov via Tarantool-patches
2024-11-11 13:04     ` mandesero--- via Tarantool-patches [this message]
2024-11-01 12:51 ` [Tarantool-patches] [PATCH v3 luajit 0/2] Enable running tests with Valgrind, add CI " Sergey Bronnikov via Tarantool-patches

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=20241111130450.8311-1-mandesero@gmail.com \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=mandesero@gmail.com \
    --subject='Re: [Tarantool-patches] [PATCH v3 luajit 2/2] 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