Shape Healing - Guard ShapeFix_Face::Perform against non-face context replacement#1323
Draft
gsdali wants to merge 1 commit into
Draft
Conversation
… replacement When a prior fix in the shared ReShape context replaced the face with a non-face (e.g. a self-intersecting face split into a compound), Context()->Apply(myFace) returns a non-face and the TopoDS::Face casts in Perform build an invalid handle, corrupting the shape and crashing. Skip Perform in that case. Added GTest ShapeFix_ShapeTest.HealPrismFromSelfIntersectingFace.
e7cee19 to
5ee3535
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1322.
Problem
ShapeFix_Face::PerformcastsContext()->Apply(myFace)toTopoDS_FaceviaTopoDS::Face(S.EmptyCopied())in three places, none of which check the shape type:When a fix sharing the same
ShapeBuild_ReShapecontext has already replaced the face with a compound (e.g. a self-intersecting face split into several faces),Apply()returns a non-face. The unchecked cast then builds an invalidTopoDS_Facehandle over a compoundTShape; a subsequent operation corrupts the shape and the process aborts with an uncatchable OS signal:The fault address varies run to run (SIGSEGV / SIGBUS) - a heap-corruption signature. Diagnosed by linking a debug-built
ShapeFix_Face.cxx: at-O0the cast throwsStandard_TypeMismatchat the first site withApply(myFace).ShapeType() == TopAbs_COMPOUNDwhilemyFaceis still aTopAbs_FACE.Fix
The compound replacement already exists on entry to
Perform(it was recorded by a prior face's fix), so a single guard at the top ofPerformcovers all three cast sites: ifContext()->Apply(myFace)is not a face, record it as the result and return - the replacement is already in the context, so there is nothing to fix here.Reproducer / test
Extrude a 4-point "bowtie" face (self-intersecting wire) into a prism and run
ShapeFix_Shape:Crashes on current master; fixed here. Added GTest
ShapeFix_ShapeTest.HealPrismFromSelfIntersectingFace.Validation
Verified by compiling the patched
ShapeFix_Face.cxxand linking it over the library: the bowtie reproducer and four real mesh-derived fixtures survive (5/5), and the prism now heals to a valid 8-face solid.ShapeFix_Shapeoutput on valid box/sphere/cylinder is unchanged (the guard only triggers for a non-face replacement, which never happens for a well-formed face).Reported downstream and isolated at SecondMouseAU/OCCTSwift#263.
Draft pending CLA confirmation and CI.