Add Sanitizers (UBSAN/ASAN)

.travis.yml

@@ -96,7 +96,9 @@ jobs:
     # Latest version GCC with static analysis
     - os: linux
       compiler: gcc
-      env: MATRIX_EVAL="CXX=g++-8 && CC=gcc-8" BUILD_TYPE=Debug
+      env:
+        - MATRIX_EVAL="CXX=g++-8 && CC=gcc-8"
+        - BUILD_TYPE=Debug
       addons:
         apt:
           <<: *defApt
@@ -115,10 +117,10 @@ jobs:
       env:
         - MATRIX_EVAL="CXX=clang++-8 && CC=clang-8"
         - BUILD_TYPE=Debug
-        - ADDITIONAL_CMAKE_FLAGS="-DRTTR_EXTERNAL_BUILD_TESTING=ON"
+        - ADDITIONAL_CMAKE_FLAGS="-DRTTR_EXTERNAL_BUILD_TESTING=ON -DRTTR_ENABLE_SANITIZERS=ON"
     - <<: *latest-clang-externals
       env:
         - MATRIX_EVAL="CXX=clang++-8 && CC=clang-8"
         - BUILD_TYPE=Debug
-        - ADDITIONAL_CMAKE_FLAGS="-DRTTR_EXTERNAL_BUILD_TESTING=ON"
+        - ADDITIONAL_CMAKE_FLAGS="-DRTTR_EXTERNAL_BUILD_TESTING=ON -DRTTR_ENABLE_SANITIZERS=ON"
         - BOOST_VERSION=1.70.0

CMakeLists.txt

@@ -29,6 +29,9 @@ if(DEFINED CMAKE_TOOLCHAIN_FILE)
 endif()
 
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake/Modules" "${CMAKE_SOURCE_DIR}/external/libutil/cmake")
+if(CMAKE_VERSION VERSION_LESS 3.11)
+    list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake/Modules/cmake_3.11.0")
+endif()
 
 include(CheckGitSubmodules)
 check_git_submodules()
@@ -210,16 +213,9 @@ if(HAVE_MEMCHECK_H)
     add_definitions(-DHAVE_MEMCHECK_H)
 endif()
 
-# Clang bug workaround
-if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
-    try_compile(CHECK_CLANG_INLINE "${CMAKE_CURRENT_BINARY_DIR}/cxx" "${CMAKE_SOURCE_DIR}/cmake/checkclang.cpp")
-    if(NOT ${CHECK_CLANG_INLINE})
-        add_definitions("-D__extern_always_inline=extern __always_inline __attribute__ ((__gnu_inline__))")
-    endif()
-endif()
-
 # Code coverage
 include(RttrCoverageCfg)
+include(EnableSanitizers)
 
 ################################################################################
 # Configure files

README.md

@@ -89,6 +89,14 @@ See the description in CMake-GUI/ccmake for details.
 Note that due to the use of submodules you always need to `git pull && git submodule update --init --recursive` to get the latest version.
 (The `--init` and `--recursive` arguments are only required should we add *new* submodules to the existing set.)
 
+### Tests
+Especially for developing you should build in Debug mode (`-DCMAKE_BUILD_TYPE=Debug`) and run the tests after executing `make` via `make test` or `ctest --output-on-failure`.
+There is also an option to enable checks for undefined behavior (UBSAN) and memory errors (ASAN) like use-after-free or leaks.
+Just pass `-DRTTR_ENABLE_SANITIZERS=ON` to CMake and use a recent GCC or Clang compiler to build.
+Then just run (tests or application) as usual.
+
+**Note**: Boost.Endian < 1.67 is known to have UB so use at least 1.67 when running the sanitizers.
+
 ## On Windows
 
 ### Prerequisites:

extras/CMakeLists.txt

@@ -1,3 +1,10 @@
+# Meta target that depends on all drivers
+add_custom_target(drivers)
+
+# Minimal visibility to avoid name clashes among plugins
+set(CMAKE_CXX_VISIBILITY_PRESET hidden)
+set(CMAKE_VISIBILITY_INLINES_HIDDEN ON)
+
 add_subdirectory(audioDrivers)
 add_subdirectory(videoDrivers)
 if(RTTR_BUNDLE AND APPLE)

extras/audioDrivers/SDL/AudioSDL.cpp

@@ -15,13 +15,15 @@
 // You should have received a copy of the GNU General Public License
 // along with Return To The Roots. If not, see <http://www.gnu.org/licenses/>.
 
-#include "commonDefines.h" // IWYU pragma: keep
 #include "AudioSDL.h"
+#include "RTTR_Assert.h"
 #include "SoundSDL_Effect.h"
 #include "SoundSDL_Music.h"
 #include "driver/AudioInterface.h"
 #include "driver/IAudioDriverCallback.h"
 #include "driver/Interface.h"
+#include "helpers/CIUtils.h"
+#include "helpers/LSANUtils.h"
 #include <SDL.h>
 #include <SDL_mixer.h>
 #include <iostream>
@@ -78,22 +80,27 @@ const char* AudioSDL::GetName() const
  */
 bool AudioSDL::Initialize()
 {
+    initialized = false;
+    // SDL_thread leaks a mutex on regular runs and SDL_mixer a lot on initialization error
+    rttr::ScopedLeakDisabler _;
     if(SDL_InitSubSystem(SDL_INIT_AUDIO) < 0)
     {
         std::cerr << SDL_GetError() << std::endl;
-        initialized = false;
         return false;
     }
 
     // open 44.1KHz, signed 16bit, system byte order,
     // stereo audio, using 1024 byte chunks
     if(Mix_OpenAudio(44100, AUDIO_S16LSB, 2, 4096) < 0)
     {
+        if(rttr::isRunningOnCI())
+        {
+            initialized = true;
+            return true;
+        }
         std::cerr << Mix_GetError() << std::endl;
-        initialized = false;
         return false;
     }
-
     SetNumChannels(Mix_AllocateChannels(MAX_NUM_CHANNELS));
     Mix_SetMusicCMD(nullptr);
     Mix_HookMusicFinished(AudioSDL::MusicFinished);
@@ -113,6 +120,7 @@ void AudioSDL::CleanUp()
 
     Mix_CloseAudio();
     Mix_HookMusicFinished(nullptr);
+    Mix_Quit();
     SDL_QuitSubSystem(SDL_INIT_AUDIO);
 }
 

extras/audioDrivers/SDL/CMakeLists.txt

@@ -19,4 +19,5 @@ ELSE()
 		RUNTIME DESTINATION ${RTTR_DRIVERDIR}/audio
 		LIBRARY DESTINATION ${RTTR_DRIVERDIR}/audio
 	)
+	add_dependencies(drivers audioSDL)
 ENDIF ()

extras/videoDrivers/SDL/CMakeLists.txt

@@ -7,7 +7,7 @@ IF (SDL_FOUND)
 	CORRECT_LIB(SDL_LIBRARY SDL)
 
 	ADD_LIBRARY(videoSDL SHARED ${RTTR_DRIVER_INTERFACE} VideoSDL.cpp VideoSDL.h)
-	target_link_libraries(videoSDL PRIVATE videodrv s25util ${SDL_LIBRARY} Boost::boost nowide::static)
+	target_link_libraries(videoSDL PRIVATE videodrv ${SDL_LIBRARY} nowide::static)
 	target_include_directories(videoSDL PRIVATE ${SDL_INCLUDE_DIR} ${PROJECT_SOURCE_DIR}/data/win32)
 
 	IF (WIN32)
@@ -22,6 +22,7 @@ IF (SDL_FOUND)
 		RUNTIME DESTINATION ${RTTR_DRIVERDIR}/video
 		LIBRARY DESTINATION ${RTTR_DRIVERDIR}/video
 	)
+	add_dependencies(drivers videoSDL)
 ELSE ()
 	MESSAGE(WARNING ": SDL library not found: Not building SDL videodriver")
 ENDIF ()

extras/videoDrivers/SDL/VideoSDL.cpp

@@ -15,7 +15,6 @@
 // You should have received a copy of the GNU General Public License
 // along with Return To The Roots. If not, see <http://www.gnu.org/licenses/>.
 
-#include "commonDefines.h" // IWYU pragma: keep
 #include "VideoSDL.h"
 #include "driver/Interface.h"
 #include "driver/VideoDriverLoaderInterface.h"
@@ -24,6 +23,7 @@
 #include <boost/nowide/iostream.hpp>
 #include <SDL.h>
 #include <algorithm>
+#include <helpers/LSANUtils.h>
 
 #ifdef _WIN32
 #include "makeException.h"
@@ -118,7 +118,10 @@ VideoSDL::VideoSDL(VideoDriverLoaderInterface* CallBack) : VideoDriver(CallBack)
 VideoSDL::~VideoSDL()
 {
     if(initialized)
+    {
         SDL_QuitSubSystem(SDL_INIT_VIDEO);
+        SDL_Quit();
+    }
 }
 
 /**
@@ -138,6 +141,7 @@ const char* VideoSDL::GetName() const
  */
 bool VideoSDL::Initialize()
 {
+    rttr::ScopedLeakDisabler _;
     if(SDL_InitSubSystem(SDL_INIT_VIDEO) < 0)
     {
         fprintf(stderr, "%s\n", SDL_GetError());
@@ -152,7 +156,6 @@ bool VideoSDL::Initialize()
 
     // Key-Repeat einschalten
     SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, SDL_DEFAULT_REPEAT_INTERVAL);
-
     return initialized;
 }
 
@@ -309,7 +312,7 @@ bool VideoSDL::SetVideoMode(const VideoMode& newSize, bool fullscreen)
 
 void VideoSDL::PrintError(const std::string& msg)
 {
-    bnw::cerr << msg << std::endl;
+    boost::nowide::cerr << msg << std::endl;
 }
 
 void VideoSDL::HandlePaste()

extras/videoDrivers/SDL2/CMakeLists.txt

@@ -1,5 +1,5 @@
 set(SDL2_BUILDING_LIBRARY ON)
-find_package(SDL2 2.0.4)
+find_package(SDL2 2.0.2)
 
 if(SDL2_FOUND)
   add_library(videoSDL2 SHARED ${RTTR_DRIVER_INTERFACE} VideoSDL2.cpp VideoSDL2.h icon.h icon.cpp)
@@ -14,4 +14,5 @@ if(SDL2_FOUND)
     RUNTIME DESTINATION ${RTTR_DRIVERDIR}/video
     LIBRARY DESTINATION ${RTTR_DRIVERDIR}/video
   )
+  add_dependencies(drivers videoSDL2)
 endif()

extras/videoDrivers/SDL2/VideoSDL2.cpp

@@ -20,6 +20,7 @@
 #include "driver/Interface.h"
 #include "driver/VideoDriverLoaderInterface.h"
 #include "driver/VideoInterface.h"
+#include "helpers/LSANUtils.h"
 #include "helpers/containerUtils.h"
 #include "icon.h"
 #include "openglCfg.hpp"
@@ -70,6 +71,7 @@ const char* VideoSDL2::GetName() const
 bool VideoSDL2::Initialize()
 {
     initialized = false;
+    rttr::ScopedLeakDisabler _;
     if(SDL_InitSubSystem(SDL_INIT_VIDEO) < 0)
     {
         PrintError(SDL_GetError());
@@ -90,6 +92,7 @@ void VideoSDL2::CleanUp()
     if(window)
         SDL_DestroyWindow(window);
     SDL_QuitSubSystem(SDL_INIT_VIDEO);
+    SDL_Quit();
     initialized = false;
 }
 
@@ -181,7 +184,9 @@ bool VideoSDL2::ResizeScreen(const VideoMode& newSize, bool fullscreen)
         isFullscreen_ = (SDL_GetWindowFlags(window) & SDL_WINDOW_FULLSCREEN) != 0;
         if(!isFullscreen_)
         {
+#if SDL_VERSION_ATLEAST(2, 0, 5)
             SDL_SetWindowResizable(window, SDL_TRUE);
+#endif
             MoveWindowToCenter();
         }
     }
@@ -383,11 +388,11 @@ bool VideoSDL2::MessageLoop()
                     CallBack->Msg_RightUp(mouse_xy);
                 }
                 break;
-            case SDL_MOUSEWHEEL:
-            {
-                int y = ev.wheel.y;
+            case SDL_MOUSEWHEEL: { int y = ev.wheel.y;
+#if SDL_VERSION_ATLEAST(2, 0, 4)
                 if(ev.wheel.direction == SDL_MOUSEWHEEL_FLIPPED)
                     y = -y;
+#endif
                 if(y > 0)
                     CallBack->Msg_WheelUp(mouse_xy);
                 else if(y < 0)
@@ -469,11 +474,18 @@ void* VideoSDL2::GetMapPointer() const
 void VideoSDL2::MoveWindowToCenter()
 {
     SDL_Rect usableBounds;
+#if SDL_VERSION_ATLEAST(2, 0, 5)
     CHECK_SDL(SDL_GetDisplayUsableBounds(SDL_GetWindowDisplayIndex(window), &usableBounds));
     int top, left, bottom, right;
     CHECK_SDL(SDL_GetWindowBordersSize(window, &top, &left, &bottom, &right));
     usableBounds.w -= left + right;
     usableBounds.h -= top + bottom;
+#else
+    CHECK_SDL(SDL_GetDisplayBounds(SDL_GetWindowDisplayIndex(window), &usableBounds));
+    // rough estimates
+    usableBounds.w -= 10;
+    usableBounds.h -= 30;
+#endif
     if(usableBounds.w < GetWindowSize().width || usableBounds.h < GetWindowSize().height)
     {
         SDL_SetWindowSize(window, usableBounds.w, usableBounds.h);

extras/videoDrivers/WinAPI/CMakeLists.txt

@@ -7,4 +7,5 @@ IF (WIN32)
     target_include_directories(videoWinAPI SYSTEM PRIVATE ${OPENGL_INCLUDE_DIR} ${PROJECT_SOURCE_DIR}/data/win32)
 
 	INSTALL(TARGETS videoWinAPI RUNTIME DESTINATION ${RTTR_DRIVERDIR}/video)
+	add_dependencies(drivers videoWinAPI)
 ENDIF ()

libs/common/include/RTTR_Assert.h

@@ -42,11 +42,11 @@ extern void __cdecl __debugbreak();
 #endif
 
 [[noreturn]] void RTTR_AssertFailure(const char* condition, const char* file, int line, const char* function);
-bool RTTR_IsBreakOnAssertFailureEnabled();
 /// If true(default), a breakpoint is triggered on assert (if available)
 /// Note: This breakpoint can be globally disabled by setting the environment variable
 ///       RTTR_DISABLE_ASSERT_BREAKPOINT to "1" or "yes" which overrides this setting
-extern bool RTTR_AssertEnableBreak;
+bool RTTR_IsBreakOnAssertFailureEnabled();
+bool RTTR_SetBreakOnAssertFailure(bool enabled);
 
 /* Some aspects about RTTR_Assert:
     - do-while(false) so it can be used in conditions: if(foo) RTTR_Assert(bar);

libs/common/include/helpers/CIUtils.h

@@ -0,0 +1,35 @@
+// Copyright (c) 2018 - 2019 Settlers Freaks (sf-team at siedler25.org)
+//
+// This file is part of Return To The Roots.
+//
+// Return To The Roots is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 2 of the License, or
+// (at your option) any later version.
+//
+// Return To The Roots is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Return To The Roots. If not, see <http://www.gnu.org/licenses/>.
+
+#ifndef CI_Utils_h__
+#define CI_Utils_h__
+
+#include <cstdlib>
+#include <string>
+
+namespace rttr {
+inline bool isRunningOnCI()
+{
+    const auto* ciPtr = std::getenv("CI");
+    if(!ciPtr)
+        return false;
+    const std::string ci = ciPtr;
+    return ci == "true" || ci == "True";
+}
+} // namespace rttr
+
+#endif // CI_Utils_h__