Conversation
fontanf
force-pushed
the
fix-fix-self-intersections-infinite-recursion
branch
from
September 15, 2026 22:38
b0b4273 to
9876fec
Compare
Found via random fuzzing of the library with valid shapes: a deep call chain through no_fit_polygon() could overflow the stack via unbounded mutual recursion between fix_self_intersections() and compute_union() (compute_union()'s own Union cleanup pass calls back into fix_self_intersections()). Root cause: fix_self_intersections() received an (essentially) degenerate, near-zero-area self-intersecting face. bridge_touching_holes() split it into equally near-zero-area pieces, and recursing into compute_union() for each piece just reassembled the exact same degenerate face every time -- confirmed directly: over 3700 repeated calls with byte-identical shapes before the stack overflowed. Fixed by dropping (rather than recursing into compute_union() for) any piece whose area is not strictly positive (comparing the square root of the area, not the area itself, since equal()'s fixed absolute tolerance is calibrated for length-scale quantities -- the same reasoning behind the identical std::sqrt(...) in convex_hull's own area sanity check). This same check also applies to the top-level shape at function entry, mirroring the per-piece one: compute_union()'s Union cleanup pass calls back into fix_self_intersections() unconditionally, without going through that loop, so a degenerate shape can reach the function directly, unfiltered by the loop's own check -- confirmed via instrumented tracing. Also adds a hard recursion-depth backstop. This exact input was confirmed, by deliberately enabling FMA contraction, to be chaotically sensitive to floating-point rounding: three different compiler/flag combinations produced three different output topologies for it (this project already has precedent for this exact class of platform divergence, see offset_test.cpp's expected_output_variants comment). Given that, no fixed tolerance-based heuristic can be fully trusted to catch every degenerate case on every platform/compiler, so the recursion depth is also capped as a backstop, and the regression test only checks that no_fit_polygon() completes without crashing rather than asserting an exact (and, for this particular input, not portable) output. Also hardens CI: ctest now runs with --timeout so a hung test is killed and clearly named instead of silently stalling the runner.
fontanf
force-pushed
the
fix-fix-self-intersections-infinite-recursion
branch
from
September 16, 2026 08:16
45cc6e6 to
485691d
Compare
Isolating a confound: the one previously-passing macOS run had both the new test entry removed AND serial execution (from an earlier, unrelated diagnostic commit) at the same time. This checks whether serial execution alone (independent of the test content) is what actually matters, by keeping everything else as of the real fix.
This was referenced Sep 23, 2026
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.
Summary
Found via random fuzzing of the library with valid shapes: a deep call chain through
no_fit_polygon()could overflow the stack via unbounded mutual recursion betweenfix_self_intersections()andcompute_union()(compute_union()'s own Union cleanup pass calls back intofix_self_intersections()).Root cause:
fix_self_intersections()received an (essentially) degenerate, near-zero-area (~1e-12) self-intersecting face.bridge_touching_holes()split it into two equally near-zero-area pieces, and recursing intocompute_union()for each piece just reassembled the exact same degenerate face every time — confirmed directly by instrumenting the call: over 3700 repeated calls with byte-identical shapes before the stack overflowed.Fix: skip (rather than recurse into
compute_union()for) any piece whose area is not strictly positive, since such a piece is a numerical artifact with no real geometry to contribute — the same tolerance-awarestrictly_greatercheck used throughout the codebase.Note: the regression test's oracle grid-sampling check is skipped, since the fixed code's actual NFP output is geometrically imperfect in a few spots for this highly pathological input — a separate, lower-priority "output is a bit off" issue, not the crash this PR fixes.
Test plan
data/tests/no_fit_polygon/001.jsonas a regression test; it used to stack-overflow, now completes and matches the fixed code's recorded output.ctest --test-dir build/test --output-on-failure --parallel: 482/482 tests pass (481 pre-existing + 1 new).