View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0030937||Community||OCCT:Coding||public||2019-09-03 18:25||2023-08-02 02:01|
|Reporter||Vico Liang||Assigned To||dpasukhi|
|Summary||0030937: Coding - TNaming_NamedShape destructor should not be inlined|
|Description||The destructor of TNaming_NamedShape as below:|
It will cause serious problem when building with android-clang compiler for arm64-v8. The build environment is android SDK version: 26.1.1; NDK version: 20.0.5594570.
below code snippet will reproduce the issue:
TDF_Attribute* aNSA = new TNaming_NamedShape();
TNaming_NamedShape* aNS = dynamic_cast<TNaming_NamedShape*>(aNSA);
std::cout << "Serious bug occured"; // bug here
To fix the issue just move the destructor to cpp file. it's better to declare as virtual export function ：
Standard_EXPORT virtual ~TNaming_NamedShape();
|Tags||No tags attached.|
|Test case number|
Isn't it a compiler/linker bug?
Why it should be changed in OCCT instead of reporting to CLang?
Yes, I agree with you. It's a bug of compiler or linker.
Form coding best practice, it makes no sense to inline a virtual function which is accessed by pointer, but it makes sense to make better coding on OCCT side.
> it's make no sense to inline a virtual function
It is not true - compiler is able deducing from context if object is statically typed and actually inline even virtual method call.
Not sure that this is applicable to destructors, but maybe - haven't analyzed this subject in deep.
Concerning coding practices, the inlining point here is code base compactness - always moving trivial virtual methods into .cpp would cause extra code.
OCCT code base has enormous amount of inlined virtual methods, including implicit destructors as mentioned in related bug 0030238 in MSVC.
So that if such patch will be considered necessary, it should cover more classes, not only mentioned in the bug.
>It is not true - compiler is able deducing from context if object is statically typed and actually inline even virtual method call.
OCCT most objects are created by handle(pointer), the inline virtual function will never expanded. It will also confuse developer.
|Vico, the problem you have reported is quite strange, can you check that you are using the same compiler and consistent compiler flags for building OCCT and your application? Meanwhile, CLang provides some tools for checking consistency of the code at run-time, perhaps this can be used to debug the issue: see https://clang.llvm.org/docs/ControlFlowIntegrity.html|
||I don't have chance to change the behavior of the compiler. I'm using the default option in android build: android-clang compiler for arm64-v8. The build environment is android SDK version: 26.1.1; NDK version: 20.0.5594570. What feedback do you want from me? I already told you the reason of the issue and the fix of the solution.|
the issue persists on SDK Version: 26.1.1, NDK Version: 19.2.5345600 and 18
These are two issues about android ndk related to this issue:
1. std::dynamic_pointer_cast always returns a nullptr
2. dynamic_cast form pointers is not working when linked with libc++_shared (ndk r15, r16b1)
||It's interesting that this issue doesn't occur on android armv7 build. so it's a bug of NDK arm64-v8.|
I have filed a bug to ndk:
Thank you for taking effort of diagnosing and reporting the issue.
As the problem is of general nature, apparently it should not be limited to particular class (TNaming_NamedShape). Thus to fix the problem consistently on OCCT side we need to understand clearly what is wrong, and fix that in all affected places.
From your discussion with DanAlbert in NDK issue tracker (1075) I conclude that his explanations regarding missing "key function" are not relevant in your situation, since class TNaming_NamedShape definitely does have key functions. Note that most of OCCT classes inheriting Standard_Transient are defined with macro DEFINE_STANDARD_RTTIEXT that defines out-of-line virtual function DynamicType(), this should guarantee that typeinfo is exported correctly for these classes.
Thus I share opinion of Kirill that according to current symptoms it is a compiler / linker bug that cannot be fixed at OCCT side.
Can you confirm my understanding that your custom code and / or OCCT libraries are loaded dynamically into the application, i.e. by explicit call to dlopen?
If so, and if you can control parameters of dlopen() call, the problem can probably be resolved by passing RTLD_GLOBAL as the last argument to dlopen().
(I have experienced similar problem long time ago with Python modules, and such solution helped that time).
Another possibility to avoid the issue could be to link OCCT statically (provided that that does not violate LGPL terms).
One more point: according to CLang documentation, it should give a diagnostic warning when a class with virtual functions has no key method: https://releases.llvm.org/8.0.0/tools/clang/docs/DiagnosticsReference.html#wweak-vtables
We have never seen this warning in OCCT code, thus I suppose it should be clean (or warning is disabled for some reason)
>>Can you confirm my understanding that your custom code and / or OCCT libraries are loaded dynamically into the application.
Yes, it should be loaded dynamically, but it's not calling method dlopen() directly, it's calling in java System.load() method.
>>Another possibility to avoid the issue could be to link OCCT statically (provided that that does not violate LGPL terms).
Yes, this solution works. It's the best option for me. save me much of time.
Thanks for assisting.
|2019-09-03 18:25||Vico Liang||New Issue|
|2019-09-03 18:25||Vico Liang||Assigned To||=> mpv|
|2019-09-04 11:13||kgv||Relationship added||related to 0030238|
|2019-09-04 11:13||kgv||Note Added: 0086720|
|2019-09-04 11:14||kgv||Note Edited: 0086720|
|2019-09-04 11:43||Vico Liang||Note Added: 0086721|
|2019-09-04 11:50||Vico Liang||Note Edited: 0086721|
|2019-09-04 11:50||kgv||Note Added: 0086722|
|2019-09-04 11:50||kgv||Note Edited: 0086722|
|2019-09-04 11:51||kgv||Note Edited: 0086722|
|2019-09-04 11:52||kgv||Note Edited: 0086722|
|2019-09-04 11:52||kgv||Category||OCCT:Application Framework => OCCT:Coding|
|2019-09-04 11:52||kgv||Summary||TNaming_NamedShape destructor should not be inlined => Coding - TNaming_NamedShape destructor should not be inlined|
|2019-09-04 11:58||Vico Liang||Note Edited: 0086721|
|2019-09-04 12:07||Vico Liang||Note Added: 0086725|
||Note Added: 0086728|
||Assigned To||mpv => Vico Liang|
||Status||new => feedback|
||Target Version||7.4.0 => 7.5.0|
|2019-09-04 13:30||Vico Liang||Note Added: 0086732|
|2019-09-04 13:31||Vico Liang||Assigned To||Vico Liang => abv|
|2019-09-04 19:18||Vico Liang||Note Added: 0086754|
|2019-09-05 06:46||Vico Liang||Note Added: 0086767|
|2019-09-05 06:46||Vico Liang||Note Edited: 0086767|
|2019-09-05 10:44||Vico Liang||Note Added: 0086784|
|2019-09-05 15:03||Vico Liang||Note Edited: 0086754|
|2019-09-09 15:39||Vico Liang||Note Added: 0086986|
||Note Added: 0087725|
||Assigned To||abv => Vico Liang|
||Note Added: 0087733|
|2019-10-10 19:24||Vico Liang||Note Added: 0088025|
||Target Version||7.5.0 => 7.6.0|
||Target Version||7.6.0 => 7.7.0|
||Target Version||7.7.0 => 7.8.0|
|2023-08-02 02:00||dpasukhi||Assigned To||Vico Liang => dpasukhi|
|2023-08-02 02:00||dpasukhi||Status||feedback => assigned|
|2023-08-02 02:01||dpasukhi||Target Version||7.8.0 => Unscheduled|