Building core::sync::atomic for BPF target fails #106795
Comments
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this issue
Jan 23, 2023
…omic-cas-bpf, r=bjorn3 BPF: Disable atomic CAS Enabling CAS for BPF targets (rust-lang#105708) breaks the build of core library. The failure occurs both when building rustc for BPF targets and when building crates for BPF targets with the current nightly. The LLVM BPF backend does not correctly lower all `atomicrmw` operations and crashes for unsupported ones. Before we can enable CAS for BPF in Rust, we need to fix the LLVM BPF backend first. Fixes rust-lang#106795 Signed-off-by: Michal Rostecki <[email protected]>
|
Re-opening as #106856 will need to land too. |
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this issue
Jan 27, 2023
… r=joshtriplett core: Support variety of atomic widths in width-agnostic functions Before this change, the following functions and macros were annotated with `#[cfg(target_has_atomic = "8")]` or `#[cfg(target_has_atomic_load_store = "8")]`: * `atomic_int` * `strongest_failure_ordering` * `atomic_swap` * `atomic_add` * `atomic_sub` * `atomic_compare_exchange` * `atomic_compare_exchange_weak` * `atomic_and` * `atomic_nand` * `atomic_or` * `atomic_xor` * `atomic_max` * `atomic_min` * `atomic_umax` * `atomic_umin` However, none of those functions and macros actually depend on 8-bit width and they are needed for all atomic widths (16-bit, 32-bit, 64-bit etc.). Some targets might not support 8-bit atomics (i.e. BPF, if we would enable atomic CAS for it). This change fixes that by removing the `"8"` argument from annotations, which results in accepting the whole variety of widths. Fixes rust-lang#106845 Fixes rust-lang#106795 Signed-off-by: Michal Rostecki <[email protected]>
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this issue
Jan 27, 2023
… r=joshtriplett core: Support variety of atomic widths in width-agnostic functions Before this change, the following functions and macros were annotated with `#[cfg(target_has_atomic = "8")]` or `#[cfg(target_has_atomic_load_store = "8")]`: * `atomic_int` * `strongest_failure_ordering` * `atomic_swap` * `atomic_add` * `atomic_sub` * `atomic_compare_exchange` * `atomic_compare_exchange_weak` * `atomic_and` * `atomic_nand` * `atomic_or` * `atomic_xor` * `atomic_max` * `atomic_min` * `atomic_umax` * `atomic_umin` However, none of those functions and macros actually depend on 8-bit width and they are needed for all atomic widths (16-bit, 32-bit, 64-bit etc.). Some targets might not support 8-bit atomics (i.e. BPF, if we would enable atomic CAS for it). This change fixes that by removing the `"8"` argument from annotations, which results in accepting the whole variety of widths. Fixes rust-lang#106845 Fixes rust-lang#106795 Signed-off-by: Michal Rostecki <[email protected]>
workingjubilee
added
the
O-eBPF
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Starting from the following commit and PR, which enabled CAS for BPF target and got included in nightly:
#105708
11331b1
building the rustc library with the following config:
fails with:
https://gist.github.com/vadorovsky/c9a3892a85ea4425082d91a729ffcfb2
A similar failure can be observed when building an eBPF program based on https://github.com/aya-rs/aya:
https://gist.github.com/vadorovsky/7aaf1bbbb78ab0c0371fe65d90105007
That error occurs, because all these macros and functions which "can't be found", are annotated with
#[cfg(target_has_atomic = "8")]or#[cfg(target_has_atomic_load_store = "8")].Example:
rust/library/core/src/sync/atomic.rs
Lines 3014 to 3016 in bfffe40
Those annotations prevent those macros and functions to be built for BPF, because we enable only 64-bit atomics for BPF:
rust/compiler/rustc_target/src/spec/bpf_base.rs
Lines 25 to 26 in bfffe40
A quick fix to make the build fix successful is changing annotations to:
I made this change on my local branch:
vadorovsky@041978b
It fixes the core library build, however, it doesn't make the most of atomic operations actually working. I made a test Aya-based project where I tried to use all atomic operations on
AtomicU64:https://github.com/vadorovsky/aya-examples/tree/main/atomic
https://github.com/vadorovsky/aya-examples/blob/main/atomic/atomic-ebpf/src/main.rs
But pretty much every atomic operation except
store,swapand assigning to mutable raw pointer triggers various bpf-linker / LLVM errors like this one (when usingfetch_add):Or this one (when using
compare_exchange):I didn't investigate the bpf-linker / LLVM failures deeper yet. It might be related to the fact that BPF target is calling intrinsics manually and was written only with clang in mind. But again, I need more time to investigate.
But at this point I'm quite confident that we should just revert #105708 and reconsider such change after we make sure it actually works and all the mentioned issues are fixed.
/cc @tomerze @alessandrod
The text was updated successfully, but these errors were encountered: