View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0025720 | Open CASCADE | OCCT:Modeling Algorithms | public | 2015-01-19 20:57 | 2015-05-14 15:32 |
| Reporter | Assigned To | bugmaster | |||
| Priority | normal | Severity | minor | ||
| Status | closed | Resolution | fixed | ||
| Product Version | 6.8.0 | ||||
| Target Version | 6.9.0 | Fixed in Version | 6.9.0 | ||
| Summary | 0025720: Incorrect code of math classes can lead to unpredicted behavior of algorithms | ||||
| Description | This 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 Reproduce | not applicable | ||||
| Tags | No tags attached. | ||||
| Test case number | Not needed | ||||
| related to | 0025719 | closed | bugmaster | Community | Boolean operations can crash |
| related to | 0025622 | closed | bugmaster | Open CASCADE | CAST analysis: Avoid invocation of virtual Methods of the declared Class in a Constructor or Destructor |
| related to | 0025577 | closed | Open CASCADE | Avoid invocation of virtual Methods of the declared Class in a Constructor or Destructor |
|
|
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 |
|
|
Dear Mikhail, Please review brunch CR25720 |
|
|
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". |
|
|
Branch CR25720 has been updated forcibly by azn. SHA-1: cfed4e3b1912e8635a607780a877f61d477fc4e0 |
|
|
Please review. |
|
|
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. |
|
|
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". |
|
|
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. |
|
|
There is no direct dependencies between occt and products. |
|
|
Reviewed. |
|
|
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. |
|
|
Dear AZN, Could you please to rebase CR25720 to current master. I got a lot of conflicts. |
|
|
Branch CR25720 has been updated forcibly by azn. SHA-1: 19dbf0b5ba7ef7e5294cf2c734e992a1e3aff910 |
|
|
Brunch has been rebased. Please check. |
|
|
Please squash commits into one and make actual comment. |
|
|
Branch CR25720 has been updated forcibly by azn. SHA-1: 0efb3fcea74ba7b0569cf145a9e06e5be76a4c05 |
|
|
Reviewed. |
|
|
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 |
|
|
Branch CR25720 has been updated forcibly by msv. SHA-1: 63341728cbd734e38313528e6475f6e11edaec2f |
|
|
Branch CR25720 has been deleted by inv. SHA-1: 63341728cbd734e38313528e6475f6e11edaec2f |
|
occt: master 859a47c3 2015-01-22 12:19:05
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 | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2015-01-19 20:57 |
|
New Issue | |
| 2015-01-19 20:57 |
|
Assigned To | => msv |
| 2015-01-19 21:01 |
|
Relationship added | related to 0025719 |
| 2015-01-19 21:01 |
|
Assigned To | msv => azv |
| 2015-01-19 21:01 |
|
Status | new => assigned |
| 2015-01-20 08:30 |
|
Relationship added | related to 0025622 |
| 2015-01-22 11:05 |
|
Assigned To | azv => azn |
| 2015-01-22 15:19 | git | Note Added: 0036397 | |
| 2015-01-22 15:20 |
|
Note Added: 0036398 | |
| 2015-01-22 15:20 |
|
Assigned To | azn => msv |
| 2015-01-22 15:20 |
|
Status | assigned => resolved |
| 2015-01-22 15:20 |
|
Steps to Reproduce Updated | |
| 2015-01-22 16:20 |
|
Note Added: 0036409 | |
| 2015-01-22 16:20 |
|
Assigned To | msv => azn |
| 2015-01-22 16:20 |
|
Status | resolved => assigned |
| 2015-01-26 10:38 |
|
Relationship added | related to 0025577 |
| 2015-01-28 15:37 | git | Note Added: 0036762 | |
| 2015-01-28 15:38 |
|
Note Added: 0036763 | |
| 2015-01-28 15:38 |
|
Assigned To | azn => msv |
| 2015-01-28 15:38 |
|
Status | assigned => resolved |
| 2015-01-28 17:45 |
|
Note Added: 0036782 | |
| 2015-01-28 17:45 |
|
Assigned To | msv => azn |
| 2015-01-28 17:45 |
|
Status | resolved => assigned |
| 2015-01-28 18:20 |
|
Note Added: 0036784 | |
| 2015-01-29 08:54 | git | Note Added: 0036792 | |
| 2015-01-29 13:47 |
|
Note Added: 0036814 | |
| 2015-01-29 13:47 |
|
Assigned To | azn => msv |
| 2015-01-29 13:47 |
|
Status | assigned => resolved |
| 2015-01-29 14:39 |
|
Note Added: 0036823 | |
| 2015-01-29 14:39 |
|
Assigned To | msv => bugmaster |
| 2015-01-29 14:39 |
|
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 |
|
Note Added: 0037227 | |
| 2015-02-09 13:31 |
|
Note Edited: 0037227 | |
| 2015-02-09 15:35 |
|
Note Added: 0037238 | |
| 2015-02-09 15:35 |
|
Assigned To | bugmaster => azn |
| 2015-02-09 15:35 |
|
Status | tested => assigned |
| 2015-02-10 14:18 | git | Note Added: 0037284 | |
| 2015-02-10 14:23 |
|
Assigned To | azn => msv |
| 2015-02-10 14:23 |
|
Status | assigned => resolved |
| 2015-02-10 15:39 |
|
Note Added: 0037291 | |
| 2015-02-10 15:39 |
|
Assigned To | msv => bugmaster |
| 2015-02-10 15:39 |
|
Status | resolved => reviewed |
| 2015-02-10 16:44 |
|
Assigned To | bugmaster => apv |
| 2015-02-12 17:57 |
|
Note Added: 0037465 | |
| 2015-02-12 17:57 |
|
Assigned To | apv => bugmaster |
| 2015-02-12 17:57 |
|
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 |
|
Status | verified => closed |
| 2015-05-14 15:32 |
|
Fixed in Version | => 6.9.0 |
