Skip to content

Keep bootstrap confidence interval indices in range - #3216

Merged
horenmar merged 1 commit into
catchorg:develfrom
fhgffy:fix/bootstrap-index
Oct 30, 2026
Merged

horenmar merged 1 commit into
catchorg:develfrom
fhgffy:fix/bootstrap-index

Conversation

@fhgffy

@fhgffy fhgffy commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Detail::bootstrap() could index past the end of the resample, as reported in #3214.

If all jackknife estimates are equal, sum_squares is 0 and the acceleration is 0 / 0 = NaN. That makes a1/a2 NaN, std::lround(NaN) returns LONG_MIN, and since hi was only clamped from above it becomes a huge size_t. On Linux with a 64-bit long the index times sizeof(double) wraps around to resample[0], so the upper bound silently comes out below the point estimate; with a 32-bit long (MinGW, as in the issue) it is a real out-of-bounds read and crashes. The same thing happens when every resampled estimate is below the point estimate and the acceleration is non-zero, because bias is then infinite and a(b) turns into inf/inf.

Changes:

  • zero jackknife variance now means zero acceleration instead of NaN
  • both indices are clamped to [0, n - 1]; if the adjusted quantile is still NaN, lo falls back to 0 and hi to n - 1

Added a test case to InternalBenchmark.tests.cpp that calls bootstrap() with both inputs. Before the change it fails with upper_bound 22.0 == 24 and 0.5 == 2 (the wrapped-around resample[0]); with it, it passes. Approval baselines are updated for the new test case, and the full ctest run passes on Linux (GCC 14).

When all jackknife estimates are equal, the acceleration was computed as
0 / 0, the adjusted quantiles became NaN and std::lround(NaN) gave
LONG_MIN. The upper index was only clamped from above, so it ended up as
a huge size_t and resample[hi] read out of bounds. With a 64-bit long
this happens to wrap around to resample[0], with a 32-bit long (MinGW)
it crashes.

Treat zero jackknife variance as zero acceleration, and clamp both
indices to [0, n - 1], falling back to the ends of the resample if the
quantile is still NaN.

Fixes catchorg#3214
@codecov

codecov Bot commented Oct 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.42%. Comparing base (0809213) to head (bb06231).

Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #3216      +/-   ##
==========================================
+ Coverage   91.18%   91.42%   +0.24%     
==========================================
  Files         205      205              
  Lines        9101     9104       +3     
==========================================
+ Hits         8298     8323      +25     
+ Misses        803      781      -22     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@horenmar
horenmar merged commit a1a991f into catchorg:devel Oct 30, 2026
84 checks passed
@horenmar

Copy link
Copy Markdown
Member

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants