I have no objections, so LGTM.
One question: there are some ASAN supressions related to our own code
without reference to tickets or any other resource with explanation why
we have added this supression or when we want to fix it. Looks like we
decided to silently mute warnings and forget about it. Please clarify.
Sergey
On 13:50 Mon 20 Jan , Alexander V. Tikhonov wrote:
> 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.
>
> Close #2058
> ---
> .travis.mk | 17 ++++--
> asan/asan.supp | 17 ++++++
> asan/lsan.supp | 105 ++++++++++++++++++++++++++++++++++
> cmake/profile.cmake | 4 +-
> test/box/on_shutdown.skipcond | 7 +++
> 5 files changed, 142 insertions(+), 8 deletions(-)
> create mode 100644 asan/asan.supp
> create mode 100644 asan/lsan.supp
> create mode 100644 test/box/on_shutdown.skipcond
>
> diff --git a/.travis.mk b/.travis.mk
> index 42969ff56..efbc00ba1 100644
> --- a/.travis.mk
> +++ b/.travis.mk
> @@ -111,15 +111,20 @@ 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}
> + 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:
> + # - To exclude tests from ASAN checks the asan/asan.supp file
> + # was set at the build time in cmake/profile.cmake file.
> + # - To exclude tests from LSAN checks the asan/lsan.supp file
> + # was set in environment options to be used at run time.
> + cd test && ASAN=ON \
> + LSAN_OPTIONS=suppressions=${PWD}/asan/lsan.supp \
> + 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
>
> diff --git a/asan/asan.supp b/asan/asan.supp
> new file mode 100644
> index 000000000..2a20114f4
> --- /dev/null
> +++ b/asan/asan.supp
> @@ -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/lsan.supp b/asan/lsan.supp
> new file mode 100644
> index 000000000..a1237fb86
> --- /dev/null
> +++ b/asan/lsan.supp
> @@ -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..bc4bf67f5 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/asan.supp")
> 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/asan.supp)
> 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
>
--
sergeyb@