1.0.1 does not compile for arm-thumb-v6m targets (Cortex-M0)

[henrikssn]

You can reproduce by cloning cortex-m-quickstart and setting target accordingly in .cargo/config

$ rustc --version
rustc 1.48.0 (7eac88abb 2020-11-16)
$ cargo --version
cargo 1.48.0 (65cbdd2dc 2020-10-14)

error: could not compile `bytes`

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed
error[E0599]: no method named `fetch_add` found for struct `AtomicUsize` in the current scope
   --> /home/erik/.cargo/registry/src/github.com-1ecc6299db9ec823/bytes-1.0.1/src/bytes.rs:970:38
    |
970 |     let old_size = (*shared).ref_cnt.fetch_add(1, Ordering::Relaxed);
    |                                      ^^^^^^^^^ method not found in `AtomicUsize`

error[E0599]: no method named `compare_exchange` found for reference `&AtomicPtr<()>` in the current scope
    --> /home/erik/.cargo/registry/src/github.com-1ecc6299db9ec823/bytes-1.0.1/src/bytes.rs:1030:16
     |
1030 |     match atom.compare_exchange(ptr as _, shared as _, Ordering::AcqRel, Ordering::Acquire) {
     |                ^^^^^^^^^^^^^^^^ method not found in `&AtomicPtr<()>`

error[E0599]: no method named `fetch_sub` found for struct `AtomicUsize` in the current scope
    --> /home/erik/.cargo/registry/src/github.com-1ecc6299db9ec823/bytes-1.0.1/src/bytes.rs:1058:23
     |
1058 |     if (*ptr).ref_cnt.fetch_sub(1, Ordering::Release) != 1 {
     |                       ^^^^^^^^^ method not found in `AtomicUsize`

error[E0599]: no method named `fetch_add` found for struct `AtomicUsize` in the current scope
    --> /home/erik/.cargo/registry/src/github.com-1ecc6299db9ec823/bytes-1.0.1/src/bytes_mut.rs:1201:37
     |
1201 |     let old_size = (*ptr).ref_count.fetch_add(1, Ordering::Relaxed);
     |                                     ^^^^^^^^^ method not found in `AtomicUsize`

error[E0599]: no method named `fetch_sub` found for struct `AtomicUsize` in the current scope
    --> /home/erik/.cargo/registry/src/github.com-1ecc6299db9ec823/bytes-1.0.1/src/bytes_mut.rs:1210:25
     |
1210 |     if (*ptr).ref_count.fetch_sub(1, Ordering::Release) != 1 {
     |                         ^^^^^^^^^ method not found in `AtomicUsize`

error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0599`.
       Error Failed to run cargo build: exit code = Some(101)

[taiki-e]

This is because those targets do not support atomic CAS. See also https://github.com/rust-lang/rust/pull/51953.

The feature to handle this correctly on the library side is unstable, and some crates support it via the unstable feature flag (cfg-target-has-atomic feature in futures-rs, nightly feature in crossbeam, etc.).

Technically bytes can do the same, but it may not be very helpful as Bytes, BytesMut, etc. are not available on those platforms. (If crates that depend on bytes are using it, the compile will fail anyway.) Whether it can fix the problem you are encountering depends on how bytes are actually used in your project's dependencies.

[taiki-e]

@henrikssn I have created a patch to support those targets (https://github.com/taiki-e/bytes/tree/cfg_target_has_atomic). Could you try if it solves the problem in your project? (in the way described below)

1. Add the following block to your project's root Cargo.toml:

   [patch.crates-io]
   bytes = { git = "https://github.com/taiki-e/bytes", branch = "cfg_target_has_atomic" }

2. And run cargo with RUSTFLAGS="--cfg=bytes_unstable", like:

RUSTFLAGS="--cfg=bytes_unstable" cargo build

[henrikssn]

Works like a charm!

The packages I depend only use Buf / BufMut from this package and we are always passing byte slices to them.

[dunmatt]

Is it possible to do a similar magic to allow bytes::Bytes to be used? I'm trying to get prost to build for thumbv6m-none-eabi and it's complaining...

[niclashoyer]

while the patch allows compilation it now requires nightly, doesn't it? Would it instead be possible to just add a feature flag for Bytes, so one can choose this by using Cargo features? Or maybe put the traits in another crate?

[taiki-e]

@dunmatt

Is it possible to do a similar magic to allow bytes::Bytes to be used?

atomic CAS (compare and swap) cannot be used in thumbv6m-none-eabi and Bytes uses it. It may be possible to rewrite the implementation to use only atomic load/store, but that probably not be easy.

---

@niclashoyer

while the patch allows compilation it now requires nightly, doesn't it? Would it instead be possible to just add a feature flag for Bytes, so one can choose this by using Cargo features?

Adding an enabled by default feature that enables Bytes/BytesMut is a breaking change, and adding a disabled by default feature that disables Bytes/BytesMut violates the rules of cargo features that "features should be additive".

There is a way to avoid using unstable features, but there are some problems.

Or maybe put the traits in another crate?

I don't think it's possible without breaking changes. Buf uses Bytes in public API.

[carllerche]

If it never compiled, I don't think it would be a breaking change to support Bytes as a !Send and !Sync type on some platforms. That said, I'm not sure if we want to do that :) It would probably be better to support compilation of bytes on those platforms w/o the type entirely and someone could implement a single-threaded Bytes type in a separate crate.

Thoughts?

[dunmatt]

Separate create wouldn't help me since my hypothetical bytes dependence would be transitive. What I really wanted to use was prost.

On Fri, Feb 12, 2021, 11:39 Carl Lerche notifications@github.com wrote:

If it never compiled, I don't think it would be a breaking change to support Bytes as a !Send and !Sync type on some platforms. That said, I'm not sure if we want to do that :) It would probably be better to support compilation of bytes on those platforms w/o the type entirely and someone could implement a single-threaded Bytes type in a separate crate.

Thoughts?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tokio-rs/bytes/issues/461#issuecomment-778370470, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIN7S7OO7G2FN4IYBG7C2TS6VYWLANCNFSM4V77THFA .

[taiki-e]

If it never compiled, I don't think it would be a breaking change to support Bytes as a !Send and !Sync type on some platforms.

I don't think this is preferable. This can make crates that currently manually implement Send on types containing Bytes/BytesMut unsound on some platforms.

[taiki-e]

while the patch allows compilation it now requires nightly, doesn't it?

Updated #467 to support stable Rust.

[marius-meissner]

Maybe I'm missing something, then I apologize upfront.

Could using the following polyfill help in any way? https://crates.io/crates/atomic-polyfill

[taiki-e]

Could using the following polyfill help in any way? https://crates.io/crates/atomic-polyfill

That is unsound on multi-core systems.

[marius-meissner]

That is unsound on multi-core systems.

Right, but what about adding it as an optional feature? In this case, single thread applications (including interrupts) could make safe use of Bytes/BytesMut, which would be a huge benefit for systems like Cortex M0.

[taiki-e]

Supporting it as an optional dependency is a bad idea if enabling it could cause unsoundness.

One of the approachs to handle such cases soundly is to use the optional cfg as my portable-atomic crate does.

This is intentionally not an optional feature. (If this is an optional feature, dependencies can implicitly enable the feature, resulting in the use of unsound code without the end-user being aware of it.)

This prevents them from accidentally being compiled against a multi-core target unless the end-user explicitly guarantees that the target is single core.

[marius-meissner]

One of the approachs to handle such cases soundly is to use the optional cfg

Thanks, makes absolute sense.

as my portable-atomic crate does.

Wow, what an awesome crate! Are there reasons against integrating this in bytes?

My naive draft would be the following extension of your PR https://github.com/tokio-rs/bytes/pull/467

#[cfg(not(all(test, loom)))]
pub(crate) mod sync {
    pub(crate) mod atomic {
        #[cfg(not(bytes_no_atomic_cas))]
        pub(crate) use core::sync::atomic::{fence, AtomicPtr, AtomicUsize, Ordering};

        #[cfg(bytes_no_atomic_cas)]
        pub(crate) use core::sync::atomic::{fence, Ordering};
        #[cfg(bytes_no_atomic_cas)]
        pub(crate) use portable_atomic::{AtomicUsize, AtomicPtr};

[taiki-e]

@marius-meissner Sorry for the late response.

as my portable-atomic crate does.

Wow, what an awesome crate! Are there reasons against integrating this in bytes?

I was hesitant to use this with bytes because of tokio's strict dependency policy and the fact that there were few users at the time, but now that crates like metrics and spin are using portable-atomic, I think we can progress this. (Also, the increase of dependencies can be avoided in most cases by using portable-atomic as an optional dependency, as spin did.)

@carllerche @Darksonn: Any objection to adding an optional dependency (portable-atomic) to support these targets? I'm the (only) maintainer of that crate and can add others from the tokio core team as backup maintainers if needed. Also, since it is an optional dependency for no-std users, it is unlikely that it will actually appear in tokio's dependency tree. EDIT: Also, portable-atomic is a pre-1.0 crate, but it will not be used in the public API and I expect it will be treated like tokio's parking_lot feature.

[carllerche]

I think it would be fine to add it as an optional dependency. Maybe call it something like extra-platforms or something.