Fix audit findings: compile-time guards, denormal thresholds, discrete fit, decodePosterior#57
Merged
Merged
Conversation
C-1 (critical): Fix cache-fill data race in DistributionBase - Add mutable std::mutex cacheMutex_ to DistributionBase<Derived,Obs> - Add ensureCache() CRTP helper: lock-free atomic fast path, mutex- serialized fill path, double-checked locking under the lock - Add friend class DistributionBase<Derived> to all 16 scalar distributions so ensureCache() can call private updateCache() - Replace every if (!isCacheValid()) updateCache() call site in scalar distributions with ensureCache() (~50 sites across 23 files) - DiagonalGaussian and FullCovGaussian already had their own per-class mutex inside updateCache(); left unchanged - Update distribution_base.h comment and STYLE_GUIDE.md to accurately reflect the now-correct thread-safety guarantee H-1 (high): Guard Weibull MLE solver against exponent overflow - Clamp k*log_x[i] to [-700, 700] before std::exp in both the Newton-Raphson loop and the final lambda computation - Extend the break condition to !isfinite(s0) || s0 <= 0 - Add init_k fallback when k diverges to non-finite or non-positive H-2 (high): Remove stale Boost dependency from installed CMake config - Delete find_dependency(Boost REQUIRED) from libhmmConfig.cmake.in; Boost was removed in Phase 8.3 but the install config was not updated, breaking find_package(libhmm) on machines without Boost M-2 (medium): Add ThreadSanitizer CI job - New tsan job on ubuntu-latest/GCC with -fsanitize=thread -g -O1 - Would have caught C-1 directly; prevents future regressions Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses findings 3, 6, 7, 8, 11 from the 2026-07-04 audit, plus pre-existing tool build warnings and a benchmark CMake correctness fix. This is the libhmm half of the fix; the pylibhmm half (Finding 1 — UAF Holder pattern) follows in a separate PR against that repo.
Changes
Finding 3 — Compile-time guard against temporary observation sequences
Add deleted
SeqType&&constructor overloads toBasicForwardBackwardCalculatorandBasicViterbiCalculator. The base-class deletion was ineffective: a temporary bound toconst SeqType&in a derived constructor reached the base as a named lvalue, bypassing the deleted overload. The new derived-class overloads close the gap and will cause the pylibhmm_core.cppbindings to fail to compile at the exact UAF sites (Finding 1), making them self-announcing.Finding 6 — Denormal guard inconsistency in M-step
m_step_transitions(BW trainer) used> 0.0wherem_step_picorrectly uses>= precision::ZERO. Same hazard inm_step_pi_mapandm_step_transitions_map(MAP trainer). All three aligned to>= precision::ZERO.Finding 7 — DiscreteDistribution weighted fit under-normalises
fit(data, weights)divided bysumW(total weight, including out-of-range observations), leaving the pmf summing to < 1 when any observation fell outside the symbol range. Now accumulatesbinnedW(weight that landed in bins) and divides by that. Removes the compensating renormalisation pass fromapply_discrete_smoothingin the MAP trainer.Finding 8 — decodePosterior silent failure on zero-probability sequences
When
logP = -inf, thelogAlpha + logBeta - logPscores were NaN; all comparisons were false, silently returning all-state-0. Now throwsstd::runtime_errorwhen!isfinite(logProbability_).Finding 11 — Hygiene
Add
install/to.gitignore.Benchmark CMake — HMMLib Boost detection
The previous version incorrectly claimed HMMLib has no Boost dependency and set
HMMLIB_READY ONunconditionally whenhmm.hppexisted. HMMLib usesboost::shared_ptrinternally. Now guards withfind_package(Boost QUIET)and propagates${Boost_INCLUDE_DIRS}throughenable_hmmlib().Tool warnings
Fix 5 instances of deprecated compound assignment on
volatile-qualified types (C++20) inhotspot_breakdown,bw_hotspot, andfb_contour_sweep.Verification
Related
_core.cpprefactor/estep-dedupbranchWarp conversation