ModelingData, ModelingAlgorithms - Extrema_ExtCC::Points() indexes an empty sequence on some parallel-curve results

[gsdali]

Extrema_ExtCC::NbExt() counts one container (mySqDist); Extrema_ExtCC::Points() reads a
different one (mypoints), but bounds-checks the request against NbExt():

int Extrema_ExtCC::NbExt() const { ...; return mySqDist.Length(); }

void Extrema_ExtCC::Points(const int N, Extrema_POnCurv& P1, Extrema_POnCurv& P2) const {
  if (N < 1 || N > NbExt()) throw Standard_OutOfRange();
  P1 = mypoints.Value(2 * N - 1);
  P2 = mypoints.Value(2 * N);
}

Several branches of PrepareParallelResult (called whenever the two curves are found to be
parallel) append a distance to mySqDist with no matching pair appended to mypoints, because in
those branches there is no discrete answer to give: the curves are parallel over a continuous range
(or unbounded), every point in it is equally close, and there is no unique "the" closest pair, only
a distance. NbExt() reports 1 in exactly the cases this happens, so Points(1) indexes
mypoints at an index past its actual length.

Points()'s own bounds check does not catch this because it checks the wrong container's length
(NbExt(), not mypoints.Length()). The check that would catch it sits one level down, inside
NCollection_Sequence::Value(size_t):

const TheItemType& Value(const size_t theIndex) const {
  Standard_OutOfRange_Raise_if(theIndex == 0 || theIndex > mySize, "NCollection_Sequence::Value");
  ...
}

Under a Release build configured with BUILD_RELEASE_DISABLE_EXCEPTIONS (-DNo_Exception), that
macro-based check compiles to nothing, and indexing a zero-length sequence walks a null node and
dereferences it: a segmentation fault, not a C++ exception. Confirmed with a standalone reproducer
linked directly against a No_Exception-configured build.

GeomAPI_ExtremaCurveCurve::Points(), the public wrapper most callers actually use, has the
identical shape one level up: its own bounds check is also built from
Standard_OutOfRange_Raise_if and is also compiled away under No_Exception, so nothing stands
between a caller of the public API and the crash in that configuration.

Fix

Bound Points() against the container it actually reads:

void Extrema_ExtCC::Points(const int N, Extrema_POnCurv& P1, Extrema_POnCurv& P2) const
{
  if (N < 1 || 2 * N > mypoints.Length())
  {
    throw Standard_OutOfRange();
  }
  P1 = mypoints.Value(2 * N - 1);
  P2 = mypoints.Value(2 * N);
}

NbExt() is left unchanged: it is also used by SquareDistance(), and legitimate callers
(GeomAPI_ExtremaCurveCurve::LowerDistance()) rely on getting a real distance back in exactly the
parallel-distance-only case this fix's Points() now refuses — the perpendicular distance between
two parallel lines is a well-defined number even though there is no unique closest point pair.
Redefining NbExt() to track mypoints instead would have broken that caller; this was verified by
tracing LowerDistance()'s call path (SquareDistance(myIndex), which bounds against NbExt()
too) before deciding, not assumed. A short //! Exceptions line is added to the header declaration.

Also includes a companion, behavior-neutral one-line addition to Geom2dAPI_ExtremaCurveCurve,
which does not have this defect (its own NbExtrema() already reports 0 in the equivalent
parallel case, confirmed by measurement) but was missing the IsParallel() convenience its 3D
sibling GeomAPI_ExtremaCurveCurve has:

//! Returns True if the two curves are parallel.
bool IsParallel() const { return myExtCC.IsParallel(); }

(Extrema_ExtCC2d::IsParallel() was already public and already reachable through the existing
Extrema() accessor, so this is a convenience, not a new capability.)

Reproducer

Four fixtures, run through both GeomAPI_ExtremaCurveCurve (the typical entry point) and
Extrema_ExtCC directly:

1. Two finite, parallel line segments whose projected ranges overlap over a genuine interval —
   crashes.
2. Two finite, parallel line segments whose projected ranges are disjoint — does not crash, has
   a real unique nearest-point answer, unaffected by this fix.
3. Two unbounded, parallel Geom_Lines — crashes.
4. Two finite, parallel line segments whose projected ranges touch at exactly one point — does
   not crash, has a real unique answer, unaffected by this fix. (Included because a first read of
   the source suggested this might be a case where IsParallel() is true but a point pair still
   exists; measuring it showed the source resets IsParallel() to false here, so it is not such a
   case — reported since the reasoning is useful even though it turned out not to be a
   counterexample.)

// Case 1 (crashes before this patch):
Handle(Geom_TrimmedCurve) c1 = GC_MakeSegment(gp_Pnt(0, 0, 0), gp_Pnt(10, 0, 0)).Value();
Handle(Geom_TrimmedCurve) c2 = GC_MakeSegment(gp_Pnt(3, 1, 0), gp_Pnt(13, 1, 0)).Value();
GeomAPI_ExtremaCurveCurve ext(c1, c2);
// ext.IsParallel() == true, ext.NbExtrema() == 1, ext.LowerDistance() == 1.0 (fine)
gp_Pnt p1, p2;
ext.Points(1, p1, p2);  // before: SIGSEGV.  after: throws Standard_OutOfRange.

fixture	IsParallel()	NbExtrema()	Points(1) before	Points(1) after
1. overlapping	1	1	SIGSEGV	Standard_OutOfRange
2. disjoint	0	1	real pair	unchanged
3. unbounded	1	1	SIGSEGV	Standard_OutOfRange
4. touching	0	1	real pair	unchanged

LowerDistance()/Distance(1) return the identical values before and after in every fixture
(measured, since this fix does not touch mySqDist or NbExt()).

Validation

Compiled the patched Extrema_ExtCC.cxx standalone with -DNDEBUG -DNo_Exception (matching a
Release, exceptions-disabled configuration) and linked it ahead of the stock archive. Fixtures 1 and
3 go from a deterministic SIGSEGV (raw exit 139) to a caught Standard_OutOfRange; fixtures 2 and 4
are byte-identical before and after, in both the returned points and the distance values. Full
before/after transcripts and the reproducer source are in the downstream OCCTSwift wrapper's
Scripts/repro/636-extrema-parallel/ (lands on main with that wrapper's v2.0.0)
(issue #636 there).

[gkv311]

@gsdali , it is better to synchronize Extrema_ExtCC2d::Points() implementations with consistent checks.

[dpasukhi]

Dear @gsdali please add GTest to cover your changes, you can follow the logic of all exited GTests.
Previously you already added them.

[dpasukhi]

Too much text which is unreadable without PR context, please concentrate the description in shorter form

[dpasukhi]

Too much text which is unreadable without PR context, please concentrate the description in shorter form

[gsdali]

I've seen the comment, we've just worked through our release against 8.0.1 and can now take a look at this.

[gsdali]

Thanks both — pushed an update.

@gkv311 on synchronizing Extrema_ExtCC2d::Points(): measured before changing, since the two classes aren't actually built the same way. Extrema_ExtCC2d's mynbext and mypoints.Length()/2 are an invariant — both Results() overloads only ever touch mynbext, mySqDist and mypoints together, inside the same conditional, so there's no branch where mynbext advances without a matching point pair (unlike Extrema_ExtCC, where NbExt() reads a separate container, mySqDist, that legitimately gets a parallel-only entry mypoints doesn't). So this wasn't reachable as a crash. Applied your suggestion anyway since it's provably a no-op: Points() now checks 2 * N > mypoints.Length() like the 3D fix, confirmed byte-identical output against the old check across four fixtures (intersecting, parallel overlapping/disjoint/touching) with a standalone equivalence probe before pushing.

@dpasukhi: added Extrema_ExtCC_Test.cxx and Extrema_ExtCC2d_Test.cxx in src/ModelingData/TKGeomBase/GTests/. Verified both ways: against the current pinned kernel (predates this fix), the two parallel-crash fixtures SIGSEGV; linked with the patched translation units ahead of the archive, all 7 tests pass.