From 0dd7d75878f6102123a1fa0fbfc6df5b1619ab0c Mon Sep 17 00:00:00 2001 From: Dan Crosby Date: Fri, 19 Jun 2026 22:11:02 -0700 Subject: [PATCH] Improve Shardok C++ safety checks (#7345) --- .bazelrc | 26 +++++++++++++++ .clang-tidy | 32 +++++++++++++++++++ .../shardok/server/ShardokGamesManager.cpp | 5 +-- 3 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 .clang-tidy diff --git a/.bazelrc b/.bazelrc index bfaf86683d..28b32a0af9 100644 --- a/.bazelrc +++ b/.bazelrc @@ -22,6 +22,32 @@ common --per_file_copt=src/test/cpp/.*@-Wno-sign-compare common --per_file_copt=src/test/cpp/.*@-Wno-unknown-warning-option common --host_cxxopt="--std=c++23" +# C++ sanitizer configs for targeted Shardok safety checks. +# Example: bazel test --config=asan //src/test/cpp/net/eagle0/shardok/library/... +build:asan --compilation_mode=dbg +build:asan --strip=never +build:asan --copt=-fno-omit-frame-pointer +build:asan --copt=-Wno-macro-redefined +build:asan --copt=-fsanitize=address +build:asan --linkopt=-fsanitize=address +test:asan --test_env=ASAN_OPTIONS=detect_leaks=0:strict_init_order=1 + +build:ubsan --compilation_mode=dbg +build:ubsan --strip=never +build:ubsan --copt=-fno-omit-frame-pointer +build:ubsan --copt=-Wno-macro-redefined +build:ubsan --copt=-fsanitize=undefined +build:ubsan --linkopt=-fsanitize=undefined +test:ubsan --test_env=UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1 + +build:tsan --compilation_mode=dbg +build:tsan --strip=never +build:tsan --copt=-fno-omit-frame-pointer +build:tsan --copt=-Wno-macro-redefined +build:tsan --copt=-fsanitize=thread +build:tsan --linkopt=-fsanitize=thread +test:tsan --test_env=TSAN_OPTIONS=halt_on_error=1 + common --javacopt="-Xlint:-options" # Prefer protobuf's prebuilt protoc toolchain instead of building protoc from diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000000..13c03ff6a3 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,32 @@ +Checks: > + -*, + bugprone-*, + performance-*, + readability-*, + modernize-*, + cppcoreguidelines-*, + -bugprone-easily-swappable-parameters, + -cppcoreguidelines-avoid-magic-numbers, + -cppcoreguidelines-macro-usage, + -cppcoreguidelines-owning-memory, + -cppcoreguidelines-pro-bounds-array-to-pointer-decay, + -cppcoreguidelines-pro-bounds-constant-array-index, + -cppcoreguidelines-pro-bounds-pointer-arithmetic, + -cppcoreguidelines-pro-type-const-cast, + -cppcoreguidelines-pro-type-reinterpret-cast, + -cppcoreguidelines-pro-type-union-access, + -cppcoreguidelines-pro-type-vararg, + -modernize-use-trailing-return-type, + -readability-convert-member-functions-to-static, + -readability-function-cognitive-complexity, + -readability-identifier-length, + -readability-magic-numbers +WarningsAsErrors: '' +HeaderFilterRegex: 'src/(main|test)/cpp/net/eagle0/(shardok|common)/.*' +FormatStyle: file +CheckOptions: + readability-braces-around-statements.ShortStatementLines: '1' + readability-function-size.LineThreshold: '160' + readability-function-size.StatementThreshold: '80' + readability-function-size.BranchThreshold: '20' + modernize-use-nullptr.NullMacros: 'NULL' diff --git a/src/main/cpp/net/eagle0/shardok/server/ShardokGamesManager.cpp b/src/main/cpp/net/eagle0/shardok/server/ShardokGamesManager.cpp index bacf29168c..bae8603bd0 100644 --- a/src/main/cpp/net/eagle0/shardok/server/ShardokGamesManager.cpp +++ b/src/main/cpp/net/eagle0/shardok/server/ShardokGamesManager.cpp @@ -9,6 +9,7 @@ #include "ShardokGamesManager.hpp" #include +#include #include #include @@ -68,7 +69,7 @@ auto ShardokGamesManager::UnlockedCreateFromGameStateBytes( const byte_vector &gameStateBytes, int32_t startingHistoryCount, const string &mapName) -> GameId { - std::shared_lock lk(runningControllersLock); + std::unique_lock lk(runningControllersLock); auto gs = GameStateW::FromByteVector(gameStateBytes); @@ -90,7 +91,7 @@ auto ShardokGamesManager::UnlockedCreateSpecifiedGame( const string &serializedRequest, const std::vector>> &watcherAllies) -> GameId { // Check for existing game with this ID and early-exit - std::shared_lock lk(runningControllersLock); + std::unique_lock lk(runningControllersLock); const auto existingGame = runningControllers.find(gameId); // If it exists, register new handlers and early-exit