Add Sanitizers (UBSAN/ASAN) #1085

Flamefire · 2019-08-03T11:52:17Z

Undefined behavior and memory violation can cause hard to find bugs. Those sanitizers make this easier as they can be run with the code and tests.

It adds the potential for adding fuzzers (fuzzy testing) via -fsanitize=fuzzing for Clang to find even more problems.

Solution for now:

Enable with -DRTTR_ENABLE_SANITIZERS=ON
build and run (tests and/or exe)

Other changes:

Removed some obsolete workaround
Added FindBoost of CMake 3.11 if lower version cmake is used. This allows to use targets of newer boost versions avoiding the need to upgrade cmake when upgrading boost which eases development

stefson · 2019-08-11T10:15:04Z

Could you please explain how to use these, as a user? I can make use of the normal testsuite by typing make test, are those sanitizers included into the testsuite and executed by the same command? :-)

edit: I just noticed that you triggered a bug in gcc-8.3.0, is there an open bug for it?

Flamefire · 2019-08-11T11:23:59Z

See above: Enable with -DRTTR_ENABLE_SANITIZERS=ON, then
build and run (tests and/or exe) normally

I recommend a recent clang for that.

I haven't found a bug report although it's easy to reproduce

stefson · 2019-08-11T11:41:47Z

So the way how this works seems to be: make test executes all the binaries found in the test folder, where each of those run different tests on their own. It's also possible to run each test on it's own, for instance only network or IO based, but make test runs them all in a batch mode.

Executing the tests is actually really fast, given the complexity of the project.

Flamefire · 2019-08-11T16:13:42Z

Almost: make test or equivalently ctest inside the build folder runs all defined tests (inside the CMake files). Each cmake test is using Boost.Test to define individual test cases. One could put all tests into a single binary but splitting them allows to focus them and building in parallel which is faster.

The test must be fast as they are meant to be run very often during developing. Read about Test Driven Development (TDD) which would be the ideal. The current tests don't cover the whole project though, see the badges in the readme: We are currently at ~57% of the main project (which excludes all submodules)

stefson · 2019-08-11T17:14:46Z

I intended to run this with user priviliges, but I'm lacking of the correct git vodoo to checkout your sanitizers branch correctly. git checkout sanitizers doesn't pull in the correct submodules, can you please help me out?

this is from the package manager, run by the root user:

>>> Test phase: games-strategy/s25rttr-9999-r4
>>> Working in BUILD_DIR: "/var/tmp/portage/games-strategy/s25rttr-9999-r4/work/s25rttr-9999_build"
ctest -j 12 --output-on-failure --test-load 999
Test project /var/tmp/portage/games-strategy/s25rttr-9999-r4/work/s25rttr-9999_build
      Start  1: Test_common
      Start  2: Test_gameData
      Start  3: Test_samplerate_cpp
      Start  4: Test_sounds
      Start  5: Test_drivers
      Start  6: Test_integration
      Start  7: Test_IO
      Start  8: Test_locale
      Start  9: Test_lua
      Start 10: Test_mapGenerator
      Start 11: Test_network
      Start 12: Test_simple
 1/13 Test  #1: Test_common ......................   Passed    0.04 sec
      Start 13: Test_UI
 2/13 Test  #2: Test_gameData ....................   Passed    0.05 sec
 3/13 Test  #3: Test_samplerate_cpp ..............***Failed    0.05 sec
Running 6 test cases...
unknown location(0): fatal error: in "SimpleCallback": std::runtime_error: SRC_DATA->data_out or SRC_DATA->data_in is NULL.
/var/tmp/portage/games-strategy/s25rttr-9999-r4/work/s25rttr-9999/tests/libsamplerate_cpp/testSamplerate.cpp(30): last checkpoint

*** 1 failure is detected in the test module "RTTR_Samplerate_CPP"

 4/13 Test  #4: Test_sounds ......................   Passed    0.06 sec
 5/13 Test  #7: Test_IO ..........................   Passed    0.06 sec
 6/13 Test  #5: Test_drivers .....................SIGPIPE***Exception:   0.08 sec
ALSA lib /var/tmp/portage/media-libs/alsa-lib-1.1.8/work/alsa-lib-1.1.8/src/confmisc.c:767:(parse_card) cannot find card '0'
ALSA lib /var/tmp/portage/media-libs/alsa-lib-1.1.8/work/alsa-lib-1.1.8/src/conf.c:4568:(_snd_config_evaluate) function snd_func_card_driver returned error: No such file or directory
ALSA lib /var/tmp/portage/media-libs/alsa-lib-1.1.8/work/alsa-lib-1.1.8/src/confmisc.c:392:(snd_func_concat) error evaluating strings
ALSA lib /var/tmp/portage/media-libs/alsa-lib-1.1.8/work/alsa-lib-1.1.8/src/conf.c:4568:(_snd_config_evaluate) function snd_func_concat returned error: No such file or directory
ALSA lib /var/tmp/portage/media-libs/alsa-lib-1.1.8/work/alsa-lib-1.1.8/src/confmisc.c:1246:(snd_func_refer) error evaluating name
ALSA lib /var/tmp/portage/media-libs/alsa-lib-1.1.8/work/alsa-lib-1.1.8/src/conf.c:4568:(_snd_config_evaluate) function snd_func_refer returned error: No such file or directory
ALSA lib /var/tmp/portage/media-libs/alsa-lib-1.1.8/work/alsa-lib-1.1.8/src/conf.c:5047:(snd_config_expand) Evaluate error: No such file or directory
ALSA lib /var/tmp/portage/media-libs/alsa-lib-1.1.8/work/alsa-lib-1.1.8/src/pcm/pcm.c:2565:(snd_pcm_open_noupdate) Unknown PCM default
Running 3 test cases...
No available audio device
/var/tmp/portage/games-strategy/s25rttr-9999-r4/work/s25rttr-9999/tests/s25Main/drivers/testDriverWrapper.cpp(43): fatal error: in "AllAudioDriversAreLoadable": critical check AudioDriverWrapper::inst().LoadDriver(preference) has failed
No protocol specified
No protocol specified
AddressSanitizer:DEADLYSIGNAL
=================================================================
==43==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000030 (pc 0x7fdc417f75a4 bp 0x000000000005 sp 0x7ffc75d93030 T0)
==43==The signal is caused by a READ memory access.
==43==Hint: address points to the zero page.
AddressSanitizer:DEADLYSIGNAL
AddressSanitizer: nested bug in the same thread, aborting.
==43==WARNING: external symbolizer didn't start up correctly!

 7/13 Test  #8: Test_locale ......................   Passed    0.09 sec
 8/13 Test #12: Test_simple ......................   Passed    0.18 sec
 9/13 Test #11: Test_network .....................***Failed    0.22 sec
Running 2 test cases...
Connection to 127.0.0.1:1.337
Successfully connected to localhost:1.337
Connection to 127.0.0.1:1.337
Successfully connected to localhost:1.337
Connection to 127.0.0.1:1.337
Successfully connected to localhost:1.337
/usr/include/boost/endian/buffers.hpp:313:9: runtime error: store to misaligned address 0x7ffe7cfeade2 for type 'int', which requires 4 byte alignment
0x7ffe7cfeade2: note: pointer points here
 00 00  01 40 fe 7c fe 7f 00 00  20 ae fe 7c fe 7f 00 00  10 ae fe 7c fe 7f 00 00  19 e3 5e 00 00 00
              ^ 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/include/boost/endian/buffers.hpp:313:9 in 

10/13 Test  #9: Test_lua .........................   Passed    0.34 sec
11/13 Test #10: Test_mapGenerator ................   Passed    0.71 sec
12/13 Test #13: Test_UI ..........................   Passed    2.57 sec
13/13 Test  #6: Test_integration .................   Passed    5.67 sec

77% tests passed, 3 tests failed out of 13

Total Test time (real) =   5.69 sec

The following tests FAILED:
	  3 - Test_samplerate_cpp (Failed)
	  5 - Test_drivers (SIGPIPE)
	 11 - Test_network (Failed)
Errors while running CTest

Flamefire · 2019-08-11T18:35:25Z

Generic workflow (replace <foo>, rest is verbatim):

git fetch <remote>
git checkout <branch>
git reset --hard <remote>/<branch> # ALL LOCAL CHANGES ARE LOST
git submodule update --init

Edit: On the errors:

Test_samplerate_cpp is 1/12 test fail with -DRTTR_USE_SYSTEM_SAMPLERATE=ON #1090
Test_drivers crash is an issue with GCC ubsan that I'Ve seen on CI before... Maybe it happens when running as root...
network is a bug in boost until 1.67. Will add to suppressions as it is mostly harmless

stefson · 2019-08-11T18:50:22Z

Could you tell me which boost bug that is? If I knew it I could try to patch it locally?

Flamefire · 2019-08-11T18:57:37Z

boostorg/endian#21

stefson · 2019-08-11T19:18:29Z

okay, so via this config: CC=clang CXX=clang++ cmake -DCMAKE_BUILD_TYPE=Release .. -DRTTR_BUILD_UPDATER=OFF -DRTTR_USE_SYSTEM_SAMPLERATE=ON -DRTTR_ENABLE_SANITIZERS=ON

and with running as a user, the Test_drivers crash is gone. You may want to know that not all distros use fakeroot, but stick to real root access, or use a hybrid one: I used a root shell to fire up the process, but the package manager process has an own user called portage, which is somewhere in between root and normal user. Just wanted to say that it might ring some bells at distro's without the intention.

That network error might be the bug that stops me from opening a new game on the rip2, you remember? :-) I backported the fix to v1.65, will be right back with the results.

stefson · 2019-08-11T19:31:43Z

Alright, when executing as a normal user and with the boost fix backported, it's all set, only #1090 remains.

Fix or suppress found issues

Workaround buggy GCC in previous distros: https://stackoverflow.com/q/50024731/1930508

Don't use capture-less lambda with auto* param decaying into a function pointer

g++ doesn't seem to work on CI (segfault on caca_free_display)

On CI initialization errors and SDL_mixer seems to leak a lot then...

Flamefire · 2019-08-14T06:29:16Z

@Flow86 Ready to merge

stefson · 2019-08-14T14:42:52Z

I noticed that you added some commits to this branch wich likely fix #1089 and gave it a go on arm with this config:

>>> Working in BUILD_DIR: "/var/tmp/portage/games-strategy/s25rttr-9999-r4/work/s25rttr-9999_build"
cmake -C /var/tmp/portage/games-strategy/s25rttr-9999-r4/work/s25rttr-9999_build/gentoo_common_config.cmake -G Unix Makefiles -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_SKIP_RPATH=ON -DRTTR_DRIVERDIR=lib/s25rttr -DRTTR_GAMEDIR=share/s25rttr/S2/ -DRTTR_LIBDIR=lib/s25rttr -DRTTR_BUILD_UPDATER=OFF -DRTTR_USE_SYSTEM_SAMPLERATE=ON -DBUILD_TESTING=ON -DRTTR_ENABLE_SANITIZERS=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_TOOLCHAIN_FILE=/var/tmp/portage/games-strategy/s25rttr-9999-r4/work/s25rttr-9999_build/gentoo_toolchain.cmake  /var/tmp/portage/games-strategy/s25rttr-9999-r4/work/s25rttr-9999

but this compile fails during linking:

cat arm-with-sanitizers.log | grep error: -b1
388850-/usr/lib/llvm/8/bin/armv7a-unknown-linux-gnueabihf-clang++  -mcpu=cortex-a7 -pipe -O2 -mfpu=neon-vfpv4 -mfloat-abi=hard  -Wl,-O1 -Wl,--as-needed -fsanitize=address,undefined -fno-sanitize-recover=all -fno-omit-frame-pointer -fsanitize-recover=enum,alignment -Wl,--export-dynamic -rdynamic CMakeFiles/Test_common.dir/commonTestsuite.cpp.o CMakeFiles/Test_common.dir/testContainerUtils.cpp.o CMakeFiles/Test_common.dir/testMakeException.cpp.o CMakeFiles/Test_common.dir/testMathHelpers.cpp.o CMakeFiles/Test_common.dir/testMultiArray.cpp.o CMakeFiles/Test_common.dir/testPoint.cpp.o CMakeFiles/Test_common.dir/testRect.cpp.o CMakeFiles/Test_common.dir/testStrUtils.cpp.o  -o ../../bin/Test_common ../../libs/common/libs25Common.a ../testHelpers/libtestHelpers.a /usr/lib/libboost_unit_test_framework-mt.so ../../libs/rttrConfig/librttrConfig.a ../../libs/common/libs25Common.a ../../external/libutil/src/libs25util.a ../../external/libendian/libendian_static.a /usr/lib/libboost_filesystem-mt.so /usr/lib/libboost_system-mt.so /usr/lib/libboost_filesystem-mt.so /usr/lib/libboost_system-mt.so /usr/lib/libminiupnpc.so /usr/lib/libboost_filesystem-mt.so /usr/lib/libboost_system-mt.so /usr/lib/libboost_unit_test_framework-mt.so 
390077:CMakeFiles/Test_common.dir/testPoint.cpp.o:testPoint.cpp:function auto operator*<long long, unsigned long long>(Point<long long> const&, Point<unsigned long long> const&): error: undefined reference to '__mulodi4'
390291:CMakeFiles/Test_common.dir/testPoint.cpp.o:testPoint.cpp:function auto operator*<long long, unsigned long long>(Point<long long> const&, Point<unsigned long long> const&): error: undefined reference to '__mulodi4'
390505:CMakeFiles/Test_common.dir/testPoint.cpp.o:testPoint.cpp:function auto operator*<long long, long long>(Point<long long> const&, Point<long long> const&): error: undefined reference to '__mulodi4'
390701:CMakeFiles/Test_common.dir/testPoint.cpp.o:testPoint.cpp:function auto operator*<long long, long long>(Point<long long> const&, Point<long long> const&): error: undefined reference to '__mulodi4'
390897:clang-8: error: linker command failed with exit code 1 (use -v to see invocation)

full build log.zip

However, this gets triggered by the sanitizer function -DRTTR_ENABLE_SANITIZERS=ON, compiling without them is working.

Flamefire · 2019-08-14T15:00:03Z

This is a known bug in Clang. See android/ndk#184, https://bugs.llvm.org/buglist.cgi?quicksearch=__mulodi4, https://bugs.llvm.org/show_bug.cgi?id=17693

CI found a visibility issue though. Fix incoming.

stefson · 2019-08-14T19:06:19Z

That commit 376bc85 isn't related to these __mulodi4 errors, right?

Flamefire · 2019-08-15T07:47:29Z

No. As the message says it comes from testPoint. We could probably fix this by removing the tests for Point<int64> which shouldn't be required. Can you continue building and see if this is the only error?

stefson · 2019-08-15T20:05:10Z

I'm sorry, but that doesn't work with make. Once there is an error, the job server halts. But you might stub out the responsible test and attach the patch seperatly in your next posting - however, you must decide how much of additional work that is going to be for you.

Flamefire · 2019-08-15T20:33:22Z

No. You can use -i or -k to continue on error. See make --help

stefson · 2019-08-15T20:34:57Z

hmm, okay, I'll checkout tomorrow how to convince the package manager of that :-)

stefson · 2019-08-16T16:24:21Z

there you go: build-arm-sanitizers.log.zip

I count 50 errors, where 40 of them are undefined reference to '__mulodi4', and 10 are linker command failed with exit code 1

Flamefire · 2019-08-19T12:23:53Z

there you go

Alright then officially: Bug in Clang, go to their sources and try to fix it if you must, otherwise just run the sanitizers on non-special systems where it works or don't run them at all. It's mostly for CI.

stefson · 2019-09-09T14:54:31Z

Not to annoy you, but can we go ahead? :-)

Flamefire · 2019-09-29T11:38:50Z

I can't merge this without an approving review by e.g. @Flow86

Flamefire force-pushed the sanitizers branch 7 times, most recently from 469c0b0 to 8626359 Compare August 9, 2019 16:42

Flamefire requested a review from Flow86 August 10, 2019 10:09

Flamefire changed the title ~~Add Sanitizers (UBSAN/ASAN)~~ WIP: Add Sanitizers (UBSAN/ASAN) Aug 11, 2019

Return-To-The-Roots deleted a comment from stefson Aug 11, 2019

Flamefire force-pushed the sanitizers branch from efaf1a8 to ebe8e45 Compare August 12, 2019 14:43

Flamefire changed the title ~~WIP: Add Sanitizers (UBSAN/ASAN)~~ Add Sanitizers (UBSAN/ASAN) Aug 12, 2019

Flamefire force-pushed the sanitizers branch from ebe8e45 to 62c09b6 Compare August 13, 2019 06:45

Flamefire mentioned this pull request Aug 13, 2019

RTTR_OPTIMIZATION_TUNE doesnt work #1094

Closed

Flamefire force-pushed the sanitizers branch 2 times, most recently from b0480ab to 50bf2e5 Compare August 13, 2019 13:32

Flamefire mentioned this pull request Aug 13, 2019

Fix tune flags #1095

Merged

1 task

Flamefire added 3 commits August 13, 2019 16:19

Remove obsolate (and buggy) clang bug workaround

3a7a85e

Include FindBoost of CMake 3.11

fb8d3e9

Add option to enable sanitizers (ASAN, UBSAN) and fix them

0d1de63

Fix or suppress found issues

Flamefire added 9 commits August 13, 2019 16:23

Report CMake logs on CI error

34e0dd5

Use Ubuntu 18.04 for GCC tests using sanitizers

69dfe3a

Workaround buggy GCC in previous distros: https://stackoverflow.com/q/50024731/1930508

Workaround for GCC 8.3 sanitizer bug

9120739

Don't use capture-less lambda with auto* param decaying into a function pointer

Remove GCC sanitizer test

36a5643

g++ doesn't seem to work on CI (segfault on caca_free_display)

Suppress more AudioSDL leaks

1ac4702

On CI initialization errors and SDL_mixer seems to leak a lot then...

Allow SDL2 2.0.2 (travis version)

ab16c70

Rework ColorARGB to ColorBGRA

053adb1

Update libutil

f15b82c

Don't require boost 1.67 on CI anymore

0bd6a90

Flamefire force-pushed the sanitizers branch from 50bf2e5 to 0bd6a90 Compare August 13, 2019 14:26

Flamefire added this to the 0.9.0 - bugfixes and new compile environment milestone Aug 13, 2019

Enable hidden visibility for drivers

376bc85

Flamefire force-pushed the sanitizers branch from c989cd2 to 376bc85 Compare August 14, 2019 15:04

Merge branch 'master' into sanitizers

da8cd29

Flow86 approved these changes Oct 4, 2019

View reviewed changes

Flow86 merged commit a039ffa into Return-To-The-Roots:master Oct 4, 2019

Flamefire deleted the sanitizers branch October 4, 2019 08:54

Flamefire mentioned this pull request Oct 5, 2019

s25client fails to compile on arm with: "RTTR_USE_UNALIGNED_ACCESS" is not defined, evaluates to 0 [-Werror=undef] #if RTTR_USE_UNALIGNED_ACCESS" #1089

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Sanitizers (UBSAN/ASAN) #1085

Add Sanitizers (UBSAN/ASAN) #1085

Flamefire commented Aug 3, 2019

stefson commented Aug 11, 2019 •

edited

Loading

Flamefire commented Aug 11, 2019

stefson commented Aug 11, 2019

Flamefire commented Aug 11, 2019

stefson commented Aug 11, 2019

Flamefire commented Aug 11, 2019 •

edited

Loading

stefson commented Aug 11, 2019

Flamefire commented Aug 11, 2019

stefson commented Aug 11, 2019 •

edited

Loading

stefson commented Aug 11, 2019

Flamefire commented Aug 14, 2019

stefson commented Aug 14, 2019

Flamefire commented Aug 14, 2019

stefson commented Aug 14, 2019

Flamefire commented Aug 15, 2019

stefson commented Aug 15, 2019

Flamefire commented Aug 15, 2019

stefson commented Aug 15, 2019

stefson commented Aug 16, 2019

Flamefire commented Aug 19, 2019

stefson commented Sep 9, 2019

Flamefire commented Sep 29, 2019

Add Sanitizers (UBSAN/ASAN) #1085

Add Sanitizers (UBSAN/ASAN) #1085

Conversation

Flamefire commented Aug 3, 2019

stefson commented Aug 11, 2019 • edited Loading

Flamefire commented Aug 11, 2019

stefson commented Aug 11, 2019

Flamefire commented Aug 11, 2019

stefson commented Aug 11, 2019

Flamefire commented Aug 11, 2019 • edited Loading

stefson commented Aug 11, 2019

Flamefire commented Aug 11, 2019

stefson commented Aug 11, 2019 • edited Loading

stefson commented Aug 11, 2019

Flamefire commented Aug 14, 2019

stefson commented Aug 14, 2019

Flamefire commented Aug 14, 2019

stefson commented Aug 14, 2019

Flamefire commented Aug 15, 2019

stefson commented Aug 15, 2019

Flamefire commented Aug 15, 2019

stefson commented Aug 15, 2019

stefson commented Aug 16, 2019

Flamefire commented Aug 19, 2019

stefson commented Sep 9, 2019

Flamefire commented Sep 29, 2019

stefson commented Aug 11, 2019 •

edited

Loading

Flamefire commented Aug 11, 2019 •

edited

Loading

stefson commented Aug 11, 2019 •

edited

Loading