MantisBT - Open CASCADE
View Issue Details
0031756Open CASCADE[OCCT] OCCT:Data Exchangepublic2020-09-07 13:202020-12-29 12:55
abv 
bugmaster 
normalminor 
verifiedfixed 
 
[OCCT] 7.6.0* 
Not required
0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
The problem has been discovered during analysis of regression manifested on test case bugs step bug26451 with a fix made for #31489.

The STEP entity #71919 (and many others) in the file attached to issue 0026451 has last parameter missing, like this:

#71919=EDGE_CURVE('',#84041,#84041,#0, );
#71920=FILL_AREA_STYLE('',(#84042));

When this STEP file is read, such entities are parsed incorrectly: instead of incomplete list of parameters, they get extra parameters by including ones from the next entity, and that next entity gets ignored:

~~~~~
Draw[1]> test bugs step bug26451
...
Draw[2]> entity #71919
  -- DUMP Entity n0 71355 level 1
 --- (STEP) Entity 71355:#71919 Type cdl : StepShape_EdgeCurve
 *** NOT WELL LOADED : CONTENT FROM FILE ***
In dump, iii:#jjj means : entity rank iii has step ident #jjj
71355:#71919 = EDGE_CURVE('',83437:#84041,83437:#84041,#0,#0,/* (SUB) */
  (83438:#84042));
  /* On Entity above, Fail Messages recorded at Read time :
Count of Parameters is not 5 for edge_curve
A reference to another entity is unresolved */
 ** Data Check (One Entity) **
Check:1 -- Entity (n0:id) 71355:#71919 Type:EDGE_CURVE
Count of Parameters is not 5 for edge_curve
A reference to another entity is unresolved

Draw[3]> entity #71920
Error in Command : entity #71920
~~~~~

I have prepared synthetic test file for this case, attached as bug31756.stp.
Desired behavior is that the file should be read successfully (it is just a sphere) but error shall be generated on entity 0000015.

Current behavior is:
- on master branch: translation fails, with error on entity 0000015 (wrong nb. of parameters)
- on CR31489_1: translation succeeds without any messages

Note that while work-around implemented in CR31489_1 effectively works in this particular case, this issue shows that our STEP parser can be broken easily by incorrect data, and its behavior becomes unpredictable. We need to understand how original problem occurs and prevent this in more generic way.
pload ALL
stepread [locate_data_file bug31756.stp] a *
No tags attached.
related to 0026451closed bugmaster Community Crash importing STeP file 
parent of 0032028verified bugmaster Open CASCADE Coding Rules, StepFile - eliminate CLang warning -Wmisleading-indentation 
? bug31756.stp (2,175) 2020-09-07 14:39
https://tracker.dev.opencascade.org/
Issue History
2020-09-07 13:20abvNew Issue
2020-09-07 13:20abvAssigned To => dpasukhi
2020-09-07 13:20abvRelationship addedrelated to 0031489
2020-09-07 13:20abvRelationship addedrelated to 0026451
2020-09-07 14:39abvFile Added: bug31756.stp
2020-09-07 14:45abvDescription Updatedbug_revision_view_page.php?rev_id=23356#r23356
2020-09-07 14:45abvSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=23358#r23358
2020-09-17 19:49gkaStatusnew => assigned
2020-09-22 18:28szyTarget Version7.5.0 => 7.6.0*
2020-11-30 11:42gitNote Added: 0097155
2020-12-03 02:00gitNote Added: 0097255
2020-12-04 15:07gitNote Added: 0097323
2020-12-04 17:18dpasukhiNote Added: 0097328
2020-12-04 17:18dpasukhiAssigned Todpasukhi => abv
2020-12-04 17:18dpasukhiStatusassigned => resolved
2020-12-06 18:25abvNote Added: 0097398
2020-12-06 18:25abvAssigned Toabv => dpasukhi
2020-12-06 18:25abvStatusresolved => assigned
2020-12-08 18:08gitNote Added: 0097428
2020-12-09 00:01gitNote Added: 0097434
2020-12-09 11:20gitNote Added: 0097435
2020-12-09 11:34dpasukhiNote Added: 0097436
2020-12-09 11:34dpasukhiAssigned Todpasukhi => abv
2020-12-09 11:34dpasukhiStatusassigned => resolved
2020-12-11 18:57gitNote Added: 0097507
2020-12-11 19:20gitNote Added: 0097510
2020-12-11 22:21gitNote Added: 0097512
2020-12-12 07:44abvNote Added: 0097515
2020-12-12 07:44abvAssigned Toabv => bugmaster
2020-12-12 07:44abvStatusresolved => reviewed
2020-12-16 11:53gitNote Added: 0097628
2020-12-16 12:05gitNote Added: 0097631
2020-12-17 11:00gitNote Added: 0097656
2020-12-17 12:47gitNote Added: 0097672
2020-12-18 12:32gitNote Added: 0097702
2020-12-18 14:32gitNote Added: 0097711
2020-12-18 19:26bugmasterAssigned Tobugmaster => dpasukhi
2020-12-18 19:26bugmasterStatusreviewed => feedback
2020-12-21 14:10gitNote Added: 0097829
2020-12-23 19:32bugmasterAssigned Todpasukhi => bugmaster
2020-12-23 19:32bugmasterStatusfeedback => reviewed
2020-12-26 12:15bugmasterNote Added: 0097916
2020-12-26 12:15bugmasterStatusreviewed => tested
2020-12-26 12:21bugmasterTest case number => Not required
2020-12-26 14:40bugmasterChangeset attached => occt master 2057c37b
2020-12-26 14:40bugmasterStatustested => verified
2020-12-26 14:40bugmasterResolutionopen => fixed
2020-12-26 14:52gitNote Added: 0097936
2020-12-26 14:53gitNote Added: 0097937
2020-12-26 14:53gitNote Added: 0097938
2020-12-26 14:53gitNote Added: 0097939
2020-12-26 14:53gitNote Added: 0097940
2020-12-26 14:53gitNote Added: 0097941
2020-12-26 14:53gitNote Added: 0097942
2020-12-26 14:53gitNote Added: 0097943
2020-12-26 14:53gitNote Added: 0097944
2020-12-26 14:53gitNote Added: 0097945
2020-12-29 12:55kgvRelationship addedparent of 0032028

Notes
(0097155)
git   
2020-11-30 11:42   
Branch CR31756_testParser has been created by dpasukhi.

SHA-1: 225b5fa264dcbc01d8edfe2e4c56775917347288


Detailed log of new commits:

Author: dpasukhi
Date: Fri Nov 27 22:32:18 2020 +0300

    0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
    
    Recorder made thread safe (converted from c to c ++)
    Delete useless files ( stepread.cxx, stepread.ph )
(0097255)
git   
2020-12-03 02:00   
Branch CR31756_1 has been created by dpasukhi.

SHA-1: 8c27092e8209ceeeb6a97a5e33a19db099619de0


Detailed log of new commits:

Author: dpasukhi
Date: Fri Nov 27 22:32:18 2020 +0300

    0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
    
    - Recorder may safely be used in a multi-threaded environment.
     - Transform "C" recfile to "C++" RecordFile
    - Fix problem with missing last parameters
    - Rename files and methods of recfile ( RecordFile )
    - Delete useless files ( stepread.cxx, stepread.ph )
(0097323)
git   
2020-12-04 15:07   
Branch CR31756_2 has been created by dpasukhi.

SHA-1: 67ab474f6b69bf2882d1e30b9984d13307ddd9f6


Detailed log of new commits:

Author: dpasukhi
Date: Fri Nov 27 22:32:18 2020 +0300

    0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
    
    - Recorder may safely be used in a multi-threaded environment.
     - Transform "C" recfile to "C++" RecordFile
    - Fix problem with missing last parameters
    - Rename files and methods of recfile ( RecordFile )
    - Delete useless files ( stepread.cxx, stepread.ph )
    - Add control of arguments count ( special for error argument )

Author: dpasukhi
Date: Tue Nov 24 15:52:39 2020 +0300

    0031000: Data Exchange - STEP file cannot be opened due to lower-case special token End-ISO-10303-21
    
    Update flex lexical scanner (step.lex lex.step.cxx) to parse special tokens in a case insensitive manner
(0097328)
dpasukhi   
2020-12-04 17:18   
Dear ABV,
Please review CR31756_2.
This branch contains two resolved issue: current and 0031000 ( touch same files )

Current branch fix undefined behavior of parser ( skip or rewrite entity )

RecordFile.cxx:71 contains non-ASCI symbol ('C'), I'll fix it with rebase to new master or with remarks

Rebase "recfile" from "C" to "C++" don't have regressions for time or memory.

All tests are OK, no regressions, see:
http://occt-tests/CR31756-master-dpasukhi-OCCT/Debian80-64/diff_summary.html [^]
http://occt-tests/CR31756-master-dpasukhi-OCCT/Windows-64-VC14/diff_summary.html [^]
http://occt-tests/CR31756-master-dpasukhi-Products/Debian80-64/diff_summary.html [^]
http://occt-tests/CR31756-master-dpasukhi-Products/Windows-64-VC14/diff_summary.html [^]
(0097398)
abv   
2020-12-06 18:25   
Please consider some remarks:

- New class "RecordFile" should be named according to OCCT coding conventions, with package prefix "StepFile_". I would suggest name "StepFile_Model" or "StepFile_DataModel" or similar, reflecting the fact that this class defines data structures representing the contents of the STEP file (if my understanding is correct indeed).
- It would be nice to add some description of this class (logical: purpose, way of using, including any relevant notes on use in multithreaded contexts) as the class documentation.
- In StepFile_Read, instance of RecordFile object can be made a local variable to avoid need of error-prone new/delete actions
- Method RecordDebList() can be better named RecordListStart() to exclude use of legacy French terms (deb = debut I guess)
(0097428)
git   
2020-12-08 18:08   
Branch CR31756_3 has been created by dpasukhi.

SHA-1: 1c902294bab7f96b9c7b4096e0f64baee19c0818


Detailed log of new commits:

Author: dpasukhi
Date: Fri Nov 27 22:32:18 2020 +0300

    0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
    
    - Recorder may safely be used in a multi-threaded environment.
     - Transform "C" recfile to "C++" StepFile_DataModel
    - Fix problem with missing last parameters
    - Rename files and methods of recfile ( StepFile_DataModel )
    - Delete useless files ( stepread.cxx, stepread.ph )
    - Add control of arguments count ( special for error argument )
    - Add description of StepFile_DataModel class

Author: dpasukhi
Date: Tue Nov 24 15:52:39 2020 +0300

    0031000: Data Exchange - STEP file cannot be opened due to lower-case special token End-ISO-10303-21
    
    Update flex lexical scanner (step.lex lex.step.cxx) to parse special tokens in a case insensitive manner
(0097434)
git   
2020-12-09 00:01   
Branch CR31756_3 has been updated forcibly by dpasukhi.

SHA-1: 734a0192983bda65a3e304397b753a24ba43cbf5
(0097435)
git   
2020-12-09 11:20   
Branch CR31756_3 has been updated forcibly by dpasukhi.

SHA-1: 9d531d0fb491e8b5f8e8806bf2f11e7b501e9729
(0097436)
dpasukhi   
2020-12-09 11:34   
Dear ABV,
All remarks have been done.
Please review CR31756_3.

All test are OK, see,
- restart:
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31756-master-dpasukhi/view/TESTING/job/CR31756-master-dpasukhi-OCCT-Windows-64-VC14-opt-test-restart/HTML_20Report/ [^]
- tests:
http://occt-tests/CR31756-master-dpasukhi-OCCT/Debian80-64/diff_summary.html [^]
http://occt-tests/CR31756-master-dpasukhi-OCCT/Windows-64-VC14/diff_summary.html [^]
http://occt-tests/CR31756-master-dpasukhi-Products/Debian80-64/diff_summary.html [^]
http://occt-tests/CR31756-master-dpasukhi-Products/Windows-64-VC14/diff_summary.html [^]
(0097507)
git   
2020-12-11 18:57   
Branch CR31756_3 has been updated by abv.

SHA-1: b0ad5789e7988a63f65fc66684d36e1c26d43dfa


Detailed log of new commits:

Author: abv
Date: Fri Dec 11 18:50:43 2020 +0300

    // StepFile_DataModel renamed to StepFile_ReadData

(0097510)
git   
2020-12-11 19:20   
Branch CR31756_3 has been updated forcibly by abv.

SHA-1: a022b5e458ece2597d213fee763e3ad1d3c88ed3
(0097512)
git   
2020-12-11 22:21   
Branch CR31756_4 has been created by abv.

SHA-1: a524d2359be3ede0b4807b09739cffa3c7548e0b


Detailed log of new commits:

Author: dpasukhi
Date: Fri Nov 27 22:32:18 2020 +0300

    0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
    
    - Parser is corrected to handle case of missing arguments properly (report error without corruption of the next entity)
    - Added counter of entity arguments for appropriate error messages
    - Plain C tools and data structures (recfile.*, stepfile.*) are converted to C++ class (StepFile_ReadData) to avoid use of static variables, so that reader can be safely used in a multi-threaded environment
(0097515)
abv   
2020-12-12 07:44   
Reviewed with minor amendments (new class renamed) and tested, see Jenkins job CR31756-master-dpasukhi.
Please integrate:
OCCT branch CR31756_4
Products: none
(0097628)
git   
2020-12-16 11:53   
Branch CR31756_5 has been created by dpasukhi.

SHA-1: 726f0fe206884aeb0950cd278f86babdbda6b1fc


Detailed log of new commits:

Author: dpasukhi
Date: Fri Nov 27 22:32:18 2020 +0300

    0031756: Data Exchange - broken parsing of STEP entity in case of missing last parameter
    
    - Parser is corrected to handle case of missing arguments properly (report error without corruption of the next entity)
    - Added counter of entity arguments for appropriate error messages
    - Plain C tools and data structures (recfile.*, stepfile.*) are converted to C++ class (StepFile_ReadData) to avoid use of static variables, so that reader can be safely used in a multi-threaded environment
(0097631)
git   
2020-12-16 12:05   
Branch CR31756_4_1 has been created by dpasukhi.

SHA-1: 271f53ddd82e3e2b6a5c0c0ee9fcae865a3d8c0e


Detailed log of new commits:

Author: dpasukhi
Date: Wed Dec 16 12:05:36 2020 +0300

    // Fix "-Wmisleading-indentation" warning
(0097656)
git   
2020-12-17 11:00   
Branch CR31756_4_2 has been created by dpasukhi.

SHA-1: 128182ac39bf781c4822f4621fa02964e105e163


Detailed log of new commits:

Author: dpasukhi
Date: Wed Dec 16 12:05:36 2020 +0300

    // Fix "-Wmisleading-indentation" and "-Wunknown-pragmas" warnings
(0097672)
git   
2020-12-17 12:47   
Branch CR31756_4_2 has been updated forcibly by dpasukhi.

SHA-1: 1bb778940b1d8a521c964b18518716898f0890ef
(0097702)
git   
2020-12-18 12:32   
Branch CR31756_4_3 has been created by dpasukhi.

SHA-1: b1019f95e9f602a6764b0ccb869871c8dac9a821


Detailed log of new commits:

Author: dpasukhi
Date: Wed Dec 16 12:05:36 2020 +0300

    // Fix "-Wmisleading-indentation" and "-Wunknown-pragmas" warnings
(0097711)
git   
2020-12-18 14:32   
Branch CR31756_4_4 has been created by dpasukhi.

SHA-1: b5769181b2000bc1058e681918840a519cf22c51


Detailed log of new commits:

Author: dpasukhi
Date: Wed Dec 16 12:05:36 2020 +0300

    // Fix "-Wmisleading-indentation" warning
(0097829)
git   
2020-12-21 14:10   
Branch CR31756_4_4 has been updated forcibly by dpasukhi.

SHA-1: 1fa1ebcfa3b2b1e530eb1fd843e2377cb0f36b50
(0097916)
bugmaster   
2020-12-26 12:15   
Combination -
OCCT branch : WEEK-52
master SHA - 41046145c4a15f5cedf5f3c5877952ee00d568b4
a206de37fbfa0bf71bd534ae47192bbec23b8522
Products branch : WEEK-52 SHA - 290e5c74e8fef71947cadf90acb8e43c81ed10a1
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: 17722.770000000193 / 17710.950000000055 [+0.07%]
Products
Total CPU difference: 12416.490000000109 / 12412.520000000126 [+0.03%]
Windows-64-VC14:
OCCT
Total CPU difference: 19321.5625 / 19274.6875 [+0.24%]
Products
Total CPU difference: 13844.421875 / 13853.046875 [-0.06%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0097936)
git   
2020-12-26 14:52   
Branch CR31756_1 has been deleted by inv.

SHA-1: 8c27092e8209ceeeb6a97a5e33a19db099619de0
(0097937)
git   
2020-12-26 14:53   
Branch CR31756_2 has been deleted by inv.

SHA-1: 67ab474f6b69bf2882d1e30b9984d13307ddd9f6
(0097938)
git   
2020-12-26 14:53   
Branch CR31756_3 has been deleted by inv.

SHA-1: a022b5e458ece2597d213fee763e3ad1d3c88ed3
(0097939)
git   
2020-12-26 14:53   
Branch CR31756_4 has been deleted by inv.

SHA-1: a524d2359be3ede0b4807b09739cffa3c7548e0b
(0097940)
git   
2020-12-26 14:53   
Branch CR31756_4_1 has been deleted by inv.

SHA-1: 271f53ddd82e3e2b6a5c0c0ee9fcae865a3d8c0e
(0097941)
git   
2020-12-26 14:53   
Branch CR31756_4_2 has been deleted by inv.

SHA-1: 1bb778940b1d8a521c964b18518716898f0890ef
(0097942)
git   
2020-12-26 14:53   
Branch CR31756_4_3 has been deleted by inv.

SHA-1: b1019f95e9f602a6764b0ccb869871c8dac9a821
(0097943)
git   
2020-12-26 14:53   
Branch CR31756_4_4 has been deleted by inv.

SHA-1: 1fa1ebcfa3b2b1e530eb1fd843e2377cb0f36b50
(0097944)
git   
2020-12-26 14:53   
Branch CR31756_5 has been deleted by inv.

SHA-1: 726f0fe206884aeb0950cd278f86babdbda6b1fc
(0097945)
git   
2020-12-26 14:53   
Branch CR31756_testParser has been deleted by inv.

SHA-1: 225b5fa264dcbc01d8edfe2e4c56775917347288