Skip to content

Commit efbd798

Browse files
committed
refactor PointeeInfo
Make `size`/`align` always correct rather than conditionally on the `safe` field. This makes it less error prone and easier to work with for `MaybeDangling` / potential future pointer kinds like `Aligned<_>`.
1 parent 4efbe0c commit efbd798

7 files changed

Lines changed: 125 additions & 105 deletions

File tree

compiler/rustc_abi/src/lib.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2145,22 +2145,22 @@ pub enum PointerKind {
21452145
}
21462146

21472147
/// Encodes extra information we have about a pointer.
2148+
///
21482149
/// Note that this information is advisory only, and backends are free to ignore it:
21492150
/// if the information is wrong, that can cause UB, but if the information is absent,
21502151
/// that must always be okay.
21512152
#[derive(Copy, Clone, Debug)]
21522153
pub struct PointeeInfo {
2153-
/// If this is `None`, then this is a raw pointer, so size and alignment are not guaranteed to
2154-
/// be reliable.
2154+
/// If this is `None`, then this is a raw pointer.
21552155
pub safe: Option<PointerKind>,
2156-
/// If `safe` is `Some`, then the pointer is either null or dereferenceable for this many bytes.
2156+
/// If `size` is not zero, then the pointer is either null or dereferenceable for this many bytes.
2157+
///
21572158
/// On a function argument, "dereferenceable" here means "dereferenceable for the entire duration
21582159
/// of this function call", i.e. it is UB for the memory that this pointer points to be freed
21592160
/// while this function is still running.
2160-
/// The size can be zero if the pointer is not dereferenceable.
21612161
pub size: Size,
2162-
/// If `safe` is `Some`, then the pointer is aligned as indicated.
2163-
pub align: Align,
2162+
/// If `Some`, then the pointer is aligned as indicated.
2163+
pub align: Option<Align>,
21642164
}
21652165

21662166
impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {

compiler/rustc_codegen_cranelift/src/base.rs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use rustc_data_structures::profiling::SelfProfilerRef;
1010
use rustc_index::IndexVec;
1111
use rustc_middle::ty::TypeVisitableExt;
1212
use rustc_middle::ty::adjustment::PointerCoercion;
13-
use rustc_middle::ty::layout::FnAbiOf;
13+
use rustc_middle::ty::layout::{FnAbiOf, HasTypingEnv as _};
1414
use rustc_middle::ty::print::with_no_trimmed_paths;
1515
use rustc_session::config::OutputFilenames;
1616
use rustc_span::Symbol;
@@ -924,19 +924,26 @@ fn codegen_stmt<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, cur_block: Block, stmt:
924924
count,
925925
}) => {
926926
let dst = codegen_operand(fx, dst);
927-
let pointee = dst
928-
.layout()
929-
.pointee_info_at(fx, rustc_abi::Size::ZERO)
930-
.expect("Expected pointer");
927+
928+
let &ty::RawPtr(pointee, _) = dst.layout().ty.kind() else {
929+
bug!("expected pointer")
930+
};
931+
let pointee_layout = fx
932+
.tcx
933+
.layout_of(fx.typing_env().as_query_input(pointee))
934+
.expect("expected pointee to have a layout");
935+
let elem_size: u64 = pointee_layout.layout.size().bytes();
936+
931937
let dst = dst.load_scalar(fx);
932938
let src = codegen_operand(fx, src).load_scalar(fx);
933939
let count = codegen_operand(fx, count).load_scalar(fx);
934-
let elem_size: u64 = pointee.size.bytes();
940+
935941
let bytes = if elem_size != 1 {
936942
fx.bcx.ins().imul_imm(count, elem_size as i64)
937943
} else {
938944
count
939945
};
946+
940947
fx.bcx.call_memcpy(fx.target_config, dst, src, bytes);
941948
}
942949
},

compiler/rustc_codegen_gcc/src/type_of.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,10 @@ impl<'tcx> LayoutGccExt<'tcx> for TyAndLayout<'tcx> {
288288
Float(f) => cx.type_from_float(f),
289289
Pointer(address_space) => {
290290
// If we know the alignment, pick something better than i8.
291-
let pointee = if let Some(pointee) = self.pointee_info_at(cx, offset) {
292-
cx.type_pointee_for_align(pointee.align)
291+
let pointee = if let Some(pointee) = self.pointee_info_at(cx, offset)
292+
&& let Some(align) = pointee.align
293+
{
294+
cx.type_pointee_for_align(align)
293295
} else {
294296
cx.type_i8()
295297
};

compiler/rustc_codegen_llvm/src/builder.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -714,9 +714,9 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
714714
}
715715

716716
if let Some(pointee) = layout.pointee_info_at(bx, offset)
717-
&& let Some(_) = pointee.safe
717+
&& let Some(align) = pointee.align
718718
{
719-
bx.align_metadata(load, pointee.align);
719+
bx.align_metadata(load, align);
720720
}
721721
}
722722
abi::Primitive::Float(_) => {}

compiler/rustc_codegen_ssa/src/mir/statement.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use rustc_middle::mir::{self, NonDivergingIntrinsic, StmtDebugInfo};
2-
use rustc_middle::span_bug;
2+
use rustc_middle::{bug, span_bug, ty};
33
use tracing::instrument;
44

55
use super::{FunctionCx, LocalRef};
@@ -77,15 +77,22 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
7777
let dst_val = self.codegen_operand(bx, dst);
7878
let src_val = self.codegen_operand(bx, src);
7979
let count = self.codegen_operand(bx, count).immediate();
80-
let pointee_layout = dst_val
81-
.layout
82-
.pointee_info_at(bx, rustc_abi::Size::ZERO)
83-
.expect("Expected pointer");
84-
let bytes = bx.mul(count, bx.const_usize(pointee_layout.size.bytes()));
8580

86-
let align = pointee_layout.align;
81+
let &ty::RawPtr(pointee, _) = dst_val.layout.ty.kind() else {
82+
bug!("expected pointer")
83+
};
84+
let pointee_layout = bx
85+
.tcx()
86+
.layout_of(bx.typing_env().as_query_input(pointee))
87+
.expect("expected pointee to have a layout");
88+
let elem_size = pointee_layout.layout.size().bytes();
89+
let bytes =
90+
if elem_size != 1 { bx.mul(count, bx.const_usize(elem_size)) } else { count };
91+
92+
let align = pointee_layout.layout.align.abi;
8793
let dst = dst_val.immediate();
8894
let src = src_val.immediate();
95+
8996
bx.memcpy(dst, align, src, align, bytes, crate::MemFlags::empty(), None);
9097
}
9198
mir::StatementKind::FakeRead(..)

compiler/rustc_middle/src/ty/layout.rs

Lines changed: 55 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,40 +1023,66 @@ where
10231023
let typing_env = cx.typing_env();
10241024

10251025
let pointee_info = match *this.ty.kind() {
1026-
ty::RawPtr(p_ty, _) if offset.bytes() == 0 => {
1027-
tcx.layout_of(typing_env.as_query_input(p_ty)).ok().map(|layout| PointeeInfo {
1028-
size: layout.size,
1029-
align: layout.align.abi,
1030-
safe: None,
1031-
})
1026+
ty::RawPtr(_, _) | ty::FnPtr(..) if offset.bytes() == 0 => {
1027+
Some(PointeeInfo { safe: None, size: Size::ZERO, align: None })
10321028
}
1033-
ty::FnPtr(..) if offset.bytes() == 0 => {
1034-
tcx.layout_of(typing_env.as_query_input(this.ty)).ok().map(|layout| PointeeInfo {
1035-
size: layout.size,
1036-
align: layout.align.abi,
1037-
safe: None,
1029+
ty::Ref(_, ty, mt) if offset.bytes() == 0 => {
1030+
tcx.layout_of(typing_env.as_query_input(ty)).ok().map(|layout| {
1031+
// Use conservative pointer kind if not optimizing. This saves us the
1032+
// Freeze/Unpin queries, and can save time in the codegen backend (noalias
1033+
// attributes in LLVM have compile-time cost even in unoptimized builds).
1034+
let optimize = tcx.sess.opts.optimize != OptLevel::No;
1035+
1036+
let size;
1037+
let kind = match mt {
1038+
hir::Mutability::Not => {
1039+
let frozen = optimize && ty.is_freeze(tcx, typing_env);
1040+
1041+
// Non-frozen shared references are not necessarily dereferenceable for the entire duration of the function
1042+
// (see <https://github.com/rust-lang/rust/pull/98017>)
1043+
// (if we had "dereferenceable on entry", we could support this)
1044+
size = if frozen { layout.size } else { Size::ZERO };
1045+
1046+
PointerKind::SharedRef { frozen }
1047+
}
1048+
hir::Mutability::Mut => {
1049+
let unpin = optimize
1050+
&& ty.is_unpin(tcx, typing_env)
1051+
&& ty.is_unsafe_unpin(tcx, typing_env);
1052+
1053+
// Mutable references to potentially self-referential types are not
1054+
// necessarily dereferenceable for the entire duration of the function
1055+
// (see <https://github.com/rust-lang/unsafe-code-guidelines/issues/381>)
1056+
// (if we had "dereferenceable on entry", we could support this)
1057+
size = if unpin { layout.size } else { Size::ZERO };
1058+
1059+
PointerKind::MutableRef { unpin }
1060+
}
1061+
};
1062+
PointeeInfo { safe: Some(kind), size, align: Some(layout.align.abi) }
10381063
})
10391064
}
1040-
ty::Ref(_, ty, mt) if offset.bytes() == 0 => {
1041-
// Use conservative pointer kind if not optimizing. This saves us the
1042-
// Freeze/Unpin queries, and can save time in the codegen backend (noalias
1043-
// attributes in LLVM have compile-time cost even in unoptimized builds).
1065+
1066+
ty::Adt(..)
1067+
if offset.bytes() == 0
1068+
&& let Some(pointee) = this.ty.boxed_ty() =>
1069+
{
10441070
let optimize = tcx.sess.opts.optimize != OptLevel::No;
1045-
let kind = match mt {
1046-
hir::Mutability::Not => {
1047-
PointerKind::SharedRef { frozen: optimize && ty.is_freeze(tcx, typing_env) }
1048-
}
1049-
hir::Mutability::Mut => PointerKind::MutableRef {
1071+
1072+
tcx.layout_of(typing_env.as_query_input(pointee)).ok().map(|layout| PointeeInfo {
1073+
safe: Some(PointerKind::Box {
10501074
unpin: optimize
1051-
&& ty.is_unpin(tcx, typing_env)
1052-
&& ty.is_unsafe_unpin(tcx, typing_env),
1053-
},
1054-
};
1075+
&& pointee.is_unpin(tcx, typing_env)
1076+
&& pointee.is_unsafe_unpin(tcx, typing_env),
1077+
global: this.ty.is_box_global(tcx),
1078+
}),
10551079

1056-
tcx.layout_of(typing_env.as_query_input(ty)).ok().map(|layout| PointeeInfo {
1057-
size: layout.size,
1058-
align: layout.align.abi,
1059-
safe: Some(kind),
1080+
// `Box` are not necessarily dereferenceable for the entire duration of the function as
1081+
// they can be deallocated at any time.
1082+
// (if we had "dereferenceable on entry", we could support this)
1083+
size: Size::ZERO,
1084+
1085+
align: Some(layout.align.abi),
10601086
})
10611087
}
10621088

@@ -1098,7 +1124,7 @@ where
10981124
}
10991125
}
11001126
Variants::Multiple { .. } => None,
1101-
_ => Some(this),
1127+
Variants::Empty | Variants::Single { .. } => Some(this),
11021128
};
11031129

11041130
if let Some(variant) = data_variant
@@ -1135,24 +1161,6 @@ where
11351161
}
11361162
}
11371163

1138-
// Fixup info for the first field of a `Box`. Recursive traversal will have found
1139-
// the raw pointer, so size and align are set to the boxed type, but `pointee.safe`
1140-
// will still be `None`.
1141-
if let Some(ref mut pointee) = result {
1142-
if offset.bytes() == 0
1143-
&& let Some(boxed_ty) = this.ty.boxed_ty()
1144-
{
1145-
debug_assert!(pointee.safe.is_none());
1146-
let optimize = tcx.sess.opts.optimize != OptLevel::No;
1147-
pointee.safe = Some(PointerKind::Box {
1148-
unpin: optimize
1149-
&& boxed_ty.is_unpin(tcx, typing_env)
1150-
&& boxed_ty.is_unsafe_unpin(tcx, typing_env),
1151-
global: this.ty.is_box_global(tcx),
1152-
});
1153-
}
1154-
}
1155-
11561164
result
11571165
}
11581166
};

compiler/rustc_ty_utils/src/abi.rs

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -302,42 +302,31 @@ fn arg_attrs_for_rust_scalar<'tcx>(
302302

303303
let tcx = cx.tcx();
304304

305-
if let Some(pointee) = layout.pointee_info_at(&cx, offset) {
306-
let kind = if let Some(kind) = pointee.safe {
307-
Some(kind)
308-
} else if let Some(pointee) = drop_target_pointee {
309-
assert_eq!(pointee, layout.ty.builtin_deref(true).unwrap());
310-
assert_eq!(offset, Size::ZERO);
311-
// The argument to `drop_in_place` is semantically equivalent to a mutable reference.
312-
let mutref = Ty::new_mut_ref(tcx, tcx.lifetimes.re_erased, pointee);
313-
let layout = cx.layout_of(mutref).unwrap();
314-
layout.pointee_info_at(&cx, offset).and_then(|pi| pi.safe)
315-
} else {
316-
None
305+
let drop_target_pointee_info = drop_target_pointee.and_then(|pointee| {
306+
assert_eq!(pointee, layout.ty.builtin_deref(true).unwrap());
307+
assert_eq!(offset, Size::ZERO);
308+
// The argument to `drop_in_place` is semantically equivalent to a mutable reference.
309+
let mutref = Ty::new_mut_ref(tcx, tcx.lifetimes.re_erased, pointee);
310+
let layout = cx.layout_of(mutref).unwrap();
311+
layout.pointee_info_at(&cx, offset)
312+
});
313+
314+
if let Some(pointee) = drop_target_pointee_info.or_else(|| layout.pointee_info_at(&cx, offset))
315+
{
316+
if let Some(align) = pointee.align {
317+
attrs.pointee_align = Some(align.min(cx.tcx().sess.target.max_reliable_alignment()));
318+
}
319+
320+
// LLVM dereferenceable attribute has unclear semantics on the return type,
321+
// they seem to be "dereferenceable until the end of the program", which is
322+
// generally, not valid for references. See
323+
// <https://rust-lang.zulipchat.com/#narrow/channel/136281-t-opsem/topic/LLVM.20dereferenceable.20on.20return.20type/with/563001493>
324+
attrs.pointee_size = match is_return {
325+
false => pointee.size,
326+
true => Size::ZERO,
317327
};
318-
if let Some(kind) = kind {
319-
attrs.pointee_align =
320-
Some(pointee.align.min(cx.tcx().sess.target.max_reliable_alignment()));
321-
322-
attrs.pointee_size = match kind {
323-
// LLVM dereferenceable attribute has unclear semantics on the return type,
324-
// they seem to be "dereferenceable until the end of the program", which is
325-
// generally, not valid for references. See
326-
// <https://rust-lang.zulipchat.com/#narrow/channel/136281-t-opsem/topic/LLVM.20dereferenceable.20on.20return.20type/with/563001493>
327-
_ if is_return => Size::ZERO,
328-
// `Box` are not necessarily dereferenceable for the entire duration of the function as
329-
// they can be deallocated at any time. Same for non-frozen shared references (see
330-
// <https://github.com/rust-lang/rust/pull/98017>), and for mutable references to
331-
// potentially self-referential types (see
332-
// <https://github.com/rust-lang/unsafe-code-guidelines/issues/381>). If LLVM had a way
333-
// to say "dereferenceable on entry" we could use it here.
334-
PointerKind::Box { .. }
335-
| PointerKind::SharedRef { frozen: false }
336-
| PointerKind::MutableRef { unpin: false } => Size::ZERO,
337-
PointerKind::SharedRef { frozen: true }
338-
| PointerKind::MutableRef { unpin: true } => pointee.size,
339-
};
340328

329+
if let Some(kind) = pointee.safe {
341330
// The aliasing rules for `Box<T>` are still not decided, but currently we emit
342331
// `noalias` for it. This can be turned off using an unstable flag.
343332
// See https://github.com/rust-lang/unsafe-code-guidelines/issues/326
@@ -554,7 +543,14 @@ fn fn_abi_new_uncached<'tcx>(
554543
};
555544

556545
Ok(ArgAbi::new(cx, layout, |scalar, offset| {
557-
arg_attrs_for_rust_scalar(*cx, scalar, layout, offset, is_return, drop_target_pointee)
546+
arg_attrs_for_rust_scalar(
547+
*cx,
548+
scalar,
549+
layout,
550+
offset,
551+
is_return,
552+
drop_target_pointee.filter(|_| offset == Size::ZERO),
553+
)
558554
}))
559555
};
560556

0 commit comments

Comments
 (0)