MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0025720Open CASCADE[OCCT] OCCT:Modeling Algorithmspublic2015-01-19 20:572015-05-14 15:32
Reportermsv 
Assigned Tobugmaster 
PrioritynormalSeverityminor 
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version[OCCT] 6.8.0 
Target Version[OCCT] 6.9.0Fixed in Version[OCCT] 6.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
Attached Files

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

-  Notes
(0036397)
git (administrator)
2015-01-22 15:19

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
(0036398)
azn (developer)
2015-01-22 15:20

Dear Mikhail,
Please review brunch CR25720
(0036409)
msv (developer)
2015-01-22 16:20

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".
(0036762)
git (administrator)
2015-01-28 15:37

Branch CR25720 has been updated forcibly by azn.

SHA-1: cfed4e3b1912e8635a607780a877f61d477fc4e0
(0036763)
azn (developer)
2015-01-28 15:38

Please review.
(0036782)
msv (developer)
2015-01-28 17:45

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.
(0036784)
abv (manager)
2015-01-28 18:20

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".
(0036792)
git (administrator)
2015-01-29 08:54

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.

(0036814)
azn (developer)
2015-01-29 13:47

There is no direct dependencies between occt and products.
(0036823)
msv (developer)
2015-01-29 14:39

Reviewed.
(0036956)
apn (administrator)
2015-02-02 13:33
edited on: 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.

(0037121)
bugmaster (administrator)
2015-02-05 14:43

Dear AZN,

Could you please to rebase CR25720 to current master. I got a lot of conflicts.
(0037226)
git (administrator)
2015-02-09 13:27

Branch CR25720 has been updated forcibly by azn.

SHA-1: 19dbf0b5ba7ef7e5294cf2c734e992a1e3aff910
(0037227)
azn (developer)
2015-02-09 13:31
edited on: 2015-02-09 13:31

Brunch has been rebased. Please check.

(0037238)
msv (developer)
2015-02-09 15:35

Please squash commits into one and make actual comment.
(0037284)
git (administrator)
2015-02-10 14:18

Branch CR25720 has been updated forcibly by azn.

SHA-1: 0efb3fcea74ba7b0569cf145a9e06e5be76a4c05
(0037291)
msv (developer)
2015-02-10 15:39

Reviewed.
(0037465)
apv (tester)
2015-02-12 17:57

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
(0037495)
git (administrator)
2015-02-13 11:36

Branch CR25720 has been updated forcibly by msv.

SHA-1: 63341728cbd734e38313528e6475f6e11edaec2f
(0038564)
git (administrator)
2015-03-18 13:36

Branch CR25720 has been deleted by inv.

SHA-1: 63341728cbd734e38313528e6475f6e11edaec2f

- Related Changesets
occt: master 859a47c3
Timestamp: 2015-01-22 12:19:05
Author: 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
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 View Revisions
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 View Revisions
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 View Revisions
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


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker