Rollup of 5 pull requests - #160324
Closed
jhpratt wants to merge 17 commits into
Closed
Conversation
Co-authored-by: Jacob Lifshay <programmerjake@gmail.com>
When we suggest implementing a missing trait item (or group of items from `must_implement_one_of`), label any unstable items, so the user doesn't get confused by suggestions to implement an unstable item parallel to suggestions to implement stable items.
Includes a test for unstable trait methods.
test case due to @maxdexh
stabilize size_of_val_raw, align_of_val_raw, Layout::for_value_raw
Stabilizes versions of `size_of_val_raw` and friends that can be invoked on raw pointers, which means they can be used even if one does not have a pointer to a "valid value". This was held up for a while because figuring out the exact safety requirements is tricky, but I think the ones we now have had for a while (plus the minor clarifications in this PR) are "good enough": we basically do case distinction on the unsized tail of the type, and spell out the appropriate requirement for each case. This avoids making blanket statements about future kinds of DST that we may introduce eventually.
These are the requirements I should we should stabilize:
```
/// - If `T` is `Sized`, this function is always safe to call.
/// - If the unsized tail of `T` is:
/// - a [slice] `[U]`, `str`, or a [trait object] `dyn Trait`, then the size of the *entire value*
/// (dynamic tail length + statically sized prefix) must fit in `isize`.
/// For the special case where the dynamic tail length is 0, this function
/// is safe to call.
// NOTE: the reason this is safe is that if an overflow were to occur already with size 0,
// then we would stop compilation as even the "statically known" part of the type would
// already be too big (or the call may be in dead code and optimized away, but then it
// doesn't matter).
/// - No other kind of unsized tail currently exists that satisfies the trait bounds for this
/// function. If more kinds of unsized tails get introduced in the future, the documentation
/// of this function will have to be extended before it can be used for such types.
```
Going over the open questions from the [tracking issue](rust-lang#69835):
- What should the exact safety requirements of these functions be? -> the currently documented requirement look good to me.
- How should this interact with extern types? -> what we document looks good here; we can still adjust this until extern types are stabilized.
- Are the functions sound to call on invalid data pointers? -> for the unsized tails that currently exist: yes; that's kind of the only reason to use them.
Cc @rust-lang/opsem @rust-lang/lang
(FCP should probably include both teams, unless lang is okay with fully delegating this to t-opsem)
I'll nominate the tracking issue for libs-api so we can get their approval as well for how the operation is exposed.
Fixes rust-lang#69835
miri: ensure validity of references and pointers we dereference and cast
Conceptually, when we evaluate the place expression `*place`, there are two steps that happen:
- perform a (typed) load from `place`, which yields a value of pointer/ref type
- construct a new place from that value
Since this is a typed load, we have to ensure that the value satisfies the validity invariant. We forgot to do that, which led to Miri missing a bunch of UB. This PR fixes that by adding the missing checks.
Triggering this UB is a bit non-trivial: you cannot just store an invalid value into `place` using regular assignment (that would already be caught as part of the assignment). However, you can take a raw pointer to `place` and then use that to mutate the contents of `place` in a way that its validity invariant no longer holds. For example:
```rust
fn main() {
let mut x = &();
// Overwrite `x` with null.
unsafe { (&raw mut x).cast::<usize>().write(0) };
let _val = *x; //~ERROR: null reference
}
```
This program clearly (IMO) should have UB, and has UB in MiniRust, but Miri accepted that program before this PR. The same applies to references that are invalid because they are unaligned (even if we end up only accessing a 1-aligned field, i.e. the rest of the place expression evaluates just fine), and to references that are invalid because they are not dereferenceable (even if we end up projecting to a zero-sized field, i.e., the rest of the place expression evaluates just fine).
Even raw pointers have this problem: a raw pointer can be invalid if its vtable pointer is wrong, and we did not check that.
And finally, this also affects `Box`, and this is where things get tricky. `Box` derefs are not even present any more in the MIR Miri sees; they have been replaced by derefs of the underlying raw pointer. But that means we have no way to know that we should check all those things. So to fix that I adjusted the ElaborateBoxDerefs to emit a new special kind of cast that's almost a transmute but also checks `Box` validity.
Fixes rust-lang/miri#5226
Fixes rust-lang/unsafe-code-guidelines#617
Cc @rust-lang/opsem
…eyouxu Update Enzyme to resolve one of the open bugs
…acrum allocations: document that they can be read-only Miri currently tracks "read-only" as an explicit flag on allocations that exists independent of provenance. It essentially corresponds to a read-only mapping in the page table. Let's make this officially part of our model. Cc @rust-lang/lang @rust-lang/opsem
…-no-unstable-suggestions, r=JohnTitor When issuing suggestions for missing trait items, label unstable items When we suggest implementing a missing trait item (or group of items from `must_implement_one_of`), label any unstable items, so the user doesn't get confused by suggestions to implement an unstable item parallel to suggestions to implement stable items.
rustbot
added
F-autodiff
Member
Author
|
@bors r+ rollup=never p=5 |
Contributor
rust-bors
Bot
added
S-waiting-on-bors
rust-bors
Bot
added
the
S-waiting-on-author
Contributor
|
This pull request was unapproved due to being closed. |
rust-bors
Bot
removed
the
S-waiting-on-bors
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Successful merges:
r? @ghost
Create a similar rollup