Skip to content

Fix audit findings L3, L5: LIBSTATS_PORTABLE, cache doc accuracy, remove CachedProperty #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
OldCrow merged 1 commit into from
Jul 5, 2026
Merged

Fix audit findings L3, L5: LIBSTATS_PORTABLE, cache doc accuracy, remove CachedProperty #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 36 additions & 21 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ endif()

# Verbose messaging control
option(LIBSTATS_VERBOSE_BUILD "Enable verbose build messages for debugging" OFF)
# LIBSTATS_PORTABLE: skip the global /arch:AVX512 / /arch:AVX2 flag on Windows MSVC / Clang-cl
# builds so that the binary is safe to distribute across machines with different ISA capabilities.
# Per-file ISA flags on the SIMD kernel TUs (applied by
# cmake/SIMDDetection.cmake:apply_simd_source_flags) remain active and unaffected. Default OFF
# (native performance build). Recommended ON for pylibstats wheel builds. See libstats issue
# tracker for the long-term plan to extend per-file flags to all TUs on Windows.
option(
LIBSTATS_PORTABLE
"Skip global /arch: SIMD flag on Windows builds (safe for distributable wheels; OFF by default)"
OFF)

# Threading system preference control
option(LIBSTATS_FORCE_TBB
Expand Down Expand Up @@ -682,28 +692,33 @@ endif()
# source-file-specific flags (cmake/SIMDDetection.cmake) - All platforms: Definitions are set by
# SIMDDetection.cmake based on detection

# Windows compilers: Use highest detected SIMD level as global flag. SIMDDetection.cmake has already
# run by this point and set LIBSTATS_HAS_AVX512 etc.
if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC" AND CMAKE_SIZEOF_VOID_P EQUAL 8)
if(LIBSTATS_HAS_AVX512)
add_compile_options(/arch:AVX512)
message(STATUS "Applied MSVC x64 SIMD flags: /arch:AVX512")
else()
add_compile_options(/arch:AVX2)
message(STATUS "Applied MSVC x64 SIMD flags: /arch:AVX2")
endif()

elseif(
CMAKE_CXX_COMPILER_ID MATCHES "Clang"
AND WIN32
AND CMAKE_SIZEOF_VOID_P EQUAL 8)
if(LIBSTATS_HAS_AVX512)
add_compile_options(-mavx512f)
message(STATUS "Applied Clang-cl x64 SIMD flags: -mavx512f")
else()
add_compile_options(-mavx2)
message(STATUS "Applied Clang-cl x64 SIMD flags: -mavx2")
# Windows compilers: apply the build-machine SIMD level as a global flag. LIBSTATS_PORTABLE=ON skips
# this block so the binary is distributable to machines with lower ISA than the build host. Per-file
# flags from cmake/SIMDDetection.cmake:apply_simd_source_flags() remain active.
if(NOT LIBSTATS_PORTABLE)
if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC" AND CMAKE_SIZEOF_VOID_P EQUAL 8)
if(LIBSTATS_HAS_AVX512)
add_compile_options(/arch:AVX512)
message(STATUS "Applied MSVC x64 SIMD flags: /arch:AVX512")
else()
add_compile_options(/arch:AVX2)
message(STATUS "Applied MSVC x64 SIMD flags: /arch:AVX2")
endif()
elseif(
CMAKE_CXX_COMPILER_ID MATCHES "Clang"
AND WIN32
AND CMAKE_SIZEOF_VOID_P EQUAL 8)
if(LIBSTATS_HAS_AVX512)
add_compile_options(-mavx512f)
message(STATUS "Applied Clang-cl x64 SIMD flags: -mavx512f")
else()
add_compile_options(-mavx2)
message(STATUS "Applied Clang-cl x64 SIMD flags: -mavx2")
endif()
endif()
else()
message(STATUS "LIBSTATS_PORTABLE=ON: skipping global Windows /arch: flag "
"(SIMD kernel TUs still use per-file flags from SIMDDetection.cmake)")
endif()

# IMPORTANT: SIMD compile definitions are handled by cmake/SIMDDetection.cmake That system detects
Expand Down
59 changes: 17 additions & 42 deletions include/core/distribution_cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,23 @@ namespace stats {
*
* ### Dual-flag pattern (cache_valid_ + cacheValidAtomic_)
* Two flags represent the same state deliberately:
* - cacheValidAtomic_ enables a lock-free fast path in getCachedValue(). An
* acquire load synchronizes-with the release store in updateCacheUnsafe(),
* ensuring cache_valid_ and the cached values are visible before the atomic
* is seen as true.
* - cache_valid_ is the plain bool read under shared_lock in the slow path.
* - cacheValidAtomic_ enables a contention-reduced fast path in
* getCachedValue(): the atomic load (acquire) avoids a lock entirely when
* the cache is stale (proceeding directly to the unique-lock slow path);
* when valid it still acquires shared_lock before reading values. This is
* NOT a fully lock-free read — it is a contention-reduced path that avoids
* the exclusive (write) lock on the hot path.
* - cache_valid_ is the plain bool read under shared_lock; it serves as the
* ground-truth flag for code paths that already hold cache_mutex_.
*
* ### Per-parameter atomic fast paths (derived classes)
* Some derived classes (e.g. GaussianDistribution) expose individual
* parameter atomics (atomicMean_, atomicStdDev_) for lock-free
* getMean()/getStdDev() calls. Each value is individually consistent, but
* reading two atomics across separate calls does not form an atomic pair:
* a concurrent setParameters() between the two reads yields a torn snapshot.
* Callers that need a consistent pair of parameters should use the snapshot
* pattern (shared_lock, read both under the same lock).
*
* v2.0.0 removed the old derived-class cacheValidAtomic_ shadows; this base
* member is now the single atomic cache-validity flag for all distributions.
Expand Down Expand Up @@ -77,41 +89,4 @@ class ThreadSafeCacheManager {
auto getCachedValue(Func&& accessor) const -> decltype(accessor());
};

// =============================================================================
// CACHED PROPERTY TEMPLATE
// =============================================================================

/**
* @brief Template helper for cached statistical properties
* @tparam PropertyType Type of cached property
*
* @warning **Not thread-safe.** This class has no synchronisation of its own.
* Use it only within a scope that already holds an appropriate lock from
* ThreadSafeCacheManager (e.g. under cache_mutex_ unique_lock). Concurrent
* access from multiple threads without external locking is a data race.
* Full per-property atomic protection can be considered in a future v2.x
* cache redesign. v2.0.0 guarantees noexcept moves by not moving mutex/cache
* state and by invalidating/rebuilding caches after moves.
*/
template <typename PropertyType>
class CachedProperty {
private:
mutable PropertyType value_;
mutable bool valid_{false};

public:
template <typename ComputeFunc>
PropertyType get(ComputeFunc&& compute_func) const {
if (!valid_) {
value_ = compute_func();
valid_ = true;
}
return value_;
}

void invalidate() noexcept { valid_ = false; }

bool isValid() const noexcept { return valid_; }
};

} // namespace stats
Loading