From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 3B06EE8094D; Fri, 13 Dec 2024 16:23:44 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3B06EE8094D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1734096224; bh=iSeb6G9SFsGd0G8ye06vDgoNzt9mi7Ttju8lkMmKOKg=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=XrHaPCPNnppqWFNfzYV87bPwhfeCJ6BF4MIfqL1rO5/dAsfKckPoWBHLR+8NlKdV2 yKD9/2Rx89EVaHojbgOFWnBRYiTEaTXZrdUSgel3DuSbG4gyKPTZHhcj+yDFbAQGhz 5L6R+QOFumQ7Pjh0oYIJU3E3oQ0Wp7LgNfJXnywQ= Received: from send279.i.mail.ru (send279.i.mail.ru [95.163.59.118]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 5A285E80947 for ; Fri, 13 Dec 2024 16:23:43 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5A285E80947 Received: by exim-smtp-bf6fb4755-962wm with esmtpa (envelope-from ) id 1tM5du-00000000J0F-1GrI; Fri, 13 Dec 2024 16:23:42 +0300 Content-Type: multipart/alternative; boundary="------------qzPH9lXdjhxRGR07iL9Qj7pv" Message-ID: Date: Fri, 13 Dec 2024 16:23:42 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Sergey Kaplun , Maksim Tiushev Cc: tarantool-patches@dev.tarantool.org References: In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD911C40A3FA7A6588311CA95751A16EE21F6DC58E6DB62A7BD182A05F5380850405B76770160D4CCAE3DE06ABAFEAF67056720BE08A7158D656F5C15B752694997EFBA94EB484632BD X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE702C8878A98595F58EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637FDB3827A455F08028638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8CAEE735A1DE600A32BF2E7CCCA509FF619C5A58D8551AF16CC7F00164DA146DAFE8445B8C89999728AA50765F79006375FFD5C25497261569FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C327ED053E960B195E117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CF1DD302B0B79430F8BA3038C0950A5D36C8A9BA7A39EFB766D91E3A1F190DE8FDBA3038C0950A5D36D5E8D9A59859A8B63A07C165BFF3A40C76E601842F6C81A1F004C906525384303E02D724532EE2C3F43C7A68FF6260569E8FC8737B5C2249D082881546D93491E827F84554CEF50127C277FBC8AE2E8BF1175FABE1C0F9B6AAAE862A0553A39223F8577A6DFFEA7CEB8BE9EE02D59E2D43847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A50AA0ABD4C80F90B35002B1117B3ED696EDA21C7F6518D428A13BD6A4B0E00B96823CB91A9FED034534781492E4B8EEADADEF88395FA75C5FBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFDAB8C49134F52A726258AD6B769FFC35073033D0ECE531B9C213FCFB3438BE6DA53151558601347D8E708AA2021A9CC2A2DF60AFFD2C450941203E977425CBEFEC0160F9C3BA048B5F4332CA8FE04980913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojgFHDrhqZ0P92kc+biNmZdQ== X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D6140904D6C03FA06A08B8E7FD94675F33565657C87FA8476E3B40152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v5 luajit 3/3] ci: add Valgrind testing workflow X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------qzPH9lXdjhxRGR07iL9Qj7pv Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi, Sergey, thanks for the patch! LGTM On 11.12.2024 16:21, Sergey Kaplun wrote: > From: Maksim Tiushev > > 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 }} --------------qzPH9lXdjhxRGR07iL9Qj7pv Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

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 }}
--------------qzPH9lXdjhxRGR07iL9Qj7pv--