chore(#512) + fix(#518, #555, #522): rebuild the kernel, redesign 0016 after upstream review, add 0018 and 0019

[gsdali]

Four changes that have to ship in one kernel binary: the #512 rebuild, #518's redesign of patch 0016 after upstream review, #555's new patch 0018, and #522's new patch 0019. Closes nothing on its own, since the remaining items on #512's checklist are release-time steps, listed at the bottom.

What this does

Scripts/patches/0017-* (the #484 null-context guard for ShapeFix_ComposeShell::Perform(), ShapeFix_ComposeShell::SplitEdges() and ShapeUpgrade_WireDivide::Perform()) was carried but inert. build-occt.sh applies every carried patch before each cmake build, so a patch only reaches callers once Libraries/OCCT.xcframework is rebuilt from source. It is now rebuilt from V8_0_0_p1 + all 19 carried patches.

No API change and no behaviour change through this wrapper: both bridge call sites already call SetContext(new ShapeBuild_ReShape()), so nothing here could reach the crash before or after, and those workarounds stay in place (same PR1→PR2 pattern as #298/#341/#344/#349). The rebuild closes the crash for code reaching those OCCT classes through a path this package does not control.

Base

0017 arrived on refactor/381-pass1b when #511 merged partway through the rebuild, so there is no
stacking left. Rebased three times as #516 (#487), #517 (#488) and #509 (#485) landed in the base;
each conflicted on the same docs/CHANGELOG.md insertion point, resolved by keeping both entries with
#### items inside the Pass 1b group and these two ### sections after them.

Why the two commits are one PR

They produce a single OCCT.xcframework. Splitting them would mean publishing a checksum for a binary
that the second commit immediately replaces. #518's kernel change had to land in the same rebuild, so
it rides here rather than in a follow-up.

Verification

Everything was measured against the real rebuilt binary with no override-linked TUs. That distinction is the point of this PR: all prior #484 evidence was override-linked, which proves the patch compiles and works, not that it shipped.

Before the rebuild, V8_0_0_p1 + patches 0001-0016:

ShapeUpgrade_WireDivide::Perform   ctx=NO  :   *** KILLED BY SIGNAL 11 ***
ShapeFix_ComposeShell::Perform     ctx=NO  :   *** KILLED BY SIGNAL 11 ***

After, same source file, same compile command:

ShapeUpgrade_WireDivide::Perform   ctx=NO  : Perform done, wireNull=0
ShapeFix_ComposeShell::Perform     ctx=NO  : Perform=1 resultNull=0

The other six cases in repro_484_crash.mm are unchanged. repro_484_equivalence.mm: all four ctx=yes fingerprints byte-identical to the pre-rebuild values, and all four ctx=NO fingerprints now complete and match their ctx=yes counterpart.

The checklist expected those fingerprints to already be recorded in the reproducer's README. They were not, so I captured them against the old kernel before rebuilding and have now written them into that README as the reference values for any future rebuild:

WireDivide   planar       ctx=yes : type=5 n4=0 n5=1 n6=4 n7=8 brep=19494c8eaac44ef7 len=582
WireDivide   cylindrical  ctx=yes : type=5 n4=0 n5=1 n6=4 n7=8 brep=ea26831beb4fa5fb len=667
ComposeShell planar       ctx=yes : ok=1 type=4 n4=1 n5=1 n6=4 n7=8 brep=a06f56d38503cd48 len=752
ComposeShell cylindrical  ctx=yes : ok=1 type=4 n4=1 n5=1 n6=4 n7=8 brep=aa5c2d38eac7d8b3 len=699

Other gates:

• Full swift test: 4842 tests in 1346 suites passed, clean, with OCCTSWIFT_BRIDGE_PREBUILT unset so the bridge compiled from source (it was ambiently set in the shell, which would have masked fix(#484): two upstream null-context SIGSEGVs patched, Face.fixed given a context, connectedFaces covered and de-first-of-N'd #511's own bridge changes).
• Scripts/tsan-stress.sh deliberately not run, per the checklist: 0017 is a null-handle guard on a single-threaded path.

Build hygiene

The checklist's two build traps were both checked rather than assumed:

• occt-src contents. Modified in exactly the 47 files patches 0001-0017 touch, no stray files, no untracked probes, and all 17 reverse-apply cleanly. So the tree that got compiled is the pinned tag plus the carried patch set and nothing else, which matters because sessions do compile diagnostic probes into occt-src. All 17 logged as already applied (the source was patched by fix(#484): two upstream null-context SIGSEGVs patched, Face.fixed given a context, connectedFaces covered and de-first-of-N'd #511's override-link work), which is the expected wording, not a skipped patch.
• Stale objects. The build was interrupted twice by session limits and resumed in place, exactly the case where a fresh-timestamped but stale artifact can appear. Objects for both patched TUs are newer than the patched sources in all three slices, and the resume ran from the same checkout that configured the build dir.

#518: Storage_Schema's scratch handle becomes a member, not a guarded global

Patch 0016 (#374) fixed two kernel races. Its Resource_Manager::Debug half (a file-scope static bool made std::atomic<bool>) is untouched. Reviewing our upstream PR OCCT#1399, maintainer gkv311 pointed out the Storage_Schema half needs no lock at all if ICurrentData() becomes a mutable Handle(Storage_Data) field on the class.

That checks out against the source, and I verified the premise before writing any code:

• No Storage_Schema instance is ever shared. All four constructions in the tree are function-local (PCDM_StorageDriver::Write, PCDM_ReadWriter_1 at three sites). None is cached, unlike the driver instances Concurrent Save/SaveAs to the same format corrupts a shared, cached storage driver instance (SIGSEGV) #349 had to deal with.
• Every callback re-entry lands on the same instance. Storage_CallBack::Add/Write/Read all take the driving schema as an argument, so the BindType/AddPersistent/PersistentToAdd calls made from callbacks go through *this.
• Removing the statics breaks nothing. ICurrentData() and ISetCurrentData() were both private with no reference anywhere outside the class. Other Storage_Schema users in the tree (BinLDrivers, XmlMDF, StdLDrivers) touch only its statics CheckTypeMigration() and ICreationDate(), neither of which reads the current data.
• mutable is required, because Write, Clear, HasTypeBinding, BindType, TypeBinding, AddPersistent and PersistentToAdd are all const.

The field is also strictly stronger on the failure #374 reported. Under the mutex, a throwaway Storage_Schema built by PCDM_ReadWriter_1 during an unrelated Open() still nullified an in-flight Write()'s current data, it just did so without a data race. Under the field it cannot reach another instance's data at all. It narrows exactly one thing, two threads driving the same instance, which no caller does.

One extra change, flagged rather than buried. The removed mutex was incidentally covering AddPersistent()'s static TCollection_AsciiString aTypeName, a second process-wide mutable object in the same class (the 374 reproducer's own README had noted it). The static is dropped: the value is assigned from the tName argument and read two lines later, so it only ever saved an allocation. Currently unreachable in-tree, since both concrete PCDM_StorageDriver subclasses override Write, but AddPersistent is public and a third-party subclass can reach it.

Patch number stays 0016, a corrected design rather than a new fix, matching the 0011 precedent from #341 to #363. The file is renamed to ...-Storage_Schema-per-instance-374.patch since the old name says mutex.

Verification, all against the rebuilt kernel:

• Scripts/repro/374 harness, the unguarded variant: 0 races, 0 save/load/verify failures at 8×50, 8×30 and 10×60. Same result the mutex version gave; stock is 13 races plus SIGABRT.
• Full Scripts/tsan-stress.sh run gate, 10/10 scenarios clean. Run twice, the second time against the exact final patch after the aTypeName change.
• clang-format clean on all three touched files (OCCT's own .clang-format).
• Stale ShapeFix_Face/ShapeFix_FaceConnect cross-reference index entries hide a real, unpatched #317-class SIGSEGV site and an untested wrapper #484's two reproducers re-run on the repackaged binary: unchanged from what Rebuild OCCT.xcframework with patch 0017 (#484 null-context guard) and ship a patch release #512 recorded, all four ctx=yes fingerprints identical.

One full-suite run during this validation hit a pre-existing timing flake in #300's stepRobustRepairCancellation, which reports .importFailed where it expects .cancelled about 1 run in 9 in isolation. Unrelated to either change here (Storage_Schema is only reachable from OCAF store/retrieve, not STEP import), so it is filed as #525 rather than absorbed, and the count above is from a clean run.

Upstream OCCT#1399 is updated to match. Force-pushed the redesigned single commit (retitled to OCCT's Category - Summary convention as Foundation Classes - ...), rewrote the PR description, and replied to gkv311's review, including an offer to split the aTypeName change into its own commit and to split Resource_Manager from Storage_Schema into two PRs if preferred. clang-format and the non-ASCII check are clean locally against OCCT's own .clang-format; CI is running.

The upstream branch's diff and our carried 0016 are the same change, verified by diffing them (identical apart from one hunk-header context label, which differs because our tree also carries 0012 in Resource_Manager.cxx). The myCurrentData doc comment is one line rather than three, per okf/policies/upstream-occt-style.md: mechanism only, rationale in the PR description. Trimming it changed a packaged header, so the artifact was repackaged and the checksum above is the result; the reproducers and the full suite were re-run against it.

#555: two GCPnts point-count defects, patched as 0018

GCPnts_UniformAbscissa / GCPnts_QuasiUniformAbscissa. #501 closed the OCCTSwift-reachable half of the first one at the bridge layer (merged as #544 while this was in progress, and unaffected by the patch); this is the kernel side, which every other OCCT consumer still had.

NbPoints() was not bounded by the requested count. The array is sized theNbPoints + 5 and the walk fills it until it reaches the end or runs out of room. The cause is a tolerance mismatch, not an off-by-one: Perform terminates on a parametric epsilon that Resolution() derives from the curve's largest derivative, so on an ellipse with major radius 1e6 and minor radius 1e-3 it is about 1e-13 where the locally correct value is about 1e-4. The walk stops 1.557e-08 short, refuses to call that done, takes one more step and snaps it to the end.

The probe added the fact that decides the fix: the surplus point is a duplicate, 1.175e-10 from its neighbour in 3D. So the fix is not to clamp the count, which would drop the exact end parameter and leave the distribution stopping short (precisely the defect #501 found in the one bridge function that already clamped), but to accept a point that coincides with the end in 3D within the caller's tolerance. The 3D tolerance is threaded in beside the parametric one, the end point is evaluated once outside the walk, and the distance test sits behind a cheap aUU2 - aUi < aDelta gate so it runs on the final step rather than every step.

A point count below 2 stored out of bounds. Both classes document theNbPoints >= 2 and enforce it with Standard_ConstructionError_Raise_if, which compiles to nothing under No_Exception, which is how the shipped Release kernel is built (#487). GCPnts_QuasiUniformAbscissa's Bezier/BSpline branch then allocated an empty array and unconditionally stored into index 1: uncatchable SIGSEGV, same class as #263/#310/#317/#318. Both classes now leave the object not done for such a count; the Raise_if stays, so an exceptions-enabled build throws exactly as before.

Verification, against the rebuilt kernel with no override-linked TUs:

• 17 curve types x counts 2..200 x both classes, 6766 configurations: 232 lines change and they are exactly the 232 that were over-requesting. Everything else is byte-identical, across a line, four circles from radius 1e-6 to 1e7, a 5x2 ellipse, a hyperbola, a parabola, a 2-pole and a 4-pole Bezier, an 8-point and a 40-point BSpline, an offset circle, an offset BSpline and a trimmed circle. On the changed lines the last parameter is still exactly the end.
• Over-request goes from 22 counts to 0; every degenerate count on every curve now returns IsDone() == false for both classes, in place of a SIGSEGV, a 5-point answer or a 1-point answer depending on which curve and class you called.
• clang-format clean on both touched files, against OCCT's own config.

No OCCTSwift API changes behaviour: #544's bridge guards already rejected degenerate counts and clamped the overflow. The patch closes both for code reaching those OCCT classes through a path this package does not control. Its guards stay in place regardless, since they are what makes the bridge correct against an unpatched kernel.

Filed upstream as OCCT#1417, a fix PR with no companion reproducer issue, per the rule maintainer dpasukhi gave on OCCT#1409.

One trap is recorded in the reproducer README because it cost me a confusing run: building the override-linked TUs without -DNo_Exception brings the Raise_if back, so the degenerate cases abort with an uncaught Standard_ConstructionError rather than crashing (stock) or returning not-done (patched). It reads like a broken patch, and it is measuring a kernel nobody ships.

fix(#522): the Jacobi maxima that were never written, and the zero error that followed

New carried patch 0019, found while building #491's surface-approximation parity tests and folded in here because it needs the same binary.

A radius-10 sphere approximated at C0 and tolerance 1e-3 came back as a degree-1, 2-pole-in-U B-spline: a straight line across the full 2*pi of its longitude, deviating by the sphere's own diameter of 20, while isDone said the tolerance was met and maxError said 1.07e-4. A bicubic Bezier at C0 collapsed to a 2x2 bilinear patch reporting 4.08e-15 at every tolerance from 1e-1 down to 1e-7, so tightening the request changed nothing.

One line explains both. AdvApp2Var_ApproxF2var::mma2ce1_ partitions one scratch allocation into seven consecutive buffers, ipt4 for XMAXJU (the maxima of the U Jacobi polynomials) and ipt5 for XMAXJV, then fills both from ipt5:

AdvApp2Var_ApproxF2var::mma2jmx_(ndjacu, iordru, &wrkar_off[ipt5]);   /* -> should be ipt4 */
AdvApp2Var_ApproxF2var::mma2jmx_(ndjacv, iordrv, &wrkar_off[ipt5]);

XMAXJU is never written; mma2ce2_ reads whatever the allocation left at ipt4, in practice zeros. Every truncation error the approximator computes is |PATJAC(i,j)| * XMAXJU(i - 2*(IORDRU+1)) * XMAXJV(j - 2*(IORDRV+1)), so a zero XMAXJU zeroes every term. The interior error of every patch then evaluates to exactly 0, which means mma2ce2_'s tolerance test can never fire on it and MaxError() only ever describes the boundary iso-curves; and mma2er2_, asked for the lowest degree whose truncation error still fits, always answers NDMINU, the floor from the constraint order and the neighbouring isos, because every candidate scores 0.

Where that floor is low the fit collapses onto it. C0 gives IORDRU = 0, and a full sphere's V-boundary isos degenerate to its two poles, one coefficient each, so NDMINU is 1. C1 and C2 hide the collapse, their floor already being 8, but not the misreported error, which was never specific to C0. Degree collapse per se was never the defect — a cylinder trimmed in V legitimately fits at vDegree = 1, and does so before and after.

The write also overruns: mma2jmx_ writes ndjacu + 1 - 2*(IORDRU+1) doubles into a slot sized for the ndjacv equivalent, running into VECERR when MaxDegU >> MaxDegV. Benign, since VECERR is re-zeroed on entry to mma2ce2_, but out of bounds.

Fix: target ipt4 from the U call. One character. AdvApp2Var_Context's own two mma2jmx_ calls, the only others in the tree, already write to separate per-direction arrays.

Verification, against the rebuilt kernel with no override-linked TUs:

	before	after
real deviation > 10x reported maxError	12 of 98	0 of 98
real deviation > reported maxError at all	17 of 98	1 of 98
worst ratio of real deviation to reported error	3.4e13	1.0018

98 requests: 7 surface families x all 9 (uContinuity, vContinuity) combinations of C0/C1/C2 at tolerance 1e-3, plus C0/C0 across five tolerances, each comparing the reported error against the real maximum deviation over a 21x21 grid. The one row still over the line afterwards is a Bezier reproduced exactly, reporting 9.95221e-15 against a measured 9.96978e-15. Every reported error rises slightly, which is the interior contribution being counted for the first time; degrees rise only where the collapse was happening. Full before/after transcripts committed as sweep-before.txt / sweep-after.txt. The mechanism was confirmed directly by dumping the buffer through an -O0 override-link: xmaxju[8] = 0 0 0 0 0 0 0 0 stock, 0.9682 0.986 1.078 ... fixed.

This one does change OCCTSwift behaviour, unlike 0017 and 0018. Surface.approximated(tolerance:continuity:) and Surface.approxWithDetails(...) at C0 previously returned a collapsed surface and an error that described neither the input nor the output; both now return a usable fit and a maxError that bounds the real deviation. Issue491SurfaceApproxParityTests.maxErrorDescribesTheSharedFit had to exclude .c0 when it was written for exactly this reason; that exclusion is gone and it now checks every request. Four new tests in Issue522ApproxC0CollapseTests were run against a deliberately reverted kernel and all four fail there with the numbers above; the fifth, a V-linear cylinder control, passes either way.

The blast radius inside the kernel is wider than the C0 collapse. GeomFill_Sweep, BRepOffset_Offset, GeomLib, ShapeCustom_BSplineRestriction, ShapeConstruct and GeomConvert_1 construct a GeomConvert_ApproxSurface, ShapeCustom_ConvertToBSpline and ShapeUpgrade_UnifySameDomain reach one, and GeomPlate_MakeApprox drives AdvApp2Var_ApproxAFunc2Var directly. Most request C1 or C2, so the collapse could not reach them, but the always-zero interior error could — and ShapeConstruct::ConvertSurfaceToBSpline and ShapeCustom_BSplineRestriction both loop the requested continuity down to 0 on failure, then accept the result on MaxError() <= tol. ShapeCustom_ConvertToBSpline starts at C0 for any offset surface (ShapeCustom_ConvertToBSpline.cxx:148, a 1999 workaround for a hang) before handing off to the first of those. Two mentions are not callers: BRepFill_Sweep.cxx:1162 sits inside a /* */ block and BRepFill_Filling.cxx:712 is //-commented — a filename-level grep counts both, which is how an earlier revision of this description listed BRepFill_Sweep. Follow-ups filed per consumer.

Full swift test after: 4847 tests in 1347 suites, clean. Filed upstream as OCCT#1418, fix-only, based on b8f597c6; the touched file is byte-identical between upstream master and our V8_0_0_p1 pin. Reproducers, root-cause walkthrough and transcripts: Scripts/repro/522-approx-c0-collapse/.

Release-time steps, deliberately not in this PR

Package.swift's url:/checksum: pin is untouched on purpose. It has to resolve to a release asset that exists, and ci.yml runs swift build against it on every PR to refactor/**, so pointing it at an unpublished tag would 404 the PR. The bump belongs to the release commit that ships this branch:

1. Attach OCCT.xcframework.zip to the new tag's release. Built artifact: 149,078,261 bytes, sha256 2ccb194e11a675ba6c2769f7a13821da1d75787aa7d8a37e07905ff433915d53. This is the post-GeomConvert_ApproxSurface at GeomAbs_C0 returns a degree-1 collapse while reporting IsDone and a maxError five orders of magnitude too small #522 binary; earlier checksums in this PR's history are superseded. It is sitting at Libraries/OCCT.xcframework.zip.
2. Package.swift: bump the OCCT url: to that tag and checksum: to the sha256 above, and extend the carried-patch comment block (it currently ends at 0016) to name 0017.
3. docs/CHANGELOG.md: add #484 to the kernel-patch list on the ## Current: line.

Until then the two consumer paths diverge on purpose: this checkout and every sibling repo path-depending on its Libraries/OCCT.xcframework already have the new kernel, while anything resolving the remote url: stays on v1.15.18.

Also in here

Three things the rebuild surfaced:

• docs/guides/building-occt.md gains a "Shipping a rebuild" section. The rebuild-to-release sequence existed only as hand-written checklists inside issues, which is how the "bump both url: and checksum:" step gets dropped. It now covers verifying the patch set, verifying object freshness, resuming an interrupted build without rm -rfing the progress, proving the fix reached the binary, and packaging plus pinning.
• Scripts/patches/README.md gains the 0016 (Resource_Manager::Debug and Storage_Schema::ICurrentData() races surfaced by #371 (upstream OCCT#1398) #374) entry it never got. That README documents every carried patch and jumped straight from 0015 to 0017.
• Two stale workflow references corrected. ci.yml credited kernel rebuilds to a kernel-rebuild.yml that has never existed in this repo's history, and building-occt.md pointed readers at a nonexistent build-occt.yml build service. Kernel rebuilds are a manual local Scripts/build-occt.sh run; there is no CI job for them.

Pre-existing em-dashes in the large files touched here (docs/CHANGELOG.md, Scripts/patches/README.md) were left alone. Stripping 774 and 92 of them would bury this change in unrelated churn, and OKF-STANDARD.md does not require a backfill pass. Everything added here is em-dash free.

Refs #512, #484, #518, #374, #555, #501

Closes #522

🤖 Generated with Claude Code