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 Oct 10, 2024
2 parents d0141af + 6671c90 commit 9a4ff20
Show file tree
Hide file tree
Showing 21 changed files with 339 additions and 75 deletions.
8 changes: 7 additions & 1 deletion compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4076,11 +4076,17 @@ mod size_asserts {
use rustc_data_structures::static_assert_size;

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))]
// tidy-alphabetical-start
static_assert_size!(ExprKind<'_>, 40);
static_assert_size!(FnDecl<'_>, 40);
static_assert_size!(ForeignItem<'_>, 88);
static_assert_size!(ForeignItemKind<'_>, 56);
Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_hir_typeck/src/intrinsicck.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use hir::HirId;
use rustc_abi::Primitive::Pointer;
use rustc_abi::Size;
use rustc_errors::codes::*;
use rustc_errors::struct_span_code_err;
use rustc_hir as hir;
Expand Down Expand Up @@ -88,8 +89,22 @@ 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, 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, _)) => {
if let Some(v) = u128::from(size.bytes()).checked_mul(8) {
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 @@ -7,6 +7,7 @@ use rustc_abi::{
Abi, AddressSpace, Align, FieldsShape, HasDataLayout, Integer, LayoutCalculator, LayoutS,
PointeeInfo, PointerKind, ReprOptions, Scalar, Size, TagEncoding, TargetDataLayout, Variants,
};
use rustc_ast::Mutability;
use rustc_error_messages::DiagMessage;
use rustc_errors::{
Diag, DiagArgValue, DiagCtxtHandle, Diagnostic, EmissionGuarantee, IntoDiagArg, Level,
Expand Down Expand Up @@ -326,6 +327,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 @@ -384,7 +387,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),
})
}
ty::Error(guar) => {
// Fixes ICE #124031
Expand Down Expand Up @@ -462,7 +481,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 @@ -472,6 +491,7 @@ impl<'tcx> SizeSkeleton<'tcx> {
}
_ => false,
},
known_size,
tail,
});
} else {
Expand All @@ -482,9 +502,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 @@ -505,7 +525,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
209 changes: 202 additions & 7 deletions compiler/rustc_ty_utils/src/layout.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::fmt::Debug;
use std::iter;
use std::ops::ControlFlow;
use std::{cmp, iter};

use hir::def_id::DefId;
use rustc_abi::Integer::{I8, I32};
Expand All @@ -18,12 +19,14 @@ use rustc_middle::ty::layout::{
};
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{
self, AdtDef, CoroutineArgsExt, EarlyBinder, GenericArgsRef, Ty, TyCtxt, TypeVisitableExt,
self, AdtDef, CoroutineArgsExt, EarlyBinder, GenericArgsRef, ParamEnv, Ty, TyCtxt,
TypeVisitableExt,
};
use rustc_session::{DataTypeKind, FieldInfo, FieldKind, SizeKind, VariantInfo};
use rustc_span::sym;
use rustc_span::symbol::Symbol;
use rustc_target::abi::{FIRST_VARIANT, FieldIdx, Layout, VariantIdx};
use rustc_type_ir::DynKind;
use tracing::{debug, instrument, trace};
use {rustc_abi as abi, rustc_hir as hir};

Expand Down Expand Up @@ -161,7 +164,7 @@ fn layout_of_uncached<'tcx>(
};
debug_assert!(!ty.has_non_region_infer());

Ok(match *ty.kind() {
let layout = match *ty.kind() {
ty::Pat(ty, pat) => {
let layout = cx.layout_of(ty)?.layout;
let mut layout = LayoutS::clone(&layout.0);
Expand Down Expand Up @@ -198,7 +201,6 @@ fn layout_of_uncached<'tcx>(
}
}
}

// Basic scalars.
ty::Bool => tcx.mk_layout(LayoutS::scalar(cx, Scalar::Initialized {
value: Int(I8, false),
Expand Down Expand Up @@ -269,10 +271,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_for_codegen(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_for_codegen(pointee, param_env);
Expand All @@ -281,7 +305,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 @@ -673,7 +718,157 @@ 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)
}
(n @ (NonZst::False | NonZst::Uninhabited), false) => {
bug!("{:?} is not a ZST but ty_is_non_zst() thinks it is NonZst::{:?}", ty, n)
}
(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::Pat(ty, _) => ty_is_non_zst(*ty, param_env, tcx),
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_aliases() {
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() {
// repr(C) enums can never be ZSTs or uninhabited.
// They must have at least one variant and even if the variant has a payload that is uninhabited,
// the tag is still there.
if def.repr().c() {
return NonZst::True;
}

if def.variants().len() == 0 {
return NonZst::Uninhabited;
}
// An enum is !ZST if
// * it has a repr(int) 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,
},
ty::Error(_) => NonZst::Unknown,
_ => bug!("is_non_zst not implemented for this kind {:?}", ty),
}
}

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

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

0 comments on commit 9a4ff20

Please sign in to comment.