test(threading): harden ScryptControlledByCheckPowFlag against sanitizer timing jitter - #483
Merged
Merged
Conversation
…zer jitter The wall-clock guard asserted EXPECT_LT(t_fast_us, t_scrypt_us) over only 5 reps with zero margin. The healthy fast-path cost (non-scrypt verify work W) is empirically ~equal to the scrypt baseline (S), so the ratio sits right on the 1.0x boundary and a 2.3% scheduling wobble under ASan/UBSan flips it red (observed 2785us vs 2721us), false-stopping every lane on the shared ASan leg. Harden three ways, test-only and fenced: - Assert the fast loop stays under 1.5x the scrypt baseline (integer form t_fast*2 < t_scrypt*3). A removed check_pow guard makes each fast call run scrypt too (W+S, ~2.0x); 1.5x sits midway between healthy ~1.0x and the regression ~2.0x, absorbing jitter the zero-margin compare could not. - Raise reps 5 -> 40 so per-call scheduling noise averages out. - GTEST_SKIP under sanitizer builds (__SANITIZE_ADDRESS__ / __has_feature), where instrumentation distorts the verify and scrypt paths unequally; the guard stays live on the non-sanitized legs. Stale header comment (20 reps / >1 scrypt call) corrected to match.
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
1 participant
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.
Problem
test/test_threading.cpp::VerifyShareThreading.ScryptControlledByCheckPowFlagis a fragile shared sanitizer test that has been red-ing the ASan+UBSan leg on every lane's PRs (diagnosed on PR #451 — not an nmc regression).It asserted
EXPECT_LT(t_fast_us, t_scrypt_us)over only 5 reps with zero margin. The healthy fast-path cost (non-scrypt verify workW) is empirically ~equal to the scrypt baseline (S), so the ratio sits right on the 1.0x boundary. A 2.3% scheduling wobble under sanitizer instrumentation flips it red (observed 2785µs vs 2721µs) — pure noise, recurring false-stop on the whole fleet.Fix (test-only, fenced)
t_fast*2 < t_scrypt*3). A removedcheck_powguard makes each fast call run scrypt too (W+S, ~2.0x); the 1.5x ceiling sits midway between healthy ~1.0x and the regression ~2.0x, absorbing jitter the zero-margin compare could not.__SANITIZE_ADDRESS__/__has_feature), where instrumentation distorts the verify vs scrypt paths unequally. The guard stays live on the non-sanitized legs, so regression coverage is preserved.Stale header comment (20 reps / ">1 scrypt call") corrected to match.
Verification (workstation, GCC 13, Release)
test_threadingbuilds clean; full suite 7/7 PASS, target test stable across 3 consecutive runs.__SANITIZE_ADDRESS__forced on → compiles (GTEST_SKIP taken on the ASan leg).Label: SAFE — test-only, no production code touched. Commit GPG-signed.