From 154c51fdc752bae8726df274aca4286841e23edd Mon Sep 17 00:00:00 2001 From: Sander Adamson Date: Fri, 18 Jul 2025 12:38:14 +0100 Subject: [PATCH] 0033895: fix null surface crash in fixshape ShapeFix_Shape crashes with a segfault when given a shape that contains faces without a surface pointer, for example if the shape is tessellated or triangulated, or contains tessellated / triangulated subshapes. I added null checks to the `const Handle(Geom_Surface)&` parameter of the ShapeAnalysis_Surface constructor, and to the analogous parameter of the Init method, in each case throwing a Standard_NullObject exception. Now, ShapeFix_Shape::Perform will throw an exception instead of crashing with a segfault whenever it encounters a face without a surface. I have CLA 1124. --- data/step/tessellated_tetrahedron_ap242.step | 83 ++++++++++++ .../TKShHealing/GTests/FILES.cmake | 3 +- .../GTests/ShapeHealing_TessellatedTest.cxx | 119 ++++++++++++++++++ .../ShapeAnalysis/ShapeAnalysis_Surface.cxx | 5 + tests/bugs/step/bug33895 | 39 ++++++ 5 files changed, 248 insertions(+), 1 deletion(-) create mode 100644 data/step/tessellated_tetrahedron_ap242.step create mode 100644 src/ModelingAlgorithms/TKShHealing/GTests/ShapeHealing_TessellatedTest.cxx create mode 100644 tests/bugs/step/bug33895 diff --git a/data/step/tessellated_tetrahedron_ap242.step b/data/step/tessellated_tetrahedron_ap242.step new file mode 100644 index 00000000000..277c8273bb0 --- /dev/null +++ b/data/step/tessellated_tetrahedron_ap242.step @@ -0,0 +1,83 @@ +ISO-10303-21; +HEADER; +/* Generated by OCCT for testing shape healing with tessellated geometry */ +FILE_DESCRIPTION(('Tessellated tetrahedron in AP242'),'2;1'); +FILE_NAME('tessellated_tetrahedron_ap242.step','2025-06-19T10:00:00',('OpenCASCADE'),('Organization'),'7.8.0','OpenCASCADE STEP Writer',''); +FILE_SCHEMA(('AP242_MANAGED_MODEL_BASED_3D_ENGINEERING_MIM_LF { 1 0 10303 442 1 1 4 }')); +ENDSEC; +DATA; +/* Application context */ +#1 = APPLICATION_PROTOCOL_DEFINITION('international standard','ap242_managed_model_based_3d_engineering',2014,#2); +#2 = APPLICATION_CONTEXT('managed model based 3d engineering'); + +/* Product definition structure */ +#3 = SHAPE_DEFINITION_REPRESENTATION(#4,#10); +#4 = PRODUCT_DEFINITION_SHAPE('tessellated shape','shape definition for tessellated tetrahedron',#5); +#5 = PRODUCT_DEFINITION('design','',#6,#9); +#6 = PRODUCT_DEFINITION_FORMATION('','',#7); +#7 = PRODUCT('Tetrahedron','Tetrahedron','',(#8)); +#8 = PRODUCT_CONTEXT('',#2,'mechanical'); +#9 = PRODUCT_DEFINITION_CONTEXT('part definition',#2,'design'); + +/* Shape representation with tessellated solid */ +#10 = TESSELLATED_SHAPE_REPRESENTATION('tessellated representation',(#11,#12),#16); +#11 = AXIS2_PLACEMENT_3D('',#13,#14,#15); +#12 = TESSELLATED_SOLID('tetrahedron solid',(#17,#18,#19,#20),.T.); + +/* Coordinate system */ +#13 = CARTESIAN_POINT('',(0.,0.,0.)); +#14 = DIRECTION('',(0.,0.,1.)); +#15 = DIRECTION('',(1.,0.,0.)); + +/* Representation context with units */ +#16 = ( GEOMETRIC_REPRESENTATION_CONTEXT(3) +GLOBAL_UNCERTAINTY_ASSIGNED_CONTEXT((#21)) +GLOBAL_UNIT_ASSIGNED_CONTEXT((#22,#23,#24)) +REPRESENTATION_CONTEXT('Context','3D Context with UNIT and UNCERTAINTY') ); + +/* Tetrahedron faces */ +/* Base triangle - vertices 1,2,3 */ +#17 = TRIANGULATED_FACE('Base',#25,3,( + (0.,0.,-1.), /* Normal pointing down */ + (0.,0.,-1.), + (0.,0.,-1.) +),$,(1,3,2),((1,3,2))); + +/* Side face 1 - vertices 1,2,4 */ +#18 = TRIANGULATED_FACE('Side1',#25,3,( + (0.,-0.942809,0.333333), /* Outward normals */ + (0.,-0.942809,0.333333), + (0.,-0.942809,0.333333) +),$,(1,2,4),((1,2,4))); + +/* Side face 2 - vertices 2,3,4 */ +#19 = TRIANGULATED_FACE('Side2',#25,3,( + (0.816497,0.471405,0.333333), + (0.816497,0.471405,0.333333), + (0.816497,0.471405,0.333333) +),$,(2,3,4),((2,3,4))); + +/* Side face 3 - vertices 3,1,4 */ +#20 = TRIANGULATED_FACE('Side3',#25,3,( + (-0.816497,0.471405,0.333333), + (-0.816497,0.471405,0.333333), + (-0.816497,0.471405,0.333333) +),$,(3,1,4),((3,1,4))); + +/* Units and uncertainty */ +#21 = UNCERTAINTY_MEASURE_WITH_UNIT(LENGTH_MEASURE(1.E-07),#22, + 'distance_accuracy_value','confusion accuracy'); +#22 = ( LENGTH_UNIT() NAMED_UNIT(*) SI_UNIT(.MILLI.,.METRE.) ); +#23 = ( NAMED_UNIT(*) PLANE_ANGLE_UNIT() SI_UNIT($,.RADIAN.) ); +#24 = ( NAMED_UNIT(*) SI_UNIT($,.STERADIAN.) SOLID_ANGLE_UNIT() ); + +/* Tetrahedron vertex coordinates */ +#25 = COORDINATES_LIST('vertices',4,( + (0.,0.,0.), /* Base vertex 1 */ + (100.,0.,0.), /* Base vertex 2 */ + (50.,86.602540378,0.), /* Base vertex 3 */ + (50.,28.867513459,81.649658093) /* Apex */ +)); + +ENDSEC; +END-ISO-10303-21; \ No newline at end of file diff --git a/src/ModelingAlgorithms/TKShHealing/GTests/FILES.cmake b/src/ModelingAlgorithms/TKShHealing/GTests/FILES.cmake index 8c15769379b..3b540d02723 100644 --- a/src/ModelingAlgorithms/TKShHealing/GTests/FILES.cmake +++ b/src/ModelingAlgorithms/TKShHealing/GTests/FILES.cmake @@ -2,4 +2,5 @@ set(OCCT_TKShHealing_GTests_FILES_LOCATION "${CMAKE_CURRENT_LIST_DIR}") set(OCCT_TKShHealing_GTests_FILES -) + ShapeHealing_TessellatedTest.cxx +) \ No newline at end of file diff --git a/src/ModelingAlgorithms/TKShHealing/GTests/ShapeHealing_TessellatedTest.cxx b/src/ModelingAlgorithms/TKShHealing/GTests/ShapeHealing_TessellatedTest.cxx new file mode 100644 index 00000000000..b1a1d985fa1 --- /dev/null +++ b/src/ModelingAlgorithms/TKShHealing/GTests/ShapeHealing_TessellatedTest.cxx @@ -0,0 +1,119 @@ +// Copyright (c) 2025 OPEN CASCADE SAS +// +// This file is part of Open CASCADE Technology software library. +// +// This library is free software; you can redistribute it and/or modify it under +// the terms of the GNU Lesser General Public License version 2.1 as published +// by the Free Software Foundation, with special exception defined in the file +// OCCT_LGPL_EXCEPTION.txt. Consult the file LICENSE_LGPL_21.txt included in OCCT +// distribution for complete text of the license and disclaimer of any warranty. +// +// Alternatively, this file may be used under the terms of Open CASCADE +// commercial license or contractual agreement. + +// OCCT headers +#include +#include +#include +#include +#include +#include +#include +#include + +// GoogleTest headers +#include + +// System headers +#include + +//! Test fixture for shape healing operations on tessellated geometry. +//! This fixture loads a tessellated tetrahedron from a STEP file +//! and provides it for testing shape healing algorithms. +//! Related to bug 33895: fixshape crashes on tessellated geometry without surfaces. +class ShapeHealing_TessellatedTest : public ::testing::Test +{ +protected: + TopoDS_Shape myShape; + + void SetUp() override + { + // Load the tessellated tetrahedron STEP file + STEPControl_Reader aReader; + + // Use relative path from test execution directory + const char* aFileName = "data/step/tessellated_tetrahedron_ap242.step"; + + // Check if file exists + std::ifstream aFile(aFileName); + if (!aFile.good()) + { + GTEST_SKIP() << "Test file not found: " << aFileName; + return; + } + aFile.close(); + + IFSelect_ReturnStatus aStatus = aReader.ReadFile(aFileName); + ASSERT_EQ(aStatus, IFSelect_RetDone) << "Failed to read STEP file"; + + Standard_Integer aNbRoots = aReader.NbRootsForTransfer(); + ASSERT_GT(aNbRoots, 0) << "No roots found in STEP file"; + + aReader.TransferRoots(); + + // Get the shape + myShape = aReader.OneShape(); + ASSERT_FALSE(myShape.IsNull()) << "Loaded shape is null"; + } +}; + +//! Test that the shape loads successfully and has expected structure. +//! Verifies that the tessellated tetrahedron has 4 faces, 1 shell, and 1 solid. +//! Also checks that the shape is considered invalid due to missing surfaces. +TEST_F(ShapeHealing_TessellatedTest, LoadAndValidateShape) +{ + // Count sub-shapes + Standard_Integer aNbFaces = 0; + Standard_Integer aNbShells = 0; + Standard_Integer aNbSolids = 0; + + for (TopExp_Explorer anExp(myShape, TopAbs_FACE); anExp.More(); anExp.Next()) + { + aNbFaces++; + } + + for (TopExp_Explorer anExp(myShape, TopAbs_SHELL); anExp.More(); anExp.Next()) + { + aNbShells++; + } + + for (TopExp_Explorer anExp(myShape, TopAbs_SOLID); anExp.More(); anExp.Next()) + { + aNbSolids++; + } + + // Expected: 4 faces (tetrahedron), 1 shell, 1 solid + // Note: This test verifies the specific tessellated tetrahedron structure + EXPECT_EQ(aNbFaces, 4) << "Tessellated tetrahedron should have 4 faces"; + EXPECT_EQ(aNbShells, 1) << "Should have 1 shell"; + EXPECT_EQ(aNbSolids, 1) << "Should have 1 solid"; + + // Check shape validity + BRepCheck_Analyzer aChecker(myShape); + EXPECT_FALSE(aChecker.IsValid()) << "Tessellated shape without surfaces should not be valid"; +} + +//! Test that ShapeFix_Shape throws exception on tessellated geometry. +//! Verifies that attempting to heal a tessellated shape without surfaces +//! throws a Standard_NullObject exception instead of crashing. +//! This test reproduces bug 33895. +TEST_F(ShapeHealing_TessellatedTest, ShapeFixThrowsException) +{ + Handle(ShapeFix_Shape) aFixer = new ShapeFix_Shape(myShape); + aFixer->SetPrecision(1.0e-6); + aFixer->SetMaxTolerance(1.0); + + // This should throw Standard_NullObject exception instead of crashing + EXPECT_THROW({ aFixer->Perform(); }, Standard_NullObject) + << "ShapeFix_Shape should throw exception for tessellated geometry"; +} diff --git a/src/ModelingAlgorithms/TKShHealing/ShapeAnalysis/ShapeAnalysis_Surface.cxx b/src/ModelingAlgorithms/TKShHealing/ShapeAnalysis/ShapeAnalysis_Surface.cxx index 002729168a7..4332a03afa6 100644 --- a/src/ModelingAlgorithms/TKShHealing/ShapeAnalysis/ShapeAnalysis_Surface.cxx +++ b/src/ModelingAlgorithms/TKShHealing/ShapeAnalysis/ShapeAnalysis_Surface.cxx @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -105,6 +106,8 @@ ShapeAnalysis_Surface::ShapeAnalysis_Surface(const Handle(Geom_Surface)& S) myUCloseVal(-1), myVCloseVal(-1) { + // Bug 33895: Prevent crash when surface is null + Standard_NullObject_Raise_if(mySurf.IsNull(), "ShapeAnalysis_Surface: Cannot create with null surface"); mySurf->Bounds(myUF, myUL, myVF, myVL); myAdSur = new GeomAdaptor_Surface(mySurf); } @@ -115,6 +118,8 @@ void ShapeAnalysis_Surface::Init(const Handle(Geom_Surface)& S) { if (mySurf == S) return; + // Bug 33895: Prevent crash when surface is null + Standard_NullObject_Raise_if(S.IsNull(), "ShapeAnalysis_Surface::Init: Cannot initialize with null surface"); myExtOK = Standard_False; //: 30 mySurf = S; myNbDeg = -1; diff --git a/tests/bugs/step/bug33895 b/tests/bugs/step/bug33895 new file mode 100644 index 00000000000..5e8c37d4776 --- /dev/null +++ b/tests/bugs/step/bug33895 @@ -0,0 +1,39 @@ +puts "========" +puts "bug33895" +puts "========" +puts "" +################################################# +# Bug 33895: fixshape crashes on tessellated geometry without surfaces +# Test fixshape on tessellated tetrahedron in AP242 +# Expect exception on tessellated geometry without surfaces +################################################# + +pload XDE MODELING + +stepread [locate_data_file tessellated_tetrahedron_ap242.step] a * + +# Check what kind of shape we get +checknbshapes a_1 -t + +# Validate shape - will show BRepCheck_NoSurface errors +checkshape a_1 + +# Test fixshape - expect exception +set exception_thrown 0 +if {[catch {fixshape result a_1} err]} { + puts "EXPECTED: Exception caught during fixshape: $err" + if {[string match "*Standard_NullObject*" $err]} { + puts "SUCCESS: Correct exception type (Standard_NullObject)" + set exception_thrown 1 + } else { + puts "ERROR: Wrong exception type" + } +} else { + puts "ERROR: No exception thrown - fixshape should fail on tessellated geometry" +} + +if {$exception_thrown == 0} { + puts "TEST FAILED: Expected Standard_NullObject exception" +} else { + puts "TEST PASSED: Exception properly thrown for tessellated geometry" +} \ No newline at end of file