-
Notifications
You must be signed in to change notification settings - Fork 488
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
update aliasing rules section of the reference #1290
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc also @rust-lang/wg-unsafe-code-guidelines
Just to log it, I'm weakly against merging this without also providing a solution like rust-lang/rfcs#3336 to the question of Box
-just-as-a-Drop
-token. Even though the struct { mem: Box<T>, ptr: &'self.mem T }
pattern is currently UB, it is in practice used for lack of a reasonable1 alternative.
At this point, though, I do strongly agree that this is the correct way to go. The optimizations that these requirements provide are significant.
musings about the future
I also think that Vec
should get the same treatment as Box
(though noting so can be deferred until later) even though IIRC there is more code misusing Vec
(including stdlib) than Box
(in part because miri doesn't complain about Vec
(yet)), although I no longer think this is a hard requirement. (However, if we do make Vec
enforce uniqueness even when not reallocating, then I expect we'll need to expose some form of RawVec
which doesn't in order to cover the gap.)
This definitely does make Rust unsafe
significantly more treacherous to use than C, but noalias
analysis is getting better to the point where Rust could be more performant than C (without restrict
) while also being safer. If we manage to make working with raw pointers more ergonomic to the point where it's not significantly worse than C, then a simple first-order rule of "if you're managing an object with raw pointers, only use raw pointers" prevents the issues of mixing strong references and permissive pointers.
Footnotes
-
into_raw
ing the owner is possible but makes interacting with the type much more difficult. ↩
src/behavior-considered-undefined.md
Outdated
lifetime assigned by the borrow checker. When a reference is passed to a | ||
function, it is live at least as long as that function call, again except if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... Does this apply to Box
? (The intent is obviously no, but what about for what we're telling LLVM?)
Because it's obviously safe to deallocate a Box
allocation within a function,
fn example(x: Box<Data>) {
drop(x);
// here -- is `x` still considered `dereferencable`?
}
does this mean that Box
needs to be marked with the hypothetical LLVM-dereferenceable_on_entry
instead of just LLVM-dereferenceable
? I suspect this is a nonissue in practice (memory allocation is either a very side-effectful built-in or an opaque environment call preventing code motion over it) but it's definitely relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See rust-lang/rust#66600 (linked in the PR description) :)
But I will clarify the text, thanks!
So now we have a circular dependency. 😂 I opened this because that RFC has a diff for what it does to the reference, and the diff used to include the clarifications of this PR, which is misleading since that part is not new from the RFC. So I think this here needs to be merged for that RFC to even be able to discuss the aliasing rules in the level of detail that is needed to say what Also as I said above, for better or worse, what this PR describes is the baseline that future changes will have to compare against. Not merging this PR won't make any code less likely to break. Basically, this PR is descriptive in nature, not prescriptive. It describes the world as it is, whether we like it or not. We can change the world later, but first we need to agree on where we are. |
Would it be legal to take a |
No. LLVM recognizes allocation level provenance, meaning the resulting |
Co-authored-by: Josh Triplett <josh@joshtriplett.org>
@rfcbot fcp merge We discussed this in the @rust-lang/lang meeting today and arrived at the consensus that it was a good idea to describe current behavior, with the caveat that some of what we define as UB here may become legal behavior in the future (perhaps via language changes, or as part of adopting a more precise operational semantics). |
Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
Checking my box for documenting the current behavior. Not committing to wanting this to be the long-term behavior; I expect to have further thoughts on the associated RFC. |
@joshtriplett your box isn't checked though?
I assume you specifically mean the "outlives the function call" part? Yes I do expect that to be an important point in a future discussion, when we are ready to start deciding on actual aliasing model details. This is usually a knob that is fairly easy to turn (we can just have or not have that specific rule without surprising interactions elsewhere), but it has a lot of consequences for LLVM attributes and MIR optimizations. |
@joshtriplett Your box is still unchecked. Is that intentional? |
Not sure why my checkbox didn't stick. Transient network issue? Checked now, in any case. |
So FCP should start now, right? "Once a majority of reviewers approve (and at most 2 approvals are outstanding)"... let's see if the bot jumps into action... |
🔔 This is now entering its final comment period, as per the review above. 🔔 psst @nikomatsakis, I wasn't able to add the |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. psst @nikomatsakis, I wasn't able to add the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Update books ## rust-lang/book 1 commits in 3f64052c048c6def93b94a2b514ee88bba918744..a60f4316ec923a5ac2ed6a2eba6960edb832d855 2022-11-16 15:07:18 UTC to 2022-11-16 15:07:18 UTC - Fix Install MdBook command (rust-lang/book#3424) ## rust-embedded/book 4 commits in c533348edd69f11a8f4225d633a05d7093fddbf3..19f798d448835a4888e3b3eae7fe69f1d61d8681 2022-11-17 15:08:11 UTC to 2022-11-08 23:27:57 UTC - start/hardware.md: Fix typo (rust-embedded/book#336) - doc: Fix `arm-none-eabi-gdb` installation instruction for Fedora 27 or newer to just use `gdb` (rust-embedded/book#335) - Update singletons.md (rust-embedded/book#334) - Remove incorrect claim HashMap is avail in no_std (rust-embedded/book#333) ## rust-lang/nomicon 2 commits in 05532356e7a4dbea2330aabb77611f5179493bb8..ae406aa5287a9e025abb72343aaceec98458c117 2022-11-21 22:48:20 UTC to 2022-11-15 00:29:20 UTC - Improve chapter about `Vec<T>` (rust-lang/nomicon#381) - Grammar change for 3.4: Limits of Lifetimes (lifetime-mismatch.md) (rust-lang/nomicon#386) ## rust-lang/reference 9 commits in 9f0cc13ffcd27c1fbe1ab766a9491e15ddcf4d19..3ae62681ff236d5528ef7c8c28ba7c6b2ecc6731 2022-12-05 00:51:50 UTC to 2022-11-15 20:43:30 UTC - Document that type parameter `Self` is unsized by default (rust-lang/reference#1285) - replace `crateid` term with `crate_name` (rust-lang/reference#1310) - Document native library modifier `verbatim` (rust-lang/reference#1299) - Update literal suffix docs for rust-lang#102944 (rust-lang/reference#1305) - update aliasing rules section of the reference (rust-lang/reference#1290) - Document RFC 2867: instruction_set attribute (rust-lang/reference#1253) - Fix a minor typo in the "Higher-ranked trait bounds" section (rust-lang/reference#1288) - Disallow newline directly following `//` (rust-lang/reference#1294) - Add an anchor to the "forwarding macro fragments" paragraph (rust-lang/reference#1300) ## rust-lang/rust-by-example 5 commits in 2b15c0abf2bada6e00553814336bc3e2d8399097..a9869b4a3c4cac3bc6099b41f088679e268400b8 2022-11-27 19:03:05 UTC to 2022-11-11 18:54:53 UTC - Migrate from highfive to triagebot (rust-lang/rust-by-example#1647) - Simpler version of the read_lines script. (rust-lang/rust-by-example#1641) - Fix comment in "Formatted print" example code (rust-lang/rust-by-example#1638) - Added a missing backtick in a comment in chapter 15.4. (rust-lang/rust-by-example#1642) - Clarify the confusing closure example rust-lang#1611 (rust-lang/rust-by-example#1643) ## rust-lang/rustc-dev-guide 13 commits in d0dc6c97a6486f68bac782fff135086eae6d77ec..e269950a57fa6fcda356426545fb5aa3691a7ced 2022-12-03 23:09:24 UTC to 2022-11-08 21:35:38 UTC - Remove duplicate paragraph (rust-lang/rustc-dev-guide#1523) - clarify subtree tool policy (rust-lang/rustc-dev-guide#1518) - Typo (rust-lang/rustc-dev-guide#1520) - Link directly to the section on `--keep-stage` (rust-lang/rustc-dev-guide#1515) - do an actual link to detect if it breaks in future (rust-lang/rustc-dev-guide#1517) - Triage some date-check items (rust-lang/rustc-dev-guide#1513) - Update path for `try_mark_green` implementation (rust-lang/rustc-dev-guide#1512) - Fix a broken design docs link about unused substs bug (rust-lang/rustc-dev-guide#1511) - updating-llvm: keep a calm tone (rust-lang/rustc-dev-guide#1449) - date-check: updating-llvm (rust-lang/rustc-dev-guide#1424) - rewrite the section about regions in the trait solver (rust-lang/rustc-dev-guide#1508) - Consistent ordered list indexing (rust-lang/rustc-dev-guide#1509) - Document multiple alternative suggestions on diagnostic structs (rust-lang/rustc-dev-guide#1486)
Update books ## rust-lang/book 1 commits in 3f64052c048c6def93b94a2b514ee88bba918744..a60f4316ec923a5ac2ed6a2eba6960edb832d855 2022-11-16 15:07:18 UTC to 2022-11-16 15:07:18 UTC - Fix Install MdBook command (rust-lang/book#3424) ## rust-embedded/book 4 commits in c533348edd69f11a8f4225d633a05d7093fddbf3..19f798d448835a4888e3b3eae7fe69f1d61d8681 2022-11-17 15:08:11 UTC to 2022-11-08 23:27:57 UTC - start/hardware.md: Fix typo (rust-embedded/book#336) - doc: Fix `arm-none-eabi-gdb` installation instruction for Fedora 27 or newer to just use `gdb` (rust-embedded/book#335) - Update singletons.md (rust-embedded/book#334) - Remove incorrect claim HashMap is avail in no_std (rust-embedded/book#333) ## rust-lang/nomicon 2 commits in 05532356e7a4dbea2330aabb77611f5179493bb8..ae406aa5287a9e025abb72343aaceec98458c117 2022-11-21 22:48:20 UTC to 2022-11-15 00:29:20 UTC - Improve chapter about `Vec<T>` (rust-lang/nomicon#381) - Grammar change for 3.4: Limits of Lifetimes (lifetime-mismatch.md) (rust-lang/nomicon#386) ## rust-lang/reference 9 commits in 9f0cc13ffcd27c1fbe1ab766a9491e15ddcf4d19..3ae62681ff236d5528ef7c8c28ba7c6b2ecc6731 2022-12-05 00:51:50 UTC to 2022-11-15 20:43:30 UTC - Document that type parameter `Self` is unsized by default (rust-lang/reference#1285) - replace `crateid` term with `crate_name` (rust-lang/reference#1310) - Document native library modifier `verbatim` (rust-lang/reference#1299) - Update literal suffix docs for rust-lang#102944 (rust-lang/reference#1305) - update aliasing rules section of the reference (rust-lang/reference#1290) - Document RFC 2867: instruction_set attribute (rust-lang/reference#1253) - Fix a minor typo in the "Higher-ranked trait bounds" section (rust-lang/reference#1288) - Disallow newline directly following `//` (rust-lang/reference#1294) - Add an anchor to the "forwarding macro fragments" paragraph (rust-lang/reference#1300) ## rust-lang/rust-by-example 5 commits in 2b15c0abf2bada6e00553814336bc3e2d8399097..a9869b4a3c4cac3bc6099b41f088679e268400b8 2022-11-27 19:03:05 UTC to 2022-11-11 18:54:53 UTC - Migrate from highfive to triagebot (rust-lang/rust-by-example#1647) - Simpler version of the read_lines script. (rust-lang/rust-by-example#1641) - Fix comment in "Formatted print" example code (rust-lang/rust-by-example#1638) - Added a missing backtick in a comment in chapter 15.4. (rust-lang/rust-by-example#1642) - Clarify the confusing closure example rust-lang#1611 (rust-lang/rust-by-example#1643) ## rust-lang/rustc-dev-guide 13 commits in d0dc6c97a6486f68bac782fff135086eae6d77ec..e269950a57fa6fcda356426545fb5aa3691a7ced 2022-12-03 23:09:24 UTC to 2022-11-08 21:35:38 UTC - Remove duplicate paragraph (rust-lang/rustc-dev-guide#1523) - clarify subtree tool policy (rust-lang/rustc-dev-guide#1518) - Typo (rust-lang/rustc-dev-guide#1520) - Link directly to the section on `--keep-stage` (rust-lang/rustc-dev-guide#1515) - do an actual link to detect if it breaks in future (rust-lang/rustc-dev-guide#1517) - Triage some date-check items (rust-lang/rustc-dev-guide#1513) - Update path for `try_mark_green` implementation (rust-lang/rustc-dev-guide#1512) - Fix a broken design docs link about unused substs bug (rust-lang/rustc-dev-guide#1511) - updating-llvm: keep a calm tone (rust-lang/rustc-dev-guide#1449) - date-check: updating-llvm (rust-lang/rustc-dev-guide#1424) - rewrite the section about regions in the trait solver (rust-lang/rustc-dev-guide#1508) - Consistent ordered list indexing (rust-lang/rustc-dev-guide#1509) - Document multiple alternative suggestions on diagnostic structs (rust-lang/rustc-dev-guide#1486)
Update books ## rust-lang/book 1 commits in 3f64052c048c6def93b94a2b514ee88bba918744..a60f4316ec923a5ac2ed6a2eba6960edb832d855 2022-11-16 15:07:18 UTC to 2022-11-16 15:07:18 UTC - Fix Install MdBook command (rust-lang/book#3424) ## rust-embedded/book 4 commits in c533348edd69f11a8f4225d633a05d7093fddbf3..19f798d448835a4888e3b3eae7fe69f1d61d8681 2022-11-17 15:08:11 UTC to 2022-11-08 23:27:57 UTC - start/hardware.md: Fix typo (rust-embedded/book#336) - doc: Fix `arm-none-eabi-gdb` installation instruction for Fedora 27 or newer to just use `gdb` (rust-embedded/book#335) - Update singletons.md (rust-embedded/book#334) - Remove incorrect claim HashMap is avail in no_std (rust-embedded/book#333) ## rust-lang/nomicon 2 commits in 05532356e7a4dbea2330aabb77611f5179493bb8..ae406aa5287a9e025abb72343aaceec98458c117 2022-11-21 22:48:20 UTC to 2022-11-15 00:29:20 UTC - Improve chapter about `Vec<T>` (rust-lang/nomicon#381) - Grammar change for 3.4: Limits of Lifetimes (lifetime-mismatch.md) (rust-lang/nomicon#386) ## rust-lang/reference 9 commits in 9f0cc13ffcd27c1fbe1ab766a9491e15ddcf4d19..3ae62681ff236d5528ef7c8c28ba7c6b2ecc6731 2022-12-05 00:51:50 UTC to 2022-11-15 20:43:30 UTC - Document that type parameter `Self` is unsized by default (rust-lang/reference#1285) - replace `crateid` term with `crate_name` (rust-lang/reference#1310) - Document native library modifier `verbatim` (rust-lang/reference#1299) - Update literal suffix docs for rust-lang#102944 (rust-lang/reference#1305) - update aliasing rules section of the reference (rust-lang/reference#1290) - Document RFC 2867: instruction_set attribute (rust-lang/reference#1253) - Fix a minor typo in the "Higher-ranked trait bounds" section (rust-lang/reference#1288) - Disallow newline directly following `//` (rust-lang/reference#1294) - Add an anchor to the "forwarding macro fragments" paragraph (rust-lang/reference#1300) ## rust-lang/rust-by-example 5 commits in 2b15c0abf2bada6e00553814336bc3e2d8399097..a9869b4a3c4cac3bc6099b41f088679e268400b8 2022-11-27 19:03:05 UTC to 2022-11-11 18:54:53 UTC - Migrate from highfive to triagebot (rust-lang/rust-by-example#1647) - Simpler version of the read_lines script. (rust-lang/rust-by-example#1641) - Fix comment in "Formatted print" example code (rust-lang/rust-by-example#1638) - Added a missing backtick in a comment in chapter 15.4. (rust-lang/rust-by-example#1642) - Clarify the confusing closure example rust-lang#1611 (rust-lang/rust-by-example#1643) ## rust-lang/rustc-dev-guide 13 commits in d0dc6c97a6486f68bac782fff135086eae6d77ec..e269950a57fa6fcda356426545fb5aa3691a7ced 2022-12-03 23:09:24 UTC to 2022-11-08 21:35:38 UTC - Remove duplicate paragraph (rust-lang/rustc-dev-guide#1523) - clarify subtree tool policy (rust-lang/rustc-dev-guide#1518) - Typo (rust-lang/rustc-dev-guide#1520) - Link directly to the section on `--keep-stage` (rust-lang/rustc-dev-guide#1515) - do an actual link to detect if it breaks in future (rust-lang/rustc-dev-guide#1517) - Triage some date-check items (rust-lang/rustc-dev-guide#1513) - Update path for `try_mark_green` implementation (rust-lang/rustc-dev-guide#1512) - Fix a broken design docs link about unused substs bug (rust-lang/rustc-dev-guide#1511) - updating-llvm: keep a calm tone (rust-lang/rustc-dev-guide#1449) - date-check: updating-llvm (rust-lang/rustc-dev-guide#1424) - rewrite the section about regions in the trait solver (rust-lang/rustc-dev-guide#1508) - Consistent ordered list indexing (rust-lang/rustc-dev-guide#1509) - Document multiple alternative suggestions on diagnostic structs (rust-lang/rustc-dev-guide#1486)
Update books ## rust-lang/book 1 commits in 3f64052c048c6def93b94a2b514ee88bba918744..a60f4316ec923a5ac2ed6a2eba6960edb832d855 2022-11-16 15:07:18 UTC to 2022-11-16 15:07:18 UTC - Fix Install MdBook command (rust-lang/book#3424) ## rust-embedded/book 4 commits in c533348edd69f11a8f4225d633a05d7093fddbf3..19f798d448835a4888e3b3eae7fe69f1d61d8681 2022-11-17 15:08:11 UTC to 2022-11-08 23:27:57 UTC - start/hardware.md: Fix typo (rust-embedded/book#336) - doc: Fix `arm-none-eabi-gdb` installation instruction for Fedora 27 or newer to just use `gdb` (rust-embedded/book#335) - Update singletons.md (rust-embedded/book#334) - Remove incorrect claim HashMap is avail in no_std (rust-embedded/book#333) ## rust-lang/nomicon 2 commits in 05532356e7a4dbea2330aabb77611f5179493bb8..ae406aa5287a9e025abb72343aaceec98458c117 2022-11-21 22:48:20 UTC to 2022-11-15 00:29:20 UTC - Improve chapter about `Vec<T>` (rust-lang/nomicon#381) - Grammar change for 3.4: Limits of Lifetimes (lifetime-mismatch.md) (rust-lang/nomicon#386) ## rust-lang/reference 9 commits in 9f0cc13ffcd27c1fbe1ab766a9491e15ddcf4d19..3ae62681ff236d5528ef7c8c28ba7c6b2ecc6731 2022-12-05 00:51:50 UTC to 2022-11-15 20:43:30 UTC - Document that type parameter `Self` is unsized by default (rust-lang/reference#1285) - replace `crateid` term with `crate_name` (rust-lang/reference#1310) - Document native library modifier `verbatim` (rust-lang/reference#1299) - Update literal suffix docs for rust-lang#102944 (rust-lang/reference#1305) - update aliasing rules section of the reference (rust-lang/reference#1290) - Document RFC 2867: instruction_set attribute (rust-lang/reference#1253) - Fix a minor typo in the "Higher-ranked trait bounds" section (rust-lang/reference#1288) - Disallow newline directly following `//` (rust-lang/reference#1294) - Add an anchor to the "forwarding macro fragments" paragraph (rust-lang/reference#1300) ## rust-lang/rust-by-example 5 commits in 2b15c0abf2bada6e00553814336bc3e2d8399097..a9869b4a3c4cac3bc6099b41f088679e268400b8 2022-11-27 19:03:05 UTC to 2022-11-11 18:54:53 UTC - Migrate from highfive to triagebot (rust-lang/rust-by-example#1647) - Simpler version of the read_lines script. (rust-lang/rust-by-example#1641) - Fix comment in "Formatted print" example code (rust-lang/rust-by-example#1638) - Added a missing backtick in a comment in chapter 15.4. (rust-lang/rust-by-example#1642) - Clarify the confusing closure example rust-lang#1611 (rust-lang/rust-by-example#1643) ## rust-lang/rustc-dev-guide 13 commits in d0dc6c97a6486f68bac782fff135086eae6d77ec..e269950a57fa6fcda356426545fb5aa3691a7ced 2022-12-03 23:09:24 UTC to 2022-11-08 21:35:38 UTC - Remove duplicate paragraph (rust-lang/rustc-dev-guide#1523) - clarify subtree tool policy (rust-lang/rustc-dev-guide#1518) - Typo (rust-lang/rustc-dev-guide#1520) - Link directly to the section on `--keep-stage` (rust-lang/rustc-dev-guide#1515) - do an actual link to detect if it breaks in future (rust-lang/rustc-dev-guide#1517) - Triage some date-check items (rust-lang/rustc-dev-guide#1513) - Update path for `try_mark_green` implementation (rust-lang/rustc-dev-guide#1512) - Fix a broken design docs link about unused substs bug (rust-lang/rustc-dev-guide#1511) - updating-llvm: keep a calm tone (rust-lang/rustc-dev-guide#1449) - date-check: updating-llvm (rust-lang/rustc-dev-guide#1424) - rewrite the section about regions in the trait solver (rust-lang/rustc-dev-guide#1508) - Consistent ordered list indexing (rust-lang/rustc-dev-guide#1509) - Document multiple alternative suggestions on diagnostic structs (rust-lang/rustc-dev-guide#1486)
This makes explicit some de-facto rules that existed at least since Rust 1.0:
dereferenceable
attribute was added, given that the attribute was also added toBox
which can definitely be deallocated by functions they are passed to. But when that got fixed inBox
is marked as "dereferenceable" for the duration of the call rust#66600, nobody suggested to also remove the attribute from references.)Irrespective of whether we actually want these rules, they do reflect our current requirements and code not following them risks miscompilations. As such I think we should document them here and consider them the baseline for changes to the aliasing model. It's not great to have a baseline that was never carefully reviewed or designed, but for better or worse that's where we are at.
Also incorporates rust-lang/rust#98017.
Rendered
Cc @rust-lang/lang