[Tarantool-patches] [PATCH v1 2/2] gitlab-ci: ASAN testing memory leaks add

Alexander Tikhonov avtikhon at tarantool.org
Mon Jan 20 08:26:55 MSK 2020


Igor,

Thanks for the review, I've made all the changes as you suggested.


>Среда, 15 января 2020, 2:26 +03:00 от Igor Munkin <imun at 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. 
Rewrote as you suggested.
>
>
>> ---
>>  .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.
Previously it had some strange issues that were not reproduced for now,
so the command changed as it should be from the very start.
>
>
>> +	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. 
Added comments about ASAN/LSAN excluding tests files at build/run times.
>
>
>> +	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/ 
Renamed.
>
>
>> 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 <sanitizer/asan_interface.h>
>>          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. 
Ok.
>
>
>-- 
>Best regards,
>IM


-- 
Alexander Tikhonov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20200120/78469c50/attachment.html>


More information about the Tarantool-patches mailing list