Commit 97e1b5e
fix(bb): clamp BatchMergeProver degree-check loop to fix nightly debug SIGABRT (#23019)
## Summary
Unblocks the `Nightly Debug Build` workflow, which has been failing
again on `next` after #22937 landed. With the `gemini_masking_poly` fix
in #22937 the dsl_tests abort site is gone; the next abort site is in
`BatchMergeProver`, hit by
`BatchMergeTests/{2,3}.TooManySubtablesFails`.
Most recent failing run:
https://github.com/AztecProtocol/aztec-packages/actions/runs/25478845904
(b30fe8f, exit 134, 7m46s).
## Root cause
`TooManySubtablesFails` deliberately calls `BB_DISABLE_ASSERTS()` so it
can drive the verifier with `N = max_subtables + 1` subtables. With BB
asserts demoted to warnings, control flow falls past the
`BB_ASSERT_LTE(N, M, ...)` guard at the top of
`BatchMergeProver::construct_proof` and reaches
`compute_degree_check_polynomial`, which iterates over
`flattened_columns.size()` (sized for `N`) but indexes
`degree_check_challenges` (sized for `(M+1) * NUM_WIRES`).
In release the over-read returns garbage that the verifier rejects —
exactly what the test wants. In debug, libstdc++'s `_GLIBCXX_DEBUG` (set
by the `debug` preset's `CXXFLAGS`) makes `std::vector::operator[]`
bounds-checked and the OOB at `idx == 40` traps:
```
Error: attempt to subscript container with out-of-bounds index 40,
but container only holds 40 elements.
timeout: the monitored command dumped core
```
`BB_DISABLE_ASSERTS()` can't suppress this — the bounds check is
libstdc++'s, not bb's.
## Fix
Clamp the loop to `min(flattened_columns.size(),
degree_check_challenges.size())`. Normal paths are unaffected because
both sizes equal `(M+1) * NUM_WIRES` whenever `N <= M` (the assert
holds). The misuse path now produces a partial degree-check polynomial
that the verifier still rejects in both Debug and Release — same
observable behaviour, no UB.
## Verification
Reproduced inside the ClaudeBox container on `b30fe8f401` with the same
flags as the nightly:
```bash
cd barretenberg/cpp
rm -rf build-debug
CXXFLAGS="-gdwarf-4 -D_GLIBCXX_DEBUG" CC=clang-20 CXX=clang++-20 \
cmake -S . -B build-debug -G Ninja \
-DCMAKE_BUILD_TYPE=Debug -DENABLE_ASAN=OFF -DDISABLE_ASM=OFF -DENABLE_STACKTRACES=ON
cd build-debug && ninja goblin_tests
./bin/goblin_tests --gtest_filter='*TooManySubtablesFails*'
```
- Pre-fix: aborts on `BatchMergeTests/2.TooManySubtablesFails` with the
OOB shown above (exit 134).
- Post-fix: 2 passed, 2 skipped (the native-curve cases self-skip).
Full `goblin_tests`: 68 passed / 7 skipped / 0 failed.
## Relationship to other PRs
- #22937 (merged) fixed the prior abort site (`gemini_masking_poly`
virtual size). Without it the build never reached `BatchMergeProver`;
with it, this is the next abort site.
- #22976 is a draft from an earlier ClaudeBox session carrying the same
one-file change against an older base. This PR re-opens the fix on top
of current `next` (`b30fe8f401`) and is ready for review so it doesn't
sit silently in draft. Once this merges, #22976 can be closed.
- #22811 (BB_DISABLE_ASSERTS workaround for
`HonkRecursionConstraintTestWithoutPredicate.GenerateVKFromConstraints`)
is obsoleted by #22937 and intentionally not included.
Detailed analysis:
https://gist.github.com/AztecBot/3ea741b9545337f7a3adcfb60b088378
ClaudeBox log: https://claudebox.work/s/6f2f86264b1de77e?run=1
ClaudeBox log: https://claudebox.work/s/6f2f86264b1de77e?run=1
---------
Co-authored-by: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com>1 parent b30fe8f commit 97e1b5e
1 file changed
Lines changed: 14 additions & 0 deletions
Lines changed: 14 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
238 | 238 | | |
239 | 239 | | |
240 | 240 | | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
241 | 248 | | |
242 | 249 | | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
243 | 257 | | |
244 | 258 | | |
245 | 259 | | |
| |||
0 commit comments