Enable valgrind annotations for gcc build in jenkins

This works around a weird error I was getting when trying to link a
translation unit that included Internal.h with libconflict-set-static.a
with clang + gnu objcopy

CMakeLists.txt

@@ -31,11 +31,24 @@ if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
                                                "MinSizeRel" "RelWithDebInfo")
 endif()
 
-add_compile_options(-fdata-sections -ffunction-sections -Wswitch-enum
+add_compile_options(
-                    -Werror=switch-enum -fPIC)
+  -Werror=switch-enum
+  -Wswitch-enum
+  -Wunused-variable
+  -fPIC
+  -fdata-sections
+  -ffunction-sections
+  -fno-jump-tables # https://github.com/llvm/llvm-project/issues/54247
+)
 
 if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
   add_link_options("-Wno-unused-command-line-argument")
+  find_program(LLVM_OBJCOPY llvm-objcopy)
+  if(LLVM_OBJCOPY)
+    set(CMAKE_OBJCOPY
+        ${LLVM_OBJCOPY}
+        CACHE FILEPATH "path to objcopy binary" FORCE)
+  endif()
 endif()
 
 if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
@@ -56,6 +69,21 @@ if(HAS_FULL_RELRO)
 endif()
 cmake_pop_check_state()
 
+if(CMAKE_SYSTEM_PROCESSOR STREQUAL aarch64)
+  add_compile_options(-mbranch-protection=standard)
+else()
+  add_compile_options(-fcf-protection)
+  set(rewrite_endbr_flags "-fuse-ld=mold;LINKER:-z,rewrite-endbr")
+  cmake_push_check_state()
+  list(APPEND CMAKE_REQUIRED_LINK_OPTIONS ${rewrite_endbr_flags})
+  check_cxx_source_compiles("int main(){}" HAS_REWRITE_ENDBR FAIL_REGEX
+                            "warning:")
+  if(HAS_REWRITE_ENDBR)
+    add_link_options(${rewrite_endbr_flags})
+  endif()
+  cmake_pop_check_state()
+endif()
+
 set(version_script_flags
     LINKER:--version-script=${CMAKE_CURRENT_SOURCE_DIR}/linker.map)
 cmake_push_check_state()
@@ -81,19 +109,11 @@ else()
   add_link_options(-Wl,--gc-sections)
 endif()
 
-if(NOT USE_SIMD_FALLBACK)
+if(USE_SIMD_FALLBACK)
-  cmake_push_check_state()
+  add_compile_definitions(USE_SIMD_FALLBACK)
-  list(APPEND CMAKE_REQUIRED_FLAGS -mavx)
+else()
-  check_include_file_cxx("immintrin.h" HAS_AVX)
+  if(CMAKE_SYSTEM_PROCESSOR STREQUAL x86_64)
-  if(HAS_AVX)
     add_compile_options(-mavx)
-    add_compile_definitions(HAS_AVX)
-  endif()
-  cmake_pop_check_state()
-
-  check_include_file_cxx("arm_neon.h" HAS_ARM_NEON)
-  if(HAS_ARM_NEON)
-    add_compile_definitions(HAS_ARM_NEON)
   endif()
 endif()
 
@@ -323,7 +343,8 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
   # c++98
   add_executable(conflict_set_cxx_api_test conflict_set_cxx_api_test.cpp)
   target_compile_options(conflict_set_cxx_api_test PRIVATE ${TEST_FLAGS})
-  target_link_libraries(conflict_set_cxx_api_test PRIVATE ${PROJECT_NAME})
+  target_link_libraries(conflict_set_cxx_api_test
+                        PRIVATE ${PROJECT_NAME}-static)
   set_target_properties(conflict_set_cxx_api_test PROPERTIES CXX_STANDARD 98)
   set_target_properties(conflict_set_cxx_api_test
                         PROPERTIES CXX_STANDARD_REQUIRED ON)
@@ -356,6 +377,15 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR AND BUILD_TESTING)
         ${symbol_imports})
   endif()
 
+  if(NOT CMAKE_CROSSCOMPILING)
+    find_program(HARDENING_CHECK hardening-check)
+    if(HARDENING_CHECK)
+      add_test(NAME hardening_check
+               COMMAND ${HARDENING_CHECK} $<TARGET_FILE:${PROJECT_NAME}>
+                       --nofortify --nostackprotector)
+    endif()
+  endif()
+
   # bench
   add_executable(conflict_set_bench Bench.cpp)
   target_link_libraries(conflict_set_bench PRIVATE ${PROJECT_NAME} nanobench)

ConflictSet.cpp

@@ -14,6 +14,16 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+#if !defined(USE_SIMD_FALLBACK) && defined(__has_include)
+#if __has_include("immintrin.h")
+#define HAS_AVX 1
+#include <immintrin.h>
+#elif __has_include("arm_neon.h")
+#define HAS_ARM_NEON 1
+#include <arm_neon.h>
+#endif
+#endif
+
 #include "ConflictSet.h"
 #include "Internal.h"
 #include "LongestCommonPrefix.h"
@@ -34,12 +44,6 @@ limitations under the License.
 #include <type_traits>
 #include <utility>
 
-#ifdef HAS_AVX
-#include <immintrin.h>
-#elif defined(HAS_ARM_NEON)
-#include <arm_neon.h>
-#endif
-
 #ifndef __SANITIZE_THREAD__
 #if defined(__has_feature)
 #if __has_feature(thread_sanitizer)
@@ -696,8 +700,6 @@ constexpr int64_t kMaxFreeListBytes = 1 << 20;
 // doesn't meet the capacity constraints, it's freed and a new node is allocated
 // with the minimum capacity. The hope is that "unfit" nodes don't get stuck in
 // the free list.
-//
-// TODO valgrind annotations
 template <class T> struct NodeAllocator {
 
   static_assert(std::derived_from<T, Node>);
@@ -734,6 +736,7 @@ template <class T> struct NodeAllocator {
     p->parent = freeList;
     freeList = p;
     freeListSize += sizeof(T) + p->partialKeyCapacity;
+    VALGRIND_MAKE_MEM_NOACCESS(p, sizeof(T) + p->partialKeyCapacity);
   }
 
   void deferRelease(T *p, Node *forwardTo) {
@@ -755,6 +758,13 @@ template <class T> struct NodeAllocator {
   void releaseDeferred() {
     if (deferredList != nullptr) {
       deferredListFront->parent = freeList;
+#ifndef NVALGRIND
+      for (auto *iter = deferredList; iter != freeList;) {
+        auto *tmp = iter;
+        iter = (T *)iter->parent;
+        VALGRIND_MAKE_MEM_NOACCESS(tmp, sizeof(T) + tmp->partialKeyCapacity);
+      }
+#endif
       freeList = std::exchange(deferredList, nullptr);
     }
     for (T *n = std::exchange(deferredListOverflow, nullptr); n != nullptr;) {
@@ -775,6 +785,7 @@ template <class T> struct NodeAllocator {
     assert(deferredList == nullptr);
     assert(deferredListOverflow == nullptr);
     for (T *iter = freeList; iter != nullptr;) {
+      VALGRIND_MAKE_MEM_DEFINED(iter, sizeof(T));
       auto *tmp = iter;
       iter = (T *)iter->parent;
       removeNode(tmp);
@@ -792,6 +803,7 @@ private:
 
   T *allocate_helper(int minCapacity, int maxCapacity) {
     if (freeList != nullptr) {
+      VALGRIND_MAKE_MEM_DEFINED(freeList, sizeof(T));
       freeListSize -= sizeof(T) + freeList->partialKeyCapacity;
       assume(freeList->partialKeyCapacity >= 0);
       assume(minCapacity >= 0);
@@ -800,6 +812,11 @@ private:
           freeList->partialKeyCapacity <= maxCapacity) {
         auto *result = freeList;
         freeList = (T *)freeList->parent;
+        VALGRIND_MAKE_MEM_UNDEFINED(result,
+                                    sizeof(T) + result->partialKeyCapacity);
+        VALGRIND_MAKE_MEM_DEFINED(&result->partialKeyCapacity,
+                                  sizeof(result->partialKeyCapacity));
+        VALGRIND_MAKE_MEM_DEFINED(&result->type, sizeof(result->type));
         return result;
       } else {
         auto *p = freeList;
@@ -3183,6 +3200,12 @@ Node *firstGeqPhysical(Node *n, const TrivialSpan key) {
 #define PRESERVE_NONE
 #endif
 
+#if __has_attribute(musttail) && __has_attribute(preserve_none)
+constexpr bool kEnableInterleaved = true;
+#else
+constexpr bool kEnableInterleaved = false;
+#endif
+
 namespace check {
 
 typedef PRESERVE_NONE void (*Continuation)(struct Job *, struct Context *);
@@ -4969,51 +4992,50 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     check::Context context;
     context.readContext.impl = this;
 
-#if __has_attribute(musttail)
+    if constexpr (kEnableInterleaved) {
-    if (count == 1) {
+      if (count == 1) {
-      useSequential(reads, result, count, context);
+        useSequential(reads, result, count, context);
-    } else {
+      } else {
-      constexpr int kConcurrent = 16;
+        constexpr int kConcurrent = 16;
-      check::Job inProgress[kConcurrent];
+        check::Job inProgress[kConcurrent];
-      context.count = count;
+        context.count = count;
-      context.oldestVersionFullPrecision = oldestVersionFullPrecision;
+        context.oldestVersionFullPrecision = oldestVersionFullPrecision;
-      context.root = root;
+        context.root = root;
-      context.queries = reads;
+        context.queries = reads;
-      context.results = result;
+        context.results = result;
-      int64_t started = std::min(kConcurrent, count);
+        int64_t started = std::min(kConcurrent, count);
-      context.started = started;
+        context.started = started;
-      for (int i = 0; i < started; i++) {
+        for (int i = 0; i < started; i++) {
-        inProgress[i].init(reads + i, result + i, root,
+          inProgress[i].init(reads + i, result + i, root,
-                           oldestVersionFullPrecision);
+                             oldestVersionFullPrecision);
-      }
+        }
-      for (int i = 0; i < started - 1; i++) {
+        for (int i = 0; i < started - 1; i++) {
-        inProgress[i].next = inProgress + i + 1;
+          inProgress[i].next = inProgress + i + 1;
-      }
+        }
-      for (int i = 1; i < started; i++) {
+        for (int i = 1; i < started; i++) {
-        inProgress[i].prev = inProgress + i - 1;
+          inProgress[i].prev = inProgress + i - 1;
-      }
+        }
-      inProgress[0].prev = inProgress + started - 1;
+        inProgress[0].prev = inProgress + started - 1;
-      inProgress[started - 1].next = inProgress;
+        inProgress[started - 1].next = inProgress;
 
-      // Kick off the sequence of tail calls that finally returns once all jobs
+        // Kick off the sequence of tail calls that finally returns once all
-      // are done
+        // jobs are done
-      inProgress->continuation(inProgress, &context);
+        inProgress->continuation(inProgress, &context);
 
 #ifndef NDEBUG
-      Arena arena;
+        Arena arena;
-      auto *results2 = new (arena) Result[count];
+        auto *results2 = new (arena) Result[count];
-      check::Context context2;
+        check::Context context2;
-      context2.readContext.impl = this;
+        context2.readContext.impl = this;
-      useSequential(reads, results2, count, context2);
+        useSequential(reads, results2, count, context2);
-      assert(memcmp(result, results2, count) == 0);
+        assert(memcmp(result, results2, count) == 0);
-      assert(context.readContext == context2.readContext);
+        assert(context.readContext == context2.readContext);
 #endif
+      }
+    } else {
+      useSequential(reads, result, count, context);
     }
 
-#else
-    useSequential(reads, result, count, context);
-#endif
-
     for (int i = 0; i < count; ++i) {
       assert(reads[i].readVersion >= 0);
       assert(reads[i].readVersion <= newestVersionFullPrecision);
@@ -5186,11 +5208,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     assert(allPointWrites || sorted);
 #endif
 
-#if __has_attribute(musttail)
-    constexpr bool kEnableInterleaved = true;
-#else
-    constexpr bool kEnableInterleaved = false;
-#endif
     if (kEnableInterleaved && count > 1) {
       interleavedWrites(writes, count, InternalVersionT(writeVersion));
     } else {
@@ -5858,13 +5875,13 @@ void checkVersionsGeqOldestExtant(Node *n,
   case Type_Node0: {
   } break;
   case Type_Node3: {
-    auto *self = static_cast<Node3 *>(n);
+    [[maybe_unused]] auto *self = static_cast<Node3 *>(n);
     for (int i = 0; i < 3; ++i) {
       assert(self->childMaxVersion[i] >= oldestExtantVersion);
     }
   } break;
   case Type_Node16: {
-    auto *self = static_cast<Node16 *>(n);
+    [[maybe_unused]] auto *self = static_cast<Node16 *>(n);
     for (int i = 0; i < 16; ++i) {
       assert(self->childMaxVersion[i] >= oldestExtantVersion);
     }
@@ -5874,7 +5891,7 @@ void checkVersionsGeqOldestExtant(Node *n,
     for (int i = 0; i < 48; ++i) {
       assert(self->childMaxVersion[i] >= oldestExtantVersion);
     }
-    for (auto m : self->maxOfMax) {
+    for ([[maybe_unused]] auto m : self->maxOfMax) {
       assert(m >= oldestExtantVersion);
     }
   } break;
@@ -5883,7 +5900,7 @@ void checkVersionsGeqOldestExtant(Node *n,
     for (int i = 0; i < 256; ++i) {
       assert(self->childMaxVersion[i] >= oldestExtantVersion);
     }
-    for (auto m : self->maxOfMax) {
+    for ([[maybe_unused]] auto m : self->maxOfMax) {
       assert(m >= oldestExtantVersion);
     }
   } break;

Dockerfile

@@ -13,12 +13,14 @@ RUN TZ=America/Los_Angeles DEBIAN_FRONTEND=noninteractive apt-get install -y \
     ccache \
     cmake \
     curl \
+    devscripts \
     g++-aarch64-linux-gnu \
     gcovr \
     git \
     gnupg \
     libc6-dbg \
     lsb-release \
+    mold \
     ninja-build \
     pre-commit \
     python3-requests \

Jenkinsfile

@@ -91,7 +91,7 @@ pipeline {
                 minio bucket: 'jenkins', credentialsId: 'jenkins-minio', excludes: '', host: 'minio.weaselab.dev', includes: 'build/*.deb,build/*.rpm,paper/*.pdf', targetFolder: '${JOB_NAME}/${BUILD_NUMBER}/${STAGE_NAME}/'
             }
         }
-        stage('Release [gcc]') {
+        stage('gcc') {
             agent {
                 dockerfile {
                     args '-v /home/jenkins/ccache:/ccache'
@@ -99,7 +99,7 @@ pipeline {
                 }
             }
             steps {
-                CleanBuildAndTest("-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DCMAKE_CXX_FLAGS=-DNVALGRIND")
+                CleanBuildAndTest("-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++")
                 recordIssues(tools: [gcc()])
             }
         }

RealDataBench.cpp

@@ -5,7 +5,7 @@
 #include <cstdio>
 #include <cstring>
 #include <fcntl.h>
-#include <string_view>
+#include <span>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <unistd.h>
@@ -64,7 +64,7 @@ int main(int argc, const char **argv) {
     auto *const mapOriginal = begin;
     const auto sizeOriginal = size;
 
-    using StringView = std::basic_string_view<uint8_t>;
+    using StringView = std::span<const uint8_t>;
 
     StringView write;
     std::vector<StringView> reads;
@@ -78,9 +78,9 @@ int main(int argc, const char **argv) {
       end = (uint8_t *)memchr(begin, '\n', size);
 
       if (line.size() > 0 && line[0] == 'P') {
-        write = line.substr(2, line.size());
+        write = line.subspan(2, line.size());
       } else if (line.size() > 0 && line[0] == 'L') {
-        reads.push_back(line.substr(2, line.size()));
+        reads.push_back(line.subspan(2, line.size()));
       } else if (line.empty()) {
         {
           readRanges.resize(reads.size());

ServerBench.cpp

@@ -64,31 +64,47 @@ template <class... Ts> std::string tupleKey(const Ts &...ts) {
   return result;
 }
 
-constexpr int kWindowSize = 300000;
+constexpr int kTotalKeyRange = 1'000'000'000;
+constexpr int kWindowSize = 1'000'000;
+constexpr int kNumKeys = 10;
 
 void workload(weaselab::ConflictSet *cs) {
   int64_t version = kWindowSize;
-  constexpr int kNumWrites = 16;
   for (;; transactions.fetch_add(1, std::memory_order_relaxed)) {
     std::vector<int64_t> keyIndices;
-    for (int i = 0; i < kNumWrites; ++i) {
+    for (int i = 0; i < kNumKeys; ++i) {
-      keyIndices.push_back(rand() % 100'000'000);
+      keyIndices.push_back(rand() % kTotalKeyRange);
     }
     std::sort(keyIndices.begin(), keyIndices.end());
     std::vector<std::string> keys;
-    std::vector<weaselab::ConflictSet::WriteRange> writes;
+    constexpr std::string_view fullString =
-    constexpr std::string_view suffix = "this is a suffix";
+        "this is a string, where a prefix of it is used as an element of the "
-    for (int i = 0; i < kNumWrites; ++i) {
+        "tuple forming the key";
-      keys.push_back(tupleKey(0x100, i, keyIndices[i],
+    for (int i = 0; i < kNumKeys; ++i) {
-                              suffix.substr(0, rand() % suffix.size()),
+      keys.push_back(
-                              rand()));
+          tupleKey(0x100, keyIndices[i] / fullString.size(),
+                   fullString.substr(0, keyIndices[i] % fullString.size())));
       // printf("%s\n", printable(keys.back()).c_str());
     }
-    for (int i = 0; i < kNumWrites; ++i) {
+
+    std::vector<weaselab::ConflictSet::ReadRange> reads;
+    std::vector<weaselab::ConflictSet::WriteRange> writes;
+    std::vector<weaselab::ConflictSet::Result> results;
+    for (int i = 0; i < kNumKeys; ++i) {
       writes.push_back({{(const uint8_t *)keys[i].data(), int(keys[i].size())},
                         {nullptr, 0}});
+      reads.push_back({{(const uint8_t *)keys[i].data(), int(keys[i].size())},
+                       {nullptr, 0},
+                       version - kWindowSize});
     }
-    cs->addWrites(writes.data(), writes.size(), version);
+    results.resize(reads.size());
+
+    cs->check(reads.data(), results.data(), reads.size());
+    bool ok = true;
+    for (auto result : results) {
+      ok &= result == weaselab::ConflictSet::Commit;
+    }
+    cs->addWrites(writes.data(), ok ? writes.size() : 0, version);
     cs->setOldestVersion(version - kWindowSize);
     ++version;
   }

SkipList.cpp

@@ -767,7 +767,9 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
           false, true);
     }
 
-    sortPoints(points);
+    if (!std::is_sorted(points.begin(), points.end())) {
+      sortPoints(points);
+    }
 
     int activeWriteCount = 0;
     std::vector<std::pair<StringRef, StringRef>> combinedWriteConflictRanges;
@@ -794,7 +796,6 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     int temp[stripeSize];
     int stripes = (stringCount + stripeSize - 1) / stripeSize;
     StringRef values[stripeSize];
-    int64_t writeVersions[stripeSize / 2];
     int ss = stringCount - (stripes - 1) * stripeSize;
     int64_t entryDelta = 0;
     for (int s = stripes - 1; s >= 0; s--) {

apple-symbol-imports.txt

@@ -1,3 +1,4 @@
+___chkstk_darwin
 ___stack_chk_fail
 ___stack_chk_guard
 __tlv_bootstrap
@@ -5,6 +6,7 @@ _abort
 _bzero
 _free
 _malloc
+_memcmp
 _memcpy
 _memmove
 dyld_stub_binder

package_macos.sh

@@ -8,7 +8,7 @@ SRC_DIR="${0%/*}"
 BUILD_ARM="$(mktemp -d -t conflict-set-arm)"
 BUILD_X86="$(mktemp -d -t conflict-set-x86)"
 
-cmake_args=(-DCMAKE_CXX_FLAGS=-DNVALGRIND -DCPACK_PACKAGING_INSTALL_PREFIX=/usr/local)
+cmake_args=(-DCMAKE_CXX_FLAGS=-DNVALGRIND -DCPACK_PACKAGING_INSTALL_PREFIX=/usr/local -DCMAKE_CXX_COMPILER=/opt/homebrew/opt/llvm/bin/clang++)
 
 cmake -S"$SRC_DIR" -B"$BUILD_ARM" -DCMAKE_OSX_ARCHITECTURES=arm64 "${cmake_args[@]}"
 cmake --build "$BUILD_ARM" --target conflict-set --target conflict-set-static