Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ suite into these targets (each `Tests/OCCT<Domain>Tests/`, declared in `Package.
- Container-overflow in NCollection on arm64 macOS — pre-existing OCCT race condition that manifests as non-deterministic SEGV under parallel test execution
- `LocOpe_SplitDrafts` throws on incompatible geometry — always wrap `Perform()` in try-catch in bridge
- `BRepOffsetAPI_ThruSections` (loft) SIGSEGV'd (null deref, "Address 8") on mismatched closed profiles — `BRepFill_CompatibleWires::SameNumberByPolarMethod` over-advanced an unguarded correspondence-list iterator. It's an OS signal, so the bridge `catch(...)` cannot save it. **Fixed upstream in OCCT 8.0.0p1** (Open-Cascade-SAS/OCCT#1298, OCCTSwift #176/#178); the previously-carried `Scripts/patches/0001-*` was dropped — the current p1 xcframework has the guard natively (regression test "Loft polar-method SIGSEGV regression (#176)" passes against it). Note: `OCC_CATCH_SIGNALS` is inert in our build (no `OCC_CONVERT_SIGNALS`) — do not rely on it for signal safety; OS signals raised inside OCCT (e.g. #234) are still uncatchable in-process.
- `ShapeAnalysis_FreeBounds` (backs `Shape.freeBoundsClosedWires`/`freeBoundsClosedCount`/`freeBoundsOpenWires`) SIGSEGVs (uncatchable) on certain shapes with multiple free-boundary components — isolated to its internal `SplitWires`/sequence-accumulation step (`NCollection_HSequence::Append`), minimally reproducible with just two disjoint planar faces in one compound; not a simple loop-count threshold (150+ loops can be fine, 2 can crash), so no reliable guard exists yet (#310, upstream filed as Open-Cascade-SAS/OCCT#1376; sibling `Standard_OutOfRange` bug in the same file is OCCT#1330, still open). No wrapper fix shipped; documented as a known crash risk only.
- `ShapeAnalysis_FreeBounds` (backs `Shape.freeBoundsClosedWires`/`freeBoundsClosedCount`/`freeBoundsOpenWires`, and `Shape.freeBounds`) used to SIGSEGV (uncatchable) on certain shapes with multiple free-boundary components — isolated via AddressSanitizer to `connectWiresToWiresImpl`'s empty-input early return (`ShapeAnalysis_FreeBounds.cxx`) leaving its `owires` out-parameter uninitialized (null), which later crashes `NCollection_HSequence::Append`. Minimally reproducible with just two disjoint planar faces in one compound; not a simple loop-count threshold (150+ loops can be fine, 2 can crash) — depends only on whether any single component's boundary closes with zero edges left over. **Fixed upstream in v1.12.6** (`Scripts/patches/0004-*`, xcframework rebuilt): `connectWiresToWiresImpl` now initializes `owires` to an empty sequence before its early return. #310, upstream repro filed as Open-Cascade-SAS/OCCT#1376, fix as [OCCT#1377](https://github.com/Open-Cascade-SAS/OCCT/pull/1377); sibling `Standard_OutOfRange` bug in the same file, OCCT#1330, is still open and unrelated to this fix.

### Carrying OCCT source patches

Expand Down
9 changes: 5 additions & 4 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,15 @@ let occtTarget: Target = useLocalBinary
name: "OCCT",
path: "Libraries/OCCT.xcframework"
)
// v1.12.3 rebuild: OCCT 8.0.0p1 + our carried patches — 0001 (ShapeFix_Face guard, #263),
// 0002 (backport of upstream OCCT#1334, #280), and 0003 (fillet TopOpeBRep thread_local, #298).
// v1.12.6 rebuild: OCCT 8.0.0p1 + our carried patches — 0001 (ShapeFix_Face guard, #263),
// 0002 (backport of upstream OCCT#1334, #280), 0003 (fillet TopOpeBRep thread_local, #298), and
// 0004 (ShapeAnalysis_FreeBounds owires init, #310).
// Bump BOTH url and checksum whenever the xcframework is rebuilt, or URL-resolving consumers
// silently keep the previous kernel while local sibling builds get the new one.
: .binaryTarget(
name: "OCCT",
url: "https://github.com/SecondMouseAU/OCCTSwift/releases/download/v1.12.3/OCCT.xcframework.zip",
checksum: "9daee3da6f0655a927ee8deebfa1306b282f2f0da459b35383ef9c7de0555cc2"
url: "https://github.com/SecondMouseAU/OCCTSwift/releases/download/v1.12.6/OCCT.xcframework.zip",
checksum: "fbd3c447769fb5d7f2f7ff53b4b9a9a59875274c422d5da7eeafa182667ac3a4"
)

let package = Package(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
From: OCCTSwift ecosystem <noreply@secondmouse.au>
Subject: [PATCH] Shape Healing - initialize owires before the empty-input early return in
connectWiresToWiresImpl

ShapeAnalysis_FreeBounds crashes (uncatchable SIGSEGV) analyzing a shape whose free
boundaries include any wire whose edges are ENTIRELY consumed by SplitWire's
closed-loop detection, leaving zero leftover ("open") edges for that component.
Minimally reproducible with two disjoint planar faces in one compound -- no shared
or even nearby geometry required, and no data-volume threshold (a shape with 150+
free-boundary loops can be fine while a 2-loop shape crashes, and vice versa,
depending only on whether any single component happens to close with nothing left
over).

Root cause: ShapeAnalysis_FreeBounds::SplitWire (the per-wire helper) finds the
closed sub-loops in a wire, then builds an "edges" list of whatever wasn't
consumed (status != 2) and hands it to ConnectEdgesToWires to chain into open
wires:

for (i = 1; i <= nbedges; i++)
if (statuses.Value(i) != 2)
edges->Append(sewd->Edge(i));
open = ShapeAnalysis_FreeBounds::ConnectEdgesToWires(edges, toler, shared);

When every edge of the wire was consumed into a closed loop, "edges" is a valid
but EMPTY sequence. ConnectEdgesToWires -> ConnectWiresToWires(3-arg) ->
ConnectWiresToWires(4-arg, with a local "occ::handle<...> owires;") ->
connectWiresToWiresImpl, whose very first statement is:

if (iwires.IsNull() || !iwires->Length())
{
return;
}

For an empty (but non-null) input this returns WITHOUT EVER ASSIGNING "owires" --
the caller's handle is left exactly as it started: null. That null propagates
back through ConnectEdgesToWires's return value into SplitWire's "open" output
parameter, into ShapeAnalysis_FreeBounds::SplitWires's per-wire loop:

SplitWire(TopoDS::Wire(wires->Value(i)), toler, shared, tmpclosed, tmpopen);
closed->Append(tmpclosed);
open->Append(tmpopen); // tmpopen is null -- crash here

NCollection_HSequence::Append(const opencascade::handle<T>&) dereferences the null
handle (theOther->ChangeSequence()) to obtain a reference to pass into
NCollection_Sequence::Append(NCollection_Sequence&), whose first statement reads
theSeq.IsEmpty() (== theSeq.mySize) through that invalid reference: SIGSEGV.

The fix is the one-line contract restoration connectWiresToWiresImpl's own
non-empty path already follows a few lines down ("owires = new
NCollection_HSequence<TopoDS_Shape>;" before populating it): "no wires to connect"
should produce a valid EMPTY result, not an untouched (and, for every caller in
this file, null-defaulted) out-parameter.

Validation (fast path, no full rebuild -- see Scripts/patches/README.md's #0001
entry for the override-link technique, extended here with AddressSanitizer):
built an ASan-instrumented macOS-arm64 OCCT (ModelingAlgorithms + ModelingData +
FoundationClasses, RelWithDebInfo, MMGT_OPT=0 at runtime to route allocations
through malloc/free so ASan's redzones apply). Two disjoint 4-edge planar faces in
one compound SIGSEGV 100% of the time on stock V8_0_0_p1 (confirmed identical
crash signature -- same function, same NCollection_Sequence::Append call, same
0xfffffffffffffff8 fault address -- at both -O2 and -O0); after this patch the
same input returns the correct "2 closed, 0 open" and the process exits 0. On a
608 KB real-world fixture (a 150-face analytic BRep compound, sewn with
BRepBuilderAPI_Sewing): tol=0.05 gives 152 closed/0 open both before and after the
patch (byte-identical -- confirms no behavior change on the working path); tol=0.10
crashes on stock p1 and returns 144 closed/0 open after the patch.

Reported downstream and isolated at SecondMouseAU/OCCTSwift#310. Filed upstream as
Open-Cascade-SAS/OCCT#1376 (repro only, filed before the root cause was pinned
down); this patch supersedes that repro with the actual fix.
---
src/ModelingAlgorithms/TKShHealing/ShapeAnalysis/ShapeAnalysis_FreeBounds.cxx | 1 +
1 file changed, 1 insertion(+)

diff --git a/src/ModelingAlgorithms/TKShHealing/ShapeAnalysis/ShapeAnalysis_FreeBounds.cxx b/src/ModelingAlgorithms/TKShHealing/ShapeAnalysis/ShapeAnalysis_FreeBounds.cxx
index a22f25a1..3662905a 100644
--- a/src/ModelingAlgorithms/TKShHealing/ShapeAnalysis/ShapeAnalysis_FreeBounds.cxx
+++ b/src/ModelingAlgorithms/TKShHealing/ShapeAnalysis/ShapeAnalysis_FreeBounds.cxx
@@ -207,6 +207,7 @@ static void connectWiresToWiresImpl(
{
if (iwires.IsNull() || !iwires->Length())
{
+ owires = new NCollection_HSequence<TopoDS_Shape>;
return;
}
occ::handle<NCollection_HArray1<TopoDS_Shape>> arrwires =
23 changes: 23 additions & 0 deletions Scripts/patches/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,26 @@ This is the same class of fix, in the same engine, as [Open-Cascade-SAS/OCCT#118
**Validation:** the isolated pure-C++ repro (`fuse` two/three prisms → fillet the seam, 8 threads) returns BRepCheck-invalid solids across several wrong volumes on stock p1, and 0/1600 invalid with a single correct volume after the patch. ThreadSanitizer reports the `STATIC_SOLIDINDEX` and `BlendFunc` scratch races on stock p1 and is clean on the fillet path after the patch (only an unrelated benign `BOPAlgo_InitMessages` lazy-init race remains, in the boolean path — orthogonal). While this patch is carried, the in-wrapper `occtFilletMutex` shipped in v1.12.1 serialises fillet/chamfer builds so the corruption cannot reach consumers.

**Retire** once the bundled OCCT includes these `thread_local` conversions (upstream PR pending), at which point the `occtFilletMutex` guard can also be dropped.

## 0004-ShapeAnalysis_FreeBounds-init-owires-empty-input-310.patch

**Fixes the upstream OCCT crash behind [#310](https://github.com/SecondMouseAU/OCCTSwift/issues/310)** — `ShapeAnalysis_FreeBounds` (backing `Shape.freeBoundsClosedWires`/`freeBoundsClosedCount`/`freeBoundsOpenWires`) SIGSEGVs on certain shapes with multiple free-boundary components.

`ShapeAnalysis_FreeBounds::SplitWire` (its per-wire helper) finds each wire's closed sub-loops, then hands whatever edges weren't consumed to `ConnectEdgesToWires` to build the "open" result. When a wire's edges are **entirely** consumed by closed-loop detection — leaving zero leftover edges — that hand-off is an empty (but non-null) sequence. The call chain `ConnectEdgesToWires` → `ConnectWiresToWires` → `connectWiresToWiresImpl` starts with:

```cpp
if (iwires.IsNull() || !iwires->Length())
{
return;
}
```

For empty input this returns **without ever assigning `owires`** — every caller in the file starts from a freshly-defaulted (null) handle, so the null propagates all the way back to `SplitWire`'s `open` output parameter, then to `ShapeAnalysis_FreeBounds::SplitWires`'s `open->Append(tmpopen)` — dereferencing the null handle, an uncatchable SIGSEGV. Not a data-volume threshold: it depends only on whether *any* single free-boundary component happens to close with nothing left over, so a shape with 150+ loops can be fine while a 2-loop shape crashes (and vice versa).

**Fix:** the one-line contract restoration `connectWiresToWiresImpl`'s own non-empty path already follows a few lines down (`owires = new NCollection_HSequence<TopoDS_Shape>;` before populating it) — "nothing to connect" should produce a valid **empty** result, not an untouched out-parameter.

**Validation** (AddressSanitizer, extending the #0001 override-link technique — see the patch's own commit message for the full command sequence): an ASan-instrumented macOS-arm64 build (`ModelingAlgorithms`+`ModelingData`+`FoundationClasses`, `RelWithDebInfo`, `MMGT_OPT=0` at runtime) crashes 100% of the time on two disjoint planar faces in one compound — same function, same `NCollection_Sequence::Append` call, same `0xfffffffffffffff8` fault address at both `-O2` and `-O0` — and returns the correct `2 closed, 0 open` after the patch. On the real 150-face fixture from #310: `tol=0.05` gives `152 closed/0 open` byte-identical before and after (no behavior change on the working path); `tol=0.10` crashes on stock p1 and returns `144 closed/0 open` after.

Reported and isolated at SecondMouseAU/OCCTSwift#310; a repro-only report was filed upstream as [Open-Cascade-SAS/OCCT#1376](https://github.com/Open-Cascade-SAS/OCCT/issues/1376) before the root cause was pinned down, followed up with the fix as [OCCT#1377](https://github.com/Open-Cascade-SAS/OCCT/pull/1377).

**Retire** once the bundled OCCT includes this fix.
16 changes: 0 additions & 16 deletions Sources/OCCTSwift/Document.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6673,26 +6673,13 @@ public final class StepHeader: @unchecked Sendable {
extension Shape {

/// Count the number of closed free-boundary wires.
///
/// - Warning: **Can crash the process with an uncatchable SIGSEGV** on certain shapes with
/// multiple free-boundary components (#310, filed upstream as
/// [Open-Cascade-SAS/OCCT#1376](https://github.com/Open-Cascade-SAS/OCCT/issues/1376)) —
/// confirmed on `ShapeAnalysis_FreeBounds`'s internal `SplitWires`/sequence-accumulation step
/// (upstream kernel bug, not yet fixed; `OCC_CATCH_SIGNALS` is inert in this build so it
/// cannot be caught). Not data-volume-related — a shape with many free-boundary loops can be
/// fine while a smaller one crashes, and vice versa — so there's no reliable guard to screen
/// for it yet. Isolate calls on untrusted/large analytic-face compounds in a subprocess if a
/// crash there must not take down your process.
/// - Parameter tolerance: Sewing tolerance for boundary detection.
/// - Returns: Number of closed free-boundary wires.
public func freeBoundsClosedCount(tolerance: Double = 1e-6) -> Int {
Int(OCCTShapeFreeBoundsClosedCount(handle, tolerance))
}

/// Get the compound of closed free-boundary wires.
///
/// - Warning: Shares the same uncatchable-crash risk as ``freeBoundsClosedCount(tolerance:)`` —
/// see its doc for details (#310).
/// - Parameter tolerance: Sewing tolerance for boundary detection.
/// - Returns: Compound shape of closed wires, or nil if none.
public func freeBoundsClosedWires(tolerance: Double = 1e-6) -> Shape? {
Expand All @@ -6701,9 +6688,6 @@ extension Shape {
}

/// Get the compound of open free-boundary wires.
///
/// - Warning: Shares the same uncatchable-crash risk as ``freeBoundsClosedCount(tolerance:)`` —
/// see its doc for details (#310).
/// - Parameter tolerance: Sewing tolerance for boundary detection.
/// - Returns: Compound shape of open wires, or nil if none.
public func freeBoundsOpenWires(tolerance: Double = 1e-6) -> Shape? {
Expand Down
21 changes: 21 additions & 0 deletions Tests/OCCTShapeHealingTests/OCCTShapeHealingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,27 @@ struct FreeBoundsTests {
// Should return something even if nothing was fixed
_ = result
}

// #310 regression: ShapeAnalysis_FreeBounds crashed (uncatchable SIGSEGV) analyzing a compound
// of two or more DISJOINT closed faces — not adjacent/touching ones like
// compoundFacesHasFreeBounds above, which forms a single combined loop and never hit the bug.
// Each disjoint face's boundary is entirely consumed by its own closed-loop detection with
// nothing left over, which is exactly what tripped the kernel's uninitialized-handle bug (fixed
// upstream via Scripts/patches/0004). No #expect needed for the crash itself — a regression
// would abort the whole test process; reaching the assertions below is the real assertion.
@Test("Disjoint faces in a compound don't crash free-bounds analysis")
func issue310DisjointFacesFreeBounds() {
let face1 = Shape.face(from: Wire.rectangle(width: 1, height: 1)!)!
let face2 = Shape.face(from: Wire.rectangle(width: 1, height: 1)!)!.translated(by: SIMD3(5, 5, 0))!
let compound = Shape.compound([face1, face2])!

let closedWires = compound.freeBoundsClosedWires(tolerance: 0.01)
#expect(compound.freeBoundsClosedCount(tolerance: 0.01) == 2)
#expect(closedWires?.subShapeCount(ofType: .wire) == 2)
// A valid, empty compound (0 wires) is the correct result here, not nil — both faces
// are entirely closed, so there's nothing to report as an open free boundary.
#expect((compound.freeBoundsOpenWires(tolerance: 0.01)?.subShapeCount(ofType: .wire) ?? 0) == 0)
}
}

// MARK: - v0.41.0: Geometry Conversion
Expand Down
Loading