Skip to content

Commit

Permalink
Auto merge of rust-lang#116542 - the8472:slice-ref-len-validity, r=<try>
Browse files Browse the repository at this point in the history
Add range metadata to slice lengths

This adds range information to the slice-len in fat pointers if we can conservatively determine that the pointee is not a ZST without having to normalize the pointee type.

I only intended to pass the `!range` to llvm but apparently this also lets the length in fat pointers be used for its niches 😅.

Ideally this would use the naive-layout computation from rust-lang#113166 to calculate a better approximation of the pointee size, but that PR got reverted.
  • Loading branch information
bors committed Mar 31, 2024
2 parents bf71dae + c054bbc commit cbf560f
Show file tree
Hide file tree
Showing 16 changed files with 301 additions and 57 deletions.
14 changes: 13 additions & 1 deletion compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3765,14 +3765,26 @@ impl<'hir> Node<'hir> {
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
mod size_asserts {
use super::*;
// tidy-alphabetical-start
static_assert_size!(Block<'_>, 48);
static_assert_size!(Body<'_>, 24);
#[cfg(bootstrap)]
static_assert_size!(Expr<'_>, 64);
#[cfg(not(bootstrap))]
static_assert_size!(Expr<'_>, 56);
#[cfg(bootstrap)]
static_assert_size!(ExprKind<'_>, 48);
#[cfg(not(bootstrap))]
static_assert_size!(ExprKind<'_>, 40);
static_assert_size!(FnDecl<'_>, 40);
#[cfg(bootstrap)]
static_assert_size!(ForeignItem<'_>, 72);
#[cfg(not(bootstrap))]
static_assert_size!(ForeignItem<'_>, 64);
#[cfg(bootstrap)]
static_assert_size!(ForeignItemKind<'_>, 40);
#[cfg(not(bootstrap))]
static_assert_size!(ForeignItemKind<'_>, 32);
// tidy-alphabetical-start
static_assert_size!(GenericArg<'_>, 32);
static_assert_size!(GenericBound<'_>, 48);
static_assert_size!(Generics<'_>, 56);
Expand Down
27 changes: 16 additions & 11 deletions compiler/rustc_hir_typeck/src/intrinsicck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_hir as hir;
use rustc_index::Idx;
use rustc_middle::ty::layout::{LayoutError, SizeSkeleton};
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
use rustc_target::abi::{Pointer, VariantIdx};
use rustc_target::abi::{Pointer, Size, VariantIdx};

use super::FnCtxt;

Expand Down Expand Up @@ -84,19 +84,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

fn size_to_bits(size: Size) -> u128 {
let Some(bits) = u128::from(size.bytes()).checked_mul(8) else {
// `u128` should definitely be able to hold the size of different architectures
// larger sizes should be reported as error `are too big for the current architecture`
// otherwise we have a bug somewhere
bug!("{:?} overflow for u128", size)
};

bits
}

// Try to display a sensible error with as much information as possible.
let skeleton_string = |ty: Ty<'tcx>, sk: Result<_, &_>| match sk {
Ok(SizeSkeleton::Pointer { tail, .. }) => format!("pointer to `{tail}`"),
Ok(SizeSkeleton::Known(size)) => {
if let Some(v) = u128::from(size.bytes()).checked_mul(8) {
format!("{v} bits")
} else {
// `u128` should definitely be able to hold the size of different architectures
// larger sizes should be reported as error `are too big for the current architecture`
// otherwise we have a bug somewhere
bug!("{:?} overflow for u128", size)
}
Ok(SizeSkeleton::Pointer { tail, known_size: Some(size), .. }) => {
format!("{} bits, pointer to `{tail}`", size_to_bits(size))
}
Ok(SizeSkeleton::Pointer { tail, .. }) => format!("pointer to `{tail}`"),
Ok(SizeSkeleton::Known(size)) => format!("{} bits", size_to_bits(size)),
Ok(SizeSkeleton::Generic(size)) => {
if let Some(size) = size.try_eval_target_usize(tcx, self.param_env) {
format!("{size} bytes")
Expand Down
35 changes: 29 additions & 6 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use crate::query::TyCtxtAt;
use crate::ty::normalize_erasing_regions::NormalizationError;
use crate::ty::{self, Ty, TyCtxt, TypeVisitableExt};
use hir::Mutability;
use rustc_error_messages::DiagMessage;
use rustc_errors::{
Diag, DiagArgValue, DiagCtxt, Diagnostic, EmissionGuarantee, IntoDiagArg, Level,
Expand Down Expand Up @@ -297,6 +298,8 @@ pub enum SizeSkeleton<'tcx> {
Pointer {
/// If true, this pointer is never null.
non_zero: bool,
/// Available if the width of the pointer is known, i.e. whether it's 1 or 2 usizes
known_size: Option<Size>,
/// The type which determines the unsized metadata, if any,
/// of this pointer. Either a type parameter or a projection
/// depending on one, with regions erased.
Expand Down Expand Up @@ -334,7 +337,23 @@ impl<'tcx> SizeSkeleton<'tcx> {
match tail.kind() {
ty::Param(_) | ty::Alias(ty::Projection | ty::Inherent, _) => {
debug_assert!(tail.has_non_region_param());
Ok(SizeSkeleton::Pointer { non_zero, tail: tcx.erase_regions(tail) })
Ok(SizeSkeleton::Pointer {
non_zero,
known_size: None,
tail: tcx.erase_regions(tail),
})
}
ty::Slice(_) => {
debug_assert!(tail.has_non_region_param());
// Assumption: all slice pointers have the same size. At most they differ in niches or or ptr/len ordering
let simple_slice =
Ty::new_ptr(tcx, Ty::new_slice(tcx, tcx.types.unit), Mutability::Not);
let size = tcx.layout_of(param_env.and(simple_slice)).unwrap().size;
Ok(SizeSkeleton::Pointer {
non_zero,
known_size: Some(size),
tail: tcx.erase_regions(tail),
})
}
_ => bug!(
"SizeSkeleton::compute({ty}): layout errored ({err:?}), yet \
Expand Down Expand Up @@ -407,7 +426,7 @@ impl<'tcx> SizeSkeleton<'tcx> {
let v0 = zero_or_ptr_variant(0)?;
// Newtype.
if def.variants().len() == 1 {
if let Some(SizeSkeleton::Pointer { non_zero, tail }) = v0 {
if let Some(SizeSkeleton::Pointer { non_zero, known_size, tail }) = v0 {
return Ok(SizeSkeleton::Pointer {
non_zero: non_zero
|| match tcx.layout_scalar_valid_range(def.did()) {
Expand All @@ -417,6 +436,7 @@ impl<'tcx> SizeSkeleton<'tcx> {
}
_ => false,
},
known_size,
tail,
});
} else {
Expand All @@ -427,9 +447,9 @@ impl<'tcx> SizeSkeleton<'tcx> {
let v1 = zero_or_ptr_variant(1)?;
// Nullable pointer enum optimization.
match (v0, v1) {
(Some(SizeSkeleton::Pointer { non_zero: true, tail }), None)
| (None, Some(SizeSkeleton::Pointer { non_zero: true, tail })) => {
Ok(SizeSkeleton::Pointer { non_zero: false, tail })
(Some(SizeSkeleton::Pointer { non_zero: true, known_size, tail }), None)
| (None, Some(SizeSkeleton::Pointer { non_zero: true, known_size, tail })) => {
Ok(SizeSkeleton::Pointer { non_zero: false, known_size, tail })
}
_ => Err(err),
}
Expand All @@ -450,7 +470,10 @@ impl<'tcx> SizeSkeleton<'tcx> {

pub fn same_size(self, other: SizeSkeleton<'tcx>) -> bool {
match (self, other) {
(SizeSkeleton::Known(a), SizeSkeleton::Known(b)) => a == b,
(
SizeSkeleton::Known(a) | SizeSkeleton::Pointer { known_size: Some(a), .. },
SizeSkeleton::Known(b) | SizeSkeleton::Pointer { known_size: Some(b), .. },
) => a == b,
(SizeSkeleton::Pointer { tail: a, .. }, SizeSkeleton::Pointer { tail: b, .. }) => {
a == b
}
Expand Down
196 changes: 192 additions & 4 deletions compiler/rustc_ty_utils/src/layout.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::layout::ty::ParamEnv;
use hir::def_id::DefId;
use rustc_hir as hir;
use rustc_index::bit_set::BitSet;
Expand All @@ -13,9 +14,12 @@ use rustc_session::{DataTypeKind, FieldInfo, FieldKind, SizeKind, VariantInfo};
use rustc_span::sym;
use rustc_span::symbol::Symbol;
use rustc_target::abi::*;
use rustc_type_ir::DynKind;

use std::cmp;
use std::fmt::Debug;
use std::iter;
use std::ops::ControlFlow;

use crate::errors::{
MultipleArrayFieldsSimdType, NonPrimitiveSimdType, OversizedSimdType, ZeroLengthSimdType,
Expand Down Expand Up @@ -123,7 +127,7 @@ fn layout_of_uncached<'tcx>(
};
debug_assert!(!ty.has_non_region_infer());

Ok(match *ty.kind() {
let layout = match *ty.kind() {
// Basic scalars.
ty::Bool => tcx.mk_layout(LayoutS::scalar(
cx,
Expand Down Expand Up @@ -201,10 +205,32 @@ fn layout_of_uncached<'tcx>(
return Ok(tcx.mk_layout(LayoutS::scalar(cx, data_ptr)));
}

let Abi::Scalar(metadata) = metadata_layout.abi else {
let Abi::Scalar(mut metadata) = metadata_layout.abi else {
return Err(error(cx, LayoutError::Unknown(pointee)));
};

if !ty.is_unsafe_ptr() && metadata_ty == tcx.types.usize {
let tail = tcx.struct_tail_erasing_lifetimes(pointee, param_env);
// // eprintln!("usize-meta {:?} {}", pointee, pointee_zst);
match tail.kind() {
ty::Slice(element) => match ty_is_non_zst(*element, param_env, tcx) {
NonZst::True => {
metadata.valid_range_mut().end =
dl.ptr_sized_integer().signed_max() as u128
}
NonZst::Unknown => return Err(error(cx, LayoutError::Unknown(ty))),
_ => {}
},
ty::Str => {
metadata.valid_range_mut().end =
dl.ptr_sized_integer().signed_max() as u128;
}
_ => {
eprint!("unexpected tail {:?}", tail);
}
}
}

metadata
} else {
let unsized_part = tcx.struct_tail_erasing_lifetimes(pointee, param_env);
Expand All @@ -213,7 +239,28 @@ fn layout_of_uncached<'tcx>(
ty::Foreign(..) => {
return Ok(tcx.mk_layout(LayoutS::scalar(cx, data_ptr)));
}
ty::Slice(_) | ty::Str => scalar_unit(Int(dl.ptr_sized_integer(), false)),
ty::Slice(element) => {
let mut metadata = scalar_unit(Int(dl.ptr_sized_integer(), false));
if !ty.is_unsafe_ptr() {
match ty_is_non_zst(*element, param_env, tcx) {
NonZst::True => {
metadata.valid_range_mut().end =
dl.ptr_sized_integer().signed_max() as u128
}
NonZst::Unknown => return Err(error(cx, LayoutError::Unknown(ty))),
_ => {}
}
}
metadata
}
ty::Str => {
let mut metadata = scalar_unit(Int(dl.ptr_sized_integer(), false));
if !ty.is_unsafe_ptr() {
metadata.valid_range_mut().end =
dl.ptr_sized_integer().signed_max() as u128;
}
metadata
}
ty::Dynamic(..) => {
let mut vtable = scalar_unit(Pointer(AddressSpace::DATA));
vtable.valid_range_mut().start = 1;
Expand Down Expand Up @@ -606,7 +653,148 @@ fn layout_of_uncached<'tcx>(
ty::Placeholder(..) | ty::Param(_) => {
return Err(error(cx, LayoutError::Unknown(ty)));
}
})
};

#[cfg(debug_assertions)]
if layout.is_sized() && !layout.abi.is_uninhabited() {
match (ty_is_non_zst(ty, param_env, tcx), layout.is_zst()) {
(NonZst::Unknown, _) => {
bug!("ZSTness should not be unknown at this point {:?} {:?}", ty, layout)
}
(NonZst::False | NonZst::Uninhabited, false) => {
bug!("{:?} is not a ZST but ty_is_non_zst() thinks it is", ty)
}
(NonZst::True, true) => bug!("{:?} is a ZST but ty_is_non_zst() thinks it isn't", ty),
_ => {}
}
}

Ok(layout)
}

fn ty_is_non_zst<'tcx>(ty: Ty<'tcx>, param_env: ParamEnv<'tcx>, tcx: TyCtxt<'tcx>) -> NonZst {
fn fold_fields<'tcx>(
mut it: impl Iterator<Item = Ty<'tcx>>,
param_env: ParamEnv<'tcx>,
tcx: TyCtxt<'tcx>,
) -> NonZst {
let (ControlFlow::Break(res) | ControlFlow::Continue(res)) =
it.try_fold(NonZst::False, |acc, ty| {
if acc == NonZst::True {
return ControlFlow::Break(acc);
}

ControlFlow::Continue(cmp::max(acc, ty_is_non_zst(ty, param_env, tcx)))
});

res
}

match ty.kind() {
ty::Infer(ty::IntVar(_) | ty::FloatVar(_))
| ty::Uint(_)
| ty::Int(_)
| ty::Bool
| ty::Float(_)
| ty::FnPtr(_)
| ty::RawPtr(..)
| ty::Dynamic(_, _, DynKind::DynStar)
| ty::Char
| ty::Ref(..) => NonZst::True,

ty::Closure(_, args) => fold_fields(args.as_closure().upvar_tys().iter(), param_env, tcx),
ty::Coroutine(_, _) => NonZst::True,
ty::CoroutineClosure(_, args) => {
fold_fields(args.as_coroutine_closure().upvar_tys().iter(), param_env, tcx)
}
ty::Array(ty, len) => {
let len = if len.has_projections() {
tcx.normalize_erasing_regions(param_env, *len)
} else {
*len
};

if let Some(len) = len.try_to_target_usize(tcx) {
if len == 0 {
return NonZst::False;
}
let element_zst = ty_is_non_zst(*ty, param_env, tcx);
if element_zst != NonZst::Unknown {
return element_zst;
}
}
NonZst::Unknown
}
ty::Tuple(tys) => fold_fields(tys.iter(), param_env, tcx),
ty::Adt(def, args) => {
if ty.is_enum() {
if def.variants().len() == 0 {
return NonZst::Uninhabited;
}
// An enum is !ZST if
// * it has a repr and at least one non-uninhabited variant
// * it has at least one variant with a !ZST payload
// * it has multiple variants that are not uninhabited

let min_empty_variants = if def.repr().inhibit_enum_layout_opt() { 1 } else { 2 };

// first check without recursing
let simple_variants = def.variants().iter().filter(|v| v.fields.len() == 0).count();
if simple_variants >= min_empty_variants {
return NonZst::True;
}

let mut inhabited_zst_variants = 0;
let mut unknown = false;

for variant in def.variants().iter().filter(|v| v.fields.len() != 0) {
let variant_sized =
fold_fields(variant.fields.iter().map(|f| f.ty(tcx, args)), param_env, tcx);

match variant_sized {
// enum E { A(!, u32) } counts as !ZST for our purposes
NonZst::True => return NonZst::True,
NonZst::False => inhabited_zst_variants += 1,
NonZst::Unknown => unknown = true,
NonZst::Uninhabited => {}
}
}

if simple_variants + inhabited_zst_variants >= min_empty_variants {
return NonZst::True;
}
if unknown {
return NonZst::Unknown;
}
if simple_variants + inhabited_zst_variants == 0 {
return NonZst::Uninhabited;
}

NonZst::False
} else {
fold_fields(def.all_fields().map(|f| f.ty(tcx, args)), param_env, tcx)
}
}
ty::FnDef(..) => NonZst::False,
ty::Never => NonZst::Uninhabited,
ty::Param(..) => NonZst::Unknown,
ty::Str => NonZst::True,
// treat unsized types as potentially-ZST
ty::Dynamic(..) | ty::Slice(..) => NonZst::False,
ty::Alias(..) => match tcx.try_normalize_erasing_regions(param_env, ty) {
Ok(ty) if !matches!(ty.kind(), ty::Alias(..)) => ty_is_non_zst(ty, param_env, tcx),
_ => NonZst::Unknown,
},
_ => bug!("is_non_zst not implemented for this kind {:?}", ty),
}
}

#[derive(Clone, Copy, PartialEq, Eq, Debug, PartialOrd, Ord)]
pub enum NonZst {
False,
Uninhabited,
Unknown,
True,
}

/// Overlap eligibility and variant assignment for each CoroutineSavedLocal.
Expand Down
Loading

0 comments on commit cbf560f

Please sign in to comment.