From 00816722ddaf458f77c69b1d65553c29ba9c7903 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Mon, 1 Aug 2022 23:40:26 +0200 Subject: [PATCH] Configure Undefined Behaviour Sanitizer (#6290) --- .github/workflows/osrm-backend.yml | 19 +++++++++++++++---- CHANGELOG.md | 1 + CMakeLists.txt | 11 +++++++---- features/lib/osrm_loader.js | 7 ++++--- features/support/env.js | 3 +++ include/contractor/contractor_config.hpp | 12 +++++------- .../engine/trip/trip_farthest_insertion.hpp | 5 +++-- include/extractor/extractor_config.hpp | 15 ++++++--------- include/util/log.hpp | 3 +++ scripts/ci/undefinedsanitizer.conf | 16 ++++++++++++++++ src/util/log.cpp | 8 +++++--- 11 files changed, 68 insertions(+), 32 deletions(-) create mode 100644 scripts/ci/undefinedsanitizer.conf diff --git a/.github/workflows/osrm-backend.yml b/.github/workflows/osrm-backend.yml index b2e9947fb..5c96b7f5e 100644 --- a/.github/workflows/osrm-backend.yml +++ b/.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' }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 326d8a74d..2347d2102 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9229ac8a0..d5318344e 100644 --- a/CMakeLists.txt +++ b/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(OSRM_CXXFLAGS "${OSRM_CXXFLAGS} -fsanitize=address") - set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address") - set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -fsanitize=address") + set(SANITIZER_FLAGS "-g -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=undefined -fno-omit-frame-pointer") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${SANITIZER_FLAGS}") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${SANITIZER_FLAGS}") + 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 diff --git a/features/lib/osrm_loader.js b/features/lib/osrm_loader.js index a29d53b8e..9d409f3d6 100644 --- a/features/lib/osrm_loader.js +++ b/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 diff --git a/features/support/env.js b/features/support/env.js index 95ddf3370..eaa25fbc5 100644 --- a/features/support/env.js +++ b/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; diff --git a/include/contractor/contractor_config.hpp b/include/contractor/contractor_config.hpp index c93d2e63d..18e8ae222 100644 --- a/include/contractor/contractor_config.hpp +++ b/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"}, - {}, - {".osrm.hsgr", ".osrm.enw"}), - requested_num_threads(0) + : IOConfig( + {".osrm.ebg", ".osrm.ebg_nodes", ".osrm.properties"}, {}, {".osrm.hsgr", ".osrm.enw"}) { } @@ -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 diff --git a/include/engine/trip/trip_farthest_insertion.hpp b/include/engine/trip/trip_farthest_insertion.hpp index e4f459b77..74ed56e8c 100644 --- a/include/engine/trip/trip_farthest_insertion.hpp +++ b/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. diff --git a/include/extractor/extractor_config.hpp b/include/extractor/extractor_config.hpp index b3822d587..848d97add 100644 --- a/include/extractor/extractor_config.hpp +++ b/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"}), - requested_num_threads(0), parse_conditionals(false), use_locations_cache(true) + ".osrm.maneuver_overrides"}) { } @@ -84,14 +83,12 @@ struct ExtractorConfig final : storage::IOConfig std::vector location_dependent_data_paths; std::string data_version; - unsigned requested_num_threads; - unsigned small_component_size; + unsigned requested_num_threads = 0; + unsigned small_component_size = 1000; - bool generate_edge_lookup; - - bool use_metadata; - bool parse_conditionals; - bool use_locations_cache; + bool use_metadata = false; + bool parse_conditionals = false; + bool use_locations_cache = true; }; } // namespace extractor } // namespace osrm diff --git a/include/util/log.hpp b/include/util/log.hpp index be3650961..48bb0305f 100644 --- a/include/util/log.hpp +++ b/include/util/log.hpp @@ -85,6 +85,9 @@ class Log return *this; } + private: + void Init(); + protected: const LogLevel level; std::ostringstream buffer; diff --git a/scripts/ci/undefinedsanitizer.conf b/scripts/ci/undefinedsanitizer.conf new file mode 100644 index 000000000..fc1fd8996 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/src/util/log.cpp b/src/util/log.cpp index f8063c748..2809a406a 100644 --- a/src/util/log.cpp +++ b/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 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;