MantisBT - Open CASCADE
View Issue Details
0030557Open CASCADE[OCCT] OCCT:Codingpublic2019-03-12 04:332019-10-09 23:24
kgv 
kgv 
normalminor 
newopen 
[OCCT] 7.3.0 
[OCCT] 7.5.0* 
0030557: Coding - eliminate errors reported by -fsanitize
Compile OCCT 7.3.0 with -fsanitize=undefined, -fsanitize=address and run the test suite.
No tags attached.
parent of 0030556verified apn Community Coding - IGESData_GlobalSection missing member initialization in default constructor 
parent of 0030553verified apn Community Coding - TopOpeBRepDS_Surface missing default initalizations 
parent of 0030554verified apn Community Coding - ChFiDS_CommonPoint uninitialized member traarc 
parent of 0030552feedback abv Community Foundation Classes - Stack overflow due to math_SingleTab static array size 
parent of 0030551verified bugmaster Community Foundation Classes - Integer overflow in NCollection_CellFilter HashCode 
parent of 0030562verified apn Community Coding - TopOpeBRepBuild_Builder use of null pointer 
parent of 0030564verified apn Community Coding - math_Gauss uninitialized 'Singular' member variable 
parent of 0030565verified bugmaster Community Coding - Approx_SweepApproximation call on null pointer 
parent of 0030518verified bugmaster Community Foundation Classes - NCollection_IndexedDataMap array out of bounds 
parent of 0030550verified bugmaster Community Coding - Integer overflow in Standard_CString HashCodes 
parent of 0030603closed bugmaster Community Memory Leaks in Geom_BSplineCurve 
parent of 0030563new kgv Community Coding - Standard_Type vptr error 
parent of 0030978verified bugmaster Open CASCADE Visualization - stack-use-after-scope reported by Clang address sanitizer in OpenGl_Text.cxx 
parent of 0030980verified bugmaster Open CASCADE Data Exchange - global-buffer-overflow reported by Clang address sanitizer in iges_newchar() 
parent of 0030981verified bugmaster Open CASCADE Foundation Classes - heap-buffer-overflow reported by Clang address sanitizer in TCollection_ExtendedString 
parent of 0030985verified bugmaster Open CASCADE Modeling Algorithms - heap-use-after-free reported by Clang address sanitizer in TopOpeBRepTool_REGUW::InitBlock() 
parent of 0030986new msv Open CASCADE Modeling Algorithms - heap-buffer-overflow reported by Clang address sanitizer in HLRBRep_Data.cxx 
parent of 0030989new kgv Open CASCADE Visualization - heap-use-after-free reported by Clang address sanitizer in OpenGl_Structure::IsRaytracable() 
parent of 0030516verified bugmaster Community Visualization - Pointer to an OpenGl_Structure is deleted and accessed later after PrsMgr_Presentation::Highlight() 
parent of 0030992reviewed bugmaster Open CASCADE Foundation Classes - heap-buffer-overflow reported by Clang address sanitizer in BSplCLib::BuildKnots() 
parent of 0030993reviewed bugmaster Open CASCADE Modeling Algorithms - heap-use-after-free reported by Clang address sanitizer in BRepFeat_MakeRevolutionForm::Perform() 
parent of 0031008reviewed bugmaster Open CASCADE Application Framework - memcpy-param-overlap reported by Clang address sanitizer in LDOM_XmlReader::ReadRecord() 
parent of 0031009new msv Open CASCADE Modeling Algorithms - alloc-dealloc-mismatch reported by Clang address sanitizer in IntCurvesFace_ShapeIntersector 
parent of 0031010reviewed bugmaster Open CASCADE Foundation Classes - heap-buffer-overflow reported by Clang address sanitizer in OSD_Path::IsUncExtendedPath() 
parent of 0031024reviewed bugmaster Community Coding - invalid left shift in BVH_RadixSorter::Perform() using -fsanitize=undefined 
parent of 0031034reviewed bugmaster Open CASCADE Visualization - stack-use-after-scope reported by Clang address sanitizer in AIS_FixRelation::Compute() 
parent of 0031038new apn Open CASCADE Draw - adaptations for running tests with CLang address sanitizer 
parent of 0031048resolved abv Open CASCADE Visualization - runtime error reported by Clang undefined behavior sanitizer in Image_AlienPixMap::Save() 
Not all the children of this issue are yet resolved or closed.
Issue History
2019-03-12 04:33kgvNew Issue
2019-03-12 04:33kgvAssigned To => kgv
2019-03-12 04:33kgvRelationship addedparent of 0030556
2019-03-12 04:34kgvRelationship addedparent of 0030553
2019-03-12 04:34kgvRelationship addedparent of 0030554
2019-03-12 04:42galbramcNote Added: 0082854
2019-03-12 05:00kgvRelationship addedparent of 0030552
2019-03-12 05:00kgvSummaryCoding - eliminate errors reported by -fsanitize=undefined => Coding - eliminate errors reported by -fsanitize
2019-03-12 05:00kgvDescription Updatedbug_revision_view_page.php?rev_id=20819#r20819
2019-03-12 14:05kgvRelationship addedparent of 0030551
2019-03-12 19:47kgvRelationship addedparent of 0030562
2019-03-13 07:43kgvRelationship addedparent of 0030564
2019-03-13 07:43kgvRelationship addedparent of 0030565
2019-03-13 13:53galbramcNote Added: 0082909
2019-03-13 13:54kgvRelationship addedparent of 0030518
2019-03-20 08:40kgvRelationship addedparent of 0030550
2019-03-25 14:09kgvRelationship addedparent of 0030603
2019-03-25 14:20kgvRelationship addedparent of 0030563
2019-09-17 08:22abvNote Added: 0087181
2019-09-17 13:55galbramcNote Added: 0087201
2019-09-18 03:38abvRelationship addedparent of 0030978
2019-09-18 08:21abvNote Added: 0087240
2019-09-18 08:22abvTarget Version => 7.5.0*
2019-09-18 15:50galbramcNote Added: 0087259
2019-09-19 05:59abvNote Added: 0087282
2019-09-19 07:15abvRelationship addedparent of 0030980
2019-09-19 08:34abvRelationship addedparent of 0030981
2019-09-19 23:35galbramcNote Added: 0087333
2019-09-20 08:09abvRelationship addedparent of 0030985
2019-09-20 08:09abvRelationship addedparent of 0030986
2019-09-21 08:04abvRelationship addedparent of 0030989
2019-09-21 10:04kgvRelationship addedparent of 0030516
2019-09-24 08:28abvRelationship addedparent of 0030992
2019-09-24 08:32abvRelationship addedparent of 0030993
2019-09-28 09:13abvRelationship addedparent of 0031008
2019-09-28 09:27abvRelationship addedparent of 0031009
2019-09-28 09:37abvRelationship addedparent of 0031010
2019-10-03 06:25kgvRelationship addedparent of 0031024
2019-10-04 20:35abvRelationship addedparent of 0031034
2019-10-06 09:33abvRelationship addedparent of 0031038
2019-10-08 23:01abvRelationship addedparent of 0031048
2019-10-09 23:16abvNote Added: 0087995
2019-10-09 23:24galbramcNote Added: 0087996

Notes
(0082854)
galbramc   
2019-03-12 04:42   
Issues 0030552 and 0030551 should also be added here.
(0082909)
galbramc   
2019-03-13 13:53   
Since -fsanitize=address was added, issue 0030518 should also be a child here.
(0087181)
abv   
2019-09-17 08:22   
To use address sanitizers on my computer (Ubuntu 16.04) with CLang 6.0 I had to do the following:

1. In CMake configuration:

- set CMAKE_CXX_COMPILER to clang++-6.0 and CMAKE_C_COMPILER to clang-6.0; when using CMake GUI, this is done by choice of oprion "Specify native compilers" when you configure for the first time
- set CMAKE_LINKER to clang++-6.0; this is important to avoid linking problems
- set CMAKE_CXX_FLAGS and CMAKE_C_FLAGS to "-fsanitize=address -fno-omit-frame-pointer -fno-optimize-sibling-calls"
- set CMAKE_BUILD_TYPE to RelWithDebInfo; this is in the hope to get more informative messages

2. Build as usual (make), but times slower and more disk space consuming

3. Before running executable, make sure that "llvm-symbolizer" is in PATH; this is necessary to get human-readable stack traces. The tool must have exaclty that name. I did not had it installed with that name, so had to create a symlink with:
> sudo ln -s /usr/bin/llvm-symbolizer-6.0 /usr/bin/llvm-symbolizer

4. Set environment variable to disable memory leaks detection (they seem to be reported for every global variable, like MFC used to do in the past):
> export ASAN_OPTIONS=detect_leaks=0

5. Run DRAW (./draw.sh relwithdeb) and enjoy

So far I have been able to see the address sanitizer reporting an out-of-bound access in TCollection_ExtendedString (a problem known for long time) in XDE.

Another sanitizer I tried was "undefined", its reports are difficult to understand (perhaps will try more).
Sanitizer "memcheck" is mostly useless as it requires all libraries used by the executable to be instrumented, this requires enormous effort and time.

All this is extremely slow and takes a lot of time to build (every change is a rebuild).
From this experience I conclude that use of Valgrind is much easier and practical.

Use of compiler sanitizers requires a lot of effort (you have to build a whole environment for each sanitizer) and is worth only if there is a real strong need in making code 100% clean.
(0087201)
galbramc   
2019-09-17 13:55   
I have a few comments.

1. Did you also try the gnu compiler? g++ as of 4.9 also has the sanitizer. There is a bug in Ubuntu that requires you to use the flag -fuse-ld=gold as a link flag.

2. Using the sanitizer does make the build slower and it does take up more disk space.

3. I don't seem to need the llvm-symbolizer when using the gnu compiler.

4. The address sanitizer will report all memory that is not cleaned up before the code exits. valgrind will do the same if you use the flag --show-leak-kinds=all. You completely disabled all memory leak detection with export ASAN_OPTIONS=detect_leaks=0. I would recommend you instead suppress individual known problems with a suppression file: https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer#suppressions [^]

valgrind is only able to look for array out of bounds on heap allocated memory. The address sanitizer can also detect array out of bound errors on statically sized arrays on the stack. Bugs relate to the latter are extremely difficult to find otherwise. However, the address sanitizer cannot detect use of uninitialized data. So for our testing we use both valgrind and the sanitizer to detect the different types of errors.

While the compile time is longer with the sanitizer, the runtime of the test suite is significantly faster. Our set of tests takes about 3 hours to run with the address sanitizer, and the same suite of tests takes about 3 days to run with valgrind. We can run the sanitizer nightly to look for errors, but we only run valgrind once a month.

You do not need to instrument all libraries used by the executable, but you might need to add suppressions for libraries that you do not instrument. If the primary executable isn't instrumented you need to have
export LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libasan.so.5
(the last number on the shared library will depend on the compiler version).

5. I would encourage you to look closely at the undefined errors. They take a little getting used to deciphering, but are quite informative once you understand them.

6. I would hope that you are striving towards 100% clean code. I will continue reporting errors from the sanitizer as they show up with our testing.

Please let me know if there is anything I can help with.
(0087240)
abv   
2019-09-18 08:21   
Thank you for comments!

Your points are correct indeed, and sure we do aim at having 100% clean code... Yet systematic usage of tools like sanitizers is behind our current capacities due to amount of effort and resources needed for that.

Besides, I have been able to run OCCT tests with address sanitizers, and so far (1/5 passed during the night) only one error has been reported, thus now its usage looks more feasible.

Regarding errors reported by "undefined" sanitizer: I agree that errors like uninitialized fields are very important, but static code analyzers should be able to report them as well, and they should be much easier to use. The errors I have seen trying to run with "undefined" sanitizer looked more like in 0030563, and so far I could not understand what they mean. I will be grateful to you if you could explain the report in 0030563 - this may help us to understand other similar reports.
(0087259)
galbramc   
2019-09-18 15:50   
I appreciate that OCC is a very large code base, and I have seen a lot of improvements over the past couple revisions.

What static analyzers do you use? I'm using the clang analyzer at the moment and it catches some things, but not all the same things running with the undefined sanitizer catches.

The report 0030563 is probably the most cryptic message I have gotten so far from the undefined sanitizer, and it has me a little stumped as well. Part of the issue here is I don't completely understand the process of how aRegistry is used in OCC. The one clue is that I only get this report with running anything that does STEP file IO. I suspect there is some invalid casting that is going on in the STEP IO that the sanitizer is only catching when everything is getting cleaned up. I'm wondering if something was incorrectly cast to an NCollection_DataMap. It's either that, or there is some mismatch in the tracking of how many things are in the registry, which is why my kludge suppressed the problem.

Do you have any other undefined messages I could take a stab at?
(0087282)
abv   
2019-09-19 05:59   
For the static code analysis of OCCT code we (including some external contributors) have used (from what I know) CAST, cppcheck, PVS Studio, Visual Studio Code Analyser. All of them do their job and a few issues have been identified and solved, however we do not use them in a systematic way (i.e. in an automated build environment), due to a lots of false positives that need to be sorted out and filtered. This task is still pending.

I still have tests with address sanitizer running on my computer, apparently it will take a few days more to complete. Thus further experiments with undefined sanitizer are postponed, and I have no other messages to investigate.

I shall coment separately in 0030563 on the usage of registry of types in OCCT.
(0087333)
galbramc   
2019-09-19 23:35   
Yeah we also use cppcheck, and yes the false positives are a pain. It does not help that things change with versions of the static analyzers as well. We do our best to deal with them though.

Feel free to ask me if anything else pops up with the address or undefined sanitizers.

I will be running our suite of tests when the next version of OCCT comes out and I'll report anything that pops up (hopefully you have already got them all though!).
(0087995)
abv   
2019-10-09 23:16   
Finally I have been able to run the whole OCCT test suite under Address sanitizer, all the issues have been reported as children of this one. The total time of execution was about 8 days (on Ubuntu 16.04 running under Virtual Box VM on Windows 10 computer with i5-3590 CPU, in one thread).
(0087996)
galbramc   
2019-10-09 23:24   
Just out of curiosity, how long does the test suite toke to run without the sanitizer on Ubuntu 16.04 on the virtual box?