View Issue Details

IDProjectCategoryView StatusLast Update
0025720Open CASCADEOCCT:Modeling Algorithmspublic2015-05-14 15:32
ReportermsvAssigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version6.8.0 
Target Version6.9.0Fixed in Version6.9.0 
Summary0025720: Incorrect code of math classes can lead to unpredicted behavior of algorithms
DescriptionThis bug has been detected during discussion in the following forum topic:
http://dev.opencascade.org/index.php?q=node/1048

The quote of the user barbier:
"scan-build is able to detect such problems, it reports similar problems in
math_Powell.cxx
math_NewtonMinimum.cxx
math_NewtonFunctionSetRoot.cxx
math_BissecNewton.cxx math_FRPR.cxx
math_FunctionSetRoot.cxx
math_FunctionSetRoot.cxx
math_BrentMinimum.cxx

It also complains about Delete being called by virtual destructors."

The matter is about the calling of virtual functions from the constructor. This must be avoided, in our case mainly by suppressing call to Perform method from the constructor.
Also, of course, it is needed to avoid calling virtual functions from destructors.
Steps To Reproducenot applicable
TagsNo tags attached.
Test case numberNot needed

Relationships

related to 0025719 closedbugmaster Community Boolean operations can crash 
related to 0025622 closedbugmaster Open CASCADE CAST analysis: Avoid invocation of virtual Methods of the declared Class in a Constructor or Destructor 
related to 0025577 closedazn Open CASCADE Avoid invocation of virtual Methods of the declared Class in a Constructor or Destructor 

Activities

git

2015-01-22 15:19

administrator   ~0036397

Branch CR25720 has been created by azn.

SHA-1: cf9c341372a2c585bc486ebfaf7f04d8bb8100b2


Detailed log of new commits:

Author: azn
Date: Thu Jan 22 15:19:05 2015 +0300

    0025720: Incorrect code of math classes can lead to unpredicted behavior of algorithms
    
    The inheritance interfaces were eliminated from the following classes:
    
    math_BissecNewton
    math_BrentMinimum.cdl
    math_FRPR.cdl
    math_FRPR.cxx
    math_FunctionSetRoot
    math_FunctionSetRoot
    math_NewtonFunctionSetRoot
    math_NewtonFunctionSetRoot
    math_Powell
    math_Powell
    <-- these classes are no heirs
    
    Fixed: math_NewtonMinimum class

azn

2015-01-22 15:20

developer   ~0036398

Dear Mikhail,
Please review brunch CR25720

msv

2015-01-22 16:20

developer   ~0036409

IsSolutionReached is no more virtual, but comments say that it can be redefined.

If this method is called only from Perform and no more virtual, why not to make it inline?

src\math\math_NewtonMinimum.cdl:62 is a strange line, looks like a mistake. If you want to leave empty virtual destructor than define some non-virtual inline stub method and make alias virtual destructor calling this stub.

src\math\math_NewtonMinimum.cxx
The virtual method IsConverged is no more called. So, virtual mechanism is no more used. Why virtual destructor then is left? If there is a derived class, you need to use another approach: remove call to Perform from the constructor, and change all callers of this class, as it is done with math_BFGS in the fix for 0025719.

I suggest using the same general behavior of all math computation classes, and leaving them polymorphic. For that we should make the fix as proposed in the bug description: "suppressing call to Perform method from the constructor".

git

2015-01-28 15:37

administrator   ~0036762

Branch CR25720 has been updated forcibly by azn.

SHA-1: cfed4e3b1912e8635a607780a877f61d477fc4e0

azn

2015-01-28 15:38

developer   ~0036763

Please review.

msv

2015-01-28 17:45

developer   ~0036782

src\math\math_FRPR.cdl
- English grammar in "constructor isn't performs computations".
Correct is "constructor doesn't perform computations".
- Ctor with theStartingPoint is extra, as Perform is to be called in any case.

src\math\math_NewtonMinimum.cdl
- English grammar in "This constructor not perform computation".
Correct is "This constructor does not perform computation".

src\GeomFill\GeomFill_LocationGuide.cxx
- 360-361: indentation is broken (consider tab size = 8 characters, and not 2).

It is also needed to make fix in occt-products due to API changes.

abv

2015-01-28 18:20

manager   ~0036784

Small correction: it is better to use complete wording "does not" instead of abbreviated form "doesn't" which is used to represent spoken language. The only exclusion, to my knowledge, is "let's".

git

2015-01-29 08:54

administrator   ~0036792

Branch CR25720 has been updated by azn.

SHA-1: 277a5770773fe984e71982382dfbf7dcda2e6531


Detailed log of new commits:

Author: azn
Date: Thu Jan 29 08:54:14 2015 +0300

    0025720: Incorrect code of math classes can lead to unpredicted behavior of algorithms
    
    Small grammar fixes.

azn

2015-01-29 13:47

developer   ~0036814

There is no direct dependencies between occt and products.

msv

2015-01-29 14:39

developer   ~0036823

Reviewed.

apn

2015-02-02 13:33

administrator   ~0036956

Last edited: 2015-02-02 13:34

Dear BugMaster,
Branch CR25720 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested in Release mode.
SHA-1: 277a5770773fe984e71982382dfbf7dcda2e6531

Number of compiler warnings:

occt component :
Linux: 18 (18 on master)
Windows: 0 (0 on master)

products component :
Linux: 11 (11 on master)
Windows: 1 (1 on master)

Regressions/Differences:
No regressions

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 369654500 / 370114884
Total CPU difference: 52887.95000000013 / 51750.710000000094

Testing on Windows:
Total MEMORY difference: 276204336 / 275761696
Total CPU difference: 38970.984375 / 38148.203125

There are not differences in images found by testdiff.

bugmaster

2015-02-05 14:43

administrator   ~0037121

Dear AZN,

Could you please to rebase CR25720 to current master. I got a lot of conflicts.

git

2015-02-09 13:27

administrator   ~0037226

Branch CR25720 has been updated forcibly by azn.

SHA-1: 19dbf0b5ba7ef7e5294cf2c734e992a1e3aff910

azn

2015-02-09 13:31

developer   ~0037227

Last edited: 2015-02-09 13:31

Brunch has been rebased. Please check.

msv

2015-02-09 15:35

developer   ~0037238

Please squash commits into one and make actual comment.

git

2015-02-10 14:18

administrator   ~0037284

Branch CR25720 has been updated forcibly by azn.

SHA-1: 0efb3fcea74ba7b0569cf145a9e06e5be76a4c05

msv

2015-02-10 15:39

developer   ~0037291

Reviewed.

apv

2015-02-12 17:57

tester   ~0037465

Dear BugMaster,

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

Number of compiler warnings:
occt component:
   Linux: 18 (18 on master)
   Windows: 0 (0 on master)
products component:
   Linux: 11 (11 on master)
   Windows: 1 (1 on master)

Regressions/Differences:
Not detected

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 86055997 / 86186549
Total CPU difference: 49467.680000000124 / 47876.709999999846

Testing on Windows:
Total MEMORY difference: 40648352 / 40658496
Total CPU difference: 37627.515625 / 32754.0

git

2015-02-13 11:36

administrator   ~0037495

Branch CR25720 has been updated forcibly by msv.

SHA-1: 63341728cbd734e38313528e6475f6e11edaec2f

git

2015-03-18 13:36

administrator   ~0038564

Branch CR25720 has been deleted by inv.

SHA-1: 63341728cbd734e38313528e6475f6e11edaec2f

Related Changesets

occt: master 859a47c3

2015-01-22 12:19:05

azn


Committer: bugmaster Details Diff
0025720: Incorrect code of math classes can lead to unpredicted behavior of algorithms

The calling of virtual methods has been removed from constructors & destructors:

math_BissecNewton
math_BrentMinimum
math_FRPR
math_FunctionSetRoot
math_NewtonFunctionSetRoot
math_NewtonMinimum
math_Powell
Affected Issues
0025720
mod - src/Bisector/Bisector_BisecCC.cxx Diff File
mod - src/Bisector/Bisector_Inter.cxx Diff File
mod - src/Extrema/Extrema_ExtPExtS.cxx Diff File
mod - src/Extrema/Extrema_GenExtCS.cxx Diff File
mod - src/Extrema/Extrema_GenExtPS.cxx Diff File
mod - src/Extrema/Extrema_GenExtSS.cxx Diff File
mod - src/Extrema/Extrema_GenLocateExtCC.gxx Diff File
mod - src/Extrema/Extrema_GenLocateExtCS.cxx Diff File
mod - src/Extrema/Extrema_GenLocateExtPS.cxx Diff File
mod - src/Extrema/Extrema_GenLocateExtSS.cxx Diff File
mod - src/FairCurve/FairCurve_Newton.cdl Diff File
mod - src/FairCurve/FairCurve_Newton.cxx Diff File
mod - src/Geom2dGcc/Geom2dGcc_Circ2d2TanOnIter.cxx Diff File
mod - src/Geom2dGcc/Geom2dGcc_Circ2d3TanIter.cxx Diff File
mod - src/Geom2dGcc/Geom2dGcc_Lin2d2TanIter.cxx Diff File
mod - src/GeomFill/GeomFill_LocationDraft.cxx Diff File
mod - src/GeomFill/GeomFill_LocationGuide.cxx Diff File
mod - src/IntCurve/IntCurve_ExactIntersectionPoint.gxx Diff File
mod - src/math/math_BissecNewton.cdl Diff File
mod - src/math/math_BissecNewton.cxx Diff File
mod - src/math/math_BissecNewton.lxx Diff File
mod - src/math/math_BrentMinimum.cdl Diff File
mod - src/math/math_BrentMinimum.cxx Diff File
mod - src/math/math_BrentMinimum.lxx Diff File
mod - src/math/math_FRPR.cdl Diff File
mod - src/math/math_FRPR.cxx Diff File
mod - src/math/math_FRPR.lxx Diff File
mod - src/math/math_FunctionRoot.cxx Diff File
mod - src/math/math_FunctionSetRoot.cdl Diff File
mod - src/math/math_FunctionSetRoot.cxx Diff File
mod - src/math/math_FunctionSetRoot.lxx Diff File
mod - src/math/math_GlobOptMin.cxx Diff File
mod - src/math/math_NewtonFunctionSetRoot.cdl Diff File
mod - src/math/math_NewtonFunctionSetRoot.cxx Diff File
mod - src/math/math_NewtonFunctionSetRoot.lxx Diff File
mod - src/math/math_NewtonMinimum.cdl Diff File
mod - src/math/math_NewtonMinimum.cxx Diff File
mod - src/math/math_NewtonMinimum.lxx Diff File
mod - src/math/math_Powell.cdl Diff File
mod - src/math/math_Powell.cxx Diff File
mod - src/math/math_Powell.lxx Diff File
mod - src/ProjLib/ProjLib_PrjResolve.cxx Diff File

Issue History

Date Modified Username Field Change
2015-01-19 20:57 msv New Issue
2015-01-19 20:57 msv Assigned To => msv
2015-01-19 21:01 msv Relationship added related to 0025719
2015-01-19 21:01 msv Assigned To msv => azv
2015-01-19 21:01 msv Status new => assigned
2015-01-20 08:30 abv Relationship added related to 0025622
2015-01-22 11:05 azn Assigned To azv => azn
2015-01-22 15:19 git Note Added: 0036397
2015-01-22 15:20 azn Note Added: 0036398
2015-01-22 15:20 azn Assigned To azn => msv
2015-01-22 15:20 azn Status assigned => resolved
2015-01-22 15:20 azn Steps to Reproduce Updated
2015-01-22 16:20 msv Note Added: 0036409
2015-01-22 16:20 msv Assigned To msv => azn
2015-01-22 16:20 msv Status resolved => assigned
2015-01-26 10:38 azn Relationship added related to 0025577
2015-01-28 15:37 git Note Added: 0036762
2015-01-28 15:38 azn Note Added: 0036763
2015-01-28 15:38 azn Assigned To azn => msv
2015-01-28 15:38 azn Status assigned => resolved
2015-01-28 17:45 msv Note Added: 0036782
2015-01-28 17:45 msv Assigned To msv => azn
2015-01-28 17:45 msv Status resolved => assigned
2015-01-28 18:20 abv Note Added: 0036784
2015-01-29 08:54 git Note Added: 0036792
2015-01-29 13:47 azn Note Added: 0036814
2015-01-29 13:47 azn Assigned To azn => msv
2015-01-29 13:47 azn Status assigned => resolved
2015-01-29 14:39 msv Note Added: 0036823
2015-01-29 14:39 msv Assigned To msv => bugmaster
2015-01-29 14:39 msv Status resolved => reviewed
2015-01-29 17:58 apn Assigned To bugmaster => apn
2015-02-02 13:33 apn Note Added: 0036956
2015-02-02 13:34 apn Note Edited: 0036956
2015-02-02 13:34 apn Test case number => Not needed
2015-02-02 13:34 apn Assigned To apn => bugmaster
2015-02-02 13:34 apn Status reviewed => tested
2015-02-05 14:43 bugmaster Note Added: 0037121
2015-02-09 13:27 git Note Added: 0037226
2015-02-09 13:31 azn Note Added: 0037227
2015-02-09 13:31 azn Note Edited: 0037227
2015-02-09 15:35 msv Note Added: 0037238
2015-02-09 15:35 msv Assigned To bugmaster => azn
2015-02-09 15:35 msv Status tested => assigned
2015-02-10 14:18 git Note Added: 0037284
2015-02-10 14:23 azn Assigned To azn => msv
2015-02-10 14:23 azn Status assigned => resolved
2015-02-10 15:39 msv Note Added: 0037291
2015-02-10 15:39 msv Assigned To msv => bugmaster
2015-02-10 15:39 msv Status resolved => reviewed
2015-02-10 16:44 mkv Assigned To bugmaster => apv
2015-02-12 17:57 apv Note Added: 0037465
2015-02-12 17:57 apv Assigned To apv => bugmaster
2015-02-12 17:57 apv Status reviewed => tested
2015-02-13 11:36 git Note Added: 0037495
2015-02-20 17:21 bugmaster Changeset attached => occt master 859a47c3
2015-02-20 17:21 bugmaster Status tested => verified
2015-02-20 17:21 bugmaster Resolution open => fixed
2015-03-18 13:36 git Note Added: 0038564
2015-05-14 15:29 aiv Status verified => closed
2015-05-14 15:32 aiv Fixed in Version => 6.9.0