Skip to content

Commit

Permalink
Auto merge of #130847 - matthiaskrgr:rollup-f0n80bw, r=matthiaskrgr
Browse files Browse the repository at this point in the history
Rollup of 6 pull requests

Successful merges:

 - #130735 (Simple validation for unsize coercion in MIR validation)
 - #130781 (Fix up setting strip = true in Cargo.toml makes build scripts fail in…)
 - #130811 (add link from random() helper fn to extensive DefaultRandomSource docs)
 - #130819 (Add `must_use` attribute to `len_utf8` and `len_utf16`.)
 - #130832 (fix some cfg logic around optimize_for_size and 16-bit targets)
 - #130842 (Add tracking issue for io_error_inprogress)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Sep 25, 2024
2 parents b511753 + e805182 commit 0399709
Show file tree
Hide file tree
Showing 13 changed files with 145 additions and 48 deletions.
17 changes: 16 additions & 1 deletion compiler/rustc_codegen_cranelift/src/unsize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,22 @@ pub(crate) fn unsized_info<'tcx>(
let old_info =
old_info.expect("unsized_info: missing old info for trait upcasting coercion");
if data_a.principal_def_id() == data_b.principal_def_id() {
// A NOP cast that doesn't actually change anything, should be allowed even with invalid vtables.
// Codegen takes advantage of the additional assumption, where if the
// principal trait def id of what's being casted doesn't change,
// then we don't need to adjust the vtable at all. This
// corresponds to the fact that `dyn Tr<A>: Unsize<dyn Tr<B>>`
// requires that `A = B`; we don't allow *upcasting* objects
// between the same trait with different args. If we, for
// some reason, were to relax the `Unsize` trait, it could become
// unsound, so let's assert here that the trait refs are *equal*.
//
// We can use `assert_eq` because the binders should have been anonymized,
// and because higher-ranked equality now requires the binders are equal.
debug_assert_eq!(
data_a.principal(),
data_b.principal(),
"NOP unsize vtable changed principal trait ref: {data_a} -> {data_b}"
);
return old_info;
}

Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1087,16 +1087,17 @@ fn link_natively(
let strip = sess.opts.cg.strip;

if sess.target.is_like_osx {
let stripcmd = "/usr/bin/strip";
match (strip, crate_type) {
(Strip::Debuginfo, _) => {
strip_symbols_with_external_utility(sess, "strip", out_filename, Some("-S"))
strip_symbols_with_external_utility(sess, stripcmd, out_filename, Some("-S"))
}
// Per the manpage, `-x` is the maximum safe strip level for dynamic libraries. (#93988)
(Strip::Symbols, CrateType::Dylib | CrateType::Cdylib | CrateType::ProcMacro) => {
strip_symbols_with_external_utility(sess, "strip", out_filename, Some("-x"))
strip_symbols_with_external_utility(sess, stripcmd, out_filename, Some("-x"))
}
(Strip::Symbols, _) => {
strip_symbols_with_external_utility(sess, "strip", out_filename, None)
strip_symbols_with_external_utility(sess, stripcmd, out_filename, None)
}
(Strip::None, _) => {}
}
Expand Down
24 changes: 22 additions & 2 deletions compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,28 @@ fn unsized_info<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
let old_info =
old_info.expect("unsized_info: missing old info for trait upcasting coercion");
if data_a.principal_def_id() == data_b.principal_def_id() {
// A NOP cast that doesn't actually change anything, should be allowed even with
// invalid vtables.
// Codegen takes advantage of the additional assumption, where if the
// principal trait def id of what's being casted doesn't change,
// then we don't need to adjust the vtable at all. This
// corresponds to the fact that `dyn Tr<A>: Unsize<dyn Tr<B>>`
// requires that `A = B`; we don't allow *upcasting* objects
// between the same trait with different args. If we, for
// some reason, were to relax the `Unsize` trait, it could become
// unsound, so let's assert here that the trait refs are *equal*.
//
// We can use `assert_eq` because the binders should have been anonymized,
// and because higher-ranked equality now requires the binders are equal.
debug_assert_eq!(
data_a.principal(),
data_b.principal(),
"NOP unsize vtable changed principal trait ref: {data_a} -> {data_b}"
);

// A NOP cast that doesn't actually change anything, let's avoid any
// unnecessary work. This relies on the assumption that if the principal
// traits are equal, then the associated type bounds (`dyn Trait<Assoc=T>`)
// are also equal, which is ensured by the fact that normalization is
// a function and we do not allow overlapping impls.
return old_info;
}

Expand Down
35 changes: 32 additions & 3 deletions compiler/rustc_mir_transform/src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir::LangItem;
use rustc_index::IndexVec;
use rustc_index::bit_set::BitSet;
use rustc_infer::traits::Reveal;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::traits::{Obligation, ObligationCause, Reveal};
use rustc_middle::mir::coverage::CoverageKind;
use rustc_middle::mir::visit::{NonUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
Expand All @@ -16,6 +17,8 @@ use rustc_middle::ty::{
use rustc_middle::{bug, span_bug};
use rustc_target::abi::{FIRST_VARIANT, Size};
use rustc_target::spec::abi::Abi;
use rustc_trait_selection::traits::ObligationCtxt;
use rustc_type_ir::Upcast;

use crate::util::{is_within_packed, relate_types};

Expand Down Expand Up @@ -586,6 +589,22 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {

crate::util::relate_types(self.tcx, self.param_env, variance, src, dest)
}

/// Check that the given predicate definitely holds in the param-env of this MIR body.
fn predicate_must_hold_modulo_regions(
&self,
pred: impl Upcast<TyCtxt<'tcx>, ty::Predicate<'tcx>>,
) -> bool {
let infcx = self.tcx.infer_ctxt().build();
let ocx = ObligationCtxt::new(&infcx);
ocx.register_obligation(Obligation::new(
self.tcx,
ObligationCause::dummy(),
self.param_env,
pred,
));
ocx.select_all_or_error().is_empty()
}
}

impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
Expand Down Expand Up @@ -1202,8 +1221,18 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
CastKind::PointerCoercion(PointerCoercion::Unsize, _) => {
// This is used for all `CoerceUnsized` types,
// not just pointers/references, so is hard to check.
// Pointers being unsize coerced should at least implement
// `CoerceUnsized`.
if !self.predicate_must_hold_modulo_regions(ty::TraitRef::new(
self.tcx,
self.tcx.require_lang_item(
LangItem::CoerceUnsized,
Some(self.body.source_info(location).span),
),
[op_ty, *target_type],
)) {
self.fail(location, format!("Unsize coercion, but `{op_ty}` isn't coercible to `{target_type}`"));
}
}
CastKind::PointerCoercion(PointerCoercion::DynStar, _) => {
// FIXME(dyn-star): make sure nothing needs to be done here.
Expand Down
4 changes: 4 additions & 0 deletions library/core/src/char/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ impl char {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "const_char_len_utf", since = "1.52.0")]
#[inline]
#[must_use]
pub const fn len_utf8(self) -> usize {
len_utf8(self as u32)
}
Expand Down Expand Up @@ -637,6 +638,7 @@ impl char {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "const_char_len_utf", since = "1.52.0")]
#[inline]
#[must_use]
pub const fn len_utf16(self) -> usize {
len_utf16(self as u32)
}
Expand Down Expand Up @@ -1738,6 +1740,7 @@ impl EscapeDebugExtArgs {
}

#[inline]
#[must_use]
const fn len_utf8(code: u32) -> usize {
match code {
..MAX_ONE_B => 1,
Expand All @@ -1748,6 +1751,7 @@ const fn len_utf8(code: u32) -> usize {
}

#[inline]
#[must_use]
const fn len_utf16(code: u32) -> usize {
if (code & 0xFFFF) == code { 1 } else { 2 }
}
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/slice/sort/shared/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![cfg_attr(feature = "optimize_for_size", allow(dead_code))]
#![cfg_attr(any(feature = "optimize_for_size", target_pointer_width = "16"), allow(dead_code))]

use crate::marker::Freeze;

Expand Down
14 changes: 7 additions & 7 deletions library/core/src/slice/sort/stable/mod.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
//! This module contains the entry points for `slice::sort`.

#[cfg(not(feature = "optimize_for_size"))]
#[cfg(not(any(feature = "optimize_for_size", target_pointer_width = "16")))]
use crate::cmp;
use crate::intrinsics;
use crate::mem::{self, MaybeUninit, SizedTypeProperties};
#[cfg(not(feature = "optimize_for_size"))]
#[cfg(not(any(feature = "optimize_for_size", target_pointer_width = "16")))]
use crate::slice::sort::shared::smallsort::{
SMALL_SORT_GENERAL_SCRATCH_LEN, StableSmallSortTypeImpl, insertion_sort_shift_left,
};

pub(crate) mod merge;

#[cfg(not(feature = "optimize_for_size"))]
#[cfg(not(any(feature = "optimize_for_size", target_pointer_width = "16")))]
pub(crate) mod drift;
#[cfg(not(feature = "optimize_for_size"))]
#[cfg(not(any(feature = "optimize_for_size", target_pointer_width = "16")))]
pub(crate) mod quicksort;

#[cfg(feature = "optimize_for_size")]
#[cfg(any(feature = "optimize_for_size", target_pointer_width = "16"))]
pub(crate) mod tiny;

/// Stable sort called driftsort by Orson Peters and Lukas Bergdoll.
Expand Down Expand Up @@ -45,7 +45,7 @@ pub fn sort<T, F: FnMut(&T, &T) -> bool, BufT: BufGuard<T>>(v: &mut [T], is_less

cfg_if! {
if #[cfg(target_pointer_width = "16")] {
let heap_buf = BufT::with_capacity(alloc_len);
let mut heap_buf = BufT::with_capacity(alloc_len);
let scratch = heap_buf.as_uninit_slice_mut();
} else {
// For small inputs 4KiB of stack storage suffices, which allows us to avoid
Expand Down Expand Up @@ -85,7 +85,7 @@ pub fn sort<T, F: FnMut(&T, &T) -> bool, BufT: BufGuard<T>>(v: &mut [T], is_less
///
/// Deliberately don't inline the main sorting routine entrypoint to ensure the
/// inlined insertion sort i-cache footprint remains minimal.
#[cfg(not(feature = "optimize_for_size"))]
#[cfg(not(any(feature = "optimize_for_size", target_pointer_width = "16")))]
#[inline(never)]
fn driftsort_main<T, F: FnMut(&T, &T) -> bool, BufT: BufGuard<T>>(v: &mut [T], is_less: &mut F) {
// By allocating n elements of memory we can ensure the entire input can
Expand Down
6 changes: 3 additions & 3 deletions library/core/src/slice/sort/unstable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

use crate::intrinsics;
use crate::mem::SizedTypeProperties;
#[cfg(not(feature = "optimize_for_size"))]
#[cfg(not(any(feature = "optimize_for_size", target_pointer_width = "16")))]
use crate::slice::sort::shared::find_existing_run;
#[cfg(not(feature = "optimize_for_size"))]
#[cfg(not(any(feature = "optimize_for_size", target_pointer_width = "16")))]
use crate::slice::sort::shared::smallsort::insertion_sort_shift_left;

pub(crate) mod heapsort;
Expand Down Expand Up @@ -55,7 +55,7 @@ pub fn sort<T, F: FnMut(&T, &T) -> bool>(v: &mut [T], is_less: &mut F) {
///
/// Deliberately don't inline the main sorting routine entrypoint to ensure the
/// inlined insertion sort i-cache footprint remains minimal.
#[cfg(not(feature = "optimize_for_size"))]
#[cfg(not(any(feature = "optimize_for_size", target_pointer_width = "16")))]
#[inline(never)]
fn ipnsort<T, F>(v: &mut [T], is_less: &mut F)
where
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/io/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ pub enum ErrorKind {

/// The operation was partially successful and needs to be checked
/// later on due to not blocking.
#[unstable(feature = "io_error_inprogress", issue = "none")]
#[unstable(feature = "io_error_inprogress", issue = "130840")]
InProgress,

// "Unusual" error kinds which do not correspond simply to (sets
Expand Down
3 changes: 2 additions & 1 deletion library/std/src/random.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ impl RandomSource for DefaultRandomSource {
///
/// This is a convenience function for `T::random(&mut DefaultRandomSource)` and
/// will sample according to the same distribution as the underlying [`Random`]
/// trait implementation.
/// trait implementation. See [`DefaultRandomSource`] for more information about
/// how randomness is sourced.
///
/// **Warning:** Be careful when manipulating random values! The
/// [`random`](Random::random) method on integers samples them with a uniform
Expand Down
26 changes: 0 additions & 26 deletions tests/crashes/129219.rs

This file was deleted.

33 changes: 33 additions & 0 deletions tests/ui/mir/validate/validate-unsize-cast.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//@ compile-flags: -Zmir-opt-level=0 -Zmir-enable-passes=+Inline,+GVN -Zvalidate-mir

#![feature(unsize)]

use std::marker::Unsize;

pub trait CastTo<U: ?Sized>: Unsize<U> {}

// Not well-formed!
impl<T: ?Sized, U: ?Sized> CastTo<U> for T {}
//~^ ERROR the trait bound `T: Unsize<U>` is not satisfied

pub trait Cast {
fn cast<U: ?Sized>(&self)
where
Self: CastTo<U>;
}
impl<T: ?Sized> Cast for T {
#[inline(always)]
fn cast<U: ?Sized>(&self)
where
Self: CastTo<U>,
{
let x: &U = self;
}
}

fn main() {
// When we inline this call, then we run GVN, then
// GVN tries to evaluate the `() -> [i32]` unsize.
// That's invalid!
().cast::<[i32]>();
}
20 changes: 20 additions & 0 deletions tests/ui/mir/validate/validate-unsize-cast.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error[E0277]: the trait bound `T: Unsize<U>` is not satisfied
--> $DIR/validate-unsize-cast.rs:10:42
|
LL | impl<T: ?Sized, U: ?Sized> CastTo<U> for T {}
| ^ the trait `Unsize<U>` is not implemented for `T`
|
= note: all implementations of `Unsize` are provided automatically by the compiler, see <https://doc.rust-lang.org/stable/std/marker/trait.Unsize.html> for more information
note: required by a bound in `CastTo`
--> $DIR/validate-unsize-cast.rs:7:30
|
LL | pub trait CastTo<U: ?Sized>: Unsize<U> {}
| ^^^^^^^^^ required by this bound in `CastTo`
help: consider further restricting this bound
|
LL | impl<T: ?Sized + std::marker::Unsize<U>, U: ?Sized> CastTo<U> for T {}
| ++++++++++++++++++++++++

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.

0 comments on commit 0399709

Please sign in to comment.