From ce444d473ce0fd178e7df2984f147f46e0dbf921 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 17 Mar 2024 18:53:03 +0800 Subject: [PATCH 1/8] cmake: quote a list using quotes otherwised we'd have ``` CMake Error at cmake/modules/FindSanitizers.cmake:17 (if): if given arguments: "address" "IN_LIST" "address" "thread" "undefined_behavior" "OR" "leak" "IN_LIST" "address" "thread" "undefined_behavior" Unknown arguments specified ``` when enabling TSan with WITH_TSAN=ON. Signed-off-by: Kefu Chai --- cmake/modules/FindSanitizers.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/modules/FindSanitizers.cmake b/cmake/modules/FindSanitizers.cmake index bfb99821a9bd5..1401ca2442bf4 100644 --- a/cmake/modules/FindSanitizers.cmake +++ b/cmake/modules/FindSanitizers.cmake @@ -14,8 +14,8 @@ foreach(component ${Sanitizers_FIND_COMPONENTS}) elseif(component STREQUAL "leak") set(Sanitizers_leak_COMPILE_OPTIONS "-fsanitize=leak") elseif(component STREQUAL "thread") - if ("address" IN_LIST ${Sanitizers_FIND_COMPONENTS} OR - "leak" IN_LIST ${Sanitizers_FIND_COMPONENTS}) + if ("address" IN_LIST "${Sanitizers_FIND_COMPONENTS}" OR + "leak" IN_LIST "${Sanitizers_FIND_COMPONENTS}") message(SEND_ERROR "Cannot combine -fsanitize-leak w/ -fsanitize-thread") elseif(NOT CMAKE_POSITION_INDEPENDENT_CODE) message(SEND_ERROR "TSan requires all code to be position independent") From a13a61d63dbaa0bf294a34da3d016291dfdc5668 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 17 Mar 2024 19:35:04 +0800 Subject: [PATCH 2/8] cmake: error out on UBSan error so we can be alerted if UBSan identify something wrong. Signed-off-by: Kefu Chai --- cmake/modules/AddCephTest.cmake | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cmake/modules/AddCephTest.cmake b/cmake/modules/AddCephTest.cmake index ccd3f8dee0b55..45bb26aa6155d 100644 --- a/cmake/modules/AddCephTest.cmake +++ b/cmake/modules/AddCephTest.cmake @@ -19,6 +19,12 @@ function(add_ceph_test test_name test_path) PATH=${CMAKE_RUNTIME_OUTPUT_DIRECTORY}:${CMAKE_SOURCE_DIR}/src:$ENV{PATH} PYTHONPATH=${CMAKE_LIBRARY_OUTPUT_DIRECTORY}/cython_modules/lib.3:${CMAKE_SOURCE_DIR}/src/pybind CEPH_BUILD_VIRTUALENV=${CEPH_BUILD_VIRTUALENV}) + if(WITH_UBSAN) + set_property(TEST ${test_name} + APPEND + PROPERTY ENVIRONMENT + UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1) + endif() set_property(TEST ${test_name} PROPERTY TIMEOUT ${CEPH_TEST_TIMEOUT}) # Crimson seastar unittest always run with --smp N to start N threads. By default, crimson seastar unittest From 0dc61a4be335073e9a3047cf52ec3d12e4244f4d Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 17 Mar 2024 21:36:23 +0800 Subject: [PATCH 3/8] cmake: do not override CMAKE_EXE_LINKER_FLAGS instead of overriding CMAKE_EXE_LINKER_FLAGS, let's append to it. so that the existing `CMAKE_EXE_LINKER_FLAGS` is not overriden. this should enable us to build with Clang and with sanitizer(s) enabled. Signed-off-by: Kefu Chai --- src/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 90c9c48e06e12..149bdc45562f3 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -143,7 +143,7 @@ elseif(CMAKE_CXX_COMPILER_ID STREQUAL Clang) if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS 12) # require >= clang-12 message(FATAL_ERROR "C++20 support requires a minimum Clang version of 12.") endif() - set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_EXPORTS_C_FLAG}") + string(APPEND CMAKE_EXE_LINKER_FLAGS " ${CMAKE_EXE_EXPORTS_C_FLAG}") string(APPEND CMAKE_LINKER_FLAGS " -rdynamic -export-dynamic ${CMAKE_EXE_EXPORTS_C_FLAG}") string(PREPEND CMAKE_CXX_FLAGS_DEBUG "-g ") add_compile_options($<$:-Wno-inconsistent-missing-override>) From d22734f6cb0e11dff0d94d212f47867fe06ab0fd Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 17 Mar 2024 23:40:27 +0800 Subject: [PATCH 4/8] cmake: add ${CMAKE_SHARED_LINKER_FLAGS} to LDFLAGS when building python extensions if sanitizers are enabled, we have to populate the required link flags to python extensions's building workflow. otherwise ld would fail to link like: ``` /usr/bin/ld: /home/jenkins-build/build/workspace/ceph-pull-requests/build/lib/libceph-common.so.2: undefined reference to `__asan_stack_free_10' ``` Signed-off-by: Kefu Chai --- cmake/modules/Distutils.cmake | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmake/modules/Distutils.cmake b/cmake/modules/Distutils.cmake index daaae4ba63fdb..f3d6c41e73174 100644 --- a/cmake/modules/Distutils.cmake +++ b/cmake/modules/Distutils.cmake @@ -73,6 +73,8 @@ function(distutils_add_cython_module target name src) set(PY_CC ${compiler_launcher} ${CMAKE_C_COMPILER} ${c_compiler_arg1}) set(PY_CXX ${compiler_launcher} ${CMAKE_CXX_COMPILER} ${cxx_compiler_arg1}) set(PY_LDSHARED ${link_launcher} ${CMAKE_C_COMPILER} ${c_compiler_arg1} "-shared") + string(REPLACE " " ";" PY_LDFLAGS "${CMAKE_SHARED_LINKER_FLAGS}") + list(APPEND PY_LDFLAGS -L${CMAKE_LIBRARY_OUTPUT_DIRECTORY}) execute_process(COMMAND "${Python3_EXECUTABLE}" -c "import sysconfig; print(sysconfig.get_config_var('EXT_SUFFIX'))" @@ -98,7 +100,7 @@ function(distutils_add_cython_module target name src) CXX="${PY_CXX}" LDSHARED="${PY_LDSHARED}" OPT=\"-DNDEBUG -g -fwrapv -O2 -w\" - LDFLAGS=-L${CMAKE_LIBRARY_OUTPUT_DIRECTORY} + LDFLAGS="${PY_LDFLAGS}" CYTHON_BUILD_DIR=${CMAKE_CURRENT_BINARY_DIR} CEPH_LIBDIR=${CMAKE_LIBRARY_OUTPUT_DIRECTORY} ${Python3_EXECUTABLE} ${setup_py} @@ -130,7 +132,7 @@ function(distutils_install_cython_module name) -D'void0=dead_function\(void\)' \ -D'__Pyx_check_single_interpreter\(ARG\)=ARG\#\#0' \ ${CFLAG_DISABLE_VTA}\") - set(ENV{LDFLAGS} \"-L${CMAKE_LIBRARY_OUTPUT_DIRECTORY}\") + set(ENV{LDFLAGS} \"${PY_LDFLAGS}\") set(ENV{CYTHON_BUILD_DIR} \"${CMAKE_CURRENT_BINARY_DIR}\") set(ENV{CEPH_LIBDIR} \"${CMAKE_LIBRARY_OUTPUT_DIRECTORY}\") From f68248944815acbf5fb08ef66f36da6d5baef824 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 24 Mar 2024 20:23:00 +0800 Subject: [PATCH 5/8] pybind: use LDFLAGS in env variable when check for building env when building python bindings extensions using Cython, we first perform sanity checks by building an executable and linking it against the shared library of the C language binding. if the executable builds, we consider the sanity test passes. before this change, we don't add `LDFLAGS` specified in environmental variable to the ldflags when building the executable. but we need to link against libasan, so that the symbols used by, for instance, librados.so, can be found by the executable. in this change, we always check `LDFLAGS`, and add it to the ldflags when building the executable for testing, if it's set. Signed-off-by: Kefu Chai --- src/pybind/cephfs/setup.py | 5 +++++ src/pybind/rados/setup.py | 5 +++++ src/pybind/rbd/setup.py | 5 +++++ src/pybind/rgw/setup.py | 5 +++++ 4 files changed, 20 insertions(+) diff --git a/src/pybind/cephfs/setup.py b/src/pybind/cephfs/setup.py index f6c2025f75d35..956a4e7008c6a 100755 --- a/src/pybind/cephfs/setup.py +++ b/src/pybind/cephfs/setup.py @@ -117,10 +117,15 @@ def check_sanity(): extra_preargs=['-iquote{path}'.format(path=os.path.join(CEPH_SRC_DIR, 'include'))] ) + if ldflags := os.environ.get('LDFLAGS'): + extra_postargs = ldflags.split() + else: + extra_postargs = None compiler.link_executable( objects=link_objects, output_progname=os.path.join(tmp_dir, 'cephfs_dummy'), libraries=['cephfs'], + extra_postargs=extra_postargs, output_dir=tmp_dir, ) diff --git a/src/pybind/rados/setup.py b/src/pybind/rados/setup.py index 62b54d26b6c81..000189b7da134 100755 --- a/src/pybind/rados/setup.py +++ b/src/pybind/rados/setup.py @@ -112,10 +112,15 @@ def check_sanity(): sources=[tmp_file], output_dir=tmp_dir ) + if ldflags := os.environ.get('LDFLAGS'): + extra_postargs = ldflags.split() + else: + extra_postargs = None compiler.link_executable( objects=link_objects, output_progname=os.path.join(tmp_dir, 'rados_dummy'), libraries=['rados'], + extra_postargs=extra_postargs, output_dir=tmp_dir, ) diff --git a/src/pybind/rbd/setup.py b/src/pybind/rbd/setup.py index eeb33c73d49b3..b03d1c3fc9576 100755 --- a/src/pybind/rbd/setup.py +++ b/src/pybind/rbd/setup.py @@ -116,10 +116,15 @@ def check_sanity(): output_dir=tmp_dir ) + if ldflags := os.environ.get('LDFLAGS'): + extra_postargs = ldflags.split() + else: + extra_postargs = None compiler.link_executable( objects=link_objects, output_progname=os.path.join(tmp_dir, 'rbd_dummy'), libraries=['rbd', 'rados'], + extra_postargs=extra_postargs, output_dir=tmp_dir, ) diff --git a/src/pybind/rgw/setup.py b/src/pybind/rgw/setup.py index ed45399d39469..eb8fc554db6ee 100755 --- a/src/pybind/rgw/setup.py +++ b/src/pybind/rgw/setup.py @@ -116,10 +116,15 @@ def check_sanity(): output_dir=tmp_dir, ) + if ldflags := os.environ.get('LDFLAGS'): + extra_postargs = ldflags.split() + else: + extra_postargs = None compiler.link_executable( objects=link_objects, output_progname=os.path.join(tmp_dir, 'rgw_dummy'), libraries=['rgw', 'rados'], + extra_postargs=extra_postargs, output_dir=tmp_dir, ) From a837f494b5a82555313e7424adfaee96910a4673 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 24 Mar 2024 21:40:28 +0800 Subject: [PATCH 6/8] cmake: prevent ASAN_OPTIONS from detect_odr_violation turns out we have multiple copies of following symbol defined by rbd executable: ``` AddressSanitizer: odr-violation: global 'ceph::buffer::list::always_empty_bptr' at /home/jenkins-build/build/workspace/ceph-pull-requests/src/common/buffer.cc:1267:34 ``` before addressing it. let's disable this warning. Refs https://tracker.ceph.com/issues/65098 Signed-off-by: Kefu Chai --- cmake/modules/AddCephTest.cmake | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/cmake/modules/AddCephTest.cmake b/cmake/modules/AddCephTest.cmake index 45bb26aa6155d..4593070fe17de 100644 --- a/cmake/modules/AddCephTest.cmake +++ b/cmake/modules/AddCephTest.cmake @@ -25,6 +25,15 @@ function(add_ceph_test test_name test_path) PROPERTY ENVIRONMENT UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1) endif() + if(WITH_ASAN) + # AddressSanitizer: odr-violation: global 'ceph::buffer::list::always_empty_bptr' at + # /home/jenkins-build/build/workspace/ceph-pull-requests/src/common/buffer.cc:1267:34 + # see https://tracker.ceph.com/issues/65098 + set_property(TEST ${test_name} + APPEND + PROPERTY ENVIRONMENT + ASAN_OPTIONS=detect_odr_violation=0) + endif() set_property(TEST ${test_name} PROPERTY TIMEOUT ${CEPH_TEST_TIMEOUT}) # Crimson seastar unittest always run with --smp N to start N threads. By default, crimson seastar unittest From 02155b4ffc34c530cd42b8bc4c10540426df75e2 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sun, 24 Mar 2024 23:20:38 +0800 Subject: [PATCH 7/8] cmake: suppress LeakSanitizer reports of known leaks there are known leaks, which are tracked by qa/lsan.suppr, in Ceph. so let's reuse it so we don't see them when running unit test with ASan enabled. see also https://clang.llvm.org/docs/AddressSanitizer.html#issue-suppression Signed-off-by: Kefu Chai --- cmake/modules/AddCephTest.cmake | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmake/modules/AddCephTest.cmake b/cmake/modules/AddCephTest.cmake index 4593070fe17de..ab4dc63ca32a2 100644 --- a/cmake/modules/AddCephTest.cmake +++ b/cmake/modules/AddCephTest.cmake @@ -32,7 +32,8 @@ function(add_ceph_test test_name test_path) set_property(TEST ${test_name} APPEND PROPERTY ENVIRONMENT - ASAN_OPTIONS=detect_odr_violation=0) + ASAN_OPTIONS=detect_odr_violation=0 + LSAN_OPTIONS=suppressions=${CMAKE_SOURCE_DIR}/qa/lsan.supp) endif() set_property(TEST ${test_name} PROPERTY TIMEOUT ${CEPH_TEST_TIMEOUT}) From 1a80165e4acae2a9c2c47c9f490018462a266f6e Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Mon, 25 Mar 2024 00:15:15 +0800 Subject: [PATCH 8/8] qa/lsan.supp: suppress MallocExtension::Initialize LeakSanitizer reports ``` ==688591==ERROR: LeakSanitizer: detected memory leaks Direct leak of 45 byte(s) in 1 object(s) allocated from: #0 0x55f8dd9969dd in operator new(unsigned long) (/home/jenkins-build/build/workspace/ceph-pull-requests/build/bin/unittest_fastbmap_allocator+0x1f89dd) (BuildId: cac39eac8ef1e8774f9dd48e6e3f677fdd864776) #1 0x55f8dd99c730 in __gnu_cxx::new_allocator::allocate(unsigned long, void const*) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/ext/new_allocator.h:127:27 #2 0x55f8dd99c690 in std::allocator::allocate(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/allocator.h:185:32 #3 0x55f8dd99c690 in std::allocator_traits >::allocate(std::allocator&, unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/alloc_traits.h:464:20 #4 0x55f8dd99c393 in std::__cxx11::basic_string, std::allocator >::_M_create(unsigned long&, unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/basic_string.tcc:153:14 #5 0x55f8dda96a6c in std::__cxx11::basic_string, std::allocator >::_M_mutate(unsigned long, unsigned long, char const*, unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/basic_string.tcc:307:21 #6 0x55f8dda96852 in std::__cxx11::basic_string, std::allocator >::_M_append(char const*, unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/basic_string.tcc:395:8 #7 0x7f4a751ab6f0 in MallocExtension::Initialize() (/lib/x86_64-linux-gnu/libtcmalloc.so.4+0x2a6f0) (BuildId: eeef3d1257388a806e122398dbce3157ee568ef4) ``` this is a global object allocated by the allocator, so we can suppress this report. Signed-off-by: Kefu Chai --- qa/lsan.supp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qa/lsan.supp b/qa/lsan.supp index f63c4cf0e1e47..c7d6cf59ed113 100644 --- a/qa/lsan.supp +++ b/qa/lsan.supp @@ -3,6 +3,9 @@ # LSAN_OPTIONS="suppressions=../qa/lsan.supp" # export ASAN_OPTIONS="detect_odr_violation=0" +# gperftools allocates a singleton of MallocExtension and never frees it +leak:^MallocExtension::Initialize + # from perfglue/heap_profiler.cc # gperftools allocates a singleton and never frees it leak:^InitModule