-
Notifications
You must be signed in to change notification settings - Fork 189
Make the unit tests ASAN-clean and run on Travis #131
base: master
Are you sure you want to change the base?
Conversation
#set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address") | ||
#set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address") | ||
if (CMAKE_CXX_COMPILER_ID MATCHES "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") | ||
# Global options (these apply for other subprojects like JSObjects) and must |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is from ds2, but doesn't apply here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. Let me fix this.
set(COMMON_FLAGS "${COMMON_FLAGS} -g -fno-omit-frame-pointer") | ||
endif () | ||
|
||
if (SANITIZER STREQUAL "asan") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use "asan" here? With -DSANITIZE=address
it could just be -fsanitize=${SANITIZE}
, much less code. Or even just pass it via CMAKE_CXX_FLAGS
directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do it this way because "asan" is the name people know about, and because for other sanitizers (e.g.: ubsan), we can't just take the cmake argument and pass it down.
# Global options (these apply for other subprojects like JSObjects) and must | ||
# be set before we include subdirectories. | ||
if (SANITIZER) | ||
set(COMMON_FLAGS "${COMMON_FLAGS} -g -fno-omit-frame-pointer") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be replaced by -DCMAKE_BUILD_TYPE=Debug
in the Makefile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want the cmake to be usable standalone. Also, ASAN != debug; you could build release (with optimizations) and have asan enabled and generate debug info to print backtraces.
9acdf19
to
5ea53b0
Compare
Weird error:
Also, is there a way that Travis could check all of the sanitizers at once? |
(Google suggests the error means a missing |
This will make it easier to enable these sanitizers with a simple command-line switch, and potentially enable them on Travis.
This should prevent checking in some code with memory errors.
Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired. Before we can review or merge your code, we need you to email [email protected] with your details so we can update your status. |
No description provided.