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 5CF0FD9EDE4; Mon, 16 Sep 2024 10:25:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5CF0FD9EDE4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1726471534; bh=MnhfdcfbGl03JFkWPoyXbSqMCP6yWNXBgPHJleh7s6o=; 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=vDgUmxsh2V49byW3Mw9hyElnz7G0Cd0UL6/S1yJ3hvMRouNxZNgEHhaGyAB3uR9KJ orXCwIwXDxT7A9WXCCn0vK5AZYchj6WSoj3HFl3q7q1Lw+Q+O25mJhRndwacK3jYB1 A8yQ9hWoKy/SU3Pjgtgg82ARwuip3Qa5UdTXlSV4= Received: from smtp56.i.mail.ru (smtp56.i.mail.ru [95.163.41.94]) (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 0CFC040E76E for ; Mon, 16 Sep 2024 10:25:33 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 0CFC040E76E Received: by smtp56.i.mail.ru with esmtpa (envelope-from ) id 1sq671-0000000Bj2c-0oNj; Mon, 16 Sep 2024 10:25:31 +0300 Date: Mon, 16 Sep 2024 10:25:19 +0300 To: mandesero@gmail.com Message-ID: References: <20240912102153.163481-1-mandesero@gmail.com> <20240912102153.163481-2-mandesero@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240912102153.163481-2-mandesero@gmail.com> X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9149934E261B3C850136D25067E1DFD7C8D3A3D81EBC71DE5182A05F53808504094A019E6EC1DE9D033594132A326AF8B1F1D395C355443142120CA645B0A7377A02D50AE20F21E99 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7B9FBA884A7C9B8BAEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006370E0E628E5A2670BA8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8788951FDD2532739666C63B32158311C99263BC0170EA734CC7F00164DA146DAFE8445B8C89999728AA50765F7900637F6B57BC7E64490618DEB871D839B7333395957E7521B51C2DFABB839C843B9C08941B15DA834481F8AA50765F7900637D0FEED2715E18529389733CBF5DBD5E9B5C8C57E37DE458B9E9CE733340B9D5F3BBE47FD9DD3FB595F5C1EE8F4F765FCF1175FABE1C0F9B6E2021AF6380DFAD18AA50765F7900637F09814068C508CC822CA9DD8327EE4930A3850AC1BE2E7350555CCFDA08FA3FAC4224003CC83647689D4C264860C145E X-C1DE0DAB: 0D63561A33F958A523020700DD4514E65002B1117B3ED6961F36954AF2269305108A05421C070DB8823CB91A9FED034534781492E4B8EEADAEB4DDBFD72C8FA1BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742DC8270968E61249B1004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D3444047AE358B40754DBA5B7A26DAA2A414B9A8F53C58FB2233B9811F0531BF6CE7F34171ECA99BD841D7E09C32AA3244C19B2006B47EB291A7B0DC8E7D22A103A5B53E9BC068D1DAEEA455F16B58544A2557BDE0DD54B3590A5AE236DF995FB59829709634694AABAED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojnH7hiPSwZxAr9Xae0uwQgA== X-DA7885C5: 2321ADAB56D915A9F255D290C0D534F911F2CDB212AB053CA40344D03DE4FA1F8AF73B4A5206EC8A5B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F7393C6D0B12EA33CAA9B752FB43BB156C4562A7389938EFF2552E5552CD939D424FBE49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 luajit 1/3] cmake: run tests with Valgrind 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 Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Maksim! Thanks for the patch! Please consider my comments below. On 12.09.24, mandesero@gmail.com wrote: > From: Maksim Tiushev > > This patch adds the ability to run tests using Valgrind. Custom > Valgrind testing options can be set via the environment variable > `VALGRIND_OPTIONS`. By default, only the suppression file is set > to `src/lj.supp`. > --- > CMakeLists.txt | 5 +++++ > test/CMakeLists.txt | 24 +++++++++++++++++++++++- > 2 files changed, 28 insertions(+), 1 deletion(-) > > diff --git a/CMakeLists.txt b/CMakeLists.txt > index aa2103b3..854b3613 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt > index 0db2dd8b..280f0042 100644 > --- a/test/CMakeLists.txt > +++ b/test/CMakeLists.txt > @@ -68,7 +68,29 @@ add_custom_target(${PROJECT_NAME}-lint DEPENDS > ${PROJECT_NAME}-codespell > ) > > -set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]") > +if(LUAJIT_USE_VALGRIND) > + > + if (NOT LUAJIT_USE_SYSMALLOC) > + message(FATAL_ERROR It is better to be a warning by analog with the ASAN build. > + "LUAJIT_USE_SYSMALLOC option is mandatory for Valgrind's memcheck tool" > + " on x64 and the only way to get useful results from it for all other" > + " architectures.") > + endif() > + > + if (NOT LUAJIT_ENABLE_GC64) Why do we need this condition? > + message(FATAL_ERROR Ditto. > + "LUAJIT_USE_SYSMALLOC cannot be enabled on x64 without GC64, since" I don't get this comment. Why valgrind depends on x64, and GC64 as well? I suppose we can drop the comment and the check above. > + " realloc usually doesn't return addresses in the right address range.") > + endif() > + > + set(SUPPRESSIONS_FILE "${CMAKE_SOURCE_DIR}/src/lj.supp") Please use `LUAJIT_SOURCE_DIR` instead. > + set(VALGRIND_OPTIONS "--suppressions=${SUPPRESSIONS_FILE} $ENV{VALGRIND_OPTIONS}") Line length is more than 80 symbols. > + > + set(LUAJIT_TEST_COMMAND "valgrind ${VALGRIND_OPTIONS} ${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]") Since there are no skip conditions for this build some tests will fail or never finish on this commit. It is better to squash this commit with the last one. It is better to use a name LUAJIT_TEST_VALGRIND_OPTS to be consistent with other names (LUAJIT_TEST_BINARY LUAJIT_TEST_INIT). Line length is more than 80 symbols. > +else() > + set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]") > +endif() > + > separate_arguments(LUAJIT_TEST_COMMAND) > > set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") > -- > 2.34.1 > -- Best regards, Sergey Kaplun