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

Ch. 20: Address soundness issues and introduce Miri #4054

Closed
wants to merge 3 commits into from

Conversation

chriskrycho
Copy link
Contributor

@chriskrycho chriskrycho commented Oct 4, 2024

Important

This cannot be merged as is:

  • For keeping things easy for merging, it builds on top of the restructuring of chapters 17–20 as 18–21 to insert the new chapter on async and await.
  • It uses new syntax from Rust 1.82.0 (out in about two weeks!)

Net, we cannot merge till we merge the relevant upstream branches and have updated to require 1.82.0, but in the meantime this PR gives us a handy way to get eyes on it so we can validate that everything it says is correct.


This makes three major changes to the unsafe section:

  • It uses the newly-stabilized1 &raw borrow operator to more safely get raw pointers, with &raw const and &raw mut respectively. These provide a safe(r) way of getting raw pointers. These are part of the Rust effort to handle provenance correctly and thereby make unsafe safer and easier to work with—and while we’re not going to get into those details, this is definitely a better way to work than the cast as *const i32 and as *mut i32.

  • It updates the static mut COUNTER example to use an unsafe fn instead of a safe function around an unsafe block, since it is necessary for the caller to guarantee that the function is not called from multiple threads. To make the existing safe function actually safe, it would need to introduce some kind of locking mechanism, I think. Leaving it as an unsafe function gives us a nice opportunity to include // SAFETY: … comments, though, and thus to teach a bit more about idiomatic authoring and usage of unsafe code.

  • It introduces Miri at the end of the section! I used Miri to investigate some of the issues folks had flagged up, and credit to the Miri team: it is very easy to use. The main thing I think we should think about here is whether we need more prose or explanation around installing nightly Rust.

Footnotes

  1. as of Rust 1.82.0, arriving 2024/10/16

Note: this requires Rust 1.82.0, and will be easiest to merge after that
version is stabilized in two weeks. Since it is blocked on that anyway,
I am also basing it on top of the listing changes.
- Add `SAFETY` documentation on the unsafe function and comments on the
  unsafe invocation in the code samples.
- Discuss the soundness issues in more depth and explain the idiomatic
  use of those `SAFETY` comments.
@chriskrycho chriskrycho deleted the branch async-chapter-with-listings October 8, 2024 18:42
@chriskrycho chriskrycho closed this Oct 8, 2024
@chriskrycho
Copy link
Contributor Author

Replaced by #4062, courtesy of GitHub being confused by my branching strategy. 🙄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Listing 19-03 is potentially UB under Stacked Borrows 19.1 has an unsound example and does not explain it
1 participant