Configure Undefined Behaviour Sanitizer (#6290)

.github/workflows/osrm-backend.yml

@@ -64,7 +64,7 @@ jobs:
             CXXCOMPILER: g++-9
             ENABLE_COVERAGE: ON
 
-          - name: gcc-9-debug-asan
+          - name: gcc-9-debug-asan-ubsan
             continue-on-error: false
             node: 12
             runs-on: ubuntu-20.04
@@ -74,7 +74,9 @@ jobs:
             CUCUMBER_TIMEOUT: 20000
             CXXCOMPILER: g++-9
             ENABLE_SANITIZER: ON
-            TARGET_ARCH: x86_64-asan
+            TARGET_ARCH: x86_64-asan-ubsan
+            OSRM_CONNECTION_RETRIES: 10
+            OSRM_CONNECTION_EXP_BACKOFF_COEF: 1.5
 
           - name: clang-5.0-debug
             continue-on-error: false
@@ -95,13 +97,13 @@ jobs:
             CUCUMBER_TIMEOUT: 60000
             ENABLE_CLANG_TIDY: ON
 
-          - name: conan-linux-debug-asan
+          - name: conan-linux-debug-asan-ubsan
             continue-on-error: false
             node: 12
             runs-on: ubuntu-20.04
             BUILD_TOOLS: ON
             BUILD_TYPE: Release
-            CLANG_VERSION: 5.0.0
+            CLANG_VERSION: 11.0.0
             ENABLE_CONAN: ON
             ENABLE_SANITIZER: ON
 
@@ -382,6 +384,8 @@ jobs:
       ENABLE_SANITIZER: ${{ matrix.ENABLE_SANITIZER }}
       NODE_PACKAGE_TESTS_ONLY: ${{ matrix.NODE_PACKAGE_TESTS_ONLY }}
       TARGET_ARCH: ${{ matrix.TARGET_ARCH }}
+      OSRM_CONNECTION_RETRIES: ${{ matrix.OSRM_CONNECTION_RETRIES }}
+      OSRM_CONNECTION_EXP_BACKOFF_COEF: ${{ matrix.OSRM_CONNECTION_EXP_BACKOFF_COEF }}
     steps:
     - uses: actions/checkout@v2
 
@@ -429,6 +433,7 @@ jobs:
         if [[ "$ENABLE_SANITIZER" == 'ON' ]]; then
           # We can only set this after checkout once we know the workspace directory
           echo "LSAN_OPTIONS=print_suppressions=0:suppressions=${GITHUB_WORKSPACE}/scripts/ci/leaksanitizer.conf" >> $GITHUB_ENV
+          echo "UBSAN_OPTIONS=symbolize=1:halt_on_error=1:print_stacktrace=1:suppressions=${GITHUB_WORKSPACE}/scripts/ci/undefinedsanitizer.conf" >> $GITHUB_ENV
         fi
 
         if [[ "${RUNNER_OS}" == "Linux" ]]; then
@@ -562,6 +567,12 @@ jobs:
       if: ${{ matrix.NODE_PACKAGE_TESTS_ONLY == 'ON' }}
       run: |
         npm run nodejs-tests
+    - name: Upload test logs
+      uses: actions/upload-artifact@v3
+      if: failure()
+      with:
+        name: logs
+        path: test/logs/
 
     - name: Generate code coverage
       if: ${{ matrix.ENABLE_COVERAGE == 'ON' }}

CHANGELOG.md

@@ -5,6 +5,7 @@
     - Misc:
       - FIXED: Fix bug with reading Set values from Lua scripts. [#6285](https://github.com/Project-OSRM/osrm-backend/pull/6285)
     - Build:
+      - CHANGED: Configure Undefined Behaviour Sanitizer. [#6290](https://github.com/Project-OSRM/osrm-backend/pull/6290)
       - CHANGED: Use Conan instead of Mason to install code dependencies. [#6284](https://github.com/Project-OSRM/osrm-backend/pull/6284)
       - CHANGED: Migrate to C++17. Update sol2 to 3.3.0. [#6279](https://github.com/Project-OSRM/osrm-backend/pull/6279)
       - CHANGED: Update macOS CI image to macos-11. [#6286](https://github.com/Project-OSRM/osrm-backend/pull/6286)

CMakeLists.txt

@@ -316,11 +316,14 @@ if (ENABLE_COVERAGE)
   set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -O0 -ftest-coverage -fprofile-arcs")
 endif()
 
+
 if (ENABLE_SANITIZER)
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address")
+  set(SANITIZER_FLAGS "-g -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=undefined -fno-omit-frame-pointer")
-  set(OSRM_CXXFLAGS "${OSRM_CXXFLAGS} -fsanitize=address")
+  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${SANITIZER_FLAGS}")
-  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${SANITIZER_FLAGS}")
-  set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -fsanitize=address")
+  set(OSRM_CXXFLAGS "${OSRM_CXXFLAGS} ${SANITIZER_FLAGS}")
+  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${SANITIZER_FLAGS}")
+  set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${SANITIZER_FLAGS}")
 endif()
 
 # Configuring compilers

features/lib/osrm_loader.js

@@ -45,11 +45,12 @@ class OSRMBaseLoader{
         var retryCount = 0;
         let retry = (err) => {
           if (err) {
-            if (retryCount < 10) {
+            if (retryCount < this.scope.OSRM_CONNECTION_RETRIES) {
+              const timeoutMs = 10 * Math.pow(this.scope.OSRM_CONNECTION_EXP_BACKOFF_COEF, retryCount);
               retryCount++;
-              setTimeout(() => { tryConnect(this.scope.OSRM_IP, this.scope.OSRM_PORT, retry); }, 10);
+              setTimeout(() => { tryConnect(this.scope.OSRM_IP, this.scope.OSRM_PORT, retry); }, timeoutMs);
             } else {
-              callback(new Error("Could not connect to osrm-routed after ten retries."));
+              callback(new Error(`Could not connect to osrm-routed after ${this.scope.OSRM_CONNECTION_RETRIES} retries.`));
             }
           }
           else

features/support/env.js

@@ -40,6 +40,9 @@ module.exports = function () {
 
         this.OSRM_PORT = process.env.OSRM_PORT && parseInt(process.env.OSRM_PORT) || 5000;
         this.OSRM_IP = process.env.OSRM_IP || '127.0.0.1';
+        this.OSRM_CONNECTION_RETRIES = process.env.OSRM_CONNECTION_RETRIES && parseInt(process.env.OSRM_CONNECTION_RETRIES) || 10;
+        this.OSRM_CONNECTION_EXP_BACKOFF_COEF = process.env.OSRM_CONNECTION_EXP_BACKOFF_COEF && parseFloat(process.env.OSRM_CONNECTION_EXP_BACKOFF_COEF) || 1.0;
+        
         this.HOST = `http://${this.OSRM_IP}:${this.OSRM_PORT}`;
 
         this.OSRM_PROFILE = process.env.OSRM_PROFILE;

include/contractor/contractor_config.hpp

@@ -43,10 +43,8 @@ namespace contractor
 struct ContractorConfig final : storage::IOConfig
 {
     ContractorConfig()
-        : IOConfig({".osrm.ebg", ".osrm.ebg_nodes", ".osrm.properties"},
+        : IOConfig(
-                   {},
+              {".osrm.ebg", ".osrm.ebg_nodes", ".osrm.properties"}, {}, {".osrm.hsgr", ".osrm.enw"})
-                   {".osrm.hsgr", ".osrm.enw"}),
-          requested_num_threads(0)
     {
     }
 
@@ -62,16 +60,16 @@ struct ContractorConfig final : storage::IOConfig
     updater::UpdaterConfig updater_config;
 
     // DEPRECATED to be removed in v6.0
-    bool use_cached_priority;
+    bool use_cached_priority = false;
 
-    unsigned requested_num_threads;
+    unsigned requested_num_threads = 0;
 
     // DEPRECATED to be removed in v6.0
     // A percentage of vertices that will be contracted for the hierarchy.
     // Offers a trade-off between preprocessing and query time.
     // The remaining vertices form the core of the hierarchy
     //(e.g. 0.8 contracts 80 percent of the hierarchy, leaving a core of 20%)
-    double core_factor;
+    double core_factor = 1.0;
 };
 } // namespace contractor
 } // namespace osrm

include/engine/trip/trip_farthest_insertion.hpp

@@ -48,13 +48,14 @@ GetShortestRoundTrip(const NodeID new_loc,
 
         const auto dist_from = dist_table(*from_node, new_loc);
         const auto dist_to = dist_table(new_loc, *to_node);
-        const auto trip_dist = dist_from + dist_to - dist_table(*from_node, *to_node);
-
         // If the edge_weight is very large (INVALID_EDGE_WEIGHT) then the algorithm will not choose
         // this edge in final minimal path. So instead of computing all the permutations after this
         // large edge, discard this edge right here and don't consider the path after this edge.
         if (dist_from == INVALID_EDGE_WEIGHT || dist_to == INVALID_EDGE_WEIGHT)
             continue;
+
+        const auto trip_dist = dist_from + dist_to - dist_table(*from_node, *to_node);
+
         // This is not neccessarily true:
         // Lets say you have an edge (u, v) with duration 100. If you place a coordinate exactly in
         // the middle of the segment yielding (u, v'), the adjusted duration will be 100 * 0.5 = 50.

include/extractor/extractor_config.hpp

@@ -69,8 +69,7 @@ struct ExtractorConfig final : storage::IOConfig
                ".osrm.icd",
                ".osrm.cnbg",
                ".osrm.cnbg_to_ebg",
-               ".osrm.maneuver_overrides"}),
+               ".osrm.maneuver_overrides"})
-          requested_num_threads(0), parse_conditionals(false), use_locations_cache(true)
     {
     }
 
@@ -84,14 +83,12 @@ struct ExtractorConfig final : storage::IOConfig
     std::vector<boost::filesystem::path> location_dependent_data_paths;
     std::string data_version;
 
-    unsigned requested_num_threads;
+    unsigned requested_num_threads = 0;
-    unsigned small_component_size;
+    unsigned small_component_size = 1000;
 
-    bool generate_edge_lookup;
+    bool use_metadata = false;
-
+    bool parse_conditionals = false;
-    bool use_metadata;
+    bool use_locations_cache = true;
-    bool parse_conditionals;
-    bool use_locations_cache;
 };
 } // namespace extractor
 } // namespace osrm

include/util/log.hpp

@@ -85,6 +85,9 @@ class Log
         return *this;
     }
 
+  private:
+    void Init();
+
   protected:
     const LogLevel level;
     std::ostringstream buffer;

scripts/ci/undefinedsanitizer.conf

@@ -0,0 +1,16 @@
+enum:include/tbb/pipeline.h
+vptr:src/util/log.cpp
+vptr:include/tbb/task.h
+vptr:include/tbb/parallel_reduce.h
+vptr:include/boost/smart_ptr/detail/shared_count.hpp
+vptr:include/boost/smart_ptr/detail/sp_counted_base_gcc_atomic.hpp
+vptr:include/boost/program_options/variables_map.hpp
+vptr:include/boost/program_options/value_semantic.hpp
+vptr:src/tools/contract.cpp
+vptr:src/tools/extract.cpp
+pointer-overflow:include/partitioner/cell_storage.hpp
+signed-integer-overflow:include/engine/internal_route_result.hpp
+pointer-overflow:third_party/sol2/sol/stack_core.hpp
+pointer-overflow:third_party/rapidjson/include/rapidjson/internal/stack.h
+nonnull-attribute:third_party/microtar/src/microtar.c
+integer-divide-by-zero:unit_tests/library/route.cpp

src/util/log.cpp

@@ -63,7 +63,11 @@ std::string LogPolicy::GetLevels()
     return "NONE, ERROR, WARNING, INFO, DEBUG";
 }
 
-Log::Log(LogLevel level_, std::ostream &ostream) : level(level_), stream(ostream)
+Log::Log(LogLevel level_, std::ostream &ostream) : level(level_), stream(ostream) { Init(); }
+
+Log::Log(LogLevel level_) : level(level_), buffer{}, stream{buffer} { Init(); }
+
+void Log::Init()
 {
     std::lock_guard<std::mutex> lock(get_mutex());
     if (!LogPolicy::GetInstance().IsMute() && level <= LogPolicy::GetInstance().GetLevel())
@@ -91,8 +95,6 @@ Log::Log(LogLevel level_, std::ostream &ostream) : level(level_), stream(ostream
     }
 }
 
-Log::Log(LogLevel level_) : Log(level_, buffer) {}
-
 std::mutex &Log::get_mutex()
 {
     static std::mutex mtx;