MantisBT - Community
View Issue Details
0026042Community[OCCT] OCCT:Codingpublic2015-04-09 20:442019-03-16 17:52
Istvan Csanady 
bugmaster 
normalcrash 
closedfixed 
OS X, iOS
 
[OCCT] 7.0.0[OCCT] 7.0.0 
Not needed
0026042: OCCT won't work with the latest Xcode
Apple introduced a new optimization in the latest Xcode release, that makes OCCT unusable. The problem with this is that Apple only allows distributing apps compiled with the latest toolchain, thus currently OCCT can not be used on Apple platforms.

There is a warning about this optimization:

BSplCLib_CurveComputation.gxx:95:32: Reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true

This makes intructions like this:

if (&Weights == NULL) { //Weights is passed by reference to a function

optimized out in cases when the optimization level is >= o2

Unfortunately there are lots of codes like this in OCCT.

Since binary compatibility is not guaranteed between binaries compiled with different toolchains, compiling OCCT with an older version of Xcode is not a solution.
Compile OCCT with Xcode 6.3
No tags attached.
related to 0024682closed bugmaster Open CASCADE Move out B-spline cache from curves and surfaces to dedicated classes BSplCLib_Cache and BSplSLib_Cache 
has duplicate 0025429closed bugmaster Community Invalid use of NULL references 
related to 0025454closed bugmaster Community Large number of test suite bads/failures starting with Clang 3.4 
related to 0030582closed apn Open CASCADE Coding - avoid defining references to properties of NULL objects 
diff clang_null_reference_workaround.diff (13,333) 2015-04-17 09:53
https://tracker.dev.opencascade.org/
Issue History
2015-04-09 20:44Istvan CsanadyNew Issue
2015-04-09 20:44Istvan CsanadyAssigned To => kgv
2015-04-09 20:53Istvan CsanadyNote Added: 0039538
2015-04-09 20:55Istvan CsanadyNote Edited: 0039538bug_revision_view_page.php?bugnote_id=39538#r9907
2015-04-09 21:07kgvNote Added: 0039539
2015-04-09 21:09kgvRelationship addedrelated to 0025429
2015-04-09 21:10Istvan CsanadyNote Added: 0039540
2015-04-09 21:13kgvNote Added: 0039541
2015-04-10 11:57Istvan CsanadyNote Added: 0039556
2015-04-10 12:40Istvan CsanadyNote Added: 0039559
2015-04-10 12:48kgvNote Added: 0039560
2015-04-15 06:40abvRelationship addedrelated to 0024682
2015-04-15 10:37Istvan CsanadyNote Added: 0039738
2015-04-15 11:26abvNote Added: 0039749
2015-04-15 11:30Istvan CsanadyNote Added: 0039750
2015-04-16 17:15Istvan CsanadyFile Added: patch.diff
2015-04-16 17:15Istvan CsanadyNote Added: 0039903
2015-04-16 17:25kgvNote Added: 0039905
2015-04-17 09:53Istvan CsanadyFile Added: clang_null_reference_workaround.diff
2015-04-17 09:53Istvan CsanadyNote Added: 0039941
2015-04-17 11:40Istvan CsanadyNote Added: 0039946
2015-04-17 19:48gitNote Added: 0040010
2015-04-17 23:44kgvFile Deleted: patch.diff
2015-04-30 11:57gitNote Added: 0040463
2015-04-30 12:00gitNote Added: 0040464
2015-05-05 13:09kgvNote Added: 0040554
2015-05-05 13:10kgvNote Edited: 0040554bug_revision_view_page.php?bugnote_id=40554#r10301
2015-05-06 09:20abvTarget Version6.9.0 => 7.1.0
2015-05-06 09:34gitNote Added: 0040599
2015-05-06 09:36gitNote Added: 0040600
2015-05-06 12:05Istvan CsanadyNote Added: 0040605
2015-09-02 08:55abvTarget Version7.1.0 => 7.0.0
2015-09-02 08:55abvAssigned Tokgv => akz
2015-09-02 08:56abvStatusnew => assigned
2015-09-23 17:28gitNote Added: 0046058
2015-09-23 17:37gitNote Added: 0046060
2015-09-23 17:49akzAssigned Toakz => abv
2015-09-23 17:51akzNote Added: 0046062
2015-09-23 17:51akzStatusassigned => resolved
2015-09-23 17:52akzNote Edited: 0046062bug_revision_view_page.php?bugnote_id=46062#r11712
2015-09-23 17:53akzNote Edited: 0046062bug_revision_view_page.php?bugnote_id=46062#r11713
2015-09-24 07:26abvNote Added: 0046069
2015-09-24 07:26abvAssigned Toabv => akz
2015-09-24 07:26abvStatusresolved => assigned
2015-09-24 13:36gitNote Added: 0046082
2015-09-24 13:39akzNote Added: 0046083
2015-09-24 13:39akzAssigned Toakz => kgv
2015-09-24 13:39akzStatusassigned => resolved
2015-09-24 13:39akzAssigned Tokgv => abv
2015-09-24 17:45akzNote Edited: 0046083bug_revision_view_page.php?bugnote_id=46083#r11722
2015-09-24 18:34abvNote Added: 0046104
2015-09-24 18:34abvAssigned Toabv => bugmaster
2015-09-24 18:34abvStatusresolved => reviewed
2015-09-24 18:54gitNote Added: 0046109
2015-09-24 19:48mkvAssigned Tobugmaster => mkv
2015-09-25 14:40mkvNote Added: 0046133
2015-09-25 14:40mkvAssigned Tomkv => akz
2015-09-25 14:40mkvStatusreviewed => assigned
2015-09-25 14:40mkvNote Added: 0046134
2015-09-25 14:40mkvNote Added: 0046135
2015-09-25 16:17gitNote Added: 0046140
2015-09-25 16:17akzStatusassigned => resolved
2015-09-25 16:22akzNote Added: 0046141
2015-09-25 16:22akzAssigned Toakz => mkv
2015-09-25 16:22akzStatusresolved => reviewed
2015-09-25 16:23akzNote Edited: 0046141bug_revision_view_page.php?bugnote_id=46141#r11731
2015-09-25 18:42gitNote Added: 0046161
2015-09-29 16:38mkvNote Added: 0046275
2015-09-29 16:39mkvNote Added: 0046276
2015-09-29 16:39mkvNote Added: 0046277
2015-09-29 16:40mkvTest case number => Not needed
2015-09-29 16:40mkvAssigned Tomkv => bugmaster
2015-09-29 16:40mkvStatusreviewed => tested
2015-10-02 14:56bugmasterChangeset attached => occt master 0e14656b
2015-10-02 14:56bugmasterStatustested => verified
2015-10-02 14:56bugmasterResolutionopen => fixed
2015-10-16 16:23gitNote Added: 0046916
2015-10-16 16:23gitNote Added: 0046917
2015-10-16 16:23gitNote Added: 0046918
2015-10-16 16:28gitNote Added: 0046945
2015-11-16 16:56abvRelationship addedrelated to 0025454
2016-04-20 15:44aivFixed in Version => 7.0.0
2016-04-20 15:49aivStatusverified => closed
2017-07-20 12:33kgvRelationship replacedhas duplicate 0025429
2019-03-16 17:52kgvRelationship addedrelated to 0030582

Notes
(0039538)
Istvan Csanady   
2015-04-09 20:53   
(edited on: 2015-04-09 20:55)
Just to clarify: if we can agree in a solution, I am absolutely willing to fix this problem, I just want to get some pointers on how to fix this issue. In my opinion probably the best solution would be to pass those arguments as pointers rather then references.

(0039539)
kgv   
2015-04-09 21:07   
From my perspective, such NULL-checks should be either removed or converted to pointer arguments if there is real use cases for that.
The proper solution would depend on the context.

if (&Weights == NULL) { //Weights is passed by reference to a function

Concerning the quoted fragment - it is clear that this case is actually used, thus it would be better to change syntax to pass pointer.
(0039540)
Istvan Csanady   
2015-04-09 21:10   
Unfortunately there are lots of real use cases for those null checks.
(0039541)
kgv   
2015-04-09 21:13   
I have linked issue with other one 0025429 which seems to be related to the same problem.
(0039556)
Istvan Csanady   
2015-04-10 11:57   
Yes it is. This is a classic example where optionals could be a great solution, but I think OCCT probably don't want to introduce such a concept. Maybe later it should be considered, many algorithms could benefit from optionals.
(0039559)
Istvan Csanady   
2015-04-10 12:40   
Is it possible to define handle arguments or pointer arguments in CDL? I could not find a way.
(0039560)
kgv   
2015-04-10 12:48   
Handles are automatically used within CDL for transient classes (e.g. there no way to pass class not through the handle in this case).

Pointers should be declared as dedicated type in Package cdl file and used further with specified alias (sample from "TCollection.cdl"):
    pointer MapNodePtr to MapNode from TCollection;
(0039738)
Istvan Csanady   
2015-04-15 10:37   
I have tried to fix this issue by replacing the references with pointers, unfortunately I stuck after a day, since it would require hundreds or maybe thousands of modifications all around OCCT.
(0039749)
abv   
2015-04-15 11:26   
Have you tried to assign address of Weights to some local pointer variable, and then using it in if()? This possibly could be work around.

As for comprehensive correction, I think that replacing references by pointers is best (safest) choice, as it will break existing interface and thus we will have to update (and check) all affected code. However it requires considerable effort and even more will conflict with changes made for 0024682. Thus I suggest this issue shall be fixed after 0024682 is integrated.
(0039750)
Istvan Csanady   
2015-04-15 11:30   
Yes, this is going to be my next effort, to create a CLANG_NULL_REFERENCE_WORKARAOUND_IF macro, that can be used until a proper fix will be implemented. (I am open for any suggestions for a better name :))
(0039903)
Istvan Csanady   
2015-04-16 17:15   
I have attached a workaround that seems to be working.
(0039905)
kgv   
2015-04-16 17:25   
> diff file icon patch.diff [^] (6,698 bytes) 2015-04-16 17:15
>
> From bb50c47cac355c06040675f63235999dfe153cb7 Mon Sep 17 00:00:00 2001
> From: msv <msv@opencascade.com>
> Date: Fri, 23 Jan 2015 18:07:15 +0300
> Subject: [PATCH] 0025719: Boolean operations can crash
are you sure you have attached correct commit?
(0039941)
Istvan Csanady   
2015-04-17 09:53   
Sorry, the previous patch was another one, I have attached the correct one.
(0039946)
Istvan Csanady   
2015-04-17 11:40   
Well, that's awkward: it seems, that Clang is able even with this workaround to optimize out those ifs in some cases. The only thing that worked everywhere for me was this:

#define CLANG_WORKAROUND_REFERENCE_IS_NULL(ref) ((reinterpret_cast<size_t>(&ref) & 0xFFFFFF) == 0)
(0040010)
git   
2015-04-17 19:48   
Branch CR26042 has been created by kgv.

SHA-1: 5576665ce50396073d3dbfbf130dad8760253784


Detailed log of new commits:

(0040463)
git   
2015-04-30 11:57   
Branch CR26042 has been updated by kgv.

SHA-1: b99127c76e8524f190a857ec6079bbcd7f6976dc


Detailed log of new commits:

Author: kgv
Date: Thu Apr 30 10:33:21 2015 +0300

    Use Standard_IS_NULL_REFERENCE in BSplCLib and LDOM

(0040464)
git   
2015-04-30 12:00   
Branch CR26042_1 has been created by kgv.

SHA-1: 0bad9ce2f3f864d226479f53eb9dd3603706b6a8


Detailed log of new commits:

Author: kgv
Date: Thu Apr 30 12:03:23 2015 +0300

    0026042: OCCT won't work with the latest Xcode
    
    Workaround Clang optimization issue with NULL-reference checks
    using new macros Standard_IS_NULL_REFERENCE().
(0040554)
kgv   
2015-05-05 13:09   
(edited on: 2015-05-05 13:10)
Hi Istvan,

> The problem with this is that Apple only allows distributing apps
> compiled with the latest toolchain
though this issue is certainly should be fixed, but could you please point out where this requirement can be found?
I'm unable to find such restriction on Apple site, including "App Store Review Guidelines" document.

(0040599)
git   
2015-05-06 09:34   
Branch CR26042_1 has been updated by kgv.

SHA-1: 0c8ca9eecc2cd1a373757e71215dd7a6014e604f


Detailed log of new commits:

Author: kgv
Date: Wed May 6 09:39:43 2015 +0300

    Standard_IS_NULL_REFERENCE - use optnone attribute instead of tricky manipulations

(0040600)
git   
2015-05-06 09:36   
Branch CR26042_2 has been created by kgv.

SHA-1: 74d278318effeee6908d2fa8cf3ac9dedc601fb7


Detailed log of new commits:

Author: kgv
Date: Wed May 6 09:41:08 2015 +0300

    0026042: OCCT won't work with the latest Xcode
    
    Workaround Clang optimization issue with NULL-reference checks
    using new macros Standard_IS_NULL_REFERENCE().
(0040605)
Istvan Csanady   
2015-05-06 12:05   
It seems that this policy has changed since the last time I read it, now it is only "strongly suggested" to use the latest Xcode.
(0046058)
git   
2015-09-23 17:28   
Branch CR26042_3 has been created by akz.

SHA-1: fe811334a239acceecfa7f7f661c3bd0d9a80e7d


Detailed log of new commits:

Author: akz
Date: Wed Sep 23 16:31:15 2015 +0300

    0026042: OCCT won't work with the latest Xcode
    
    NULL references was eliminated for PLib, BSplCLib and BSplSLib. All affected code was changed accordingly.
(0046060)
git   
2015-09-23 17:37   
Branch CR26042_3 has been updated forcibly by akz.

SHA-1: 7af494563d45073f65f9037b3f1a75380517f552
(0046062)
akz   
2015-09-23 17:51   
(edited on: 2015-09-23 17:53)
All dereferenced null pointers in PLib, BSplCLib and BSplSLib was eliminated. Corresponding references replaced by pointers. I've replaced only that references which can be NULL, such as Weights and additional Multipliers.

Branch CR26042_3 is ready for review (OCCT and Products repositories).

(0046069)
abv   
2015-09-24 07:26   
Please consider some (minor) remarks:

- In AppParCurves_Gradient.gxx, AppParCurves_MultiCurve.cxx, Approx_MCurvesToBSpCurve.cxx, BSplCLib.cxx, lxx, Convert*.cxx, ProjLib*.cxx, ShapeConstruct*.cxx: I suggest that PLib::NoWeights() be either restored, or replaced by BSplCLib::NoWeights() instead of NULL, to preserve semantics. The same applies to other places where NULL is used: it is better to use NoWeights() methods everywhere (e.g. in Geom) for consistency.

- Inline methods defined in PLib.lxx can be merged to PLib.hxx, and LXX file removed (it was artifact of CDL)

- BSplCLib.cxx: when calling a method of object by pointer, it is more natural to use -> instead of (*).:
  (*Mults).Lower() -> Mults->Lower()
  
- in PLib.hxx, BSpl*Lib.hxx, please indicate in documentation comments that weight and multiplicity arrays are passed by pointer so that NULL value is valid, meaning no weights / no multiplicities (this can be done in general comment to the package)
(0046082)
git   
2015-09-24 13:36   
Branch CR26042_3 has been updated forcibly by akz.

SHA-1: 9fa96eee5fe9b2590436d47e0e7009ec0a7fc820
(0046083)
akz   
2015-09-24 13:39   
(edited on: 2015-09-24 17:45)
All remarks are handled. Branch CR26042_3.

(0046104)
abv   
2015-09-24 18:34   
No remarks, please test
(0046109)
git   
2015-09-24 18:54   
Branch CR26042_3 has been updated forcibly by mkv.

SHA-1: a5816e1b1c613df146552e128bcec220ac67dcdd
(0046133)
mkv   
2015-09-25 14:40   
Dear BugMaster,
Branch CR26049_3 was rebased on branch IR-2015-09-24 of occt git-repository.
SHA-1: a5816e1b1c613df146552e128bcec220ac67dcdd
(0046134)
mkv   
2015-09-25 14:40   
Dear BugMaster,
Branch CR26049_3 from occt git-repository (and IR-2015-09-24 from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
SHA-1: a5816e1b1c613df146552e128bcec220ac67dcdd

There are following compilation errors:
Linux:
http://jenkins-test-01.nnov.opencascade.com:8080/view/CR26042-3-master/job/CR26042-3-master_build_occt_products_linux/1/parsed_console/ [^]

../../../../src/DxfData/DxfData_TranslateCurve.cxx:429:19: error: no matching function for call to 'BSplCLib::RemoveKnot(Standard_Integer&, Standard_Integer&, Standard_Integer&, Standard_Boolean&, const TColgp_Array1OfPnt&, const TColStd_Array1OfReal&, const TColStd_Array1OfReal&, const TColStd_Array1OfInteger&, TColgp_Array1OfPnt&, TColStd_Array1OfReal&, TColStd_Array1OfReal&, TColStd_Array1OfInteger&, const Standard_Real&)'

../../../../src/DxfData/DxfData_TranslateCurve.cxx:438:19: error: no matching function for call to 'BSplCLib::RemoveKnot(Standard_Integer&, Standard_Integer&, Standard_Integer&, Standard_Boolean&, const TColgp_Array1OfPnt&, TColStd_Array1OfReal*, const TColStd_Array1OfReal&, const TColStd_Array1OfInteger&, TColgp_Array1OfPnt&, TColStd_Array1OfReal&, TColStd_Array1OfReal&, TColStd_Array1OfInteger&, const Standard_Real&)'

Windows:
http://jenkins-test-01.nnov.opencascade.com:8080/view/CR26042-3-master/job/CR26042-3-master_build_occt_products_windows_64/1/parsed_console/ [^]

26>..\..\..\src\DxfData\DxfData_TranslateCurve.cxx(429): error C2665: 'BSplCLib::RemoveKnot' : none of the 3 overloads could convert all the argument types [d:\builds\vc10\CR26042-3-master-products-64\adm\msvc\vc10\TKDXF.vcxproj]


26>..\..\..\src\DxfData\DxfData_TranslateCurve.cxx(438): error C2665: 'BSplCLib::RemoveKnot' : none of the 3 overloads could convert all the argument types [d:\builds\vc10\CR26042-3-master-products-64\adm\msvc\vc10\TKDXF.vcxproj]


..\..\..\src\DxfData\DxfData_TranslateCurve.cxx(429): error C2665: 'BSplCLib::RemoveKnot' : none of the 3 overloads could convert all the argument types [d:\builds\vc10\CR26042-3-master-products-64\adm\msvc\vc10\TKDXF.vcxproj]
..\..\..\src\DxfData\DxfData_TranslateCurve.cxx(438): error C2665: 'BSplCLib::RemoveKnot' : none of the 3 overloads could convert all the argument types [d:\builds\vc10\CR26042-3-master-products-64\adm\msvc\vc10\TKDXF.vcxproj]


Number of compiler warnings:

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

There are new additional compilation warnings on Linux platform:

http://jenkins-test-01.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_17008/job/CR26042-3-master_build_occt_linux/1/warnings17Result/package.-671474530/ [^]
BSplCLib_CurveComputation.gxx:1089, GNU C Compiler 4 (gcc), Priority: Normal
the address of 'Weights' will never be NULL [-Waddress]
BSplCLib_CurveComputation.gxx:1130, GNU C Compiler 4 (gcc), Priority: Normal
the address of 'theWeights' will never be NULL [-Waddress]
http://jenkins-test-01.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_17008/job/CR26042-3-master_build_occt_linux/1/warnings17Result/package.-605032998/ [^]
BSplCLib_2.cxx:73, GNU C Compiler 4 (gcc), Priority: Normal
the address of 'Weights' will never be NULL [-Waddress]
http://jenkins-test-01.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_17008/job/CR26042-3-master_build_occt_linux/1/warnings17Result/package.-604556342/ [^]
BSplSLib.cxx:2213, GNU C Compiler 4 (gcc), Priority: Normal
the address of 'WeightsArray' will never be NULL [-Waddress]

Regressions/Differences/Improvements:
(partially)
http://occt-tests/CR26042-3-master-occt-64/Debian70-64/summary.html [^]
http://occt-tests/CR26042-3-master-occt-64/Windows-64-VC10/summary.html [^]

Testing cases:
Not needed
(0046135)
mkv   
2015-09-25 14:40   
Dear akz,
Branch CR26049_3 has been rejected due to:
- compilation errors
- additional warnings
- regressions/differences/improvements
(0046140)
git   
2015-09-25 16:17   
Branch CR26042_3 has been updated forcibly by akz.

SHA-1: ef638e252b8655e9b09246fbe1dd2cca2547f676
(0046141)
akz   
2015-09-25 16:22   
(edited on: 2015-09-25 16:23)
Dear mkv,

I've update branch CR26042_3 on OCCT repository with minor changes that should fix warnings for GCC and probably fix regressions.

Could you please also use branch CR26042_3 on Products repository? That was pushed before testing and seems to be correct.

(0046161)
git   
2015-09-25 18:42   
Branch CR26042_3 has been updated forcibly by mkv.

SHA-1: 5021b5d03271eef1dcc09a85329b67a046bcf3b1
(0046275)
mkv   
2015-09-29 16:38   
Dear BugMaster,
Branch CR26042_3 was rebased on branch IR-2015-09-24 of occt git-repository.
SHA-1: 5021b5d03271eef1dcc09a85329b67a046bcf3b1
Branch CR26042_3 was rebased on branch IR-2015-09-24 of products git-repository.
SHA-1: c12c69d37f22cc650a0cff58f3f747849e9f1942
(0046276)
mkv   
2015-09-29 16:39   
Dear BugMaster,
Branch CR26042_3 from occt git-repository (and CR26042_3 from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
SHA-1: 5021b5d03271eef1dcc09a85329b67a046bcf3b1
SHA-1: c12c69d37f22cc650a0cff58f3f747849e9f1942

Number of compiler warnings:

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

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

Regressions/Differences/Improvements:
No regressions/differences

Testing cases:
Not needed

Testing on Linux:
occt component :
Total MEMORY difference: 92116663 / 93204908 [-1.17%]
Total CPU difference: 19606.959999999224 / 19578.25999999923 [+0.15%]
products component :
Total MEMORY difference: 26311742 / 26313327 [-0.01%]
Total CPU difference: 6572.919999999949 / 7200.489999999998 [-8.72%]

Testing on Windows:
occt component :
Total MEMORY difference: 57872215 / 57877934 [-0.01%]
Total CPU difference: 18337.37154649904 / 17715.66076119908 [+3.51%]
products component :
Total MEMORY difference: 17135379 / 17140676 [-0.03%]
Total CPU difference: 5935.354446900002 / 5623.134045499962 [+5.55%]

There are no differences in images found by testdiff.
(0046277)
mkv   
2015-09-29 16:39   
Dear BugMaster,
Branch CR26042_3 is TESTED.
(0046916)
git   
2015-10-16 16:23   
Branch CR26042 has been deleted by kgv.

SHA-1: b99127c76e8524f190a857ec6079bbcd7f6976dc
(0046917)
git   
2015-10-16 16:23   
Branch CR26042_1 has been deleted by kgv.

SHA-1: 0c8ca9eecc2cd1a373757e71215dd7a6014e604f
(0046918)
git   
2015-10-16 16:23   
Branch CR26042_2 has been deleted by kgv.

SHA-1: 74d278318effeee6908d2fa8cf3ac9dedc601fb7
(0046945)
git   
2015-10-16 16:28   
Branch CR26042_3 has been deleted by kgv.

SHA-1: 5021b5d03271eef1dcc09a85329b67a046bcf3b1