Skip to content

Stale ShapeFix_Face/ShapeFix_FaceConnect cross-reference index entries hide a real, unpatched #317-class SIGSEGV site and an untested wrapper #484

Description

@gsdali

Part of the bridge duplication audit in #381 (Pass 1b, itself a sub-issue of #377 — parallel to Pass 1a / #380). Per #377's ordering rule, triage or fix this before starting #395 (the OCCTBridge.h breakout, which depends on #381). Branch: refactor/381-pass1b — a dedicated branch for Pass 1b, separate from Pass 1a's refactor/377-segmented-audit while that branch is still under code review. PRs from this issue target refactor/381-pass1b, not main.

Duplication site

Cross-reference index (both entries stale/wrong):

  • Sources/OCCTBridge/include/OCCTBridge.h:426 — // ShapeFix_Face → OCCTShapeFixFace — this symbol does not exist anywhere in the codebase (confirmed via grep across Sources/ and Tests/).
  • Sources/OCCTBridge/include/OCCTBridge.h:427 — // ShapeFix_FaceConnect → OCCTShapeFixConnect* — this symbol also does not exist; the real function is OCCTShapeFixFaceConnect (missing the Face infix, and not a wildcard family — it's a single function).

Real ShapeFix_Face call sites (4 total, none named OCCTShapeFixFace):

  1. Sources/OCCTBridge/src/OCCTBridge_Healing.mm:299-325 — OCCTShapeRef OCCTFaceFix(OCCTFaceRef face, double tolerance), declared OCCTBridge.h:1928. Constructs Handle(ShapeFix_Face) fixer = new ShapeFix_Face(face->face) at line 303. Does NOT call SetContext().
  2. Sources/OCCTBridge/src/OCCTBridge_Healing.mm:4326-4342 — OCCTFaceFixerRef OCCTFaceFixerCreate(OCCTShapeRef face, double precision), declared OCCTBridge.h:16781 (struct OCCTFaceFixerRef typedef at 16778). Constructs ref->fixer = new ShapeFix_Face(...) at line 4334, then calls ref->fixer->SetContext(new ShapeBuild_ReShape) at line 4338 with an explicit #317 comment. One of the three sites the SIGSEGV in Wire.wireFromEdges on a single closed edge (seam-graph closed-ring rim) — v1.12.6+2; standalone repro attempt negative #317 fix (CLAUDE.md) patched.
  3. Sources/OCCTBridge/src/OCCTBridge_Modeling.mm:2239-2263 — OCCTShapeRef OCCTShapeCreateFaceFromSurfaceWire(OCCTSurfaceRef surface, OCCTWireRef wire), declared OCCTBridge.h:4536. Constructs ShapeFix_Face fixer(face) at line 2251, calls fixer.SetContext(new ShapeBuild_ReShape) at line 2252, with an explicit #317 comment. Second of the three SIGSEGV in Wire.wireFromEdges on a single closed edge (seam-graph closed-ring rim) — v1.12.6+2; standalone repro attempt negative #317-patched sites.
  4. Sources/OCCTBridge/src/OCCTBridge_Modeling.mm:2264-2298 — OCCTShapeRef OCCTShapeCreateFaceFromSurfaceWireWithHoles(...), declared OCCTBridge.h:4543. Constructs ShapeFix_Face fixer(face) at line 2291, calls fixer.SetContext(new ShapeBuild_ReShape) at line 2292, with an explicit #317 comment. Third of the three SIGSEGV in Wire.wireFromEdges on a single closed edge (seam-graph closed-ring rim) — v1.12.6+2; standalone repro attempt negative #317-patched sites.

Real ShapeFix_FaceConnect call site (1 total, not named OCCTShapeFixConnect*):

  • Sources/OCCTBridge/src/OCCTBridge_Healing.mm:2149-2178 — OCCTShapeRef OCCTShapeFixFaceConnect(OCCTShapeRef shape, double tolerance), declared OCCTBridge.h:6201.

Divergence

Two things have actually drifted, both stemming from the same stale-index root cause:

  1. The index entries are simply wrong — OCCTShapeFixFace and OCCTShapeFixConnect* are phantom symbols that were presumably renamed at some point (to the OCCTFaceFix/OCCTFaceFixer* family and to OCCTShapeFixFaceConnect respectively) without the index being updated. Anyone using the index as a map from OCCT class to bridge symbol gets zero hits searching for either name.

  2. A concrete, currently-live consequence, exactly as CLAUDE.md's own SIGSEGV in Wire.wireFromEdges on a single closed edge (seam-graph closed-ring rim) — v1.12.6+2; standalone repro attempt negative #317 entry warns about: CLAUDE.md documents that ShapeFix_Face::FixPeriodicDegenerated() SIGSEGVs (uncatchable, since it's an OS signal) via an unguarded Context()->Replace(...) unless the fixer is given a context up front via SetContext(new ShapeBuild_ReShape), and that the fix was applied to "all three ShapeFix_Face call sites." There are actually four direct ShapeFix_Face constructions in the bridge. Three (OCCTFaceFixerCreate, OCCTShapeCreateFaceFromSurfaceWire, OCCTShapeCreateFaceFromSurfaceWireWithHoles) got the SetContext guard. The fourth, OCCTFaceFix (OCCTBridge_Healing.mm:303, backing Face.fixed(tolerance:)), was never touched and still constructs ShapeFix_Face with no context — it remains exposed to the identical SIGSEGV mechanism SIGSEGV in Wire.wireFromEdges on a single closed edge (seam-graph closed-ring rim) — v1.12.6+2; standalone repro attempt negative #317 fixed everywhere else, on any face whose sole boundary wire is a closed periodic curve belting a conical/cylindrical surface's full period (the exact shape Shape.face(from:boundary:) is regression-tested against in Issue317PeriodicConicalSingleWireTests, but .fixed(tolerance:) is not). The only regression test for the SIGSEGV in Wire.wireFromEdges on a single closed edge (seam-graph closed-ring rim) — v1.12.6+2; standalone repro attempt negative #317 mechanism (Tests/OCCTSurfaceTests/Issue317PeriodicConicalSingleWireTests.swift) exercises only call site Feature Request: Coordinate System Support for CAM #3 above (face(from:boundary:)), not Face.fixed(tolerance:) — so this gap would not be caught by the existing test suite. This is precisely the failure mode the auditor flagged: someone re-auditing all ShapeFix_Face call sites for SIGSEGV in Wire.wireFromEdges on a single closed edge (seam-graph closed-ring rim) — v1.12.6+2; standalone repro attempt negative #317-class defects via the index-listed OCCTShapeFixFace symbol finds nothing, and manual grep is needed to find all four real sites — which is how this fourth, unpatched, untested site was found.

Separately, ShapeFix_FaceConnect's single real call site (OCCTShapeFixFaceConnect, backing Shape.connectedFaces(tolerance:)) has zero test coverage anywhere in Tests/ — grepping connectedFaces across the whole repo turns up only its own declaration in Shape.swift. The stale OCCTShapeFixConnect* index entry (with a wildcard, implying a family of functions) would not have led anyone to this single, untested, real function either.

Swift call sites

  • OCCTFaceFix → Sources/OCCTSwift/Shape.swift:3317, inside extension Face { public func fixed(tolerance: Double = 1e-6) -> Shape? } (declared ~Shape.swift:3316).
  • OCCTFaceFixerCreate/OCCTFaceFixerPerform/OCCTFaceFixerFixOrientation/OCCTFaceFixerFixAddNaturalBound/OCCTFaceFixerFixMissingSeam/OCCTFaceFixerFixSmallAreaWire/OCCTFaceFixerFace/OCCTFaceFixerSetMode/OCCTFaceFixerFixIntersectingWires/OCCTFaceFixerFixPeriodicDegenerated/OCCTFaceFixerFixWiresTwoCoincEdges/OCCTFaceFixerFixLoopWire/OCCTFaceFixerResult/OCCTFaceFixerStatus/OCCTFaceFixerSetMaxTolerance/OCCTFaceFixerSetMinTolerance/OCCTFaceFixerRelease → all called from public final class FaceFixer in Sources/OCCTSwift/Document.swift:12029-~12126 (despite the name, the class lives in Document.swift, not Face.swift/Shape.swift).
  • OCCTShapeCreateFaceFromSurfaceWire → Sources/OCCTSwift/Shape.swift:4822, inside public static func face(from surface: Surface, boundary: Wire) -> Shape?.
  • OCCTShapeCreateFaceFromSurfaceWireWithHoles → Sources/OCCTSwift/Shape.swift:4867, inside public static func face(from surface: Surface, outer: Wire, innerWires: [Wire]) -> Shape?.
  • OCCTShapeFixFaceConnect → Sources/OCCTSwift/Shape.swift:7507, inside public func connectedFaces(tolerance: Double = 1e-4) -> Shape?. No other Swift call site.

Associated tests

  • Face.fixed(tolerance:) (OCCTFaceFix): tested in Tests/OCCTShapeHealingTests/OCCTShapeHealingTests.swift:115 and :126, and Tests/OCCTTopologyTests/OCCTTopologyTests.swift:605,614,634 — all against ordinary boxes/wires/faces, none against a periodic-conical single-wire boundary (the SIGSEGV in Wire.wireFromEdges on a single closed edge (seam-graph closed-ring rim) — v1.12.6+2; standalone repro attempt negative #317 crash shape). No test exercises the SIGSEGV-risk scenario for this call site.
  • FaceFixer class: tested in Tests/OCCTShapeHealingTests/OCCTShapeHealingTests.swift, Tests/OCCTStressTests/StressBuilderLifecycleTests.swift, Tests/OCCTSurfaceTests/Issue266FaceAnalysisFollowupTests.swift, Tests/OCCTTopologyTests/OCCTTopologyTests.swift (this call site already has SetContext, so no SIGSEGV in Wire.wireFromEdges on a single closed edge (seam-graph closed-ring rim) — v1.12.6+2; standalone repro attempt negative #317-specific regression test is needed here, and none was found beyond ordinary functional coverage).
  • Shape.face(from:boundary:) (OCCTShapeCreateFaceFromSurfaceWire): the SIGSEGV in Wire.wireFromEdges on a single closed edge (seam-graph closed-ring rim) — v1.12.6+2; standalone repro attempt negative #317 regression test lives here — Tests/OCCTSurfaceTests/Issue317PeriodicConicalSingleWireTests.swift, test closedEdgeOnConeSurvivesHealing(), plus general coverage in Tests/OCCTSurfaceTests/Issue233FaceFromSurfaceWireTests.swift, OCCTSurfaceTests.swift, Tests/OCCTModelingTests/OCCTModelingTests.swift, Issue234DegenerateHoleTests.swift, Issue397CircularHoleTests.swift, Tests/OCCTTopologyTests/Issue211OuterShellTests.swift, Issue439OuterShellMultiSolidTests.swift, and others.
  • Shape.face(from:outer:innerWires:) (OCCTShapeCreateFaceFromSurfaceWireWithHoles): tested in Tests/OCCTSurfaceTests/Issue266FaceWithHolesTests.swift (lines 27, 42, 63, 83) and Tests/OCCTSurfaceTests/Issue266FaceAnalysisFollowupTests.swift (lines 56, 72), plus Tests/OCCTShapeHealingTests/OCCTShapeHealingTests.swift:2527. No specific periodic-conical-boundary regression test found for this call site either, though it already carries the SetContext fix.
  • Shape.connectedFaces(tolerance:) (OCCTShapeFixFaceConnect): no test coverage found anywhere in Tests/ — grepping connectedFaces repo-wide returns only the function's own declaration in Shape.swift.

Underlying OCCT functions

  • OCCTFaceFix (OCCTBridge_Healing.mm:299-325): ShapeFix_Face — constructs with new ShapeFix_Face(face->face), sets SetPrecision, FixWireMode, FixOrientationMode, FixAddNaturalBoundMode, FixMissingSeamMode, FixSmallAreaWireMode, then calls Perform() and Face().
  • OCCTFaceFixerCreate + the OCCTFaceFixer* family (OCCTBridge_Healing.mm:4330-~4463): ShapeFix_Face, wrapped per-instance to expose individual fixes: SetContext, SetPrecision, Perform, FixOrientation, FixAddNaturalBound, FixMissingSeam, FixSmallAreaWire(true), Face(), plus mode/tolerance accessors (FixIntersectingWires, FixPeriodicDegenerated, FixWiresTwoCoincEdges, FixLoopWire, Status, SetMaxTolerance, SetMinTolerance).
  • OCCTShapeCreateFaceFromSurfaceWire (OCCTBridge_Modeling.mm:2239-2263): BRepBuilderAPI_MakeFace(surf, wire->wire, Standard_True) to build the initial face, then ShapeFix_Face (SetContext, Perform, Face()) to project pcurves, then BRepLib::BuildCurves3d and BRepCheck_Analyzer to validate.
  • OCCTShapeCreateFaceFromSurfaceWireWithHoles (OCCTBridge_Modeling.mm:2264-2298): same pattern — BRepBuilderAPI_MakeFace (with .Add(hole) per inner wire, holes reversed on the first attempt), then ShapeFix_Face (SetContext, Perform, Face()), BRepLib::BuildCurves3d, BRepCheck_Analyzer.
  • OCCTShapeFixFaceConnect (OCCTBridge_Healing.mm:2149-~2178+): ShapeFix_FaceConnect — construction on a shell extracted from the input shape (TopoDS::Shell cast, or the first TopAbs_SHELL found via TopExp_Explorer), then connects face pairs (a distinct OCCT class from ShapeFix_Face, despite the near-identical name).

Original audit finding: Cross-reference index: ShapeFix_Face → OCCTShapeFixFace (line 426) — that symbol doesn't exist; ShapeFix_Face is actually wrapped by the handle-based OCCTFaceFixer* family (OCCTBridge_Healing.mm:4324-4396, header:16781 on) plus several inline uses (e.g. OCCTBridge_Modeling.mm:2251) that the index entry doesn't mention at all.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    phase:1bPass 1b — C++ bridge header duplication audit (#381)priority:P0Drop everythingrefactorPart of the #377 codebase/docs duplication-audit efforttype:bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions