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 43DB854F455; Wed, 26 Jul 2023 18:05:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 43DB854F455 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1690383938; bh=aQ8OWWT2KUcH5m2ux9SGad9bBgveGK4826gkGVWPotc=; 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=B4HLgf3+Ub48pMhCn3eUrXlGYsod6EsBjngW9fdm0ed6gZRThJhMnnSjFHo+JFl4h PHaHj5TgwpqeFaSQ+ukiLrU1kJCKKAlbif4lcfLcSm8j8ANvpZLt3+AiJPYenqAfmz WBdjyO3B90j7EcjcXe0ptwIQeHYY0l/O14YbArl8= Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [95.163.41.82]) (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 A1AD154AE1A for ; Wed, 26 Jul 2023 18:05:37 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A1AD154AE1A Received: by smtp44.i.mail.ru with esmtpa (envelope-from ) id 1qOg52-007va8-O6; Wed, 26 Jul 2023 18:05:37 +0300 Date: Wed, 26 Jul 2023 14:53:32 +0000 To: Sergey Bronnikov Message-ID: References: <97b2ba3cab77d42f00d9e2eefe331e33fe651b55.1689925402.git.imun@tarantool.org> <35cfadfe-6a66-c38b-ce05-bacf999fa0a6@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <35cfadfe-6a66-c38b-ce05-bacf999fa0a6@tarantool.org> X-Clacks-Overhead: GNU Terry Pratchett X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9F4F4431F26286E46E0D21A45EF1364E0ECF8B7538A790AE0182A05F53808504087FF0E89343D5CA9B7B6AFE1B40DDCE0863D7AA32443279F140E35989C6D4A3F X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE749E89BD568380EECC2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE768D1C4AD116E0413EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BE5CCB53A13BC8DBADACAB681709D3BB0F4F3882E849BC334CC7F00164DA146DAFE8445B8C89999728AA50765F790063783E00425F71A4181389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC87664788771C849C4F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C2A336C65186350916E0066C2D8992A164AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C32F36E1858E9CF555BA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF17B107DEF921CE79089D37D7C0E48F6C8AA50765F790063757B1FBEA53BC6EDBEFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 0D63561A33F958A52EB9A521096D6EAED2328217061F4B504C4591D4E54ABFE6F87CCE6106E1FC07E67D4AC08A07B9B065B78C30F681404DCB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF172EE44C4EE9011C7ED1109DA4F1D2C83F8723EED7DAAE01BDBDFC3761751C4ECCDF2CE9FE05F2E72D8A5E5B53F5BD0466EE50D34855DD5AD61FE7984322C85FA74DFFEFA5DC0E7F02C26D483E81D6BEECAEF3E2CCC1ED8C383653B6C8D9AE0FD16FCAA6493B703A X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojXeYlEwgoYE+RtYyBi/YM8g== X-Mailru-Sender: 2FEBA92C8E508479FE7B9A1DF348D53108BACC47B0649E24C6200682C74091FD1D9BE34E29D0AC0C2326FE6F2A341ACE0FB9F97486540B4CD9E8847AB8CFED4D9ABF8A61C016C2CFB0DAF586E7D11B3E67EA787935ED9F1B 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! See my answers inline. On 24.07.23, Sergey Bronnikov wrote: > Thanks for the patch! > > See my comments below. > > > Sergey > > On 7/21/23 11:12, Igor Munkin wrote: > > > diff --git a/.github/actions/setup-sanitizers/README.md b/.github/actions/setup-sanitizers/README.md > > new file mode 100644 > > index 00000000..3aa9e214 > > --- /dev/null > > +++ b/.github/actions/setup-sanitizers/README.md > > @@ -0,0 +1,12 @@ > > +# Setup environment for sanitizers on Linux > > + > > +Action setups the environment on Linux runners (install requirements, setup the > > +workflow environment, etc) for testing with sanitizers enabled. > > + > > +## How to use Github Action from Github workflow > > + > > +Add the following code to the running steps before LuaJIT configuration: > > +``` > > +- uses: ./.github/actions/setup-sanitizers > > + if: ${{ matrix.OS == 'Linux' }} > > Nit: I would left a comment why OSes is limited by Linux only. > > You actually said it in commit message ("I believe we will > be able to add other platforms being supported, when macOS runners > starvation is defeated."). I have literally written it here (see the first paragraph at the beginning of the README.md). | Action setups the environment on Linux runners (install requirements, | setup the workflow environment, etc) for testing with sanitizers | enabled. > > > +``` > > 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 > > @@ -0,0 +1,89 @@ > > +jobs: > > + test-asan: > > + strategy: > > + fail-fast: false > > + matrix: > > + # XXX: Let's start with only Linux/x86_64 > > + BUILDTYPE: [Debug, Release] > > + include: > > + - BUILDTYPE: Debug > > + CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON > > + - BUILDTYPE: Release > > + CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo > > + runs-on: [self-hosted, regular, Linux, x86_64] > > + name: > > > + LuaJIT with ASan (Linux/x86_64) > > + ${{ matrix.BUILDTYPE }} > > + GC64:ON SYSMALLOC:ON > > + steps: > > + - uses: actions/checkout@v3 > > + with: > > + fetch-depth: 0 > > + submodules: recursive > > + - name: setup Linux for sanitizers > > + uses: ./.github/actions/setup-sanitizers > > + - 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 > Nit: I suppose it would be easy read paragraph above if indent it by "-S", > not by "cmake". We have such alignment in all workflow files. The idea is nice, but I see no reason to touch it now. Ignoring. > > + - 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. > > + 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 \ > > + symbolize=1: \ > > + unmap_shadow_on_exit=1: \ > > + " > > This options are used in CI and doesn't present in CMake, so ASAN will work > different locally and in CI. > > Is it expected? As we discussed offline, it's not a trivial task to implement, considering integration testing with Tarantool. Since, this is not a popular configuration to be manually tested (until you poisoned the code with a leak), I suggest to return to this later. From my side, I have several ideas to try, so I'll share the results when I have some. > > > + run: cmake --build . --parallel --target LuaJIT-test > > + working-directory: ${{ env.BUILDDIR }} -- Best regards, IM