Fix fix_self_intersections/compute_union infinite mutual recursion

.github/workflows/build.yml

@@ -34,4 +34,4 @@ jobs:
         cmake --install build --config Release --prefix install
     - name: Run unit tests
       working-directory: build/test
-      run: ctest --output-on-failure --parallel
+      run: ctest --output-on-failure --timeout 120

data/tests/no_fit_polygon/001.json

@@ -0,0 +1,136 @@
+{
+    "description": "Regression test for a stack overflow found via random fuzzing of the library: no_fit_polygon() used to overflow the stack through 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 that, recursed into, just reassembled the same degenerate face every time. Fixed by dropping (rather than recursing into compute_union() for) any piece -- or the top-level shape itself, which can also reach this function directly through compute_union()'s Union cleanup pass, unfiltered by the per-piece check -- whose area is not strictly positive, plus a hard recursion-depth backstop, since 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, all differing from what's recorded below. Given that, this fixture intentionally does NOT assert exact output equality (only that no_fit_polygon() completes without crashing) -- the exact result isn't portable across platforms/compilers for this input, only the absence of a crash is.",
+    "shapes": [
+        {
+            "elements": [
+                {
+                    "center": {
+                        "x": 596.6756121721289,
+                        "y": 71.63481824124459
+                    },
+                    "end": {
+                        "x": 559.551488564542,
+                        "y": 96.33731405457867
+                    },
+                    "orientation": "Anticlockwise",
+                    "start": {
+                        "x": 614.6052878447334,
+                        "y": 30.80663455259144
+                    },
+                    "type": "CircularArc"
+                },
+                {
+                    "center": {
+                        "x": 558.9496597051063,
+                        "y": 78.14552561903446
+                    },
+                    "end": {
+                        "x": 552.0845441064494,
+                        "y": 61.28807764264352
+                    },
+                    "orientation": "Anticlockwise",
+                    "start": {
+                        "x": 559.551488564542,
+                        "y": 96.33731405457867
+                    },
+                    "type": "CircularArc"
+                },
+                {
+                    "end": {
+                        "x": 489.27750535723123,
+                        "y": 50.72199181903214
+                    },
+                    "start": {
+                        "x": 552.0845441064494,
+                        "y": 61.28807764264352
+                    },
+                    "type": "LineSegment"
+                },
+                {
+                    "center": {
+                        "x": 579.7572763643014,
+                        "y": 58.18915425144982
+                    },
+                    "end": {
+                        "x": 583.408046445965,
+                        "y": -32.524788194542566
+                    },
+                    "orientation": "Anticlockwise",
+                    "start": {
+                        "x": 489.27750535723123,
+                        "y": 50.72199181903214
+                    },
+                    "type": "CircularArc"
+                },
+                {
+                    "end": {
+                        "x": 614.6052878447334,
+                        "y": 30.80663455259144
+                    },
+                    "start": {
+                        "x": 583.408046445965,
+                        "y": -32.524788194542566
+                    },
+                    "type": "LineSegment"
+                }
+            ],
+            "is_path": false,
+            "type": "general"
+        },
+        {
+            "elements": [
+                {
+                    "center": {
+                        "x": 556.664115764325,
+                        "y": 565.2825229299746
+                    },
+                    "end": {
+                        "x": 544.4190468640979,
+                        "y": 568.023194944327
+                    },
+                    "orientation": "Anticlockwise",
+                    "start": {
+                        "x": 569.0846255855816,
+                        "y": 567.0668803049498
+                    },
+                    "type": "CircularArc"
+                },
+                {
+                    "end": {
+                        "x": 571.099732320355,
+                        "y": 521.3846451236235
+                    },
+                    "start": {
+                        "x": 544.4190468640979,
+                        "y": 568.023194944327
+                    },
+                    "type": "LineSegment"
+                },
+                {
+                    "end": {
+                        "x": 576.2041253240814,
+                        "y": 484.71998432343
+                    },
+                    "start": {
+                        "x": 571.099732320355,
+                        "y": 521.3846451236235
+                    },
+                    "type": "LineSegment"
+                },
+                {
+                    "end": {
+                        "x": 569.0846255855816,
+                        "y": 567.0668803049498
+                    },
+                    "start": {
+                        "x": 576.2041253240814,
+                        "y": 484.71998432343
+                    },
+                    "type": "LineSegment"
+                }
+            ],
+            "is_path": false,
+            "type": "general"
+        }
+    ]
+}

src/clean.cpp

@@ -910,18 +910,89 @@ std::vector<Shape> shape::clean_extreme_slopes_inner(
     return {shape};
 }
 
+namespace
+{
+
+// True iff 'shape' has no meaningful area -- a numerical artifact of
+// splitting an (essentially) degenerate self-intersecting shape, not real
+// geometry. Compares the square root of the area, not the area itself:
+// equal() et al.'s fixed absolute tolerance is calibrated for length-scale
+// quantities (see the identical std::sqrt(...) in convex_hull's own area
+// sanity check), and area, being a length squared, can carry noise far
+// above that tolerance even for a shape that is genuinely degenerate --
+// comparing it directly would make this check both too strict (rejecting a
+// coordinate-scale-appropriate sliver of area right at the tolerance
+// boundary) and too loose (missing a larger-magnitude, but still purely
+// noise-driven, area at bigger coordinate scales).
+bool is_degenerate(const ShapeWithHoles& shape)
+{
+    AreaDbl area = shape.shape.compute_area();
+    return area <= 0.0 || !strictly_greater(std::sqrt(area), 0.0);
+}
+
+}
+
 MultiShapeWithHoles shape::fix_self_intersections(
         const ShapeWithHoles& shape)
 {
     //std::cout << "fix_self_intersections" << std::endl;
     //Writer().add_shape_with_holes(shape).write_json("fix_self_intersections_input.json");
+
+    // fix_self_intersections and compute_union's own Union cleanup pass
+    // call each other; below, dropping degenerate pieces before recursing
+    // into compute_union() for them is meant to guarantee this terminates,
+    // but that comparison is a near-the-tolerance floating-point judgment
+    // call, and this codebase has already hit real, platform-dependent
+    // (e.g. FMA fusion differing between x86-64 and AArch64) differences in
+    // results this close to the noise floor for the exact same input (see
+    // offset_test.cpp's expected_output_variants) -- confirmed directly:
+    // enabling FMA contraction on this exact fuzz-found input changes which
+    // pieces bridge_touching_holes() produces entirely. Cap the recursion
+    // depth as a hard backstop so a case that dodges the heuristic below on
+    // some platform/compiler fails cleanly instead of overflowing the
+    // stack.
+    static thread_local int recursion_depth = 0;
+    struct RecursionDepthGuard
+    {
+        RecursionDepthGuard(int& depth): depth(depth) { ++depth; }
+        ~RecursionDepthGuard() { --depth; }
+        int& depth;
+    } recursion_depth_guard(recursion_depth);
+    if (recursion_depth > 256) {
+        throw std::runtime_error(
+                FUNC_SIGNATURE + ": "
+                "recursion depth exceeded; the shape's self-intersections "
+                "don't appear to converge.");
+    }
+
+    // Mirror the per-piece check in the loop below, for 'shape' itself:
+    // compute_union()'s own Union cleanup pass calls back into
+    // fix_self_intersections() unconditionally, without going through that
+    // loop, so a degenerate shape can reach here directly -- confirmed via
+    // instrumented tracing (a ~1e-12-area shape reached this exact entry
+    // point that way). Left unfiltered, its own bridge_touching_holes()
+    // call could split it into equally-degenerate pieces and cycle forever,
+    // the same as if it had arrived through the loop below.
+    if (is_degenerate(shape))
+        return {};
+
     std::vector<ShapeWithHoles> shapes = bridge_touching_holes(shape).shapes_with_holes;
     if (shapes.size() == 1)
         return {{shape}};
     MultiShapeWithHoles output;
     for (ShapePos shape_pos = 0;
             shape_pos < (ShapePos)shapes.size();
             ++shape_pos) {
+        // See is_degenerate()'s comment. Splitting a degenerate shape can,
+        // for some inputs, repeatedly reassemble into the exact same
+        // degenerate shape (found via fuzzing: bridge_touching_holes kept
+        // splitting a ~1e-12-area 7-element self-intersecting shape into
+        // the same two ~1e-12-area pieces every time), so recursing into
+        // compute_union here (whose own Union cleanup pass calls back into
+        // fix_self_intersections) never terminates. Drop degenerate pieces
+        // instead of recursing into them.
+        if (is_degenerate(shapes[shape_pos]))
+            continue;
         MultiShapeWithHoles u = compute_union({shapes[shape_pos]});
         if (!u.shapes_with_holes.empty())
             output.shapes_with_holes.push_back(u.shapes_with_holes.front());

test/no_fit_polygon_test.cpp

@@ -335,6 +335,34 @@ INSTANTIATE_TEST_SUITE_P(
             return info.param.name;
         });
 
+// Regression test for a stack overflow found via random fuzzing of the
+// library: no_fit_polygon() used to overflow the stack through unbounded
+// mutual recursion between fix_self_intersections() and compute_union()
+// (compute_union()'s own Union cleanup pass calls back into
+// fix_self_intersections()) -- see the JSON fixture's description for the
+// confirmed root cause and fix.
+//
+// Unlike the other regression tests above, this one does not assert an
+// exact expected output: 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. Only the absence of a crash is
+// portable here, so that's all this checks.
+TEST(NoFitPolygonGeneralTest, FixSelfIntersectionsInfiniteRecursionDoesNotCrash)
+{
+    std::ifstream file((fs::path("data") / "tests" / "no_fit_polygon" / "001.json").string());
+    ASSERT_TRUE(file.good());
+    nlohmann::json json;
+    file >> json;
+    ShapeWithHoles fixed_shape = ShapeWithHoles::from_json(json["shapes"][0]);
+    ShapeWithHoles orbiting_shape = ShapeWithHoles::from_json(json["shapes"][1]);
+
+    std::vector<ShapeWithHoles> nfp = no_fit_polygon(fixed_shape, orbiting_shape).shapes_with_holes;
+    std::cout << "nfp (" << nfp.size() << " component(s))" << std::endl;
+    for (const ShapeWithHoles& component: nfp)
+        std::cout << "  " << component.to_string(0) << std::endl;
+}
+
 
 ////////////////////////////////////////////////////////////////////////////////
 // Convex overload with circular arcs (decompose_into_basic_shapes' circular