Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

csky targets' atomic RMW is not lock-free #117306

Open
taiki-e opened this issue Oct 28, 2023 · 6 comments
Open

csky targets' atomic RMW is not lock-free #117306

taiki-e opened this issue Oct 28, 2023 · 6 comments
Labels
A-atomic Area: Atomics, barriers, and sync primitives C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-csky Target: glaCSKY above covers over me~ P-low Low priority

Comments

@taiki-e
Copy link
Member

taiki-e commented Oct 28, 2023 •
edited
Loading

In csky, LLVM always generates libcalls (llvm/llvm-project@ec2de74), and atomic implementations are provided by libatomic.

rust/compiler/rustc_target/src/spec/csky_unknown_linux_gnuabiv2.rs

Line 15 in c7224e3

late_link_args_static: TargetOptions::link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-l:libatomic.a"]),

However, as mentioned in #115577 (comment), the atomic RMW implementation provided in libatomic.a is not lock-free.

And it violates what the standard library is intended to guarantee.

https://doc.rust-lang.org/nightly/std/sync/atomic/index.html#portability

All atomic types in this module are guaranteed to be lock-free if they’re available. This means they don’t internally acquire a global mutex.

Also, mixing lock-free load/store and non-lock-free RMW can cause data races.

To fix this, we would need to do one of the following:

  • Fix libatomic to make the RMW implementation lock-free.
  • Fix LLVM to generate atomic instructions instead of libcalls.

It is not impossible to fix this on our end, but it is not very realistic as it would require writing a lot of inline assembly.

cc @Dirreke (mentioned because you are target maintainer)

@rustbot label A-atomic I-unsound
@taiki-e taiki-e added the C-bug Category: This is a bug. label Oct 28, 2023
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. A-atomic Area: Atomics, barriers, and sync primitives I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Oct 28, 2023
@taiki-e taiki-e mentioned this issue Oct 28, 2023
Merged
@Dirreke
Copy link
Contributor

Dirreke commented Oct 28, 2023

Thanks!

I think to fix LLVM to generate atomic instructions instead of libcalls is a better idea.

The linux system and the gcc-toolchain of csky is not well maintained and the newest version of them is not open-source. Besides, there's many exsited devices with the system which is not suitable for upgrade.

Therefore, I think to fix LLVM is a more realizable and practical way.

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 28, 2023
@bjorn3
Copy link
Member

bjorn3 commented Oct 28, 2023

(We don't have a CSky target label?)

@RalfJung
Copy link
Member

I assume this is about the csky-unknown-linux-gnuabiv2 target.

@Dirreke
Copy link
Contributor

Dirreke commented Oct 28, 2023 •
edited
Loading

I assume this is about the csky-unknown-linux-gnuabiv2 target.

and the csky-unknown-linux-gnuabiv2hf

@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@Dirreke AFAICS the compiler target csky-unknown-linux-gnuabiv2hf is not mapped on our Tier support list. Do you think it should be updated?

@rustbot label -I-prioritize P-low

@rustbot rustbot added P-low Low priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Oct 30, 2023
@Dirreke
Copy link
Contributor

Dirreke commented Oct 30, 2023

@Dirreke AFAICS the compiler target csky-unknown-linux-gnuabiv2hf is not mapped on our Tier support list. Do you think it should be updated?

csky-unknown-linux-gnuabiv2hf is introduced into rust at #117049 . The target csky-unknown-linux-gnuabiv2hf and the related docs will be released in the next version.

@workingjubilee workingjubilee added the O-csky Target: glaCSKY above covers over me~ label May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-atomic Area: Atomics, barriers, and sync primitives C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-csky Target: glaCSKY above covers over me~ P-low Low priority
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@RalfJung @apiraino @saethlin @bjorn3 @taiki-e @workingjubilee @rustbot @Dirreke