From e003a57f076bdc242ab172c9918097940c0f842a Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 8 Apr 2026 13:09:08 -0400 Subject: [PATCH] Revert "fix: Switch to boost::coroutine2 (#6372)" This reverts commit 983816248a9a05e7920cf3a432d0827d59989ba4. --- .../workflows/reusable-build-test-config.yml | 16 +++---- cmake/XrplInterface.cmake | 2 +- cmake/deps/Boost.cmake | 43 +++++++++---------- conan/profiles/sanitizers | 29 +------------ conanfile.py | 12 +----- cspell.config.yaml | 1 - include/xrpl/core/Coro.ipp | 10 ++--- include/xrpl/core/JobQueue.h | 7 ++- sanitizers/suppressions/asan.supp | 9 ++++ 9 files changed, 45 insertions(+), 84 deletions(-) diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 920b5b5278..879d0aa6c0 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -76,7 +76,7 @@ jobs: name: ${{ inputs.config_name }} runs-on: ${{ fromJSON(inputs.runs_on) }} container: ${{ inputs.image != '' && inputs.image || null }} - timeout-minutes: ${{ inputs.sanitizers != '' && 360 || 60 }} + timeout-minutes: 60 env: # Use a namespace to keep the objects separate for each configuration. CCACHE_NAMESPACE: ${{ inputs.config_name }} @@ -227,17 +227,11 @@ jobs: - name: Set sanitizer options if: ${{ !inputs.build_only && env.SANITIZERS_ENABLED == 'true' }} - env: - CONFIG_NAME: ${{ inputs.config_name }} run: | - ASAN_OPTS="include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-asan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp" - if [[ "${CONFIG_NAME}" == *gcc* ]]; then - ASAN_OPTS="${ASAN_OPTS}:alloc_dealloc_mismatch=0" - fi - echo "ASAN_OPTIONS=${ASAN_OPTS}" >> ${GITHUB_ENV} - echo "TSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-tsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp" >> ${GITHUB_ENV} - echo "UBSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-ubsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >> ${GITHUB_ENV} - echo "LSAN_OPTIONS=include=${GITHUB_WORKSPACE}/sanitizers/suppressions/runtime-lsan-options.txt:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >> ${GITHUB_ENV} + echo "ASAN_OPTIONS=print_stacktrace=1:detect_container_overflow=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/asan.supp" >> ${GITHUB_ENV} + echo "TSAN_OPTIONS=second_deadlock_stack=1:halt_on_error=0:suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/tsan.supp" >> ${GITHUB_ENV} + echo "UBSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/ubsan.supp" >> ${GITHUB_ENV} + echo "LSAN_OPTIONS=suppressions=${GITHUB_WORKSPACE}/sanitizers/suppressions/lsan.supp" >> ${GITHUB_ENV} - name: Run the separate tests if: ${{ !inputs.build_only }} diff --git a/cmake/XrplInterface.cmake b/cmake/XrplInterface.cmake index 7add613f5a..21fa76501d 100644 --- a/cmake/XrplInterface.cmake +++ b/cmake/XrplInterface.cmake @@ -23,7 +23,7 @@ target_compile_definitions( BOOST_FILESYSTEM_NO_DEPRECATED > $<$>: - BOOST_COROUTINES2_NO_DEPRECATION_WARNING + BOOST_COROUTINES_NO_DEPRECATION_WARNING BOOST_BEAST_ALLOW_DEPRECATED BOOST_FILESYSTEM_DEPRECATED > diff --git a/cmake/deps/Boost.cmake b/cmake/deps/Boost.cmake index 7594ddc806..10dc3e271a 100644 --- a/cmake/deps/Boost.cmake +++ b/cmake/deps/Boost.cmake @@ -7,7 +7,7 @@ find_package( COMPONENTS chrono container - context + coroutine date_time filesystem json @@ -26,7 +26,7 @@ target_link_libraries( Boost::headers Boost::chrono Boost::container - Boost::context + Boost::coroutine Boost::date_time Boost::filesystem Boost::json @@ -38,26 +38,23 @@ target_link_libraries( if(Boost_COMPILER) target_link_libraries(xrpl_boost INTERFACE Boost::disable_autolinking) endif() - -# GCC 14+ has a false positive -Wuninitialized warning in Boost.Coroutine2's -# state.hpp when compiled with -O3. This is due to GCC's intentional behavior -# change (Bug #98871, #119388) where warnings from inlined system header code -# are no longer suppressed by -isystem. The warning occurs in operator|= in -# boost/coroutine2/detail/state.hpp when inlined from push_control_block::destroy(). -# See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119388 -if(is_gcc AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 14) - target_compile_options(xrpl_boost INTERFACE -Wno-uninitialized) -endif() - -# Boost.Context's ucontext backend has ASAN fiber-switching annotations -# (start/finish_switch_fiber) that are compiled in when BOOST_USE_ASAN is defined. -# This tells ASAN about coroutine stack switches, preventing false positive -# stack-use-after-scope errors. BOOST_USE_UCONTEXT ensures the ucontext backend -# is selected (fcontext does not support ASAN annotations). -# These defines must match what Boost was compiled with (see conan/profiles/sanitizers). -if(enable_asan) - target_compile_definitions( - xrpl_boost - INTERFACE BOOST_USE_ASAN BOOST_USE_UCONTEXT +if(SANITIZERS_ENABLED AND is_clang) + # TODO: gcc does not support -fsanitize-blacklist...can we do something else for gcc ? + if(NOT Boost_INCLUDE_DIRS AND TARGET Boost::headers) + get_target_property( + Boost_INCLUDE_DIRS + Boost::headers + INTERFACE_INCLUDE_DIRECTORIES + ) + endif() + message(STATUS "Adding [${Boost_INCLUDE_DIRS}] to sanitizer blacklist") + file( + WRITE ${CMAKE_CURRENT_BINARY_DIR}/san_bl.txt + "src:${Boost_INCLUDE_DIRS}/*" + ) + target_compile_options( + opts + INTERFACE # ignore boost headers for sanitizing + -fsanitize-blacklist=${CMAKE_CURRENT_BINARY_DIR}/san_bl.txt ) endif() diff --git a/conan/profiles/sanitizers b/conan/profiles/sanitizers index 6d37425f43..d7a622359a 100644 --- a/conan/profiles/sanitizers +++ b/conan/profiles/sanitizers @@ -7,21 +7,16 @@ include(default) {% if compiler == "gcc" %} {% if "address" in sanitizers or "thread" in sanitizers or "undefinedbehavior" in sanitizers %} {% set sanitizer_list = [] %} - {% set defines = [] %} {% set model_code = "" %} {% set extra_cxxflags = ["-fno-omit-frame-pointer", "-O1", "-Wno-stringop-overflow"] %} {% if "address" in sanitizers %} {% set _ = sanitizer_list.append("address") %} {% set model_code = "-mcmodel=large" %} - {% set _ = defines.append("BOOST_USE_ASAN")%} - {% set _ = defines.append("BOOST_USE_UCONTEXT")%} {% elif "thread" in sanitizers %} {% set _ = sanitizer_list.append("thread") %} {% set model_code = "-mcmodel=medium" %} {% set _ = extra_cxxflags.append("-Wno-tsan") %} - {% set _ = defines.append("BOOST_USE_TSAN")%} - {% set _ = defines.append("BOOST_USE_UCONTEXT")%} {% endif %} {% if "undefinedbehavior" in sanitizers %} @@ -34,22 +29,16 @@ include(default) tools.build:cxxflags+=['{{sanitizer_flags}} {{" ".join(extra_cxxflags)}}'] tools.build:sharedlinkflags+=['{{sanitizer_flags}}'] tools.build:exelinkflags+=['{{sanitizer_flags}}'] - tools.build:defines+={{defines}} {% endif %} {% elif compiler == "apple-clang" or compiler == "clang" %} {% if "address" in sanitizers or "thread" in sanitizers or "undefinedbehavior" in sanitizers %} {% set sanitizer_list = [] %} - {% set defines = [] %} {% set extra_cxxflags = ["-fno-omit-frame-pointer", "-O1"] %} {% if "address" in sanitizers %} {% set _ = sanitizer_list.append("address") %} - {% set _ = defines.append("BOOST_USE_ASAN")%} - {% set _ = defines.append("BOOST_USE_UCONTEXT")%} {% elif "thread" in sanitizers %} {% set _ = sanitizer_list.append("thread") %} - {% set _ = defines.append("BOOST_USE_TSAN")%} - {% set _ = defines.append("BOOST_USE_UCONTEXT")%} {% endif %} {% if "undefinedbehavior" in sanitizers %} @@ -63,24 +52,8 @@ include(default) tools.build:cxxflags+=['{{sanitizer_flags}} {{" ".join(extra_cxxflags)}}'] tools.build:sharedlinkflags+=['{{sanitizer_flags}}'] tools.build:exelinkflags+=['{{sanitizer_flags}}'] - tools.build:defines+={{defines}} {% endif %} {% endif %} {% endif %} -tools.info.package_id:confs+=["tools.build:cxxflags", "tools.build:exelinkflags", "tools.build:sharedlinkflags", "tools.build:defines"] - -[options] -{% if sanitizers %} - {% if "address" in sanitizers %} - # Build Boost.Context with ucontext backend (not fcontext) so that - # ASAN fiber-switching annotations (__sanitizer_start/finish_switch_fiber) - # are compiled into the library. fcontext (assembly) has no ASAN support. - # define=BOOST_USE_ASAN=1 is critical: it must be defined when building - # Boost.Context itself so the ucontext backend compiles in the ASAN annotations. - boost/*:extra_b2_flags=context-impl=ucontext address-sanitizer=on define=BOOST_USE_ASAN=1 - boost/*:without_context=False - # Boost stacktrace fails to build with some sanitizers - boost/*:without_stacktrace=True - {% endif %} -{% endif %} +tools.info.package_id:confs+=["tools.build:cxxflags", "tools.build:exelinkflags", "tools.build:sharedlinkflags"] diff --git a/conanfile.py b/conanfile.py index 4949516bfe..537965d98d 100644 --- a/conanfile.py +++ b/conanfile.py @@ -1,4 +1,3 @@ -import os import re from conan.tools.cmake import CMake, CMakeToolchain, cmake_layout @@ -57,9 +56,6 @@ class Xrpl(ConanFile): "tests": False, "unity": False, "xrpld": False, - "boost/*:without_context": False, - "boost/*:without_coroutine": True, - "boost/*:without_coroutine2": False, "date/*:header_only": True, "ed25519/*:shared": False, "grpc/*:shared": False, @@ -129,12 +125,6 @@ class Xrpl(ConanFile): if self.settings.compiler in ["clang", "gcc"]: self.options["boost"].without_cobalt = True - # Check if environment variable exists - if "SANITIZERS" in os.environ: - sanitizers = os.environ["SANITIZERS"] - if "address" in sanitizers.lower(): - self.default_options["fPIC"] = False - def requirements(self): self.requires("boost/1.90.0", force=True, transitive_headers=True) self.requires("date/3.0.4", transitive_headers=True) @@ -201,7 +191,7 @@ class Xrpl(ConanFile): "boost::headers", "boost::chrono", "boost::container", - "boost::context", + "boost::coroutine", "boost::date_time", "boost::filesystem", "boost::json", diff --git a/cspell.config.yaml b/cspell.config.yaml index 63edba587d..fd8ad406d4 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -99,7 +99,6 @@ words: - endmacro - exceptioned - Falco - - fcontext - finalizers - firewalled - fmtdur diff --git a/include/xrpl/core/Coro.ipp b/include/xrpl/core/Coro.ipp index 38d921a658..d8c7f57915 100644 --- a/include/xrpl/core/Coro.ipp +++ b/include/xrpl/core/Coro.ipp @@ -1,5 +1,7 @@ #pragma once +#include + namespace xrpl { template @@ -8,18 +10,16 @@ JobQueue::Coro::Coro(Coro_create_t, JobQueue& jq, JobType type, std::string cons , type_(type) , name_(name) , coro_( - // Stack size of 1MB wasn't sufficient for deep calls. ASAN tests flagged the issue. Hence - // increasing the size to 1.5MB. - boost::context::protected_fixedsize_stack(1536 * 1024), [this, fn = std::forward(f)]( - boost::coroutines2::asymmetric_coroutine::push_type& do_yield) { + boost::coroutines::asymmetric_coroutine::push_type& do_yield) { yield_ = &do_yield; yield(); fn(shared_from_this()); #ifndef NDEBUG finished_ = true; #endif - }) + }, + boost::coroutines::attributes(megabytes(1))) { } diff --git a/include/xrpl/core/JobQueue.h b/include/xrpl/core/JobQueue.h index 558e55cf31..04221a44bc 100644 --- a/include/xrpl/core/JobQueue.h +++ b/include/xrpl/core/JobQueue.h @@ -7,8 +7,7 @@ #include #include -#include -#include +#include #include @@ -49,8 +48,8 @@ public: std::mutex mutex_; std::mutex mutex_run_; std::condition_variable cv_; - boost::coroutines2::coroutine::pull_type coro_; - boost::coroutines2::coroutine::push_type* yield_; + boost::coroutines::asymmetric_coroutine::pull_type coro_; + boost::coroutines::asymmetric_coroutine::push_type* yield_; #ifndef NDEBUG bool finished_ = false; #endif diff --git a/sanitizers/suppressions/asan.supp b/sanitizers/suppressions/asan.supp index e02d580aa6..ae21c316c0 100644 --- a/sanitizers/suppressions/asan.supp +++ b/sanitizers/suppressions/asan.supp @@ -1,6 +1,15 @@ # The idea is to empty this file gradually by fixing the underlying issues and removing suppressions. # # ASAN_OPTIONS="print_stacktrace=1:detect_container_overflow=0:suppressions=sanitizers/suppressions/asan.supp:halt_on_error=0" +# +# The detect_container_overflow=0 option disables false positives from: +# - Boost intrusive containers (slist_iterator.hpp, hashtable.hpp, aged_unordered_container.h) +# - Boost context/coroutine stack switching (Workers.cpp, thread.h) +# +# See: https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow + +# Boost +interceptor_name:boost/asio # Suppress false positive stack-buffer errors in thread stack allocation # Related to ASan's __asan_handle_no_return warnings (github.com/google/sanitizers/issues/189)