Skip to content

Commit

Permalink
layout computation: eagerly error for unexpected unsized fields
Browse files Browse the repository at this point in the history
  • Loading branch information
Lukas Markeffsky committed Sep 15, 2024
1 parent e0d1766 commit 10aabc4
Show file tree
Hide file tree
Showing 25 changed files with 1,224 additions and 1,093 deletions.
1,857 changes: 939 additions & 918 deletions compiler/rustc_abi/src/layout.rs

Large diffs are not rendered by default.

10 changes: 9 additions & 1 deletion compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ mod layout;
#[cfg(test)]
mod tests;

pub use layout::LayoutCalculator;
pub use layout::{LayoutCalculator, LayoutCalculatorError};

/// Requirements for a `StableHashingContext` to be used in this crate.
/// This is a hack to allow using the `HashStable_Generic` derive macro
Expand Down Expand Up @@ -393,6 +393,14 @@ impl HasDataLayout for TargetDataLayout {
}
}

// used by rust-analyzer
impl HasDataLayout for &TargetDataLayout {
#[inline]
fn data_layout(&self) -> &TargetDataLayout {
(**self).data_layout()
}
}

/// Endianness of the target, which must match cfg(target-endian).
#[derive(Copy, Clone, PartialEq, Eq)]
pub enum Endian {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/valtrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ pub fn valtree_to_const_value<'tcx>(
let branches = valtree.unwrap_branch();
// Find the non-ZST field. (There can be aligned ZST!)
for (i, &inner_valtree) in branches.iter().enumerate() {
let field = layout.field(&LayoutCx { tcx, param_env }, i);
let field = layout.field(&LayoutCx::new(tcx, param_env), i);
if !field.is_zst() {
return valtree_to_const_value(tcx, param_env.and(field.ty), inner_valtree);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
) -> Cow<'e, RangeSet> {
assert!(layout.ty.is_union());
assert!(layout.abi.is_sized(), "there are no unsized unions");
let layout_cx = LayoutCx { tcx: *ecx.tcx, param_env: ecx.param_env };
let layout_cx = LayoutCx::new(*ecx.tcx, ecx.param_env);
return M::cached_union_data_range(ecx, layout.ty, || {
let mut out = RangeSet(Vec::new());
union_data_range_uncached(&layout_cx, layout, Size::ZERO, &mut out);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use rustc_middle::bug;
use rustc_middle::ty::layout::{LayoutCx, LayoutError, LayoutOf, TyAndLayout, ValidityRequirement};
use rustc_middle::ty::layout::{
HasTyCtxt, LayoutCx, LayoutError, LayoutOf, TyAndLayout, ValidityRequirement,
};
use rustc_middle::ty::{ParamEnvAnd, Ty, TyCtxt};
use rustc_target::abi::{Abi, FieldsShape, Scalar, Variants};

Expand Down Expand Up @@ -30,7 +32,7 @@ pub fn check_validity_requirement<'tcx>(
return Ok(!layout.abi.is_uninhabited());
}

let layout_cx = LayoutCx { tcx, param_env: param_env_and_ty.param_env };
let layout_cx = LayoutCx::new(tcx, param_env_and_ty.param_env);
if kind == ValidityRequirement::Uninit || tcx.sess.opts.unstable_opts.strict_init_checks {
check_validity_requirement_strict(layout, &layout_cx, kind)
} else {
Expand All @@ -47,7 +49,7 @@ fn check_validity_requirement_strict<'tcx>(
) -> Result<bool, &'tcx LayoutError<'tcx>> {
let machine = CompileTimeMachine::new(CanAccessMutGlobal::No, CheckAlignment::Error);

let mut cx = InterpCx::new(cx.tcx, rustc_span::DUMMY_SP, cx.param_env, machine);
let mut cx = InterpCx::new(cx.tcx(), rustc_span::DUMMY_SP, cx.param_env, machine);

let allocated = cx
.allocate(ty, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap))
Expand Down
27 changes: 10 additions & 17 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::borrow::Cow;
use std::num::NonZero;
use std::ops::Bound;
use std::{cmp, fmt};
Expand Down Expand Up @@ -287,19 +286,13 @@ impl<'tcx> IntoDiagArg for LayoutError<'tcx> {

#[derive(Clone, Copy)]
pub struct LayoutCx<'tcx> {
pub tcx: TyCtxt<'tcx>,
pub calc: LayoutCalculator<TyCtxt<'tcx>>,
pub param_env: ty::ParamEnv<'tcx>,
}

impl<'tcx> LayoutCalculator for LayoutCx<'tcx> {
type TargetDataLayoutRef = &'tcx TargetDataLayout;

fn delayed_bug(&self, txt: impl Into<Cow<'static, str>>) {
self.tcx.dcx().delayed_bug(txt);
}

fn current_data_layout(&self) -> Self::TargetDataLayoutRef {
&self.tcx.data_layout
impl<'tcx> LayoutCx<'tcx> {
pub fn new(tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> Self {
Self { calc: LayoutCalculator::new(tcx), param_env }
}
}

Expand Down Expand Up @@ -576,25 +569,25 @@ impl<'tcx> HasParamEnv<'tcx> for LayoutCx<'tcx> {

impl<'tcx> HasDataLayout for LayoutCx<'tcx> {
fn data_layout(&self) -> &TargetDataLayout {
self.tcx.data_layout()
self.calc.cx.data_layout()
}
}

impl<'tcx> HasTargetSpec for LayoutCx<'tcx> {
fn target_spec(&self) -> &Target {
self.tcx.target_spec()
self.calc.cx.target_spec()
}
}

impl<'tcx> HasWasmCAbiOpt for LayoutCx<'tcx> {
fn wasm_c_abi_opt(&self) -> WasmCAbi {
self.tcx.wasm_c_abi_opt()
self.calc.cx.wasm_c_abi_opt()
}
}

impl<'tcx> HasTyCtxt<'tcx> for LayoutCx<'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx.tcx()
self.calc.cx
}
}

Expand Down Expand Up @@ -695,7 +688,7 @@ impl<'tcx> LayoutOfHelpers<'tcx> for LayoutCx<'tcx> {
_: Span,
_: Ty<'tcx>,
) -> &'tcx LayoutError<'tcx> {
self.tcx.arena.alloc(err)
self.tcx().arena.alloc(err)
}
}

Expand Down Expand Up @@ -1323,7 +1316,7 @@ impl<'tcx> TyCtxt<'tcx> {
where
I: Iterator<Item = (VariantIdx, FieldIdx)>,
{
let cx = LayoutCx { tcx: self, param_env };
let cx = LayoutCx::new(self, param_env);
let mut offset = Size::ZERO;

for (variant, field) in indices {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/layout_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn dump_layout_of(tcx: TyCtxt<'_>, item_def_id: LocalDefId, attr: &Attribute) {
}

Err(layout_error) => {
tcx.dcx().emit_fatal(Spanned { node: layout_error.into_diagnostic(), span });
tcx.dcx().emit_err(Spanned { node: layout_error.into_diagnostic(), span });
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_transmute/src/layout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub mod rustc {
use std::fmt::{self, Write};

use rustc_middle::mir::Mutability;
use rustc_middle::ty::layout::{LayoutCx, LayoutError};
use rustc_middle::ty::layout::{HasTyCtxt, LayoutCx, LayoutError};
use rustc_middle::ty::{self, Ty};
use rustc_target::abi::Layout;

Expand Down Expand Up @@ -128,7 +128,7 @@ pub mod rustc {
ty: Ty<'tcx>,
) -> Result<Layout<'tcx>, &'tcx LayoutError<'tcx>> {
use rustc_middle::ty::layout::LayoutOf;
let ty = cx.tcx.erase_regions(ty);
let ty = cx.tcx().erase_regions(ty);
cx.layout_of(ty).map(|tl| tl.layout)
}
}
12 changes: 6 additions & 6 deletions compiler/rustc_transmute/src/layout/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ pub(crate) mod rustc {
return Err(Err::TypeError(e));
}

let target = cx.tcx.data_layout();
let target = cx.data_layout();
let pointer_size = target.pointer_size;

match ty.kind() {
Expand Down Expand Up @@ -320,7 +320,7 @@ pub(crate) mod rustc {

// Computes the variant of a given index.
let layout_of_variant = |index, encoding: Option<TagEncoding<VariantIdx>>| {
let tag = cx.tcx.tag_for_variant((cx.tcx.erase_regions(ty), index));
let tag = cx.tcx().tag_for_variant((cx.tcx().erase_regions(ty), index));
let variant_def = Def::Variant(def.variant(index));
let variant_layout = ty_variant(cx, (ty, layout), index);
Self::from_variant(
Expand Down Expand Up @@ -417,7 +417,7 @@ pub(crate) mod rustc {
}
}
}
struct_tree = struct_tree.then(Self::from_tag(*tag, cx.tcx));
struct_tree = struct_tree.then(Self::from_tag(*tag, cx.tcx()));
}

// Append the fields, in memory order, to the layout.
Expand Down Expand Up @@ -509,12 +509,12 @@ pub(crate) mod rustc {
match layout.variants {
Variants::Single { index } => {
let field = &def.variant(index).fields[i];
field.ty(cx.tcx, args)
field.ty(cx.tcx(), args)
}
// Discriminant field for enums (where applicable).
Variants::Multiple { tag, .. } => {
assert_eq!(i.as_usize(), 0);
ty::layout::PrimitiveExt::to_ty(&tag.primitive(), cx.tcx)
ty::layout::PrimitiveExt::to_ty(&tag.primitive(), cx.tcx())
}
}
}
Expand All @@ -531,7 +531,7 @@ pub(crate) mod rustc {
(ty, layout): (Ty<'tcx>, Layout<'tcx>),
i: VariantIdx,
) -> Layout<'tcx> {
let ty = cx.tcx.erase_regions(ty);
let ty = cx.tcx().erase_regions(ty);
TyAndLayout { ty, layout }.for_variant(&cx, i).layout
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_transmute/src/maybe_transmutable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ mod rustc {
pub fn answer(self) -> Answer<<TyCtxt<'tcx> as QueryContext>::Ref> {
let Self { src, dst, assume, context } = self;

let layout_cx = LayoutCx { tcx: context, param_env: ParamEnv::reveal_all() };
let layout_cx = LayoutCx::new(context, ParamEnv::reveal_all());

// Convert `src` and `dst` from their rustc representations, to `Tree`-based
// representations.
Expand Down
48 changes: 26 additions & 22 deletions compiler/rustc_ty_utils/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ fn fn_abi_of_fn_ptr<'tcx>(
) -> Result<&'tcx FnAbi<'tcx, Ty<'tcx>>, &'tcx FnAbiError<'tcx>> {
let (param_env, (sig, extra_args)) = query.into_parts();

let cx = LayoutCx { tcx, param_env };
let cx = LayoutCx::new(tcx, param_env);
fn_abi_new_uncached(&cx, sig, extra_args, None, None, false)
}

Expand All @@ -347,7 +347,7 @@ fn fn_abi_of_instance<'tcx>(
instance.def.requires_caller_location(tcx).then(|| tcx.caller_location_ty());

fn_abi_new_uncached(
&LayoutCx { tcx, param_env },
&LayoutCx::new(tcx, param_env),
sig,
extra_args,
caller_location,
Expand Down Expand Up @@ -386,12 +386,14 @@ fn adjust_for_rust_scalar<'tcx>(
attrs.set(ArgAttribute::NonNull);
}

let tcx = cx.tcx();

if let Some(pointee) = layout.pointee_info_at(&cx, offset) {
let kind = if let Some(kind) = pointee.safe {
Some(kind)
} else if let Some(pointee) = drop_target_pointee {
// The argument to `drop_in_place` is semantically equivalent to a mutable reference.
Some(PointerKind::MutableRef { unpin: pointee.is_unpin(cx.tcx, cx.param_env()) })
Some(PointerKind::MutableRef { unpin: pointee.is_unpin(tcx, cx.param_env()) })
} else {
None
};
Expand All @@ -415,12 +417,12 @@ fn adjust_for_rust_scalar<'tcx>(
// The aliasing rules for `Box<T>` are still not decided, but currently we emit
// `noalias` for it. This can be turned off using an unstable flag.
// See https://github.com/rust-lang/unsafe-code-guidelines/issues/326
let noalias_for_box = cx.tcx.sess.opts.unstable_opts.box_noalias;
let noalias_for_box = tcx.sess.opts.unstable_opts.box_noalias;

// LLVM prior to version 12 had known miscompiles in the presence of noalias attributes
// (see #54878), so it was conditionally disabled, but we don't support earlier
// versions at all anymore. We still support turning it off using -Zmutable-noalias.
let noalias_mut_ref = cx.tcx.sess.opts.unstable_opts.mutable_noalias;
let noalias_mut_ref = tcx.sess.opts.unstable_opts.mutable_noalias;

// `&T` where `T` contains no `UnsafeCell<U>` is immutable, and can be marked as both
// `readonly` and `noalias`, as LLVM's definition of `noalias` is based solely on memory
Expand Down Expand Up @@ -458,6 +460,7 @@ fn fn_abi_sanity_check<'tcx>(
spec_abi: SpecAbi,
arg: &ArgAbi<'tcx, Ty<'tcx>>,
) {
let tcx = cx.tcx();
match &arg.mode {
PassMode::Ignore => {}
PassMode::Direct(_) => {
Expand All @@ -484,7 +487,7 @@ fn fn_abi_sanity_check<'tcx>(
// It needs to switch to something else before stabilization can happen.
// (See issue: https://github.com/rust-lang/rust/issues/117271)
assert!(
matches!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64")
matches!(&*tcx.sess.target.arch, "wasm32" | "wasm64")
|| matches!(spec_abi, SpecAbi::PtxKernel | SpecAbi::Unadjusted),
"`PassMode::Direct` for aggregates only allowed for \"unadjusted\" and \"ptx-kernel\" functions and on wasm\n\
Problematic type: {:#?}",
Expand Down Expand Up @@ -516,7 +519,7 @@ fn fn_abi_sanity_check<'tcx>(
// With metadata. Must be unsized and not on the stack.
assert!(arg.layout.is_unsized() && !on_stack);
// Also, must not be `extern` type.
let tail = cx.tcx.struct_tail_for_codegen(arg.layout.ty, cx.param_env());
let tail = tcx.struct_tail_for_codegen(arg.layout.ty, cx.param_env());
if matches!(tail.kind(), ty::Foreign(..)) {
// These types do not have metadata, so having `meta_attrs` is bogus.
// Conceptually, unsized arguments must be copied around, which requires dynamically
Expand Down Expand Up @@ -546,7 +549,8 @@ fn fn_abi_new_uncached<'tcx>(
// FIXME(eddyb) replace this with something typed, like an `enum`.
force_thin_self_ptr: bool,
) -> Result<&'tcx FnAbi<'tcx, Ty<'tcx>>, &'tcx FnAbiError<'tcx>> {
let sig = cx.tcx.normalize_erasing_late_bound_regions(cx.param_env, sig);
let tcx = cx.tcx();
let sig = tcx.normalize_erasing_late_bound_regions(cx.param_env, sig);

let conv = conv_from_spec_abi(cx.tcx(), sig.abi, sig.c_variadic);

Expand Down Expand Up @@ -576,7 +580,7 @@ fn fn_abi_new_uncached<'tcx>(
};

let is_drop_in_place =
fn_def_id.is_some_and(|def_id| cx.tcx.is_lang_item(def_id, LangItem::DropInPlace));
fn_def_id.is_some_and(|def_id| tcx.is_lang_item(def_id, LangItem::DropInPlace));

let arg_of = |ty: Ty<'tcx>, arg_idx: Option<usize>| -> Result<_, &'tcx FnAbiError<'tcx>> {
let span = tracing::debug_span!("arg_of");
Expand All @@ -588,8 +592,7 @@ fn fn_abi_new_uncached<'tcx>(
_ => bug!("argument to drop_in_place is not a raw ptr: {:?}", ty),
});

let layout =
cx.layout_of(ty).map_err(|err| &*cx.tcx.arena.alloc(FnAbiError::Layout(*err)))?;
let layout = cx.layout_of(ty).map_err(|err| &*tcx.arena.alloc(FnAbiError::Layout(*err)))?;
let layout = if force_thin_self_ptr && arg_idx == Some(0) {
// Don't pass the vtable, it's not an argument of the virtual fn.
// Instead, pass just the data pointer, but give it the type `*const/mut dyn Trait`
Expand Down Expand Up @@ -638,7 +641,7 @@ fn fn_abi_new_uncached<'tcx>(
fn_abi_adjust_for_abi(cx, &mut fn_abi, sig.abi, fn_def_id)?;
debug!("fn_abi_new_uncached = {:?}", fn_abi);
fn_abi_sanity_check(cx, &fn_abi, sig.abi);
Ok(cx.tcx.arena.alloc(fn_abi))
Ok(tcx.arena.alloc(fn_abi))
}

#[tracing::instrument(level = "trace", skip(cx))]
Expand Down Expand Up @@ -670,17 +673,18 @@ fn fn_abi_adjust_for_abi<'tcx>(
return Ok(());
}

let tcx = cx.tcx();

if abi == SpecAbi::Rust || abi == SpecAbi::RustCall || abi == SpecAbi::RustIntrinsic {
// Look up the deduced parameter attributes for this function, if we have its def ID and
// we're optimizing in non-incremental mode. We'll tag its parameters with those attributes
// as appropriate.
let deduced_param_attrs = if cx.tcx.sess.opts.optimize != OptLevel::No
&& cx.tcx.sess.opts.incremental.is_none()
{
fn_def_id.map(|fn_def_id| cx.tcx.deduced_param_attrs(fn_def_id)).unwrap_or_default()
} else {
&[]
};
let deduced_param_attrs =
if tcx.sess.opts.optimize != OptLevel::No && tcx.sess.opts.incremental.is_none() {
fn_def_id.map(|fn_def_id| tcx.deduced_param_attrs(fn_def_id)).unwrap_or_default()
} else {
&[]
};

let fixup = |arg: &mut ArgAbi<'tcx, Ty<'tcx>>, arg_idx: Option<usize>| {
if arg.is_ignore() {
Expand All @@ -689,7 +693,7 @@ fn fn_abi_adjust_for_abi<'tcx>(

// Avoid returning floats in x87 registers on x86 as loading and storing from x87
// registers will quiet signalling NaNs.
if cx.tcx.sess.target.arch == "x86"
if tcx.sess.target.arch == "x86"
&& arg_idx.is_none()
// Intrinsics themselves are not actual "real" functions, so theres no need to
// change their ABIs.
Expand Down Expand Up @@ -744,7 +748,7 @@ fn fn_abi_adjust_for_abi<'tcx>(
// that's how we connect up to LLVM and it's unstable
// anyway, we control all calls to it in libstd.
Abi::Vector { .. }
if abi != SpecAbi::RustIntrinsic && cx.tcx.sess.target.simd_types_indirect =>
if abi != SpecAbi::RustIntrinsic && tcx.sess.target.simd_types_indirect =>
{
arg.make_indirect();
return;
Expand Down Expand Up @@ -793,7 +797,7 @@ fn fn_abi_adjust_for_abi<'tcx>(
} else {
fn_abi
.adjust_for_foreign_abi(cx, abi)
.map_err(|err| &*cx.tcx.arena.alloc(FnAbiError::AdjustForForeignAbi(err)))?;
.map_err(|err| &*tcx.arena.alloc(FnAbiError::AdjustForForeignAbi(err)))?;
}

Ok(())
Expand Down
Loading

0 comments on commit 10aabc4

Please sign in to comment.