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 14B18D41FC1; Thu, 12 Sep 2024 21:58:20 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 14B18D41FC1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1726167500; bh=9q3ZtJgeDpXTzLRzpGESTKhW+WsHki4QOszyb0o6pWM=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=PO/vIanuAhTcdu/QIUOb/j0vS8kU0JWhgjP8z4A7fBHilkCxMCZ8iJBsKvFK45PR9 rQq3euydrcM/Upw5IT+1bGBd/nui7TVsiX8NnG2CXdCcxmCio/LTUxWzBoSkA5hqfA SQscBwl4ptFKrstLnz8jASEfT2FmELnyjrJ2ZGsE= Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [95.163.41.64]) (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 A8805D41FC1 for ; Thu, 12 Sep 2024 21:58:18 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A8805D41FC1 Received: by smtp41.i.mail.ru with esmtpa (envelope-from ) id 1sop1F-00000004PyO-2yd3; Thu, 12 Sep 2024 21:58:18 +0300 Content-Type: multipart/alternative; boundary="------------5cTcftGNfDKPfjmCO89F8lRu" Message-ID: <49b3d66a-de5f-4a0f-b574-df9af02c0dee@tarantool.org> Date: Thu, 12 Sep 2024 21:58:16 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: mandesero@gmail.com, tarantool-patches@dev.tarantool.org, skaplun@tarantool.org, m.kokryashkin@tarantool.org References: <20240912102153.163481-1-mandesero@gmail.com> <20240912102153.163481-3-mandesero@gmail.com> In-Reply-To: <20240912102153.163481-3-mandesero@gmail.com> X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD91486559A3ABF84E79604B59B75DC89D489EEBA155E7463E7182A05F5380850402AA60942A5F9F49BD4FF92D56319F19797B9C0E3C07D2E6CF74A38ABFC07646D3E3E97FC8DFF7329 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE727FD6E7FC3A8F857EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637C5BAFDCCAC60DA968638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D81957402F3A5F36D33630F7DFC7CD785C8E58D0546DA554B8CC7F00164DA146DAFE8445B8C89999728AA50765F7900637F6B57BC7E64490618DEB871D839B7333395957E7521B51C2DFABB839C843B9C08941B15DA834481F8AA50765F790063783E00425F71A4181389733CBF5DBD5E9B5C8C57E37DE458BD9DD9810294C998ED8FC6C240DEA76428AA50765F79006371A73A110E17B1318D32BA5DBAC0009BE395957E7521B51C2330BD67F2E7D9AF1090A508E0FED6299176DF2183F8FC7C0C43C4D2AE8146675B3661434B16C20ACC84D3B47A649675FE827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BA56E11165BA017C7EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 0D63561A33F958A5E5405C4BE15C403D5002B1117B3ED6964262FB0CBC29D4E803803A57F48E4E5A823CB91A9FED034534781492E4B8EEAD2739D626790C8313BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742D3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF554194081A116BF7B34B22A5A80407B164E18190130F2443A4A8B0BFFB8A5142182FB9261F6240E8BA3E0C39EEA44CB3C03567FC2F7D60BBB1BD90FE4F18F624B884D153868C24A5C226CC413062362A913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojBIBNH8BG++daFhDK82kq9A== X-Mailru-Sender: C4F68CFF4024C8867DFDF7C7F25884584C0708ECA8210E1B504BCEC120683F150FFE39BC53D7DEF41A3E4E6BA144C7CF645D15D82EE4B272BD6E4642A116CA93524AA66B5ACBE6721EF430B9A63E2A504198E0F3ECE9B5443453F38A29522196 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 luajit 2/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. --------------5cTcftGNfDKPfjmCO89F8lRu Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 }} --------------5cTcftGNfDKPfjmCO89F8lRu Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

Hi, Maxim,

thanks for the patch! See comments below:

On 12.09.2024 13:21, mandesero--- via Tarantool-patches wrote:
From: Maksim Tiushev <mandesero@gmail.com>

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 }}
--------------5cTcftGNfDKPfjmCO89F8lRu--