Repository navigation
ci: run the bwos lifo_queue tests on i386 under UBSan - #2328
Merged
Merged
Conversation
ericniebler
reviewed
Oct 12, 2026
Comment on lines
+279
to
+294
| - if: ${{ matrix.name == 'CPU (gcc 14, Debug)' }} | ||
| name: bwos tests on i386 (UBSan) | ||
| shell: bash | ||
| working-directory: /home/coder/stdexec | ||
| run: | | ||
| set -ex | ||
| apt-get update | ||
| DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends g++-14-multilib | ||
| catch=build/_deps/catch2-src/extras | ||
| mkdir -p /tmp/catch-shim/catch2 | ||
| echo '#include "catch_amalgamated.hpp"' > /tmp/catch-shim/catch2/catch_all.hpp | ||
| g++-14 -m32 -std=c++20 -fsanitize=undefined -fno-sanitize-recover=all \ | ||
| -Iinclude -Itest -I/tmp/catch-shim -I"$catch" \ | ||
| test/exec/test_bwos_lifo_queue.cpp "$catch/catch_amalgamated.cpp" \ | ||
| -o /tmp/test-bwos-i386 | ||
| /tmp/test-bwos-i386 "[bwos]" |
Collaborator
There was a problem hiding this comment.
i think maybe it would be cleaner to add an "arch = 32" to the matrix, and then add a 32-bit row to the matrix. the g++ multilib can be conditionally installed for 32-bit builds. what do you think?
Contributor
Author
There was a problem hiding this comment.
sure, I was unsure if a larger change in that regard was welcome. Let me check if anything else breaks
Contributor
Author
There was a problem hiding this comment.
Okay, I found a couple more issues that need fixing too. Commits are incomming
Contributor
Author
There was a problem hiding this comment.
@ericniebler can you approve this PR to run on the CI so we can have the fail/pass run show up here as proof or would you prefer to review it all at once and bisect manually?
No CI job builds for a 32-bit target, so code that assumes 64-bit pointers or a 64-bit std::size_t is never compiled or tested. The new "CPU (gcc 14, Debug, 32-bit)" row builds and tests everything with -m32 (arch: 32). For that row: - g++-14-multilib and gcc-multilib are installed first. gcc-multilib provides /usr/include/asm, which g++-14-multilib only recommends; without it every -m32 compile fails on <linux/errno.h>. - STDEXEC_ENABLE_TBB is off, and -D_GLIBCXX_USE_TBB_PAR_BACKEND=0 keeps libstdc++ from selecting its TBB backend because the image's 64-bit oneTBB headers are present; there is no 32-bit libtbb to link. A test is added that keeps pushing, stealing and popping across several hundred wraps of an 8-block lifo_queue ring. CI failure is expected as no fixes have been made yet.
pysco68
force-pushed
the
bwos-32bit-block-counter
branch
from
October 12, 2026 20:02
385e72f to
bbbec2b
Compare
io_uring_context stored pointers in __u64 fields (user_data, addr) with bit_cast<__u64>(pointer) and read them back with bit_cast<__task*>. A bit_cast needs both types to have the same size, so this does not compile where pointers are 4 bytes. The conversions now go through std::uintptr_t, which is the same value on 64-bit targets and widens correctly on 32-bit ones. With this change the 32-bit row builds.
Contributor
Author
|
Final fix queued here: pysco68@91ac4d9 |
ericniebler
approved these changes
Oct 13, 2026
Collaborator
|
/ok to test 4beca5c |
lifo_queue packed a round into the upper 32 bits of its queue-level block counters (last_block_, start_block_ and the values derived from them) but declared them std::size_t. Where std::size_t is 32 bits wide (i386, wasm32) every shift by 32 on them is undefined: UBSan reports "shift exponent 32 is too large for 32-bit type 'std::size_t'" from steal_front(), the queue refuses pushes as if full (i386) or never terminates (wasm32), and in a reproducer static_thread_pool ran none of 12000 tasks. The counters are now a linear std::uint64_t block counter, counter = round * num_blocks() + index. num_blocks() is a power of two, so the index is counter & mask_ and the round is counter >> round_shift_. The next and previous blocks are counter + 1 and counter - 1, and the round changes exactly when the index wraps, which removes increase_block_counter and decrease_block_counter. The counter is 64 bits wide on every target, so the round keeps 32 bits and wraps in step with the block-level rounds.
pysco68
marked this pull request as ready for review
October 14, 2026 05:53
Collaborator
|
/ok to test 4115d32 |
Contributor
Author
|
From my PoV this is a pass for the Bugfix on the 32bit lanes. LMK if you need anything else. |
Collaborator
|
thanks! |
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
2 participants
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.
Problem
exec::bwos::lifo_queue, the local queue of everystatic_thread_poolworker, packs a round number above bit 32 of its queue-level block counters (last_block_,start_block_and the values derived from them), but declares themstd::size_t. Wherestd::size_tis 32 bits wide (i386, wasm32), every<< 32/>> 32on them is undefined:bwos_lifo_queue.hpp:377:65: runtime error: shift exponent 32 is too large for 32-bit type 'unsigned int'static_thread_poolran 0 of 12000 tasks.Clang reports
-Wshift-count-overflowon these lines in 32-bit builds, but it is invisible to users who include stdexec as a system header.This was found through a wasm32 build of a library that runs its parallel work on
static_thread_pool.Fix
The queue-level counters become a linear
std::uint64_tblock counter,counter = round * num_blocks() + index. Becausenum_blocks()is a power of two, the index iscounter & mask_and the round iscounter >> round_shift_. The next and previous block arecounter + 1andcounter - 1, and the round changes exactly when the index wraps, soincrease_block_counteranddecrease_block_countergo away. The counter is 64 bits wide on every target, so the round keeps 32 bits and wraps in step with the block-level rounds. Behaviour on 64-bit targets is unchanged.Tests and CI
No CI job builds for a 32-bit target today, so the existing tests could not catch this. This PR:
[bwos]test that pushes, steals and pops across several hundred wraps of an 8-block ring, andCPU (gcc 14, Debug)job that compiles the[bwos]tests with-m32 -fsanitize=undefined -fno-sanitize-recover=all, reusing the Catch2 the build has already fetched. It installs onlyg++-14-multiliband adds about a minute.The commits are ordered so CI shows the defect: the first (CI step and test) fails on i386 with the UBSan error above, and the second (the fix) passes.
Verified locally:
rapidsai/devcontainers:26.08-cpp-gcc14-cuda12.9, i386 step: fails before the fix, "All tests passed (15054 assertions in 11 test cases)" after.[bwos]11/11, under TSan 11/11 with no reports, all oftest.exec418/418.Related
#1752, #1755 and #2172 report races and lost tasks in the same queue on 64-bit targets under TSan. This PR does not address those; it only fixes the counter width.