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.

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

static_mut_refs: Need edition migration guide documentation #123059

Open
ehuss opened this issue Mar 25, 2024 · 8 comments
Open

static_mut_refs: Need edition migration guide documentation #123059

ehuss opened this issue Mar 25, 2024 · 8 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-edition-2024 Area: The 2024 edition T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@ehuss
Copy link
Contributor

ehuss commented Mar 25, 2024

The change for static_mut_refs in the 2024 Edition needs to have documentation (here) for how a user can perform a manual migration.

This documentation should have a more complete explanation of why it is forbidden. Currently it just mentions use in multiple threads, but I don't think that is a complete description.

It also needs to explain to the user how to rewrite their code to make it work. My understanding is that someone can convert &STATIC to &*addr_of!(STATIC), which is equivalent, but still runs afoul of the exact same aliasing problems and undefined behavior, so I don't know if it makes sense to recommend that.

  • Why is &*addr_of!(…) allowed?
  • What situations are there where addr_of! is appropriate?
  • If addr_of! isn't the correct route, what should a user do to update their code?
    • Do they have to switch to interior mutability? How? What other options are there?

See also discussion at #114447 and
https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/static_mut_refs

@ehuss ehuss added T-lang Relevant to the language team, which will review and decide on the PR/issue. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-edition-2024 Area: The 2024 edition labels Mar 25, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 25, 2024
@traviscross traviscross removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 25, 2024
@traviscross
Copy link
Contributor

cc @obeis @scottmcm @RalfJung @est31

@eggyal
Copy link
Contributor

eggyal commented Apr 12, 2024

I opened rust-lang/edition-guide#299 as a step towards this.

@traviscross
Copy link
Contributor

We talked about this in the edition meeting today. The page that we merged to the edition guide in rust-lang/edition-guide#299 is a good start, but we still want to see a bit more.

We're hoping for the guide to cover an example showing why this is unsound, and then show how the simple cases could be rewritten using raw pointers, and maybe show a more complicated case showing where using a custom type instead would make sense.

Looking in the recent crater results (#125384), it seems there are a lot of calls to libc here. Perhaps we could show an example of that and how these could be addressed with raw pointers.

@dnadlinger
Copy link

dnadlinger commented Sep 26, 2024

Yes, from a user's perspective this definitely could definitely use more documentation still. For context, I am not a stranger to formally thinking about lifetimes/object provenance/… (having worked on compilers, memory model details, etc. before), but as far as I can tell not being involved in Rust language development, Rust's model here is still somewhat in flux, or at least it is not obvious to me where to go look for details. I just ran into this lint in a single-threaded embedded Rust context with some DMA buffers need to be a global static mut, when implicitly creating a reference as part of calling MaybeUninit::write. Of course, I need to make sure that the accesses are actually sound on a hardware level, including the necessary compiler barriers, etc., but I am aware that I might need to be careful to avoid accidental UB in the Rust model beyond that. But where would I find the necessary details, i.e. discussion of the relevant aliasing rules, to figure this out?

The sentence "By first obtaining a raw pointer rather than directly taking a reference, (the safety requirements of) accesses through that pointer will be more familiar to unsafe developers and can be deferred until/limited to smaller regions of code." in the current guide starts out promising, but unfortunately takes a turn from what I was hoping for half-way. I am primarily interested in what the actual rules are, not whether will be familiar to developers or not, and the guide page isn't particularly helpful there. In my case, making a pointer and immediately writing to (through) it a priori does not seem any different or safer than just implicitly creating that reference. But is it different as consequence of some weird quirk/edge case of the Rust type system? Or are they equivalent, and the goal of requiring addr_of/… gymnastics is just to make the code look more obviously dangerous (in which case an example in the docs discussing this would help)? How would I find out as a "low level expert" but non-Rust-language-lawyer user? That is, I know the problem space reasonably well in abstract terms, but am not very confident that I understand where exactly Rust drew the line on UB in various places, and the added deny-default lint suggests that code I thought is correct probably isn't, but the docs don't really help answer whether it is in fact UB or not.

@traviscross
Copy link
Contributor

traviscross commented Sep 27, 2024

If you have two pointers that alias, each in separate threads, and you're careful to only read or write through one of them at a time, that's sound. But if you create two mutable references that alias in separate threads (or otherwise), that's immediately UB, regardless of whether you ever racily read or write through those references. That's the difference.

@dnadlinger
Copy link

dnadlinger commented Sep 27, 2024

Thanks for the quick response. (I think) I understood the point about UB with two mutable references; I just wanted to illustrate how this change/the current guide page tripped me as a non-Rust-expert up with no good idea on how to follow up to confirm my interpretation.

For instance, if as mentioned in the top post,

My understanding is that someone can convert &STATIC to &*addr_of!(STATIC), which is equivalent, but still runs afoul of the exact same aliasing problems and undefined behavior

then a code example discussing this explicitly might help, i.e. that this is a way to silence the lint for cases where it really is safe to create a reference (acknowledging that that this is a non-local property of the program, etc.).

In other words: While the top-line summary says "Taking a reference to a static mut is no longer allowed", it seems like what's actually going on is not a change to aliasing rules, but just that the decision has been made to disable the most straightforward syntax for obtaining a reference, as it is easy to miss how expansive the guarantees are one needs for this not to be UB (especially when done implicitly). If this interpretation is correct, it might be helpful to state that. I suspect that the fact that the semantic rules around UB didn't change and just a particular construct was disallowed to remove a footgun may be obvious to those in the know, but wasn't to me when reading the guide page as a casual user.

@traviscross
Copy link
Contributor

Your interpretation is correct. If you're interested and feel able, submit a PR. This issue is open because we know the docs here should be improved.

@RalfJung
Copy link
Member

Implicitly creating a reference and writing through it is indeed equivalent to doing the same write through a raw pointer. So this is likely a case of the lint not being able to tell that this is a short-lived guaranteed-used reference, where a raw pointer makes no difference. (You gave no code example so I can't be sure of this though, I am just guessing.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-edition-2024 Area: The 2024 edition T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants