Rollup of 12 pull requests - #160339
Conversation
Co-authored-by: Jacob Lifshay <programmerjake@gmail.com>
This updates the crate to a version where the imported APIs use officially released/defined versions of interfaces. For the `wasm32-wasip3` target this is WASI 0.3.0. Note that `wasm32-wasip3` is still a tier 3 target, so this isn't expected to have much practical effect until it's upgraded to tier 2.
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
…null` * replace unsafe usage of `NonNull::new_unchecked` with `NonNull::from_mut` * apply review feedback * apply review feedback again!
Adding noundef to a PassMode::Cast is only sound when the cast target covers no more bytes than the layout, otherwise the extra register bytes are undef padding. In adjust_for_rust_abi this holds by construction (the cast is sized to layout.size), so the current layout_is_noundef-only check is correct but relies on that implicitly. Extract the decision into ArgAbi::cast_to_maybe_noundef, which forwards NoUndef only when layout_is_noundef(layout) && cast.size == layout.size, and switch the aggregate-immediate site to it. No behavior change.
This check is now done from `finalise_check` in the attribute parser by emitting `TrackCallerOnLangItem`. This replaces `check_track_caller` in `rust_passes`.
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
…joboet std: Update `wasip3` crate dependency This updates the crate to a version where the imported APIs use officially released/defined versions of interfaces. For the `wasm32-wasip3` target this is WASI 0.3.0. Note that `wasm32-wasip3` is still a tier 3 target, so this isn't expected to have much practical effect until it's upgraded to tier 2.
…-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.
…=jhpratt,RalfJung
Replace unsafe usage of `NonNull::new_unchecked` with `Box::into_non_null`
~~We can avoid unsafe by replacing `unsafe { NonNull::new_unchecked(Box::into_raw(..)) }` with `NonNull::from_mut(Self::leak(..))`. A simple test to demonstrate the code generation is equivalent: https://godbolt.org/z/P5q9bzPPK~~
We can avoid unsafe by replacing `unsafe { NonNull::new_unchecked(Box::into_raw(..)) }` with the (soon-to-be-stable) `Box::into_non_null(..)`
Additionally, slightly tweak documentation.
Remove final use of sealed traits from stdlib Now that stdarch has been updated, this vestigial trait is no longer needed. Since I last touched this, there was one additional usage of it added. This has been removed in favor of the native `impl(self)` syntax. r? libs
…-guard, r=RalfJung Make the noundef-on-Cast size guard explicit `ArgAbi::cast_to` drops the argument's `ArgAttributes`. rust-lang#152864 restored `noundef` on `PassMode::Cast` for the Rust ABI by re-adding it via `cast_to_with_attrs` when `layout_is_noundef` is true. This works today because `adjust_for_rust_abi` constructs a `Reg { Integer, size }` with `size == layout.size`. However, this assumption isn't guarded anywhere - if this pattern gets reused where a cast is wider than the layout (like foreign ABIs using `Uniform::new` rounding up), padding bytes would be incorrectly marked as `noundef`. This PR adds an explicit size check via a new `ArgAbi::cast_to_maybe_noundef` helper, which only emits `NoUndef` if `layout_is_noundef(layout)` and `cast.size(cx) <= layout.size`. No functional change intended. Existing tests (like `abi-noundef-cast`) continue to pass. Split out from the foreign-ABI work per the Zulip thread; foreign `Reg::iN` sites will reuse this helper in a follow-up PR. r? @RalfJung
Box::leak: tell people to avoid unleaking r? @nia-e Cc @rust-lang/opsem Note that this goes against the advice given by clippy in rust-lang/rust-clippy#17336. I think clippy should be adjusted to recommend `Box::into_non_null` instead. @ArhanChaudhary wold be great if you could make a clippy PR for that. :)
…to_attribute_parser, r=JonathanBrouwer Move `check_track_caller` into the attribute parser cc rust-lang#153101
This comment has been minimized.
This comment has been minimized.
Rollup of 12 pull requests try-job: dist-various-1 try-job: test-various try-job: x86_64-gnu-aux try-job: x86_64-gnu-llvm-21-3 try-job: x86_64-msvc-1 try-job: aarch64-apple try-job: x86_64-mingw-1 try-job: i686-msvc-*
This comment has been minimized.
This comment has been minimized.
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing c47bb2f (parent) -> b430378 (this PR) Test differencesShow 505 test diffsStage 1
Stage 2
Additionally, 502 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard b430378746a835fd5240e2c7169795d7de0547e8 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
📌 Perf builds for each rolled up PR:
previous master: c47bb2f396 In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
|
Finished benchmarking commit (b430378): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Our benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 2.2%, secondary 1.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 0.3%, secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 490.834s -> 489.711s (-0.23%) |
Successful merges:
wasip3crate dependency #160179 (std: Updatewasip3crate dependency)NonNull::new_uncheckedwithBox::into_non_null#160251 (Replace unsafe usage ofNonNull::new_uncheckedwithBox::into_non_null)check_track_callerinto the attribute parser #160328 (Movecheck_track_callerinto the attribute parser)rustc_ast_pretty#160333 (Remove itertools dependency fromrustc_ast_pretty)r? @ghost
Create a similar rollup