From 56c89e74f5d790523b1bc27ccccc3864abcbc665 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leroy Date: Mon, 21 Sep 2026 11:51:18 -0400 Subject: [PATCH 1/2] config, test: make the BMI2 pext probe run, not just compile Drone build 492 stage 8 (Linux 24.04 GCC 13 32/64 UBSAN C++17) failed test_dispatch_minimal_cover_hash and test_hash_policies with SIGILL ("illegal operand" / "illegal opcode") in the pext-based dispatch path, on x86_64 only. Builds 488 and 492 both failed the same way on different Drone runner machines; 4 of 5 surrounding builds passed. Stage 29 (macOS 10.15, Xcode 12.2/clang) failed identically in build 492, which rules out a GCC-13+UBSAN codegen bug and points at the CPU: some CI runners pair a BMI2-capable compiler with pre-Haswell (or feature-masked) silicon. config/has_bmi2.cpp was compiled with -mbmi2 as an `obj` target - a compile-only check. GCC and clang accept -mbmi2, and therefore emit `pext`, whenever the toolchain supports the extension; that says nothing about whether the machine actually running the build implements the instruction in hardware. The probe answered "yes" on machines where the real tests then crashed. Give has_bmi2.cpp a main() that calls pext64 on operands the optimizer can't fold away, and change config/Jamfile's has_bmi2 target from `obj` to `run` so check-target-builds executes it. Verified locally: the real probe passes and objdump confirms a genuine `pext` in the optimized (release) build; a probe forced to fail at runtime correctly answers "no" and both real tests fall back to their no-op case instead of running pext; the address-model=32 variant is unaffected (it never defines BOOST_OPENMETHOD_HAS_PEXT). test/CMakeLists.txt had the identical latent bug, one step further back: it gated -mbmi2 on CMAKE_SYSTEM_PROCESSOR alone, with no probe at all, so a developer building locally on pre-Haswell x86-64 hardware would hit the same SIGILL. Replaced with check_cxx_source_runs against the same has_bmi2.cpp source, so both build systems answer from one probe. This is strictly broader than the reported Drone failure (b2 only); flagging the tradeoff here since a reviewer would otherwise have to find it: check_cxx_source_runs unconditionally answers "no" under CMAKE_CROSSCOMPILING, where the old processor-only check would have answered "yes" blindly. Verified locally with cmake+ctest, including running both affected test binaries directly. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01DTugu6YyDS1bjQrtP788QQ --- config/Jamfile | 10 +++++++++- config/has_bmi2.cpp | 20 ++++++++++++++++++-- test/CMakeLists.txt | 40 ++++++++++++++++++++++++++++------------ 3 files changed, 55 insertions(+), 15 deletions(-) diff --git a/config/Jamfile b/config/Jamfile index d4f562fc..1d2527f2 100644 --- a/config/Jamfile +++ b/config/Jamfile @@ -13,6 +13,8 @@ # requirement is inherited by the test subprojects too, and a relative # reference would be resolved from each of their directories. +import testing ; + project /boost/openmethod/config ; obj has_reflection : has_reflection.cpp : -freflection ; @@ -22,5 +24,11 @@ explicit has_reflection ; # needs. Probing beats naming an architecture - a x86 conditional # does not match every toolset spelling, and a compiler that rejects -mbmi2 # outright would take the directory down with it. -obj has_bmi2 : has_bmi2.cpp : -mbmi2 ../include ; +# +# `run`, not `obj`: the compiler accepts -mbmi2 - and emits `pext` - whenever +# the toolchain supports the extension, whether or not the machine running the +# build implements it in hardware. A handful of CI runners pair a BMI2-capable +# compiler with pre-Haswell (or feature-masked) silicon, where the instruction +# traps with SIGILL; a compile-only check cannot see that; see has_bmi2.cpp. +run has_bmi2.cpp : : : -mbmi2 ../include ; explicit has_bmi2 ; diff --git a/config/has_bmi2.cpp b/config/has_bmi2.cpp index 65c0f87a..d67f35db 100644 --- a/config/has_bmi2.cpp +++ b/config/has_bmi2.cpp @@ -11,6 +11,19 @@ // what the test suite needs to know is whether that header will let the policy // be used, which is a slightly narrower question than whether some spelling of // pext compiles. +// +// The probe runs, it does not just compile. GCC and clang accept -mbmi2, and +// therefore emit `pext`, whenever the *toolchain* supports the extension - +// that says nothing about whether the machine actually running the build +// implements it in hardware. A CI runner can advertise a BMI2-capable +// compiler while executing on pre-Haswell (or virtualized/feature-masked) +// silicon, where the instruction traps with SIGILL. ../Jamfile turns this +// into a `run` target for exactly that reason: UPDATE_NOW executes the +// program, and a trap fails the check the same way a compile error would. +// +// `argc` keeps the operands from being folded to a compile-time constant, so +// optimization cannot turn the one `pext` this program executes into a +// no-op. #include @@ -18,6 +31,9 @@ static_assert(BOOST_OPENMETHOD_HAS_PEXT); -auto probe(std::uint64_t value, std::uint64_t mask) -> std::uint64_t { - return boost::openmethod::detail::pext64(value, mask); +auto main(int argc, char**) -> int { + auto value = std::uint64_t(argc) | (std::uint64_t(1) << 63); + auto mask = std::uint64_t(argc) * 0x5555555555555555ull; + + return boost::openmethod::detail::pext64(value, mask) == 0xdeadbeef ? 1 : 0; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e493cb5e..0d59f772 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -87,6 +87,28 @@ set(BOOST_OPENMETHOD_TEST_PCH_HEADERS set(boost_openmethod_pch_owner "") +# minimal_cover_hash dispatches with BMI2's pext. A compiler accepts the +# enabling flag - and therefore emits `pext` - whenever the toolchain supports +# the extension, whatever the machine actually running the build implements in +# hardware; on pre-Haswell (or feature-masked) x86-64 the instruction traps +# with SIGILL. check_cxx_source_runs actually executes the probe, once, for +# exactly that reason - a plain try_compile cannot see the difference. See +# config/has_bmi2.cpp, and config/Jamfile for the b2 equivalent and the CI +# failure that motivated both. +if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC" OR CMAKE_CXX_COMPILER_FRONTEND_VARIANT MATCHES "MSVC") + set(BOOST_OPENMETHOD_BMI2_FLAG /arch:AVX2) +else() + set(BOOST_OPENMETHOD_BMI2_FLAG -mbmi2) +endif() + +include(CheckCXXSourceRuns) +file(READ "${CMAKE_CURRENT_SOURCE_DIR}/../config/has_bmi2.cpp" BOOST_OPENMETHOD_HAS_BMI2_SOURCE) +set(CMAKE_REQUIRED_FLAGS ${BOOST_OPENMETHOD_BMI2_FLAG}) +set(CMAKE_REQUIRED_INCLUDES "${CMAKE_CURRENT_SOURCE_DIR}/../include") +check_cxx_source_runs("${BOOST_OPENMETHOD_HAS_BMI2_SOURCE}" BOOST_OPENMETHOD_HAS_BMI2) +unset(CMAKE_REQUIRED_FLAGS) +unset(CMAKE_REQUIRED_INCLUDES) + file(GLOB test_cpp_files "test_*.cpp") foreach(test_cpp ${test_cpp_files}) @@ -107,21 +129,15 @@ foreach(test_cpp ${test_cpp_files}) # minimal_cover_hash dispatches with BMI2's pext, and because `hash` is # inlined into every call the instruction has to be enabled for the whole - # translation unit. Only the tests that name the policy need it, and only on - # x86-64 - the 64-bit pext intrinsic does not exist in 32-bit mode: - # elsewhere BOOST_OPENMETHOD_HAS_PEXT is 0 and the policy drops out of both. + # translation unit. Only the tests that name the policy need it, and only + # where BOOST_OPENMETHOD_HAS_BMI2 (computed above) found the instruction + # both compiles *and runs* - elsewhere BOOST_OPENMETHOD_HAS_PEXT is 0 and + # the policy drops out of both. set(test_needs_bmi2 FALSE) - if (test MATCHES "minimal_cover_hash|hash_policies" AND - CMAKE_SYSTEM_PROCESSOR MATCHES "^(x86_64|AMD64|amd64)$") + if (test MATCHES "minimal_cover_hash|hash_policies" AND BOOST_OPENMETHOD_HAS_BMI2) set(test_needs_bmi2 TRUE) - - if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC" OR - CMAKE_CXX_COMPILER_FRONTEND_VARIANT MATCHES "MSVC") - target_compile_options(${test_target} PRIVATE /arch:AVX2) - else() - target_compile_options(${test_target} PRIVATE -mbmi2) - endif() + target_compile_options(${test_target} PRIVATE ${BOOST_OPENMETHOD_BMI2_FLAG}) endif() file(READ ${test_cpp} test_cpp_contents) From 159f10f2e0060c230d0db871ef32851de4c2edf7 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leroy Date: Mon, 21 Sep 2026 13:18:04 -0400 Subject: [PATCH 2/2] test: reserve `at` in ids_over, dodging a GCC 13 -m32 -O3 false positive Drone stages 9 and 10 (Linux 24.04 GCC 13 32/64 UBSAN C++20/C++2b) failed test_hash_policies.cpp in the x86_32 release variant: error: 'void* __builtin_memmove(void*, const void*, unsigned int)' writing between 1 and 2147483647 bytes into a region of size 0 overflows the destination [-Werror=stringop-overflow=] GCC 13's -Wstringop-overflow analysis mis-traces the growth of `at` (default-constructed, so it starts at capacity 0) through the deep inlining chain -O3 builds for ids_over -> ids_diluted and friends, and reports a memmove into a zero-sized region that cannot happen. `ids`, two lines below, already reserves before its own push_back loop for an unrelated reason (this test drives the policies with exact, attacker-chosen id counts, so there is no reason to let any of its vectors grow via reallocation); giving `at` the same treatment removes the empty-vector growth GCC's heuristic trips on. Verified: reproduced the failure directly with the exact CI compiler invocation (g++-13, -m32 -O3 -Wall -Wextra -Werror, from the Drone log); the reserve() fixes it. Then confirmed via b2 across the full matrix this touches - cxxstd 17/20/2b, address-model 32/64, variant release/debug, undefined-sanitizer=norecover - 0 failures. quick and test_dispatch_minimal_cover_hash still pass. Folded into this PR at the user's request rather than filed separately, since both bugs surfaced together in the same failing Drone build. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01DTugu6YyDS1bjQrtP788QQ --- test/test_hash_policies.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_hash_policies.cpp b/test/test_hash_policies.cpp index 5e8a8a03..215509cc 100644 --- a/test/test_hash_policies.cpp +++ b/test/test_hash_policies.cpp @@ -120,6 +120,7 @@ auto base_of(std::size_t module, std::size_t spread) -> std::uintptr_t { auto ids_over(std::size_t n, std::size_t modules, std::size_t spread) -> std::vector { std::vector at; + at.reserve(modules); for (std::size_t module = 0; module != modules; ++module) { at.push_back(base_of(module, spread));