config, test: make the BMI2 pext probe run, not just compile - #124
Merged
Merged
Conversation
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DTugu6YyDS1bjQrtP788QQ
|
An automated preview of the documentation is available at https://124.openmethod.prtest3.cppalliance.org/libs/openmethod/doc/html/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-09-21 17:21:58 UTC |
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DTugu6YyDS1bjQrtP788QQ
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #124 +/- ##
===========================================
- Coverage 93.51% 92.30% -1.21%
===========================================
Files 22 27 +5
Lines 1695 2222 +527
Branches 504 679 +175
===========================================
+ Hits 1585 2051 +466
- Misses 66 112 +46
- Partials 44 59 +15
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the SIGILL failures in Drone build 492, stage 8 (
Linux 24.04 GCC 13 32/64 UBSAN C++17):test_dispatch_minimal_cover_hashandtest_hash_policiescrashed withillegal operand/illegal opcodein the BMI2pext-based dispatch path, on x86_64 only. The same failure showed up in build 488 on a different runner, and identically in build 492's stage 29 (macOS 10.15, Xcode 12.2/clang) - which rules out a GCC-13+UBSAN codegen bug and points at the actual cause: some CI runners pair a BMI2-capable compiler with a CPU that does not implement the instruction in hardware (pre-Haswell, or feature-masked under virtualization).config/has_bmi2.cppwas compiled with-mbmi2as anobjtarget - a compile-only check. GCC/clang accept-mbmi2, and therefore emitpext, whenever the toolchain supports the extension; that says nothing about whether the machine running the build implements it. The probe answered "yes" on machines where the real tests then crashed.test/CMakeLists.txthad the identical latent bug one step further back: it gated-mbmi2onCMAKE_SYSTEM_PROCESSORalone, with no probe at all, so a developer building locally on pre-Haswell x86-64 hardware would hit the same SIGILL.Also folded in a fix for an unrelated failure that showed up in the same CI run once the SIGILL was gone (Drone stages 9/10,
Linux 24.04 GCC 13 32/64 UBSAN C++20/C++2b): a GCC 13-Wstringop-overflow=false positive intest_hash_policies.cpp,x86_32release only, confirmed present in the original failing build 492 too.Changes
config/has_bmi2.cpp: gave the probe amain()that callspext64on operands the optimizer can't fold away, so the instruction is genuinely executed.config/Jamfile:has_bmi2target changed fromobjtorun, socheck-target-buildsexecutes the probe instead of only compiling it.test/CMakeLists.txt: replaced theCMAKE_SYSTEM_PROCESSORguess withcheck_cxx_source_runsagainst the samehas_bmi2.cppsource, so both build systems answer from one probe.test/test_hash_policies.cpp:ids_over()'satvector now reserves before its push_back loop, likeidstwo lines below already does. GCC 13's-Wstringop-overflowanalysis mis-traces growth of a default-constructed (capacity-0) vector through the deep inlining-O3 -m32produces, and reports a memmove into a zero-sized region that cannot happen; starting from a reserved capacity removes the growth path its heuristic trips on.The CMake change is strictly broader than the reported (b2-only) Drone failure. Flagging the tradeoff:
check_cxx_source_runsunconditionally answers "no" underCMAKE_CROSSCOMPILING, where the old processor-only check would have answered "yes" blindly. Given the failure mode this guards against, "no" is the safe default.Test plan
b2 toolset=gcc libs/openmethod/config//has_bmi2passes on this (BMI2-capable) machineb2 toolset=gcc libs/openmethod/test//test_dispatch_minimal_cover_hash libs/openmethod/test//test_hash_policiespasses, andobjdump -dC -M intelon the release-variant probe binary confirms a realpextinstruction is emitted (not folded away)BMI2 pext : nois correctly detected and both real tests fall back to their no-op case instead of runningpextb2 toolset=gcc address-model=32 ...still passes unaffected (BOOST_OPENMETHOD_HAS_PEXTis 0 there regardless)Performing Test BOOST_OPENMETHOD_HAS_BMI2 - Success; built and ran both affected test binaries directly, both pass-Wstringop-overflow=failure directly with the exact CI compiler invocation, confirmed thereserve()fixes it, then re-verified across the full matrix it touches (cxxstd 17/20/2b, address-model 32/64, variant release/debug, undefined-sanitizer=norecover) - 0 failuresb2 toolset=gcc libs/openmethod/test//quickstill passes🤖 Generated with Claude Code