Add CMake option to enable address sanitizer checks#394
Conversation
|
After a little more tinkering and testing, I also managed to enable ASAN for our Travis build, but had to disable memory leak detection, because lots of third-party libs are generating leak errors which cause our tests to fail. So to detect leaks, a manual run is needed. For the main program (not the tests), I reduced the memory leak error dump at the end as much as possible by forcing the leak check to run before the program quits. On my Ubuntu 16.10 system here, two errors in libfontconfig.so are still there (probably real memory leaks, not only one where the developer thought that using free() at the end of the program is not necessary), but that's better than 100 or more. For Clang, I needed to reduce the ODR violation check level from 2 to 1, otherwise our Auto-Type lib throws an error. I still need to test compilation on OS X and Windows. I am not sure if LSAN is supported there (I don't think so) and if using -fsanitize=leak will produce a compiler error (it probably will) and if sanitizer/lsan_interface.h exists (most likely not). |
|
I tested it on Windows and OS X now. Windows doesn't support -fsanitize=address at all, OS X only does not support -fsanitize=leak, but -fsanitize=address somehow also doesn't do what I expect it to do and generates a lot of warnings and other weird messages during compilation. I therefore added an error message to CMake when trying to use -DWITH_ASAN=ON on a platform other than Linux. |
README.md
Outdated
| -DWITH_DEV_BUILD=[ON|OFF] Enable/Disable deprecated method warnings. (default: OFF) | ||
| -DWITH_COVERAGE=[ON|OFF] Enable/Disable coverage tests. (GCC ONLY) (default: OFF) | ||
| -DWITH_DEV_BUILD=[ON|OFF] Enable/Disable deprecated method warnings (default: OFF) | ||
| -DWITH_ASAN=[ON|OFF] Enable/Disable address sanitizer checks (default: OFF) |
There was a problem hiding this comment.
Maybe here we can add a (Linux ONLY) like for WITH_COVERAGE GCC ONLY
But It's good ✅
phoerious
force-pushed
the
feature/asan
branch
2 times, most recently
from
b1ca11b to
55c9ef8
Compare
2 participants
Description
This patch adds a CMake option
-DWITH_ASANto enable GCC/Clang address sanitizer checks. It also lists an address sanitizer check as a required merge prerequisite in the PR template checklist.Motivation and Context
Considering that pull requests keep adding third-party API dependencies which are written in C, it becomes more likely that memory management can go wrong and buffer overflows or double frees etc. occur. Running an address sanitizer as a required check before submitting a PR (or merging it) is therefore a good idea.
How Has This Been Tested?
I compiled the code with GCC on Ubuntu 16.10. I haven't yet tested it with Clang on my Arch system at home. Depending on the compiler or platform, it may be necessary to disable ODR checks, which may or may not be sensible to do directly in the code (instead of using the ASAN_OPTIONS env variable).
Types of changes
Checklist: