OCCT3D
DiscussionsIssue archiveOCCT:Modeling Algorithms

Archived issue #0031009

Modeling Algorithms - alloc-dealloc-mismatch reported by Clang address sanitizer in IntCurvesFace_ShapeIntersector

Open CASCADEOCCT:Modeling Algorithmsverified38 public notes

Search issues

Description

When running OCCT built on Linux with Clang with option -fsanitize=address, error alloc-dealloc-mismatch is reported on test bugs moddata_1 bug17424:

OCC17424 a 86.960439048069262 1028.2863798310484 -266.58526583558000 0.25881904510252074 0 -0.96592582628906831 0.
=================================================================
==3012==ERROR: AddressSanitizer: alloc-dealloc-mismatch (operator new [] vs operator delete) on 0x6040000aae10
    #0 0x5174e0 in operator delete(void*) ([user path removed]/tmp/occt-clang/lin64/clang/bini/DRAWEXE-7.4.0+0x5174e0)
    #0000001 0x7fed0aa00cda in IntCurvesFace_ShapeIntersector::Destroy() [user path removed]/occt/src/IntCurvesFace/IntCurvesFace_ShapeIntersector.cxx:59:5
    #0000002 0x7fecf96f95ce in IntCurvesFace_ShapeIntersector::~IntCurvesFace_ShapeIntersector() [user path removed]/occt/src/IntCurvesFace/IntCurvesFace_ShapeIntersector.hxx:118:3
    #3 0x7fecf96dadd5 in OCC17424(Draw_Interpretor&, int, char const**) [user path removed]/occt/src/QABugs/QABugs_11.cxx:4564:1
    #4 0x7fed0c24c349 in Draw_Interpretor::CallBackDataFunc::Invoke(Draw_Interpretor&, int, char const**) [user path removed]/occt/src/Draw/Draw_Interpretor.hxx:81:31
    #5 0x7fed0c259b7d in CommandCmd(void*, Tcl_Interp*, int, char const**) [user path removed]/occt/src/Draw/Draw_Interpretor.cxx:154:40
    #6 0x7fed065e9b95 in TclInvokeStringCommand (/usr/lib/x86_64-linux-gnu/libtcl8.6.so+0x38b95)
    #0000007 0x7fed065ebfa6 in TclNRRunCallbacks (/usr/lib/x86_64-linux-gnu/libtcl8.6.so+0x3afa6)
    #0000008 0x7fed0668a87a in Tcl_RecordAndEvalObj (/usr/lib/x86_64-linux-gnu/libtcl8.6.so+0xd987a)
    #0000009 0x7fed0668a756 in Tcl_RecordAndEval (/usr/lib/x86_64-linux-gnu/libtcl8.6.so+0xd9756)
    #10 0x7fed0c25b0bf in Draw_Interpretor::RecordAndEval(char const*, int) [user path removed]/occt/src/Draw/Draw_Interpretor.cxx:496:10
    #0000011 0x7fed0c241dad in Draw_Interprete(char const*) [user path removed]/occt/src/Draw/Draw.cxx:608:19
    #12 0x7fed0c242b90 in interpreteTclCommand(TCollection_AsciiString const&) [user path removed]/occt/src/Draw/Draw.cxx:110:5
    #0000013 0x7fed0c24037b in ReadInitFile(TCollection_AsciiString const&) [user path removed]/occt/src/Draw/Draw.cxx:121:3
    #0000014 0x7fed0c23f973 in Draw_Appli(int, char**, void (*)(Draw_Interpretor&)) [user path removed]/occt/src/Draw/Draw.cxx:497:5
    #0000015 0x7fed0c25c328 in Draw_Main(int, char**, void (*)(Draw_Interpretor&)) [user path removed]/occt/src/Draw/Draw_Main.cxx:113:3
    #0000016 0x51aaef in main [user path removed]/occt/src/DRAWEXE/DRAWEXE.cxx:33:1
    #0000017 0x7fed0501582f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #0000018 0x41aac8 in _start ([user path removed]/tmp/occt-clang/lin64/clang/bini/DRAWEXE-7.4.0+0x41aac8)

0x6040000aae10 is located 0 bytes inside of 40-byte region [0x6040000aae10,0x6040000aae38)
allocated by thread T0 here:
    #0 0x516918 in operator new[](unsigned long) ([user path removed]/tmp/occt-clang/lin64/clang/bini/DRAWEXE-7.4.0+0x516918)
    #0000001 0x7fed0aa01b9f in IntCurvesFace_ShapeIntersector::PerformNearest(gp_Lin const&, double, double) [user path removed]/occt/src/IntCurvesFace/IntCurvesFace_ShapeIntersector.cxx:98:33
    #0000002 0x7fecf96dad0c in OCC17424(Draw_Interpretor&, int, char const**) [user path removed]/occt/src/QABugs/QABugs_11.cxx:4554:15
    #3 0x7fed0c24c349 in Draw_Interpretor::CallBackDataFunc::Invoke(Draw_Interpretor&, int, char const**) [user path removed]/occt/src/Draw/Draw_Interpretor.hxx:81:31
    #4 0x7fed0c259b7d in CommandCmd(void*, Tcl_Interp*, int, char const**) [user path removed]/occt/src/Draw/Draw_Interpretor.cxx:154:40
    #5 0x7fed065e9b95 in TclInvokeStringCommand (/usr/lib/x86_64-linux-gnu/libtcl8.6.so+0x38b95)

SUMMARY: AddressSanitizer: alloc-dealloc-mismatch ([user path removed]/tmp/occt-clang/lin64/clang/bini/DRAWEXE-7.4.0+0x5174e0) in operator delete(void*)
==3012==HINT: if you don't care about these errors you may set ASAN_OPTIONS=alloc_dealloc_mismatch=0
==3012==ABORTING

Steps to reproduce

N/A

Additional information

Class IntCurvesFace_ShapeIntersector needs to be refactored as a whole:

- Use typed collections instead of void* buffers
- Use English words to name variables (instead of French)
- Follow OCCT coding rules in names of variables and formatting

Public activity

38 archived notes

Participants are labeled by their role within this record.

01Commenter 2
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: Andrei LATYSHEV
Date: Tue Feb 8 18:53:18 2022 +0300

    Refactoring of class IntCurvesFace_ShapeIntersector
    
    * Memory leaks fixes
    * Comments translated from French to English
    * Code guidelines improvements
02Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: Andrei LATYSHEV
Date: Tue Feb 8 19:34:43 2022 +0300

    Update IntCurvesFace_ShapeIntersector.cxx
03Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: Andrei LATYSHEV
Date: Wed Feb 9 10:08:04 2022 +0300

    Update FILES
04Commenter 2
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: Andrei LATYSHEV
Date: Wed Feb 9 11:45:57 2022 +0300

    Update FILES

Author: Andrei LATYSHEV
Date: Tue Feb 8 20:10:40 2022 +0300

    Removal of old-style variables
05Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: Andrei LATYSHEV
Date: Wed Feb 9 14:23:32 2022 +0300

    Merge branch 'CR31009_1' into CR31009
06Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: Andrei LATYSHEV
Date: Wed Feb 9 19:33:17 2022 +0300

    Fix for regressions
07Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: Andrei LATYSHEV
Date: Fri Feb 11 13:50:09 2022 +0300

    Update IntCurvesFace_ShapeIntersector.cxx
08Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: Andrei LATYSHEV
Date: Mon Feb 14 15:11:00 2022 +0300

    Refactoring IntCurveFace_* (next steps)
09Commenter 9
Please review branch CR31009
Test results are attached:
http://jenkins-test-occt/view/CR31009-master-ALATYSHE/view/ALL/
10Commenter 3
Dear Commenter 4, please follow bug advancement workflow.

https://dev.opencascade.org/doc/overview/html/occt_contribution__contribution_workflow.html
11Commenter 2
Branch [archived branch] has been deleted by Participant.

[revision removed]
12Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
13Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
14Commenter 3
Please do not use tabulation symbols in new code.
15Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
16Commenter 16
src/IntCurvesFace/IntCurvesFace_ShapeIntersector.hxx

85:        return(myIndexPt.Length());

No need to enclose in brackets the returned value.

===========================

92:        Handle(IntCurvesFace_Intersector) aHandle = myIntersector(myIndexFace(myIndexPt(I)));

Use more meaningful name instead of aHandle. And in all other places too.

===========================

156: protected:

To be removed.

===========================

src/IntCurvesFace/IntCurvesFace_ShapeIntersector.cxx

34: IntCurvesFace_ShapeIntersector::IntCurvesFace_ShapeIntersector() 
35: {
36:     myNbFaces = 0;
37:     myIsDone = Standard_False;
38:     myPtrNums.Nullify();
39:     myPtrIndexNums.Nullify();
40: }

Use direct initialization of members
IntCurvesFace_ShapeIntersector::IntCurvesFace_ShapeIntersector()
: myIsDone(Standard_False),
  myNbFaces(0)

and, please, keep the order of initializing members as it was defined in class declaration, because it is crucial for GCC compilers.

===========================

42: void IntCurvesFace_ShapeIntersector::Load(const TopoDS_Shape& theShape,
43:                      const Standard_Real theTol)

Use appropriate indentation when substitute tabulation with spaces.

===========================

95:            auto aPtrNums = myPtrNums;
96:            auto aPtrIndexNums = myPtrIndexNums;

Please, avoid auto variables. Also, I do not see the necessity to use these local variables.

===========================

144:        myPtrNums->ChangeValue(anIndexFace) = myPtrNums->Value(anIndexFace) + 1;

should be 
144:        myPtrNums->ChangeValue(anIndexFace) += 1;


===========================

166: //-- myPtrIntersector   : Sequence of the addresses

myIntersector?

===========================

168: //-- myIndexFace        : Number of the face (of the intersector) of the point myIndexPt(i)
169: //-- myIndexIntPnt      : Number of the point  myIndexPt(i) of the intersection myIndexFace(myIndexPt(i))

"Index" instead of "Number"?

===========================

229: // Creation le 28 jan 98
230: // Created on the January, 28th, 1998

To be removed.

src/IntCurvesFace/IntCurvesFace_Intersector.cxx
src/IntCurvesFace/IntCurvesFace_Intersector.hxx
src/IntCurvesFace/IntCurvesFace_Intersector.lxx

These changes are not relevant for the issue is being solved, except that derivation from Standard_Transient. Moreover, the necessary macros should be added to IntCurvesFace_Intersector to provide the interfaces required for HANDLE classes.


General remarks:

1. Indentation in sources should be 2 (two) spaces (https://dev.opencascade.org/doc/overview/html/occt_contribution__coding_rules.html#occt_coding_rules_3)

2. Please, rebase to the current master before testing.
17Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: Andrei LATYSHEV
Date: Tue Mar 1 16:37:55 2022 +0300

    Review fixes
18Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
19Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
20Commenter 3
+  Handle(TColStd_HArray1OfInteger) myPtrNums;

I guess TColStd_Array1OfInteger should be enough for this use case.

+  ~IntCurvesFace_ShapeIntersector()
+  {
+    Destroy();
+  }

Destroy() - is an artifact of legacy destructors and could be removed (replaced by normal C++ destructor).
21Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: Andrei LATYSHEV
Date: Thu Mar 3 22:38:46 2022 +0300

    Review fixes
22Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
23Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: Andrei LATYSHEV
Date: Fri Mar 4 13:56:41 2022 +0300

    Handles fixes
24Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
25Commenter 25
src/IntCurvesFace/IntCurvesFace_ShapeIntersector.hxx

26:#include <BRepTopAdaptor_SeqOfPtr.hxx>

No need to keep this include.


155:  TColStd_HArray1OfInteger myPtrNums;
156:  TColStd_HArray1OfInteger myPtrIndexNums;

Kirill proposed to use TColStd_Array1OfInteger instead of TColStd_HArray1OfInteger (the latter is for Handle()).


139:-  TColStd_SequenceOfInteger IndexFace;
140:-  TColStd_SequenceOfInteger IndexIntPnt;

159:+  TColStd_SequenceOfInteger myNumberFace;
160:+  TColStd_SequenceOfInteger myNumberIntPnt;

Why did you change "Index" to "Number"? My previous remarks was about comment in *.cxx file only.


src/IntCurvesFace/IntCurvesFace_ShapeIntersector.cxx

70:if(myNbFaces > 2) 
71:{  
72:  if (myPtrNums.IsEmpty())
73:  {

Wrong indentation.


src/IntCurvesFace/IntCurvesFace_Intersector.cxx

140:  myPolyhedron(NULL),
141:  myBndBounding(NULL),

To be removed

404:      IntCurveSurface_ThePolyhedronToolOfHInter::ComponentsBounding(*aPolyhedron));


469:      IntCurveSurface_ThePolyhedronToolOfHInter::ComponentsBounding(*aPolyhedron));


513: IntCurvesFace_Intersector::~IntCurvesFace_Intersector()
514: {
515: }

Update indentation.
26Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
27Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
28Commenter 28
src/IntCurvesFace/IntCurvesFace_Intersector.cxx

1. Wrong indentation
383:  if(parinf>pinf) parinf = pinf;
384:  if(parsup<psup) parsup = psup;


src/IntCurvesFace/IntCurvesFace_ShapeIntersector.cxx

2. Remove empty line 216.


src/IntCurvesFace/IntCurvesFace_ShapeIntersector.hxx

3. Should be
26:#include <TColStd_Array1OfInteger.hxx>

instead of
26:#include <TColStd_HArray1OfInteger.hxx>

29Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
30Commenter 5
Dear Commenter 5, could you assign someone to verify the reported problem disappears?

Dear Commenter 6, could you remember the OS configuration? Exact version of Linux, you used, and CLang compiler. It would really help the Test team.

Branches to integrate:
OCCT: CR31009
Products: NOT
31Commenter 31
Problem with compiling JAVA sample and CSharp sample on Windows
http://jenkins-test-08.nnov.opencascade.com/view/IR-2022-03-25/view/SAMPLES/job/IR-2022-03-25-Products-Windows-64-VC14-sample-java/5/parsed_console/
32Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
33Commenter 3
-class Bnd_BoundSortBox 
+class Bnd_BoundSortBox : public Standard_Transient
...
 
-class IntCurveSurface_ThePolyhedronOfHInter 
+DEFINE_STANDARD_HANDLE(IntCurveSurface_ThePolyhedronOfHInter, Standard_Transient)
+
+class IntCurveSurface_ThePolyhedronOfHInter : public Standard_Transient

It might be simpler using NCollection_Shared or std::unique_ptr for these local places.
34Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
35Commenter 35
Kirill,

Thanks for the hint. Unfortunately, NCollection_Shared is not applicable here, because it does not implement a constructor with large number of parameters, so, I used std::unique_ptr there. I still kept IntCurvesFace_Intersector class to be Handle.

Could you review following branches:
OCCT: CR31009
Products: CR31009
Test results: http://jenkins-test-occt/view/CR31009-CR31009-AZV/view/COMPARE/
36Commenter 3
I see there is a one more place using dangerous type `BRepTopAdaptor_SeqOfPtr` in OCCT - `IntTools_FClass2d`.
Please register a new task for the other place, or cover both places in scope of this fix.
37Commenter 37
Combination -
OCCT branch : [archived branch]
master SHA - [revision removed]
[revision removed]
Products branch : [archived branch] SHA - [revision removed]
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 18452.87000000039 / 18490.94000000032 [-0.21%]
Products
Total CPU difference: 11761.310000000152 / 11719.920000000124 [+0.35%]
Windows-64-VC14:
OCCT
Total CPU difference: 20539.828125 / 20552.71875 [-0.06%]
Products
Total CPU difference: 13248.21875 / 13242.609375 [+0.04%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
38Commenter 2
Branch [archived branch] has been deleted by Participant.

[revision removed]

Related records