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 562EF54FD03; Wed, 26 Jul 2023 19:48:02 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 562EF54FD03 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1690390082; bh=V3vjtdx/B3orjzJWwRU2GjA3Iq+ZZQ4CNckMdimg0Qw=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=hoBnqc6gDYvtnN9AY50ONbFAUs7QBdMh8YHoVIKHJsDbo/ajAr6au9Xk6aKkNmaEy LJ6IRvvGafpGWRzNpZazqqRXB8X15tMZpdrY80wPAfAAyk5FmiLexWdeWDw8Y85oH5 P/Qosy6nDGLkeSRmTvW61kVYmJjtpqq2vFDZa/Uw= Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [95.163.41.81]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id AF40954FA69 for ; Wed, 26 Jul 2023 19:48:00 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org AF40954FA69 Received: by smtp40.i.mail.ru with esmtpa (envelope-from ) id 1qOhg7-00DsFk-LW; Wed, 26 Jul 2023 19:48:00 +0300 Date: Wed, 26 Jul 2023 16:35:54 +0000 To: Sergey Kaplun Message-ID: References: <97b2ba3cab77d42f00d9e2eefe331e33fe651b55.1689925402.git.imun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9F4F4431F26286E46E06349688353877DD3F79B0846A5C917182A05F5380850409C946E11200BD97CD2150A9D98F5667B3ECB0AC228A8407FE936E53960B45C08 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE70D278D70F8433719EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063771C846A5973DEE7E8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8E91BE1343DD4DF653ED567F3902FD1C5117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BA2C6A0109559C168A471835C12D1D977C4224003CC836476EB9C4185024447017B076A6E789B0E975F5C1EE8F4F765FC998C1B47ABA640FE3AA81AA40904B5D9CF19DD082D7633A0C84D3B47A649675F3AA81AA40904B5D98AA50765F79006376F7FC77F1E52761BD81D268191BDAD3D3666184CF4C3C14F3FC91FA280E0CE3D1A620F70A64A45A98AA50765F790063735872C767BF85DA227C277FBC8AE2E8BC6A536F79815AD9275ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-B7AD71C0: 1B70FBA5C9BEEE72C9761FC34675ADEB871C96603B655635EE9D5CB6078CC77C99B689A72BC475C4135824B3FEAB4495 X-C1DE0DAB: 0D63561A33F958A5FF7C6E90DE7D289A5B681BF23CF417EA37251A34B7452FD7F87CCE6106E1FC07E67D4AC08A07B9B01F9513A7CA91E555CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0AD75DCE07D45A749953FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF617356277B1A3DDC06C6367EE8DF53FC7CDDB48C6E306A191232E7027F8DA0F6673DC415E80A8BD92D8A5E5B53F5BD044099789A5EC48AFF0ED6CFD74783244EA74DFFEFA5DC0E7F02C26D483E81D6BEECAEF3E2CCC1ED8C383653B6C8D9AE0FD16FCAA6493B703A X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojXeYlEwgoYE9iwgpS9E11lg== X-Mailru-Sender: 2FEBA92C8E508479FE7B9A1DF348D531AF6B577E41CF3C027DC28110428A1C5F6749191C9B5A18AF2326FE6F2A341ACE0FB9F97486540B4CD9E8847AB8CFED4D9ABF8A61C016C2CFB0DAF586E7D11B3E67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit v2 5/5] ci: introduce testing workflow with sanitizers 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: Igor Munkin via Tarantool-patches Reply-To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Sergey, Thanks for your review! On 26.07.23, Sergey Kaplun wrote: > Hi, Igor! > Thanks for the patch! > The patch is generally LGTM, after fixing Sergey's comments. But I'm > really concerned about the fact, that the CI don't show any error for > lj-1024-* [1], lj-128-* [2] tests, where we have obvious memleak, since > `os.exit()` is used instead of `test:done(true)` for these tests. Thanks for noticing this! Unfortunately, it was a bug in CI description, so the failures after rebasing were hidden. See the incremental patch below: ================================================================================ diff --git a/.github/workflows/sanitizers-testing.yml b/.github/workflows/sanitizers-testing.yml index 666ce6b2..28160ae3 100644 --- a/.github/workflows/sanitizers-testing.yml +++ b/.github/workflows/sanitizers-testing.yml @@ -51,22 +51,23 @@ jobs: - name: setup Linux for sanitizers uses: ./.github/actions/setup-sanitizers - 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. run: > cmake -S . -B ${{ env.BUILDDIR }} -G Ninja ${{ matrix.CMAKEFLAGS }} -DLUAJIT_USE_ASAN=ON - # XXX: 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. -DLUAJIT_USE_SYSMALLOC=ON - # XXX: 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. -DLUAJIT_ENABLE_GC64=ON - name: build run: cmake --build . --parallel ================================================================================ > > On 21.07.23, Igor Munkin wrote: > > This commit adds GitHub workflow running all available LuaJIT tests with > > LUAJIT_USE_ASAN option enabled. For now, sanitizers workflow works only > > for Linux/x86_64 as the most scaling setup in our CI. I believe we will > > be able to add other platforms being supported, when macOS runners > > starvation is defeated. There is also a separate GitHub Action > > introduced for convenient setup of the environment. > > > > Besided, internal LuaJIT memory allocator is not instrumented yet > > Typo? s/Besided/Besides/ > Typo: s/internal ... allocator/the internal ... allocator/ Fixed, thanks! > > > unfortunately, so to find any memory faults it's better to build LuaJIT > > with system provided memory allocator (i.e. run CMake configuration > > phase with -DLUAJIT_USE_SYSMALLOC=ON). However, 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. > > > > Follows up tarantool/tarantool#5878 > > > > Signed-off-by: Igor Munkin > > --- > > .github/actions/setup-sanitizers/README.md | 12 +++ > > .github/actions/setup-sanitizers/action.yml | 24 ++++++ > > .github/workflows/sanitizers-testing.yml | 89 +++++++++++++++++++++ > > > > > diff --git a/.github/actions/setup-sanitizers/action.yml b/.github/actions/setup-sanitizers/action.yml > > new file mode 100644 > > index 00000000..ca6b6b9f > > --- /dev/null > > +++ b/.github/actions/setup-sanitizers/action.yml > > @@ -0,0 +1,24 @@ > > +name: Setup CI environment for testing with sanitizers on Linux > > +description: Common part to tweak Linux CI runner environment for sanitizers > > +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 > > + shell: bash > > + - name: Install build and test dependencies > > + run: | > > + apt -y update > > + apt -y install clang-11 cmake ninja-build make perl > > + shell: bash > > + - name: Set Clang as a default toolchain > > + run: | > > + echo CC=clang-11 | tee -a $GITHUB_ENV > > + echo CXX=clang++-11 | tee -a $GITHUB_ENV > > Do we need clang++ for LuaJIT? Why? Removed. > > > + shell: bash > > diff --git a/.github/workflows/sanitizers-testing.yml b/.github/workflows/sanitizers-testing.yml > > new file mode 100644 > > index 00000000..6c345108 > > --- /dev/null > > +++ b/.github/workflows/sanitizers-testing.yml > > > > > + - name: configure > > + run: > > > + cmake -S . -B ${{ env.BUILDDIR }} > > + -G Ninja > > + ${{ matrix.CMAKEFLAGS }} > > + -DLUAJIT_USE_ASAN=ON > > + # XXX: 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. > > + -DLUAJIT_USE_SYSMALLOC=ON > > + # XXX: 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. > > + -DLUAJIT_ENABLE_GC64=ON > > + - name: build > > + run: cmake --build . --parallel > > + working-directory: ${{ env.BUILDDIR }} > > + - name: test > > + env: > > + # Enable as much checks as possible. See more info here: > > + # https://github.com/google/sanitizers/wiki/AddressSanitizerFlags. > > Please, also add the link to the [1], since heap_profile is common flag > (same for print_suppressions). Also, it is disabled by default, why do > we need to forcify it to false? I explicitly disabled these particular options, to avoid their unexpected enabling, since these options might slow or even break the tests. Besides, these flags are also disabled in Tarantool, so its another attempt to be in sync with that repo. BTW, added the link, you've mentioned: ================================================================================ diff --git a/.github/workflows/sanitizers-testing.yml b/.github/workflows/sanitizers-testing.yml index 28160ae3..4bccfcef 100644 --- a/.github/workflows/sanitizers-testing.yml +++ b/.github/workflows/sanitizers-testing.yml @@ -75,7 +75,8 @@ jobs: - name: test env: # Enable as much checks as possible. See more info here: - # https://github.com/google/sanitizers/wiki/AddressSanitizerFlags. + # https://github.com/google/sanitizers/wiki/AddressSanitizerFlags, + # https://github.com/google/sanitizers/wiki/SanitizerCommonFlags. ASAN_OPTIONS: " \ detect_invalid_pointer_pairs=1: \ detect_leaks=1: \ ================================================================================ > > > + ASAN_OPTIONS: " \ > > + detect_invalid_pointer_pairs=1: \ > > + detect_leaks=1: \ > > + detect_stack_use_after_return=1: \ > > + dump_instruction_bytes=1: \ > > + heap_profile=0: \ > > + print_suppressions=0 \ Also fixed a typo here: ================================================================================ diff --git a/.github/workflows/sanitizers-testing.yml b/.github/workflows/sanitizers-testing.yml index 6c345108..666ce6b2 100644 --- a/.github/workflows/sanitizers-testing.yml +++ b/.github/workflows/sanitizers-testing.yml @@ -81,7 +81,7 @@ jobs: detect_stack_use_after_return=1: \ dump_instruction_bytes=1: \ heap_profile=0: \ - print_suppressions=0 \ + print_suppressions=0: \ symbolize=1: \ unmap_shadow_on_exit=1: \ " ================================================================================ > > + symbolize=1: \ > > + unmap_shadow_on_exit=1: \ > > + " > > + run: cmake --build . --parallel --target LuaJIT-test > > + working-directory: ${{ env.BUILDDIR }} > > -- > > 2.30.2 > > > > [1]: https://github.com/tarantool/luajit/actions/runs/5619185186/job/15225900726#step:6:673 > [2]: https://github.com/tarantool/luajit/actions/runs/5619185186/job/15225900726#step:6:638 > [3]: https://github.com/google/sanitizers/wiki/SanitizerCommonFlags > > -- > Best regards, > Sergey Kaplun -- Best regards, IM