View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0026230CommunityOCCT:Modeling Algorithmspublic2015-05-18 16:132015-10-23 20:51
ReporterBenjaminBihler Assigned Tobugmaster  
PrioritynormalSeveritycrash 
Status closedResolutionfixed 
PlatformLinuxOSDebian 6.0 
Product Version6.7.1 
Target Version6.9.1Fixed in Version6.9.1 
Summary0026230: Segmentation fault because a NULL curve is used without precaution in case of a projection failure
DescriptionUnfortunately my example file is confidential, but I try to describe it as detailed as possible.

I call BOPTools_AlgoTools2D::CurveOnSurface(...) to get a Handle(Geom2d_Curve) for an edge on a face (but the edge is not a boundary edge of that face).

Finally we come down to BOPTools_AlgoTools2D::MakePCurveOnFace. Here it is tried three times to compute the projection of the curve onto the surface (around line 610 of BOPTools_AlgoTools2D.cxx). The first two times, if the projection fails and the result curve is still NULL, the projection is tried again with a raised tolerance. But when the third projection fails, this is not handled anymore. The resulting curve is a NULL handle and therefore I get a segmentation fault here when the statement

  BOPTools_AlgoTools2D::AdjustPCurveOnFace (aF, aFirst, aLast,
                                            aC2D, aC2DA);

(around line 621 of BOPTools_AlgoTools2D.cxx) is executed.

I would expect another aC2D.IsNull() check at the end of BOPTools_AlgoTools2D::MakePCurveOnFace and an error should be raised in this case.
Steps To ReproduceNot required
TagsNo tags attached.
Test case numberNot needed

Activities

git

2015-05-21 16:53

administrator   ~0041393

Branch CR26230 has been created by ifv.

SHA-1: 5a07bb57f17ebfb987a9bcf4364920073101c282


Detailed log of new commits:

Author: ifv
Date: Thu May 21 16:29:18 2015 +0300

    0026230: Segmentation fault because a NULL curve is used without precaution in case of a projection failure

ifv

2015-05-21 16:54

developer   ~0041394

branch CR26230 is ready for review

msv

2015-05-21 17:23

developer   ~0041395

It is needed to check the pcurve for null not only in the methods of the class BOPTools_AlgoTools2D, but follow the caller of these methods everywhere in OCCT code and protect all found places.

BenjaminBihler

2015-05-28 12:29

developer   ~0041646

Wouldn't it be sufficient to raise an error, if projection is not possible? Then the callers can handle this as they want. At least for me this would suffice.

msv

2015-05-28 12:38

developer   ~0041647

After discussion, we came to conclusion that it is better to raise a C++ exception in the case of projection failure.

BenjaminBihler

2015-05-28 12:57

developer   ~0041651

Might it be possible then to include this improvement into a release earlier than 7.1.0? It would really be helpful!

abv

2015-05-28 14:39

manager   ~0041664

Mikhail, please reconsider your idea of raising exception: in most cases it is better to produce invalid shape (without pcurve), which still can be usable, than exception (which will never be usable and has big chance to cause serious troubles on the application level). Consider returning some result and error status if problem occurs.

msv

2015-05-28 15:17

developer   ~0041671

Andrey, this method was developed to be used in the context of Boolean operations. Failure of pcurve construction cannot lead to a usable result of Boolean. Exception will be caught on the upper level of the algorithm, and error status will be set.
However, I think we should leave a corresponding warning in Doxygen documentation for all methods of this class (BOPTools), which will raise exception if projection is failure.

msv

2015-05-28 15:21

developer   ~0041673

Benjamin, if the fix will be already in master when 7.0.0 will be prepared then certainly this fix will hit in the release.

git

2015-05-29 13:55

administrator   ~0041720

Branch CR26230 has been updated forcibly by ifv.

SHA-1: 4b1a5d90d077c6eb71df42f311d7af843fdd8827

msv

2015-05-29 16:30

developer   ~0041750

Reviewed.

git

2015-05-29 17:08

administrator   ~0041756

Branch CR26230 has been updated forcibly by apv.

SHA-1: 1d42e8fa44d0e06e5dac40ec2b9c4be7d584a962

apv

2015-05-29 17:09

tester   ~0041757

Branch CR26230 has been rebased on the current master

apv

2015-06-01 18:24

tester   ~0041811

Dear BugMaster,

Branch CR26230 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: 1d42e8fa44d0e06e5dac40ec2b9c4be7d584a962

Number of compiler warnings:
occt component:
   Linux: 25 (25 on master)
   Windows: 0 (0 on master)
products component:
   Linux: 37 (37 on master)
   Windows: 0 (0 on master)

Regressions/Differences:
Not detected

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 96045957 / 96362479 [-0.33%]
Total CPU difference: 18211.99999999994 / 18304.16000000009 [-0.50%]

Testing on Windows:
Total MEMORY difference: 56696505 / 57009585 [-0.55%]
Total CPU difference: 17356.515258998883 / 17523.436328998818 [-0.95%]

git

2015-08-14 10:59

administrator   ~0044251

Branch CR26230 has been deleted by inv.

SHA-1: 1d42e8fa44d0e06e5dac40ec2b9c4be7d584a962

Related Changesets

occt: master b9a7d225

2015-05-29 08:25:56

ifv


Committer: bugmaster Details Diff		 0026230: Segmentation fault because a NULL curve is used without precaution in case of a projection failure Affected Issues
0026230
mod - src/BOPTools/BOPTools_AlgoTools2D.cdl Diff File
mod - src/BOPTools/BOPTools_AlgoTools2D.cxx Diff File

Issue History

Date Modified Username Field Change
2015-05-18 16:13 BenjaminBihler New Issue
2015-05-18 16:13 BenjaminBihler Assigned To => msv
2015-05-18 17:06 msv Assigned To msv => ifv
2015-05-18 17:08 msv Target Version => 7.1.0
2015-05-21 16:53 git Note Added: 0041393
2015-05-21 16:54 ifv Note Added: 0041394
2015-05-21 16:54 ifv Assigned To ifv => msv
2015-05-21 16:54 ifv Status new => resolved
2015-05-21 16:54 ifv Steps to Reproduce Updated
2015-05-21 17:23 msv Note Added: 0041395
2015-05-21 17:23 msv Status resolved => assigned
2015-05-21 17:23 msv Assigned To msv => ifv
2015-05-28 12:29 BenjaminBihler Note Added: 0041646
2015-05-28 12:38 msv Note Added: 0041647
2015-05-28 12:57 BenjaminBihler Note Added: 0041651
2015-05-28 14:39 abv Note Added: 0041664
2015-05-28 15:17 msv Note Added: 0041671
2015-05-28 15:21 msv Note Added: 0041673
2015-05-29 13:55 git Note Added: 0041720
2015-05-29 13:56 ifv Assigned To ifv => msv
2015-05-29 13:56 ifv Status assigned => resolved
2015-05-29 16:30 msv Note Added: 0041750
2015-05-29 16:30 msv Assigned To msv => bugmaster
2015-05-29 16:30 msv Status resolved => reviewed
2015-05-29 16:31 apv Assigned To bugmaster => apv
2015-05-29 17:08 git Note Added: 0041756
2015-05-29 17:09 apv Note Added: 0041757
2015-06-01 17:52 apv Test case number => Not needed
2015-06-01 18:24 apv Note Added: 0041811
2015-06-01 18:24 apv Assigned To apv => bugmaster
2015-06-01 18:24 apv Status reviewed => tested
2015-06-05 15:00 bugmaster Changeset attached => occt master b9a7d225
2015-06-05 15:00 bugmaster Status tested => verified
2015-06-05 15:00 bugmaster Resolution open => fixed
2015-06-08 12:32 bugmaster Target Version 7.1.0 => 7.0.0
2015-08-14 10:59 git Note Added: 0044251
2015-08-26 11:11 abv Target Version 7.0.0 => 6.9.1
2015-10-16 14:56 aiv Status verified => closed
2015-10-23 20:51 aiv Fixed in Version => 6.9.1