|Anonymous | Login||2021-02-25 22:23 MSK|
|My View | View Issues | Change Log | Roadmap|
|View Issue Details|
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0021977||Open CASCADE||[OCCT] OCCT:Application Framework||public||2010-08-18 17:54||2012-11-16 13:17|
|Product Version||[OCCT] 6.5.1|
|Target Version||[OCCT] 6.5.4||Fixed in Version||[OCCT] 6.5.4|
|Summary||0021977: Unsafe implementation of TNaming_Builder|
|Description||Class TNaming_Builder is implemented in very unsafe way: upon creation it |
creates new TNaming_Named shape attribute (which is bad on itself -- creation
of the tool should not modify the document) and remembers plain pointer to it.
The attribute can be then removed (with ForgetAttribute()), while the tool is
unable to know this; on any next call it will attempt to use this attribute as
it were alive. This can lead to crashes and unpredictable behavior.
The test case for this is chl 920 B7; see relevant code in QAAlcatel.cxx,
This problem has been diagnosed during testing of patch on OCC21961 (v6)
|Tags||No tags attached.|
|Test case number||chl 920 B7|
|Attached Files||OCC21977-patch-v1.zip (12,560 bytes) 2010-10-15 14:34|
SVN workbench http://svn/svn/occt/branches/OCC21977 [^]
has been created and ready to be revised
As far as I can see, the proposed fix does not solve the problem: class TNaming_Builder continues to hold reference to the attribute that it creates upon initialization (field myAtt), and still uses it. Regardless of the added checks for presence of this kind of attribute on the label, this myAtt field is used instead of the one actually present.
Note that test case in QAAlcatel.cxx became more complex than before, and I am sure it will fail in the same way as before if second call to Generated() is removed.
1. The proposed fix solves the problem as it is not more reproduced on the specified test case (just check the returned attribute for Null). TNaming_Builder should continue to hold reference to the attribute because of a)logic of its work and b)test cases when Builder is used in very intensive way (creating sequences of NamedShapes). Forcing each time find it in the Document leads to sufficient performance problems.
2. If the test case in QAAlcatel.cxx became more complex than before the best way is provide it and check once again. The attached patch guarantee normal behavior (without exceptions) if each after extracting it from Builder a developer will check it for Null (it seems natural action when work with handles).
Workbench KAS:dev:mkv-OCC21977-occt was created from SVN branch http://svn/svn/occt/branches/OCC21977 [^]
(and mkv-OCC21977-products from trunk) and compiled on Linux platform.
There are not regressions in mkv-OCC21977-products regarding to KAS:dev:products-20110810-opt
See results in /dn45/occttests/results/KAS/dev/mkv-OCC21977-products_27092011/lin
See reference results in /dn45/occttests/results/KAS/dev/products-20110810-opt_11082011/lin
See test cases in /dn45/occttests/tests/ED
N.B. In order to launch testing case you can make use the following instructions
The fix is integrated in branch CR21977, please review.
Note that behavior of this class is not changed; I have only replaced plain C pointers in class fields to Handles, to prevent possible use of freed memory. Notably this fix immediately caused failure of multiple tests due to bug described in 0023086. Hence this fix is put on top of the branch CR23086, and should be tested and integrated with that.
edited on: 2012-09-13 15:08
Branch CR21977 (and products from GIT master) was compiled on Linux and Windows platforms and tested.
chl 920 B7
occt: master 8b8bffc6
Timestamp: 2012-09-14 13:20:57
|0021977: Unsafe implementation of TNaming_Builder
The code is corrected to create instances of TNaming_Builder class dynamically. Note that they cannot be created as local variables as they should be instantiated only when needed and then reused for the subshapes of the same type in cycle.
Code around is cleaned from tabs and duplicated fragments.
TNaming_Builder class is changed to use Handles instead of C pointers in its fields.
This should protect from possible access to the freed memory if attribute is deleted while instance of TNaming_Builder is still alive.
In addition, method to construct dummy vertex for storing orientation is simplified.
|mod - src/TNaming/TNaming_Builder.cdl|
|mod - src/TNaming/TNaming_NamedShape.cxx|
|2010-09-06 12:08||bugmaster||Assigned To||bugmaster => szy|
|2010-09-06 12:08||bugmaster||Status||new => assigned|
|2010-10-12 19:46||szy||CC||=> abv|
|2010-10-15 16:34||szy||Status||assigned => resolved|
|2011-07-29 14:20||bugmaster||Note Added: 0017859|
|2011-07-29 14:20||bugmaster||Assigned To||szy => abv|
|2011-07-29 14:20||bugmaster||Status||resolved => assigned|
|2011-07-29 14:20||bugmaster||Status||assigned => resolved|
|2011-07-29 14:20||bugmaster||Fixed in Version||EMPTY =>|
|2011-07-29 14:20||bugmaster||Description Updated||View Revisions|
|2011-08-02 10:32||bugmaster||Category||OCCT:OCAF => OCCT:Application Framework|
|2011-08-09 12:34||bugmaster||Test case number||=> chl 920 B7|
|2011-08-22 17:12||abv||Note Added: 0018013|
|2011-08-22 17:12||abv||Assigned To||abv => szy|
|2011-08-22 17:12||abv||Status||resolved => assigned|
|2011-09-20 16:40||szy||Note Added: 0018224|
|2011-09-20 16:40||szy||Assigned To||szy => bugmaster|
|2011-09-20 16:40||szy||Status||assigned => reviewed|
|2011-09-30 12:16||mkv||Note Added: 0018297|
|2011-09-30 12:16||mkv||Status||reviewed => tested|
|2011-09-30 18:49||abv||Assigned To||bugmaster => abv|
|2011-09-30 18:49||abv||Status||tested => assigned|
|2012-03-27 12:43||abv||Relationship added||parent of 0023038|
|2012-09-07 09:22||abv||Summary||Perverse implementation of TNaming_Builder => Unsafe implementation of TNaming_Builder|
|2012-09-07 09:22||abv||Description Updated||View Revisions|
|2012-09-07 09:34||abv||Relationship added||related to 0023086|
|2012-09-07 09:57||abv||Note Added: 0021429|
|2012-09-07 09:57||abv||Assigned To||abv => szy|
|2012-09-07 09:57||abv||Status||assigned => resolved|
|2012-09-07 15:58||szy||Note Added: 0021434|
|2012-09-07 15:58||szy||Assigned To||szy => mkv|
|2012-09-07 15:58||szy||Status||resolved => reviewed|
|2012-09-10 16:28||szy||Product Version||=> 6.5.1|
|2012-09-10 16:28||szy||Target Version||=> 6.5.4|
|2012-09-13 15:07||mkv||Note Added: 0021479|
|2012-09-13 15:08||mkv||Note Edited: 0021479||View Revisions|
|2012-09-13 15:10||mkv||Assigned To||mkv => bugmaster|
|2012-09-13 15:10||mkv||Status||reviewed => tested|
|2012-09-17 17:29||szy||Changeset attached||=> occt master 8b8bffc6|
|2012-09-17 17:33||szy||Assigned To||bugmaster => szy|
|2012-09-17 17:33||szy||Status||tested => verified|
|2012-11-16 13:14||bugmaster||Fixed in Version||=> 6.5.4|
|2012-11-16 13:17||bugmaster||Status||verified => closed|
|Copyright © 2000 - 2021 MantisBT Team|