Automate Address & Thread sanitization

CMakeLists.txt

@@ -98,6 +98,7 @@ endif()
 # Find dependencies
 set(THREADS_PREFER_PTHREAD_FLAG ON)
 find_package(Threads REQUIRED)
+list(APPEND PLATFORM_LIBRARIES Threads::Threads)
 
 # Options (passed to CMake)
 option(REALM_ENABLE_ASSERTIONS "Enable assertions in release mode." OFF)

Jenkinsfile

@@ -60,8 +60,9 @@ timeout(time: 5, unit: 'HOURS') {
                              packageGeneric      : doBuildPackage('generic', 'tgz'),
                              packageCentos7      : doBuildPackage('centos-7', 'rpm'),
                              packageCentos6      : doBuildPackage('centos-6', 'rpm'),
-                             packageUbuntu1604   : doBuildPackage('ubuntu-1604', 'deb')
-                             //threadSanitizer: doBuildInDocker('jenkins-pipeline-thread-sanitizer')
+                             packageUbuntu1604   : doBuildPackage('ubuntu-1604', 'deb'),
+                             threadSanitizer     : doBuildInDocker('Debug', 'thread'),
+                             addressSanitizer    : doBuildInDocker('Debug', 'address')
             ]
 
         androidAbis = ['armeabi-v7a', 'x86', 'mips', 'x86_64', 'arm64-v8a']
@@ -151,24 +152,29 @@ def buildDockerEnv(name) {
     return docker.image(name)
 }
 
-def doBuildInDocker(String buildType) {
+def doBuildInDocker(String buildType, String sanitizeMode='') {
     return {
         node('docker') {
             getArchive()
 
             def buildEnv = docker.build 'realm-core:snapshot'
             def environment = environment()
-            if (buildType.contains('sanitizer')) {
+            def sanitizeFlags = ''
+            environment << 'UNITTEST_PROGRESS=1'
+            if (sanitizeMode.contains('thread')) {
                 environment << 'UNITTEST_THREADS=1'
-                environment << 'UNITTEST_PROGRESS=1'
+                sanitizeFlags = '-D REALM_TSAN=ON'
+            } else if (sanitizeMode.contains('address')) {
+                environment << 'UNITTEST_THREADS=1'
+                sanitizeFlags = '-D REALM_ASAN=ON'
             }
             withEnv(environment) {
                 buildEnv.inside {
                     try {
                         sh """
                            mkdir build-dir
                            cd build-dir
-                           cmake -D CMAKE_BUILD_TYPE=${buildType} -G Ninja ..
+                           cmake -D CMAKE_BUILD_TYPE=${buildType} ${sanitizeFlags} -G Ninja ..
                         """
                         runAndCollectWarnings(script: "cd build-dir && ninja")
                         sh """
@@ -193,6 +199,7 @@ def doAndroidBuildInDocker(String abi, String buildType, boolean runTestsInEmula
             def buildDir = "build-${stashName}".replaceAll('___', '-')
             def buildEnv = docker.build('realm-core-android:snapshot', '-f android.Dockerfile .')
             def environment = environment()
+            environment << 'UNITTEST_PROGRESS=1'
             withEnv(environment) {
                 if(!runTestsInEmulator) {
                     buildEnv.inside {
@@ -276,6 +283,7 @@ def buildDiffCoverage() {
 
             def buildEnv = buildDockerEnv('ci/realm-core:snapshot')
             def environment = environment()
+            environment << 'UNITTEST_PROGRESS=1'
             withEnv(environment) {
                 buildEnv.inside {
                     sh '''

test/fuzz_group.cpp

@@ -207,7 +207,7 @@ Mixed construct_mixed(State& s, util::Optional<std::ostream&> log, std::string&
         }
         case 5: {
             size_t rand_char = get_next(s);
-            size_t blob_size = get_int64(s) % ArrayBlob::max_binary_size;
+            size_t blob_size = static_cast<uint64_t>(get_int64(s)) % ArrayBlob::max_binary_size;
             buffer = std::string(blob_size, static_cast<unsigned char>(rand_char));
             if (log) {
                 *log << "std::string blob(" << blob_size << ", static_cast<unsigned char>(" << rand_char << "));\n"

test/test_shared.cpp

@@ -19,6 +19,7 @@
 #include "testsettings.hpp"
 #ifdef TEST_SHARED
 
+#include <condition_variable>
 #include <streambuf>
 #include <fstream>
 #include <tuple>
@@ -593,6 +594,8 @@ TEST(Shared_try_begin_write)
     // Create a new shared db
     SharedGroup sg(path, false, SharedGroupOptions(crypt_key()));
     std::mutex thread_obtains_write_lock;
+    std::condition_variable cv;
+    std::mutex cv_lock;
     bool init_complete = false;
 
     auto do_async = [&]() {
@@ -601,7 +604,11 @@ TEST(Shared_try_begin_write)
         bool success = sg2.try_begin_write(gw);
         CHECK(success);
         CHECK(gw != nullptr);
-        init_complete = true;
+        {
+            std::lock_guard<std::mutex> lock(cv_lock);
+            init_complete = true;
+        }
+        cv.notify_one();
         TableRef t = gw->add_table(StringData("table"));
         t->insert_column(0, type_String, StringData("string_col"));
         t->add_empty_row(1000);
@@ -615,7 +622,8 @@ TEST(Shared_try_begin_write)
     async_writer.start(do_async);
 
     // wait for the thread to start a write transaction
-    while (!init_complete) { millisleep(1); }
+    std::unique_lock<std::mutex> lock(cv_lock);
+    cv.wait(lock, [&]{ return init_complete; });
 
     // Try to also obtain a write lock. This should fail but not block.
     Group* g = nullptr;
@@ -2414,8 +2422,8 @@ TEST(Shared_EncryptionKeyCheck_3)
 {
     SHARED_GROUP_TEST_PATH(path);
     const char* first_key = crypt_key(true);
-    char second_key[32];
-    memcpy(second_key, first_key, 32);
+    char second_key[64];
+    memcpy(second_key, first_key, 64);
     second_key[3] = ~second_key[3];
     SharedGroup sg(path, false, SharedGroupOptions(first_key));
     bool ok = false;

test/test_thread.cpp

@@ -19,6 +19,7 @@
 #include "testsettings.hpp"
 #ifdef TEST_THREAD
 
+#include <condition_variable>
 #include <cstring>
 #include <algorithm>
 #include <queue>
@@ -535,6 +536,9 @@ TEST(Thread_MutexTryLock)
     Mutex base_mutex;
     std::unique_lock<Mutex> m(base_mutex, std::defer_lock);
 
+    std::condition_variable cv;
+    std::mutex cv_lock;
+
     // basic same thread try_lock
     CHECK(m.try_lock());
     CHECK(m.owns_lock());
@@ -546,7 +550,11 @@ TEST(Thread_MutexTryLock)
         std::unique_lock<Mutex> mutex2(base_mutex, std::defer_lock);
         CHECK(!mutex2.owns_lock());
         CHECK(!mutex2.try_lock());
-        init_done = true;
+        {
+            std::lock_guard<std::mutex> guard(cv_lock);
+            init_done = true;
+        }
+        cv.notify_one();
         while(!mutex2.try_lock()) { millisleep(1); }
         CHECK(mutex2.owns_lock());
         mutex2.unlock();
@@ -557,7 +565,10 @@ TEST(Thread_MutexTryLock)
     CHECK(m.try_lock());
     CHECK(m.owns_lock());
     thread.start(do_async);
-    while (!init_done) { millisleep(1); }
+    {
+        std::unique_lock<std::mutex> guard(cv_lock);
+        cv.wait(guard, [&]{return init_done;});
+    }
     m.unlock();
     thread.join();
 }
@@ -583,17 +594,27 @@ TEST(Thread_RobustMutexTryLock)
     CHECK(times_recover_function_was_called == 0);
 
     bool init_done = false;
+    std::mutex control_mutex;
+    std::condition_variable control_cv;
+
     auto do_async = [&]() {
         CHECK(!m.try_lock(recover_function));
-        init_done = true;
+        {
+            std::lock_guard<std::mutex> guard(control_mutex);
+            init_done = true;
+        }
+        control_cv.notify_one();
         while(!m.try_lock(recover_function)) { millisleep(1); }
         // exit the thread with the lock held to check robustness
     };
 
     // Check basic locking across threads.
     CHECK(m.try_lock(recover_function));
     thread.start(do_async);
-    while (!init_done) { millisleep(1); }
+    {
+        std::unique_lock<std::mutex> lock(control_mutex);
+        control_cv.wait(lock, [&]{ return init_done; });
+    }
     m.unlock();
     thread.join();
     CHECK(times_recover_function_was_called == 0);
@@ -619,20 +640,29 @@ NONCONCURRENT_TEST(Thread_InterprocessMutexTryLock)
     m.unlock();
 
     bool init_done = false;
+    std::condition_variable cv;
+    std::mutex cv_mutex;
     auto do_async = [&]() {
         InterprocessMutex m2;
         m2.set_shared_part(mutex_part, path, mutex_file_name);
 
         CHECK(!m2.try_lock());
-        init_done = true;
+        {
+            std::lock_guard<std::mutex> guard(cv_mutex);
+            init_done = true;
+        }
+        cv.notify_one();
         while(!m2.try_lock()) { millisleep(1); }
         m2.unlock();
     };
 
     // Check basic locking across threads.
     CHECK(m.try_lock());
     thread.start(do_async);
-    while (!init_done) { millisleep(1); }
+    {
+        std::unique_lock<std::mutex> ul(cv_mutex);
+        cv.wait(ul, [&]{return init_done;});
+    }
     m.unlock();
     thread.join();
     m.release_shared_part();
@@ -696,9 +726,17 @@ void waiter_with_count(bowl_of_stones_semaphore* feedback, int* wait_counter, In
 }
 
 
-void waiter(InterprocessMutex* mutex, InterprocessCondVar* cv)
+void waiter(InterprocessMutex* mutex, InterprocessCondVar* cv, std::mutex* control_mutex,
+            std::condition_variable* control_cv, size_t* num_threads_holding_lock)
 {
     std::lock_guard<InterprocessMutex> l(*mutex);
+
+    {
+        std::lock_guard<std::mutex> guard(*control_mutex);
+        *num_threads_holding_lock = (*num_threads_holding_lock) + 1;
+    }
+    control_cv->notify_one();
+
     cv->wait(*mutex, nullptr);
 }
 }
@@ -818,13 +856,26 @@ NONCONCURRENT_TEST(Thread_CondvarNotifyAllWakeup)
     SharedGroupOptions default_options;
     mutex.set_shared_part(mutex_part, path, "");
     changed.set_shared_part(condvar_part, path, "", default_options.temp_dir);
+
+    size_t num_threads_holding_lock = 0;
+    std::mutex control_mutex;
+    std::condition_variable control_cv;
+
     const int num_waiters = 10;
     Thread waiters[num_waiters];
     for (int i = 0; i < num_waiters; ++i) {
-        waiters[i].start(std::bind(waiter, &mutex, &changed));
+        waiters[i].start(std::bind(waiter, &mutex, &changed, &control_mutex, &control_cv, &num_threads_holding_lock));
+    }
+    {
+        // allow all waiters to start and obtain the InterprocessCondVar
+        std::unique_lock<std::mutex> unique_lock(control_mutex);
+        control_cv.wait(unique_lock, [&]{ return num_threads_holding_lock == num_waiters; });
     }
-    millisleep(1000); // allow time for all waiters to wait
+
+    mutex.lock();
     changed.notify_all();
+    mutex.unlock();
+
     for (int i = 0; i < num_waiters; ++i) {
         waiters[i].join();
     }

tools/cmake/SpecialtyBuilds.cmake

@@ -43,8 +43,7 @@ if(REALM_ASAN)
         message(FATAL_ERROR
                 "The Address Sanitizer is not yet supported on Visual Studio builds")
     else()
-        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -fno-omit-frame-pointern -O1 -g")
-        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address")
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -O1 -g")
     endif()
 endif()
 
@@ -57,8 +56,12 @@ if(REALM_TSAN)
         message(FATAL_ERROR
                 "The Thread Sanitizer is not yet supported on Visual Studio builds")
     else()
-        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=thread -fno-omit-frame-pointer -O2 -g -fPIE -pie")
-        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=thread")
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=thread -O2 -g -fPIE")
+        # According to the clang docs, if -fsanitize=thread is specified then compiling
+        # and linking with PIE is turned on automatically.
+        if (CMAKE_COMPILER_IS_GNUXX)
+            set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -pie")
+        endif()
     endif()
 endif()