View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0024537Open CASCADEOCCT:Foundation Classespublic2014-01-16 13:042020-11-25 17:37
ReporterabvAssigned Tokgv  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
PlatformLinuxOSDebian 6.0 
Product Version6.7.0 
Target Version7.0.0Fixed in Version7.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

Relationships

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

Activities

git

2014-09-09 14:55

administrator   ~0031515

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

git

2014-09-25 15:46

administrator   ~0032159

Branch CR24537_test has been updated forcibly by dln.

SHA-1: 8581a1d295fc6f79c8bc9033179bfcca56e49d36

git

2015-01-14 11:34

administrator   ~0036035

Branch CR24537_test has been updated forcibly by dln.

SHA-1: e1eb782e1d1b514737e15e7f705ff554c5f69c7d

dln

2015-01-14 11:35

developer   ~0036036

Dear abv,

check it please

git

2015-05-07 17:13

administrator   ~0040758

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

msv

2015-05-07 17:16

developer   ~0040759

Please review again.

msv

2015-05-07 17:52

developer   ~0040763

Sorry, the new test case fails on x64 platform. It needs to be reworked.

git

2015-05-08 17:41

administrator   ~0040809

Branch CR24537 has been updated forcibly by msv.

SHA-1: 22536d3284dadf8a6e70f49295211db77c4f6012

msv

2015-05-08 17:43

developer   ~0040810

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

Andrey, please review the branch CR24537.

abv

2015-05-12 13:03

manager   ~0040821

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

git

2015-09-16 11:43

administrator   ~0045699

Branch CR24537 has been updated forcibly by msv.

SHA-1: 729a7c27424752e546968b44eab13a77dba3b06d

msv

2015-09-16 11:47

developer   ~0045700

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.

git

2015-09-22 12:53

administrator   ~0045993

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

msv

2015-09-23 11:12

developer   ~0046024

Reviewed.

mkv

2015-09-24 12:10

tester   ~0046080

Last edited: 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.

git

2015-10-16 16:28

administrator   ~0046942

Branch CR24537 has been deleted by kgv.

SHA-1: 729a7c27424752e546968b44eab13a77dba3b06d

git

2015-10-16 16:28

administrator   ~0046943

Branch CR24537_3 has been deleted by kgv.

SHA-1: ff03877408cf1a23f6c3594c60e3e1d0b4b304cc

git

2015-10-16 16:57

administrator   ~0047059

Branch CR24537_test has been deleted by kgv.

SHA-1: e1eb782e1d1b514737e15e7f705ff554c5f69c7d

Related Changesets

occt: master 10a4116e

2015-09-24 11:17:41

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.		 Affected Issues
0024537
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
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
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
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