[KevinWMatthews]

This patch creates two options for Clang and gcc builds only, closing #275 :

• ENABLE_ASAN: enables the address sanitizer
• ENABLE_UBSAN: enables the undefined behavior sanitizer

These can be configured from the command line using:

• -DENABLE_ASAN=ON
• -DENABLE_UBSAN=ON

or by using ccmake or cmake-gui.

Unlike the thread and address sanitizers, asan and ubsan can be enabled simultaneously with no ill effect.

I've use options Google's asan docs and clang's asan docs:

• -O1 performance (don't know if it matters for our use case)
• no-omit-frame-pointer for pretty stack traces (I'm not fully up to speed on this)
• debug flag for pretty stack traces
  • Without debug symbols, stack traces point to addresses in memory
  • With debug symbols, stack traces point to specific lines of code

ubsan is also worth experimenting with. It isn't as verbose as asan but catches a different set of errors and doesn't seem to come with the same performance penalty.

• doesn't required optimization?
• doesn't require the debug flag?

asan's output changes drastically enough that I'm forcing the debug flag when this sanitizer is enabled. The most portable way to set the debug flag is to use CMake rather than directly sending -g to the compiler flags:

set_property(CACHE CMAKE_BUILD_TYPE PROPERTY VALUE "Debug")

This:

• sets a property on the CACHE variable (not, say, a target or directory) named CMAKE_BUILD_TYPE
• the PROPERTY is named VALUE (not, say, TYPE or HELPSTRING) and is set to "Debug"

There's documentation and some rather esoteric history behind this syntax that I tracked down in case you're interested ;)

If a student wants to return to a release build they'll need to:

• disable asan
• manually set CMAKE_BUILD_TYPE to "Release"

or

• clear their CMake cache

I think that's ok for our use case?

@patricksjackson Does Microsoft's C++ compiler implement the sanitizers? As far as I've read it doesn't, so I've put the CMake options inside an if(${MSVC}) conditional.

[arcuru]

A long post deserves a (somewhat) long response :) I have no issues with all the changes you've made so far.

Have you shown a failure case for both of these to sanity check they work correctly?

1. I think this also needs to be added to the compile flags for the common Catch lib?
2. We should lightly document this as "Extra Credit". No need to add a big explanation if it's too much effort, just say they catch memory errors and undefined behavior and add links.
3. We also should probably add this to CI. I think it's a two-liner change in bin/check-exercises.sh.
4. Yes, the flags you're setting are correct. We probably don't really need -O1, but we need the others for debugging. -fno-omit-frame-pointer makes sure that the compiler doesn't optimize certain functions in a way that prevents a stack trace from being accurate.
5. Visual Studio added ASan, but you are correct in ignoring it here.

There's documentation and some rather esoteric history behind this syntax that I tracked down in case you're interested ;)

I am interested, but not enough to ask you to spend time on explaining. If you had a link I'd probably read it :)

[KevinWMatthews]

You're right - I do need to show the actual use case!

I'm having second thoughts about including any optimization. I've seen a few cases where unused code that leaks memory is optimized away. Seems like I've encountered this while trying to force an error from asan. Perhaps it's better to leave the optimization unspecified?

Documentation and CI will follow.

I've run some examples locally. They're quite lengthy, so I hesitate to post them but will anyway. I'd also like to add and revert some bad code to show how Travis handles a failure.

In short, ubsan is fine because it doesn't do stack traces. asan traces look good but I'm not entirely sure why.....

[KevinWMatthews]

I've added some code that segfaults to the hello-world solution:

string hello()
{
    char *blow_up = NULL;
    cout << *blow_up;
    return "Hello, World!";
}

From a student's perspective, catch is compiled with sanitizer flags:

[ 25%] Building CXX object CMakeFiles/hello-world.dir/hello_world_test.cpp.o
/usr/bin/c++     -g   -Wall -Wextra -Wpedantic -Werror -fsanitize=address -fno-omit-frame-pointer -O1 -std=c++11 -o CMakeFiles/hello-world.dir/hello_world_test.cpp.o -c hello-world/hello_world_test.cpp
[ 50%] Building CXX object CMakeFiles/hello-world.dir/hello_world.cpp.o
/usr/bin/c++     -g   -Wall -Wextra -Wpedantic -Werror -fsanitize=address -fno-omit-frame-pointer -O1 -std=c++11 -o CMakeFiles/hello-world.dir/hello_world.cpp.o -c hello-world/hello_world.cpp
[ 75%] Building CXX object CMakeFiles/hello-world.dir/test/tests-main.cpp.o
/usr/bin/c++     -g   -Wall -Wextra -Wpedantic -Werror -fsanitize=address -fno-omit-frame-pointer -O1 -std=c++11 -o CMakeFiles/hello-world.dir/test/tests-main.cpp.o -c hello-world/test/tests-main.cpp
[100%] Linking CXX executable hello-world
/usr/bin/cmake -E cmake_link_script CMakeFiles/hello-world.dir/link.txt --verbose=1
/usr/bin/c++   -g   CMakeFiles/hello-world.dir/hello_world_test.cpp.o CMakeFiles/hello-world.dir/hello_world.cpp.o CMakeFiles/hello-world.dir/test/tests-main.cpp.o  -o hello-world -rdynamic -fsanitize=address 

We compile the solution, unit test, and tests-main code with the address sanitizer compile flags, then link with -fsanitize=address only.

Running this gives a hefty trace:

./hello-world
ASAN:SIGSEGV
=================================================================
==31112==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000043ca07 bp 0x7fffce73ebc0 sp 0x7fffce73eb40 T0)
    #0 0x43ca06 in hello_world::hello[abi:cxx11]() /home/kevin/coding/exercism/source/hello-world/hello_world.cpp:12
    #1 0x43bf93 in ____C_A_T_C_H____T_E_S_T____0 /home/kevin/coding/exercism/source/hello-world/hello_world_test.cpp:6
    #2 0x43d248 in Catch::TestInvokerAsFunction::invoke() const /home/kevin/coding/exercism/source/hello-world/test/catch.hpp:11841
    #3 0x4461b7 in Catch::TestCase::invoke() const /home/kevin/coding/exercism/source/hello-world/test/catch.hpp:11742
    #4 0x446267 in Catch::RunContext::invokeActiveTestCase() /home/kevin/coding/exercism/source/hello-world/test/catch.hpp:10601
    #5 0x45efdb in Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) /home/kevin/coding/exercism/source/hello-world/test/catch.hpp:10574
    #6 0x48cf37 in Catch::RunContext::runTest(Catch::TestCase const&) /home/kevin/coding/exercism/source/hello-world/test/catch.hpp:10344
    #7 0x4902cd in runTests /home/kevin/coding/exercism/source/hello-world/test/catch.hpp:10903
    #8 0x4902cd in Catch::Session::runInternal() /home/kevin/coding/exercism/source/hello-world/test/catch.hpp:11098
    #9 0x4909fc in Catch::Session::run() /home/kevin/coding/exercism/source/hello-world/test/catch.hpp:11055
    #10 0x490b6e in int Catch::Session::run<char>(int, char const* const*) /home/kevin/coding/exercism/source/hello-world/test/catch.hpp:10801
    #11 0x490b6e in main /home/kevin/coding/exercism/source/hello-world/test/catch.hpp:14554
    #12 0x7fecafd8682f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #13 0x43bd88 in _start (/home/kevin/coding/exercism/source/hello-world/hello-world+0x43bd88)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/kevin/coding/exercism/source/hello-world/hello_world.cpp:12 hello_world::hello[abi:cxx11]()
==31112==ABORTING

but the important info is right at the beginning:

hello_world.cpp:12
hello_world_test.cpp:6

which points to:

    cout << *blow_up;       // hello_world.cpp:12

and

    REQUIRE("Hello, World!" == hello_world::hello());   // hello_world_test.cpp:6

respectively.

asan actually traces it back to catch's main():

#11 0x490b6e in main /home/kevin/coding/exercism/source/hello-world/test/catch.hpp:14554

[KevinWMatthews]

From a maintainer's perspective, tests-main.cpp is not compiled with sanitizer flags:

[ 33%] Building CXX object CMakeFiles/catchlib.dir/test/tests-main.cpp.o
/usr/local/bin/c++     -g   -std=c++11 -o CMakeFiles/catchlib.dir/test/tests-main.cpp.o -c test/tests-main.cpp

[ 33%] Building CXX object build_exercises/hello-world/CMakeFiles/hello-world.dir/hello_world_test.cpp.o
cd hello-world && /usr/local/bin/c++   -DEXERCISM_RUN_ALL_TESTS  -g   -Wall -Wextra -Wpedantic -Werror -fsanitize=address -fno-omit-frame-pointer -O1 -std=c++11 -o CMakeFiles/hello-world.dir/hello_world_test.cpp.o -c build_exercises/hello-world/hello_world_test.cpp
[ 66%] Building CXX object build_exercises/hello-world/CMakeFiles/hello-world.dir/hello_world.cpp.o
cd hello-world && /usr/local/bin/c++   -DEXERCISM_RUN_ALL_TESTS  -g   -Wall -Wextra -Wpedantic -Werror -fsanitize=address -fno-omit-frame-pointer -O1 -std=c++11 -o CMakeFiles/hello-world.dir/hello_world.cpp.o -c build_exercises/hello-world/hello_world.cpp
[100%] Linking CXX executable hello-world
cd hello-world && /usr/bin/cmake -E cmake_link_script CMakeFiles/hello-world.dir/link.txt --verbose=1
/usr/local/bin/c++   -g   CMakeFiles/hello-world.dir/hello_world_test.cpp.o CMakeFiles/hello-world.dir/hello_world.cpp.o ../../CMakeFiles/catchlib.dir/test/tests-main.cpp.o  -o hello-world -rdynamic -fsanitize=address 

(I've trimmed the paths and am using an out-of-tree build, FYI).

The solution and unit test code are compiled with sanitizer flags. Everything is linked with sanitizer flags.

Running gives:

./hello-world
ASAN:DEADLYSIGNAL
=================================================================
==1782==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000547e09 bp 0x7ffe7492a100 sp 0x7ffe7492a030 T0)
==1782==The signal is caused by a READ memory access.
==1782==Hint: address points to the zero page.
    #0 0x547e08 in hello_world::hello[abi:cxx11]() /home/kevin/coding/exercism/source/cpp/build_exercises/hello-world/hello_world.cpp:12
    #1 0x546667 in ____C_A_T_C_H____T_E_S_T____0 /home/kevin/coding/exercism/source/cpp/build_exercises/hello-world/hello_world_test.cpp:6
    #2 0x55b669 in Catch::TestInvokerAsFunction::invoke() const /home/kevin/coding/exercism/source/cpp/test/catch.hpp:11841
    #3 0x55ae04 in Catch::TestCase::invoke() const /home/kevin/coding/exercism/source/cpp/test/catch.hpp:11742
    #4 0x555fd8 in Catch::RunContext::invokeActiveTestCase() /home/kevin/coding/exercism/source/cpp/test/catch.hpp:10601
    #5 0x555d60 in Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) /home/kevin/coding/exercism/source/cpp/test/catch.hpp:10574
    #6 0x554910 in Catch::RunContext::runTest(Catch::TestCase const&) /home/kevin/coding/exercism/source/cpp/test/catch.hpp:10344
    #7 0x55729a in runTests /home/kevin/coding/exercism/source/cpp/test/catch.hpp:10903
    #8 0x55844b in Catch::Session::runInternal() /home/kevin/coding/exercism/source/cpp/test/catch.hpp:11098
    #9 0x55822e in Catch::Session::run() /home/kevin/coding/exercism/source/cpp/test/catch.hpp:11055
    #10 0x58bc42 in int Catch::Session::run<char>(int, char const* const*) /home/kevin/coding/exercism/source/cpp/test/catch.hpp:10801
    #11 0x56b38d in main /home/kevin/coding/exercism/source/cpp/test/catch.hpp:14554
    #12 0x7f8fc99e22e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #13 0x546249 in _start (/home/kevin/coding/exercism/source/build_cpp/build_exercises/hello-world/hello-world+0x546249)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/kevin/coding/exercism/source/cpp/build_exercises/hello-world/hello_world.cpp:12 in hello_world::hello[abi:cxx11]()
==1782==ABORTING

This still points to the correct lines in the solution and test code:

hello_world.cpp:12
hello_world_test.cpp:6

asan still somehow finds catch's main():

#11 0x56b38d in main /home/kevin/coding/exercism/source/cpp/test/catch.hpp:14554

Perhaps because it's implemented in a header file?

[KevinWMatthews]

ubsan is very succinct:

hello_world.cpp:12:14: runtime error: load of null pointer of type 'char'

[arcuru]

I don't think there's a need to try to get it to not see the Catch main function (if that's what you were trying?). The traces look fine, I'm guessing it works because you are still at least compiling tests-main with -g. It still reports (even at the bottom of the output) that the issue is on hello_world.cpp:12

Adding failures to Travis and then reverting sounds good and is what I do as well. Removing optimization is also fine, I don't think we need it.

[arcuru]

Closing this. Feel free to open a new PR if anyone wants to pick this up.