Add UBSan build support and UBSan CI workflow (apache#2298)

Co-authored-by: hulk <[email protected]> Co-authored-by: Twice <[email protected]> Co-authored-by: Twice <[email protected]>
Beihao-Zhou · May 13, 2024 · a538b90 · a538b90
1 parent a0dae44
commit a538b90
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 7 deletions.
diff --git a/.github/workflows/kvrocks.yaml b/.github/workflows/kvrocks.yaml
@@ -166,6 +166,11 @@ jobs:
             without_jemalloc: -DDISABLE_JEMALLOC=ON
             compiler: clang
             ignore_when_tsan: -tags="ignore_when_tsan"
+          - 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

diff --git 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,33 @@ 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")
+        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")
+        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()
+    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")
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=thread")

diff --git 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();
@@ -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<std::chrono::microseconds>(end - start).count();
-        auto shortest = static_cast<uint64_t>(static_cast<double>(file_size) /
-                                              static_cast<double>(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<uint64_t>(static_cast<double>(file_size) /
+                                                static_cast<double>(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<std::chrono::seconds>();

diff --git a/src/search/ir_iterator.h b/src/search/ir_iterator.h
@@ -20,6 +20,7 @@
 
 #pragma once
 
+#include <array>
 #include <functional>
 #include <memory>
 #include <variant>