From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 D16BE46970E for ; Wed, 15 Jan 2020 02:26:10 +0300 (MSK) Date: Wed, 15 Jan 2020 02:23:56 +0300 From: Igor Munkin Message-ID: <20200114232356.GM31304@tarantool.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH v1 2/2] gitlab-ci: ASAN testing memory leaks add List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Alexander V. Tikhonov" Cc: tarantool-patches@dev.tarantool.org Sasha, Thanks for the patch! I left some notes below, please consider them. On 21.11.19, Alexander V. Tikhonov wrote: > gitlab-ci: ASAN testing memory leaks add IMHO the commit subject should be adjusted as the following: | gitlab-ci: add memory leaks detection via LSAN > Added memory leaks detection to ASAN testing. Added files for exceptions: > - address sanitizer on compilation: asan/ignore_asan.txt > - memory leak sanitizer on run-time: asan/ignore_lsan.txt > Cmake updated with asan/ignore_asan.txt file added and test run rules > changed with asan/ignore_lsan.txt file. Added 'engine' and replication' > suites into the testing and blocked 'box/on_shutdown.test.lua' test > that breaks the testing, all of excepted tests will be enabled durring > issue #4360. > > Close #2058 I propose to reword the commit message as the following: | The change enables memory leaks detection to existing ASAN testing | routine and introduces suppression files with the corresponding | exception list: | * address sanitizer for compile-time: asan/asan.supp | * memory leak sanitizer for run-time: asan/lsan.supp | | Furthermore, added engine and replication suites for ASAN testing | routine. | | Additionally to the tests blacklisted within #4359, | 'box/on_shutdown.test.lua' is also disabled since it fails the | introduced leak check. All blacklisted tests have to be enabled | within #4360. | | Closes #2058 Feel free to adjust it on your own way. > --- > .travis.mk | 13 +++-- > asan/ignore_asan.txt | 17 ++++++ > asan/ignore_lsan.txt | 105 ++++++++++++++++++++++++++++++++++ > cmake/profile.cmake | 4 +- > test/box/on_shutdown.skipcond | 7 +++ > 5 files changed, 138 insertions(+), 8 deletions(-) > create mode 100644 asan/ignore_asan.txt > create mode 100644 asan/ignore_lsan.txt > create mode 100644 test/box/on_shutdown.skipcond > > diff --git a/.travis.mk b/.travis.mk > index 42969ff56..c6bc82d41 100644 > --- a/.travis.mk > +++ b/.travis.mk > @@ -111,15 +111,16 @@ coverage_debian: deps_debian test_coverage_debian_no_deps > # ASAN > > build_asan_debian: > - CC=clang-8 CXX=clang++-8 cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS} > - CC=clang-8 CXX=clang++-8 cmake . -DENABLE_ASAN=ON ${CMAKE_EXTRA_PARAMS} Side note: have no idea why it had been divided into separate commands. > + CC=clang-8 CXX=clang++-8 cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON -DENABLE_ASAN=ON ${CMAKE_EXTRA_PARAMS} > make -j > > test_asan_debian_no_deps: build_asan_debian > - # temporary excluded engine/ and replication/ suites with some tests from other suites by issue #4360 > - cd test && ASAN=ON ASAN_OPTIONS=detect_leaks=0 ./test-run.py --force $(TEST_RUN_EXTRA_PARAMS) \ > - app/ app-tap/ box/ box-py/ box-tap/ engine_long/ long_run-py/ luajit-tap/ \ > - replication-py/ small/ sql/ sql-tap/ swim/ unit/ vinyl/ wal_off/ xlog/ xlog-py/ > + # temporary excluded some tests by issue #4360 > + # ASAN environment flag shows the excluded tests to skip the testing As discussed offline, please drop a comment related to the difference in applying suppressions for LSAN and ASAN. > + cd test && ASAN=ON \ > + LSAN_OPTIONS=suppressions=${PWD}/asan/ignore_lsan.txt \ > + ASAN_OPTIONS=heap_profile=0:unmap_shadow_on_exit=1:detect_invalid_pointer_pairs=1:symbolize=1:detect_leaks=1:dump_instruction_bytes=1:print_suppressions=0 \ > + ./test-run.py --force $(TEST_RUN_EXTRA_PARAMS) > > test_asan_debian: deps_debian deps_buster_clang_8 test_asan_debian_no_deps > I propose to name suppresion lists in more traditional for such tools way: * s/ignore_asan.txt/asan.supp/ * s/ignore_lsan.txt/lsan.supp/ > diff --git a/asan/ignore_asan.txt b/asan/ignore_asan.txt > new file mode 100644 > index 000000000..2a20114f4 > --- /dev/null > +++ b/asan/ignore_asan.txt > @@ -0,0 +1,17 @@ > +# File format: > +#fun:* > +#src:* > + > +# !test: app-tap/json.test.lua > +# source: third_party/lua-cjson/lua_cjson.c > +fun:json_decode > + > +# test: unit/base64.test.lua > +# source: third_party/base64.c > +fun:base64_decode_block > +# source: test/unit/base64.c > +fun:base64_test > + > +# !test: unit/msgpack.test > +# source: src/lib/msgpuck/test/msgpuck.c > +fun:test_mp_print > diff --git a/asan/ignore_lsan.txt b/asan/ignore_lsan.txt > new file mode 100644 > index 000000000..a1237fb86 > --- /dev/null > +++ b/asan/ignore_lsan.txt > @@ -0,0 +1,105 @@ > +# File format: > +#leak:* > + > +# test: app/crypto.test.lua > +# source: /usr/lib/x86_64-linux-gnu/libcrypto.so > +leak:CRYPTO_zalloc > + > +# test: app-tap/http_client.test.lua > +# source: src/tarantool > +leak:Curl_setstropt > +leak:create_conn > +leak:Curl_conncache_add_conn > +leak:alloc_addbyter > +leak:Curl_getaddrinfo_ex > +leak:Curl_cache_addr > +leak:Curl_hash_init > +leak:Curl_hash_add > +leak:Curl_he2ai > +leak:Curl_open > +leak:Curl_resolver_init > + > +# test: app-tap/iconv.test.lua > +# source: /usr/lib/x86_64-linux-gnu/gconv/UTF-16.so > +leak:gconv_init > + > +# test: box*/ > +# source: third_party/luajit > +leak:lj_BC_FUNCC > + > +# test: box/access.test.lua > +# test: box/access_bin.test.lua > +# test: box/access_misc.test.lua > +# source: src/box/error.cc > +leak:AccessDeniedError::AccessDeniedError > + > +# test: box/bitset.test.lua > +# source: src/lib/bitset/iterator.c > +leak:tt_bitset_iterator_init > + > +# test: box-py/args.test.py > +# source: /lib/x86_64-linux-gnu/libc.so* > +leak:libc.so* > + > +# test: box-tap/schema-mt.test.lua > +# source: src/lib/core/coio_task.c > +leak:coio_on_start > +# source: src/lib/salad/mhash.h > +leak:mh_i32ptr_new > + > +# test: replication/misc.test.lua > +# source: src/box/vy_log.c > +leak:vy_recovery_new_f > +# source: src/lib/salad/mhash.h > +leak:mh_i64ptr_new > + > +# test: sql-tap/gh2250-trigger-chain-limit.test.lua > +# source: src/lib/core/exception.cc > +leak:Exception::operator new > + > +# test: sql-tap/trigger9.test.lua > +# source: src/lib/core/fiber.c > +leak:cord_start > + > +# test: sql-tap/tkt-7bbfb7d442.test.lua > +# test: sql-tap/view.test.lua > +# test: sql-tap/with1.test.lua > +# test: sql-tap/with2.test.lua > +# source: src/box/sql/malloc.c > +leak:sql_sized_malloc > + > +# test: swim/errinj.test.lua > +# test: swim/swim.test.lua > +# source: src/lib/swim/swim.c > +leak:swim_member_new > +leak:swim_update_member_payload > + > +# !test: unit/bps_tree.test.lua > +# source: src/lib/salad/bps_tree.h > +leak:bps_tree_test_create_leaf > +leak:bps_tree_test_process_insert_leaf > + > +# !test: unit/heap.test.lua > +# source: test/unit/heap.c > +leak:test_random_delete_workload > +leak:test_delete_last_node > + > +# !test: unit/heap_iterator.test.lua > +# source: src/lib/salad/heap.h > +leak:test_heap_reserve > + > +# !test: unit/swim.test.lua > +# source: src/lib/swim/swim_io.c > +leak:swim_scheduler_set_codec > + > +# test: vinyl/errinj.test.lua > +# source: src/lib/core/fiber.h > +leak:fiber_cxx_invoke > + > +# test: vinyl/errinj_ddl.test.lua > +# source: src/box/vy_stmt.c > +leak:vy_stmt_alloc > + > +# test: vinyl/recover.test.lua > +# source: src/lib/core/fiber.c > +leak:cord_costart_thread_func > diff --git a/cmake/profile.cmake b/cmake/profile.cmake > index 0ba31fa2c..22e2fb2c9 100644 > --- a/cmake/profile.cmake > +++ b/cmake/profile.cmake > @@ -53,7 +53,7 @@ if (ENABLE_ASAN) > "\n") > endif() > > - set(CMAKE_REQUIRED_FLAGS "-fsanitize=address") > + set(CMAKE_REQUIRED_FLAGS "-fsanitize=address -fsanitize-blacklist=${CMAKE_SOURCE_DIR}/asan/ignore_asan.txt") > check_c_source_compiles("int main(void) { > #include > void *x; > @@ -76,5 +76,5 @@ if (ENABLE_ASAN) > message(FATAL_ERROR "Cannot enable AddressSanitizer") > endif() > > - add_compile_flags("C;CXX" -fsanitize=address) > + add_compile_flags("C;CXX" -fsanitize=address -fsanitize-blacklist=${CMAKE_SOURCE_DIR}/asan/ignore_asan.txt) > endif() > diff --git a/test/box/on_shutdown.skipcond b/test/box/on_shutdown.skipcond > new file mode 100644 > index 000000000..e46fd1088 > --- /dev/null > +++ b/test/box/on_shutdown.skipcond > @@ -0,0 +1,7 @@ > +import os > + > +# Disabled at ASAN build due to issue #4360. > +if os.getenv("ASAN") == 'ON': > + self.skip = 1 > + > +# vim: set ft=python: > -- > 2.17.1 > As discussed offline, please send the follow-up patches for all other branches if needed. -- Best regards, IM