MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0024537Open CASCADE[OCCT] OCCT:Foundation Classespublic2014-01-16 13:042020-11-25 17:37
Reporterabv 
Assigned Tokgv 
PrioritynormalSeverityminor 
StatusclosedResolutionfixed 
PlatformLinuxOSDebian 6.0OS Version64 bit
Product Version[OCCT] 6.7.0 
Target Version[OCCT] 7.0.0Fixed in Version[OCCT] 7.0.0 
Summary0024537: GCC compiler warnings in byte order reversion code
DescriptionGCC produces warnings on breakage of pointer aliasing rules in code related to conversion of data between low-endian and big-endian formats, see 0024252:

FSD_FileHeader.hxx:101: warning: dereferencing type-punned pointer will break strict-aliasing rules
BinObjMgt_Persistent.cxx:1109: warning: dereferencing type-punned pointer will break strict-aliasing rules
FSD_BinaryFile.cxx:417: warning: dereferencing type-punned pointer will break strict-aliasing rules

The code can be corrected to use unions for this conversion. The problem however is that that code is not tested now, as all currently supported platforms are based on Intel x86 architecture and thus are low-endian. Thus specific tests need to be created for this functionality.

It is also a question whether big-endian platforms need to be supported at all. As it seems, existing and potential target platforms for OCCT (including iOS and Android on ARM processors) all use little-endian convention (even if ARM is bi-endian).
Steps To Reproducetest bugs fclasses bug24537
TagsNo tags attached.
Test case numberbugs fclasses bug24537
Attached Files

- Relationships
related to 0020979assignedabv Foundation Classes, FSD_BinaryFile - conversion functions should work with pointers on values 
child of 0024252closedbugmaster GCC warnings on breakage of strict-aliasing rules 

-  Notes
(0031515)
git (administrator)
2014-09-09 14:55

Branch CR24537_test has been created by dln.

SHA-1: 237c2596f5711322edf6eb29fc27d7162ec86bf6


Detailed log of new commits:

Author: dln
Date: Tue Sep 9 14:54:08 2014 +0400

    0024537: GCC compiler warnings in byte order reversion code
    
    - preliminary version of the fix for test
(0032159)
git (administrator)
2014-09-25 15:46

Branch CR24537_test has been updated forcibly by dln.

SHA-1: 8581a1d295fc6f79c8bc9033179bfcca56e49d36
(0036035)
git (administrator)
2015-01-14 11:34

Branch CR24537_test has been updated forcibly by dln.

SHA-1: e1eb782e1d1b514737e15e7f705ff554c5f69c7d
(0036036)
dln (developer)
2015-01-14 11:35

Dear abv,

check it please
(0040758)
git (administrator)
2015-05-07 17:13

Branch CR24537 has been created by msv.

SHA-1: c468fd82b8a59892e99a5de36b78cba88027698e


Detailed log of new commits:

Author: dln
Date: Tue Sep 9 14:54:08 2014 +0400

    0024537: GCC compiler warnings in byte order reversion code
    
    - warnings in byte order inversion functionality is fixed with unions
    - test case (simulation of conversion to big endian) is added
(0040759)
msv (developer)
2015-05-07 17:16

Please review again.
(0040763)
msv (developer)
2015-05-07 17:52

Sorry, the new test case fails on x64 platform. It needs to be reworked.
(0040809)
git (administrator)
2015-05-08 17:41

Branch CR24537 has been updated forcibly by msv.

SHA-1: 22536d3284dadf8a6e70f49295211db77c4f6012
(0040810)
msv (developer)
2015-05-08 17:43

The test case has been reworked. Now it is completely hard-coded inside the draw command OCC24537.

Andrey, please review the branch CR24537.
(0040821)
abv (manager)
2015-05-12 13:03

I suggest that some corrections should be made:

1. In FSD_FileHeader.hxx:
   - it can be worth moving this code to cxx or separate header (in BinTools?), to avoid defining this stuff (very special and not used in practice) globally
   - it can be worth renaming macro DO_INVERSE to something more specific, at least prefix it with "OCCT_"
   - it can be worth adding Standard_StaticAssert to check that sizeof(ShortReal) == sizeof(Integer) and sizeof(Real) == 2 * sizeof(Integer) etc.; alternatively templated version can be used (just as for Size)
   - InverseSizeSpecialized <4> can be just a call to InverseInt()
   - InverseLong() is obviously not correct for 64-bit systems where long is 8 bytes in size; it can be removed as unused, or implemented in the same way as for Size

2. In BinObjMgt_Persistent.cxx:
   - I believe this will not work as before on 32-bit systems, since before aData was double* (incremented by 8 bytes) and now &aWrapUnion.anAddrData is void** (incremented by 4 bytes):

- aPrevPtr = &aData[aLenInPiece / BP_REALSIZE];
+ aPrevPtr = (&aWrapUnion.anAddrData)[aLenInPiece / BP_REALSIZE];

   - I deem that casting char* to double* is generally bad idea, since on some architectures this may cause alignment failure. Why not just to move along myData by 8-byte sequences, copying them to buffer, then converting and placing back?

3. In QABugs_19.cxx: I suggest using more disperse set of probing values, not just 1-2-3. For instance:

   - int: -4000000, -1234, 0, 1, 1234, 4000000
   - double: -1e300, -1e-9, 0., 1.e-9, 1., 3.1415296, 1e100
   - float: -1e-9, 0., 1.e-9, 1., 3.1415
(0045699)
git (administrator)
2015-09-16 11:43

Branch CR24537 has been updated forcibly by msv.

SHA-1: 729a7c27424752e546968b44eab13a77dba3b06d
(0045700)
msv (developer)
2015-09-16 11:47

I have considered some remarks. It is needed to continue the fix. The current version of the branch CR24537 must be re-based on current master before continuing. It can be not compilable.
(0045993)
git (administrator)
2015-09-22 12:53

Branch CR24537_3 has been created by rkv.

SHA-1: ff03877408cf1a23f6c3594c60e3e1d0b4b304cc


Detailed log of new commits:

Author: rkv
Date: Tue Sep 22 12:48:04 2015 +0300

    0024537: GCC compiler warnings in byte order reversion code
    
    - warnings in byte order inversion functionality is fixed with unions
    - test case (simulation of conversion to big endian) is added
(0046024)
msv (developer)
2015-09-23 11:12

Reviewed.
(0046080)
mkv (tester)
2015-09-24 12:10
edited on: 2015-09-24 12:10

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

Number of compiler warnings:

occt component :
Linux: 13 (15 on master)
Windows: 0 (0 on master)

products component :
Linux: 39 (39 on master)
Windows: 0 (0 on master)

There is new additional compilation warning on Linux platform:
There are compilation errors in Qt samples products on windows platform:

Regressions/Differences/Improvements:
No regressions/differences

Testing cases:
http://occt-tests/CR24537-3-master-occt-64/Debian70-64/bugs/fclasses/bug24537.html [^]
http://occt-tests/CR24537-3-master-occt-64/Windows-64-VC10/bugs/fclasses/bug24537.html [^]
bugs fclasses bug24537: OK

Testing on Linux:
occt component :
Total MEMORY difference: 92282120 / 92105726 [+0.19%]
Total CPU difference: 18763.229999999014 / 18665.439999998966 [+0.52%]
products component :
Total MEMORY difference: 25921386 / 25980951 [-0.23%]
Total CPU difference: 7418.7299999999905 / 7208.589999999996 [+2.92%]

Testing on Windows:
occt component :
Total MEMORY difference: 56806777 / 56823447 [-0.03%]
Total CPU difference: 18049.986504299017 / 16808.499346099128 [+7.39%]
products component :
Total MEMORY difference: 16785580 / 16790438 [-0.03%]
Total CPU difference: 5934.1844394 / 5526.49142599996 [+7.38%]

There are no differences in images found by testdiff.

(0046942)
git (administrator)
2015-10-16 16:28

Branch CR24537 has been deleted by kgv.

SHA-1: 729a7c27424752e546968b44eab13a77dba3b06d
(0046943)
git (administrator)
2015-10-16 16:28

Branch CR24537_3 has been deleted by kgv.

SHA-1: ff03877408cf1a23f6c3594c60e3e1d0b4b304cc
(0047059)
git (administrator)
2015-10-16 16:57

Branch CR24537_test has been deleted by kgv.

SHA-1: e1eb782e1d1b514737e15e7f705ff554c5f69c7d

- Related Changesets
occt: master 10a4116e
Timestamp: 2015-09-24 11:17:41
Author: rkv
Committer: kgv
Details ] Diff ]
0024537: GCC compiler warnings in byte order reversion code

Eliminate warnings in byte order inversion functionality by using unions.
Add test case simulating conversion to big endian.
mod - src/BinObjMgt/BinObjMgt_Persistent.cxx Diff ] File ]
mod - src/FSD/FSD_BinaryFile.cxx Diff ] File ]
mod - src/FSD/FSD_BinaryFile.hxx Diff ] File ]
mod - src/FSD/FSD_FileHeader.hxx Diff ] File ]
mod - src/QABugs/QABugs_19.cxx Diff ] File ]
add - tests/bugs/fclasses/bug24537 Diff ] File ]

- Issue History
Date Modified Username Field Change
2014-01-16 13:04 abv New Issue
2014-01-16 13:04 abv Assigned To => abv
2014-01-16 13:04 abv Relationship added child of 0024252
2014-01-16 13:48 abv Summary GCC compiler warnings in bit order reversion code => GCC compiler warnings in byte order reversion code
2014-04-04 18:16 abv Target Version 6.7.1 => 6.8.0
2014-09-03 10:11 abv Assigned To abv => dln
2014-09-09 14:55 git Note Added: 0031515
2014-09-25 15:46 git Note Added: 0032159
2014-10-29 06:58 abv Target Version 6.8.0 => 7.1.0
2015-01-14 11:34 git Note Added: 0036035
2015-01-14 11:35 dln Note Added: 0036036
2015-01-14 11:36 dln Assigned To dln => abv
2015-01-14 11:36 dln Status new => resolved
2015-01-14 11:36 dln Steps to Reproduce Updated View Revisions
2015-05-07 16:50 msv Assigned To abv => msv
2015-05-07 16:50 msv Status resolved => assigned
2015-05-07 17:13 git Note Added: 0040758
2015-05-07 17:16 msv Note Added: 0040759
2015-05-07 17:16 msv Assigned To msv => abv
2015-05-07 17:16 msv Status assigned => resolved
2015-05-07 17:17 msv Steps to Reproduce Updated View Revisions
2015-05-07 17:52 msv Note Added: 0040763
2015-05-07 17:52 msv Status resolved => assigned
2015-05-07 17:52 msv Assigned To abv => msv
2015-05-08 17:41 git Note Added: 0040809
2015-05-08 17:43 msv Note Added: 0040810
2015-05-08 17:43 msv Assigned To msv => abv
2015-05-08 17:43 msv Status assigned => resolved
2015-05-12 13:03 abv Note Added: 0040821
2015-05-12 13:03 abv Assigned To abv => msv
2015-05-12 13:03 abv Status resolved => assigned
2015-09-16 11:43 git Note Added: 0045699
2015-09-16 11:47 msv Note Added: 0045700
2015-09-18 15:54 abv Assigned To msv => rkv
2015-09-22 12:53 git Note Added: 0045993
2015-09-22 13:23 rkv Assigned To rkv => msv
2015-09-22 13:23 rkv Status assigned => resolved
2015-09-23 11:12 msv Note Added: 0046024
2015-09-23 11:12 msv Assigned To msv => bugmaster
2015-09-23 11:12 msv Status resolved => reviewed
2015-09-23 11:13 abv Target Version 7.1.0 => 7.0.0
2015-09-23 17:54 mkv Assigned To bugmaster => mkv
2015-09-24 12:10 mkv Note Added: 0046080
2015-09-24 12:10 mkv Assigned To mkv => bugmaster
2015-09-24 12:10 mkv Status reviewed => tested
2015-09-24 12:10 mkv Test case number => bugs fclasses bug24537
2015-09-24 12:10 mkv Note Edited: 0046080 View Revisions
2015-09-28 12:58 kgv Changeset attached => occt master 10a4116e
2015-09-28 12:58 kgv Assigned To bugmaster => kgv
2015-09-28 12:58 kgv Status tested => verified
2015-09-28 12:58 kgv Resolution open => fixed
2015-10-16 16:28 git Note Added: 0046942
2015-10-16 16:28 git Note Added: 0046943
2015-10-16 16:57 git Note Added: 0047059
2016-04-20 15:44 aiv Fixed in Version => 7.0.0
2016-04-20 15:48 aiv Status verified => closed
2020-11-25 17:37 kgv Relationship added related to 0020979


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker