From fa22dd09f9733d8c1304c95baef5395e70fc4489 Mon Sep 17 00:00:00 2001 From: mwish Date: Tue, 7 May 2024 13:59:03 +0800 Subject: [PATCH 01/13] Add basic ubsan support --- CMakeLists.txt | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 595665386d3..585e56626be 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -23,6 +23,7 @@ project(kvrocks option(DISABLE_JEMALLOC "disable use of the jemalloc library" OFF) option(ENABLE_ASAN "enable address sanitizer" OFF) option(ENABLE_TSAN "enable thread sanitizer" OFF) +option(ENABLE_UBSAN "enable undefined behavior sanitizer" OFF) option(ASAN_WITH_LSAN "enable leak sanitizer while address sanitizer is enabled" ON) option(ENABLE_STATIC_LIBSTDCXX "link kvrocks with static library of libstd++ instead of shared library" ON) option(ENABLE_LUAJIT "enable use of luaJIT instead of lua" ON) @@ -91,6 +92,34 @@ if(ENABLE_ASAN) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address") set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address") endif() + +# Copied from https://github.com/apache/arrow/blob/main/cpp/cmake_modules/san-config.cmake +# +# Flag to enable clang undefined behavior sanitizer +# We explicitly don't enable all of the sanitizer flags: +# - disable 'vptr' because of RTTI issues across shared libraries (?) +# - disable 'alignment' because unaligned access is really OK on Nehalem and we do it +# all over the place. +# - disable 'function' because it appears to give a false positive +# (https://github.com/google/sanitizers/issues/911) +# - disable 'float-divide-by-zero' on clang, which considers it UB +# (https://bugs.llvm.org/show_bug.cgi?id=17000#c1) +# Note: GCC does not support the 'function' flag. +if(ENABLE_UBSAN) + if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STREQUAL + "Clang") + set(CMAKE_CXX_FLAGS + "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero -fno-sanitize-recover=all" + ) + elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION + VERSION_GREATER_EQUAL "5.1") + set(CMAKE_CXX_FLAGS + "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr -fno-sanitize-recover=all" + ) + else() + message(SEND_ERROR "Cannot use UBSAN without clang or gcc >= 5.1") + endif() +endif() if(ENABLE_TSAN) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=thread") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=thread") From b74da27fac80fb9409a8c27b8a4d0f278220af1c Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 10 May 2024 13:40:34 +0800 Subject: [PATCH 02/13] Allow ubsan build --- .github/workflows/kvrocks.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/kvrocks.yaml b/.github/workflows/kvrocks.yaml index 925d8ad776b..ee4bc136584 100644 --- a/.github/workflows/kvrocks.yaml +++ b/.github/workflows/kvrocks.yaml @@ -147,22 +147,22 @@ jobs: - name: Ubuntu GCC ASan os: ubuntu-20.04 without_jemalloc: -DDISABLE_JEMALLOC=ON - with_sanitizer: -DENABLE_ASAN=ON + with_sanitizer: -DENABLE_ASAN=ON -DENABLE_UBSAN=ON compiler: gcc - name: Ubuntu Clang ASan os: ubuntu-20.04 - with_sanitizer: -DENABLE_ASAN=ON + with_sanitizer: -DENABLE_ASAN=ON -DENABLE_UBSAN=ON without_jemalloc: -DDISABLE_JEMALLOC=ON compiler: clang - name: Ubuntu GCC TSan os: ubuntu-22.04 without_jemalloc: -DDISABLE_JEMALLOC=ON - with_sanitizer: -DENABLE_TSAN=ON + with_sanitizer: -DENABLE_TSAN=ON -DENABLE_UBSAN=ON compiler: gcc ignore_when_tsan: -tags="ignore_when_tsan" - name: Ubuntu Clang TSan os: ubuntu-20.04 - with_sanitizer: -DENABLE_TSAN=ON + with_sanitizer: -DENABLE_TSAN=ON -DENABLE_UBSAN=ON without_jemalloc: -DDISABLE_JEMALLOC=ON compiler: clang ignore_when_tsan: -tags="ignore_when_tsan" From b9ed2b0ff98191d72609852fb57b5494ef0c9da2 Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 10 May 2024 23:25:20 +0800 Subject: [PATCH 03/13] add separate task --- .github/workflows/kvrocks.yaml | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/.github/workflows/kvrocks.yaml b/.github/workflows/kvrocks.yaml index ee4bc136584..4b1e5113912 100644 --- a/.github/workflows/kvrocks.yaml +++ b/.github/workflows/kvrocks.yaml @@ -147,25 +147,35 @@ jobs: - name: Ubuntu GCC ASan os: ubuntu-20.04 without_jemalloc: -DDISABLE_JEMALLOC=ON - with_sanitizer: -DENABLE_ASAN=ON -DENABLE_UBSAN=ON + with_sanitizer: -DENABLE_ASAN=ON compiler: gcc - name: Ubuntu Clang ASan os: ubuntu-20.04 - with_sanitizer: -DENABLE_ASAN=ON -DENABLE_UBSAN=ON + with_sanitizer: -DENABLE_ASAN=ON without_jemalloc: -DDISABLE_JEMALLOC=ON compiler: clang - name: Ubuntu GCC TSan os: ubuntu-22.04 without_jemalloc: -DDISABLE_JEMALLOC=ON - with_sanitizer: -DENABLE_TSAN=ON -DENABLE_UBSAN=ON + with_sanitizer: -DENABLE_TSAN=ON compiler: gcc ignore_when_tsan: -tags="ignore_when_tsan" - name: Ubuntu Clang TSan os: ubuntu-20.04 - with_sanitizer: -DENABLE_TSAN=ON -DENABLE_UBSAN=ON + with_sanitizer: -DENABLE_TSAN=ON without_jemalloc: -DDISABLE_JEMALLOC=ON compiler: clang ignore_when_tsan: -tags="ignore_when_tsan" + - name: Ubuntu GCC UBSAN + os: ubuntu-22.04 + without_jemalloc: -DDISABLE_JEMALLOC=ON + with_sanitizer: -DENABLE_UBSAN=ON + compiler: gcc + - name: Ubuntu Clang UBSAN + os: ubuntu-20.04 + with_sanitizer: -DENABLE_UBSAN=ON + without_jemalloc: -DDISABLE_JEMALLOC=ON + compiler: clang - name: Ubuntu GCC Ninja os: ubuntu-20.04 with_ninja: --ninja From b9f6ad7ae0210a630560b795985b18c22219e747 Mon Sep 17 00:00:00 2001 From: mwish Date: Sat, 11 May 2024 13:16:45 +0800 Subject: [PATCH 04/13] trying to fix the ubsan --- src/commands/cmd_replication.cc | 2 +- src/search/ir_iterator.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/commands/cmd_replication.cc b/src/commands/cmd_replication.cc index 6beffea8c31..1f7baf3f9fb 100644 --- a/src/commands/cmd_replication.cc +++ b/src/commands/cmd_replication.cc @@ -283,7 +283,7 @@ class CommandFetchFile : public Commander { if (srv->IsStopped()) break; uint64_t file_size = 0, max_replication_bytes = 0; - if (srv->GetConfig()->max_replication_mb > 0) { + if (srv->GetConfig()->max_replication_mb > 0 && srv->GetFetchFileThreadNum() != 0) { max_replication_bytes = (srv->GetConfig()->max_replication_mb * MiB) / srv->GetFetchFileThreadNum(); } auto start = std::chrono::high_resolution_clock::now(); diff --git a/src/search/ir_iterator.h b/src/search/ir_iterator.h index 2ead473c190..492735e4bdb 100644 --- a/src/search/ir_iterator.h +++ b/src/search/ir_iterator.h @@ -24,6 +24,7 @@ #include #include #include +#include #include "type_util.h" From 8f155d9e6fd468f1c5791a54eb29c83d8355c9aa Mon Sep 17 00:00:00 2001 From: mwish Date: Sat, 11 May 2024 13:31:22 +0800 Subject: [PATCH 05/13] apply clang-format --- src/search/ir_iterator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/search/ir_iterator.h b/src/search/ir_iterator.h index 492735e4bdb..0730e86ed50 100644 --- a/src/search/ir_iterator.h +++ b/src/search/ir_iterator.h @@ -20,11 +20,11 @@ #pragma once +#include #include #include #include #include -#include #include "type_util.h" From df89995ea786bd125b07b10db0cf60dcf85a01b1 Mon Sep 17 00:00:00 2001 From: mwish Date: Sat, 11 May 2024 15:02:55 +0800 Subject: [PATCH 06/13] fix replication Inf --- src/commands/cmd_replication.cc | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/commands/cmd_replication.cc b/src/commands/cmd_replication.cc index 1f7baf3f9fb..5c46e23d278 100644 --- a/src/commands/cmd_replication.cc +++ b/src/commands/cmd_replication.cc @@ -303,12 +303,14 @@ class CommandFetchFile : public Commander { // Sleep if the speed of sending file is more than replication speed limit auto end = std::chrono::high_resolution_clock::now(); uint64_t duration = std::chrono::duration_cast(end - start).count(); - auto shortest = static_cast(static_cast(file_size) / - static_cast(max_replication_bytes) * (1000 * 1000)); - if (max_replication_bytes > 0 && duration < shortest) { - LOG(INFO) << "[replication] Need to sleep " << (shortest - duration) / 1000 - << " ms since of sending files too quickly"; - usleep(shortest - duration); + if (max_replication_bytes > 0) { + auto shortest = static_cast(static_cast(file_size) / + static_cast(max_replication_bytes) * (1000 * 1000)); + if (duration < shortest) { + LOG(INFO) << "[replication] Need to sleep " << (shortest - duration) / 1000 + << " ms since of sending files too quickly"; + usleep(shortest - duration); + } } } auto now_secs = util::GetTimeStamp(); From 006492f772df1850c8b7d71e66a2b1c186e4472b Mon Sep 17 00:00:00 2001 From: mwish Date: Sat, 11 May 2024 21:29:18 +0800 Subject: [PATCH 07/13] disable gcc ubsan because rocksdb 9.0's issue --- .github/workflows/kvrocks.yaml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/workflows/kvrocks.yaml b/.github/workflows/kvrocks.yaml index 3903bba5589..4bd51b95398 100644 --- a/.github/workflows/kvrocks.yaml +++ b/.github/workflows/kvrocks.yaml @@ -166,11 +166,6 @@ jobs: without_jemalloc: -DDISABLE_JEMALLOC=ON compiler: clang ignore_when_tsan: -tags="ignore_when_tsan" - - name: Ubuntu GCC UBSAN - os: ubuntu-22.04 - without_jemalloc: -DDISABLE_JEMALLOC=ON - with_sanitizer: -DENABLE_UBSAN=ON - compiler: gcc - name: Ubuntu Clang UBSAN os: ubuntu-20.04 with_sanitizer: -DENABLE_UBSAN=ON From 000edded8571d8724462c1e13ebf1a2dc6693f5d Mon Sep 17 00:00:00 2001 From: mwish Date: Sat, 11 May 2024 23:36:54 +0800 Subject: [PATCH 08/13] Update cmake-c-flags and linker flags --- CMakeLists.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 585e56626be..37928e547e6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -111,14 +111,21 @@ if(ENABLE_UBSAN) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero -fno-sanitize-recover=all" ) + set(CMAKE_C_FLAGS + "${CMAKE_C_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero -fno-sanitize-recover=all" + ) elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "5.1") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr -fno-sanitize-recover=all" ) + set(CMAKE_C_FLAGS + "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr -fno-sanitize-recover=all" + ) else() message(SEND_ERROR "Cannot use UBSAN without clang or gcc >= 5.1") endif() + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined") endif() if(ENABLE_TSAN) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=thread") From e4bf7435cebe636b67a299ee5a52848f70473f94 Mon Sep 17 00:00:00 2001 From: mwish Date: Sun, 12 May 2024 21:20:06 +0800 Subject: [PATCH 09/13] apply suggestion --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 37928e547e6..9cdcdf2ae0f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -123,9 +123,9 @@ if(ENABLE_UBSAN) "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr -fno-sanitize-recover=all" ) else() - message(SEND_ERROR "Cannot use UBSAN without clang or gcc >= 5.1") + message(FATAL_ERROR "Cannot use UBSAN without clang or gcc >= 5.1") endif() - set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined") + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined -fno-sanitize-recover=all") endif() if(ENABLE_TSAN) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=thread") From 103eeeb3b3048422ee2ab36c7e31f17395943fed Mon Sep 17 00:00:00 2001 From: mwish Date: Sun, 12 May 2024 23:36:45 +0800 Subject: [PATCH 10/13] apply suggestions --- CMakeLists.txt | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9cdcdf2ae0f..01a5f60a614 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -109,23 +109,25 @@ if(ENABLE_UBSAN) if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang") set(CMAKE_CXX_FLAGS - "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero -fno-sanitize-recover=all" + "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero" ) set(CMAKE_C_FLAGS - "${CMAKE_C_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero -fno-sanitize-recover=all" + "${CMAKE_C_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero" ) elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "5.1") set(CMAKE_CXX_FLAGS - "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr -fno-sanitize-recover=all" + "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr" ) set(CMAKE_C_FLAGS - "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr -fno-sanitize-recover=all" + "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr" ) else() message(FATAL_ERROR "Cannot use UBSAN without clang or gcc >= 5.1") endif() - set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined -fno-sanitize-recover=all") + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-sanitize-recover=all") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-sanitize-recover=all") endif() if(ENABLE_TSAN) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=thread") From d184a4ecbb8db981cf96e6a435655c541fe7b967 Mon Sep 17 00:00:00 2001 From: Twice Date: Mon, 13 May 2024 18:01:04 +0900 Subject: [PATCH 11/13] Update CMakeLists.txt --- CMakeLists.txt | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 01a5f60a614..7b8030dc358 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -116,12 +116,8 @@ if(ENABLE_UBSAN) ) elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "5.1") - set(CMAKE_CXX_FLAGS - "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr" - ) - set(CMAKE_C_FLAGS - "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr" - ) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr") + set(CMAKE_C_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr") else() message(FATAL_ERROR "Cannot use UBSAN without clang or gcc >= 5.1") endif() From 21a101970e1e7e4cc904c2ddd8adbc5b1947c580 Mon Sep 17 00:00:00 2001 From: Twice Date: Mon, 13 May 2024 18:01:38 +0900 Subject: [PATCH 12/13] Update CMakeLists.txt --- CMakeLists.txt | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7b8030dc358..99524cb41e3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -108,12 +108,8 @@ endif() if(ENABLE_UBSAN) if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang") - set(CMAKE_CXX_FLAGS - "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero" - ) - set(CMAKE_C_FLAGS - "${CMAKE_C_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero" - ) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero") elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "5.1") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr") From de44413c9f612b90631f21e1530d0a25eb1be30e Mon Sep 17 00:00:00 2001 From: Twice Date: Mon, 13 May 2024 18:02:27 +0900 Subject: [PATCH 13/13] Update CMakeLists.txt --- CMakeLists.txt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 99524cb41e3..7db1c3aa0f6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -106,12 +106,10 @@ endif() # (https://bugs.llvm.org/show_bug.cgi?id=17000#c1) # Note: GCC does not support the 'function' flag. if(ENABLE_UBSAN) - if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STREQUAL - "Clang") + if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr,function,float-divide-by-zero") - elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION - VERSION_GREATER_EQUAL "5.1") + elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "5.1") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr") set(CMAKE_C_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined -fno-sanitize=alignment,vptr") else()