Skip to content

CTFE/Miri engine Pointer type overhaul #87123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Jul 17, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,20 +193,21 @@ pub(crate) fn codegen_const_value<'tcx>(
place.to_cvalue(fx)
}
}
Scalar::Ptr(ptr) => {
let alloc_kind = fx.tcx.get_global_alloc(ptr.alloc_id);
Scalar::Ptr(ptr, _size) => {
let (alloc_id, offset) = ptr.into_parts(); // we know the `offset` is relative
let alloc_kind = fx.tcx.get_global_alloc(alloc_id);
let base_addr = match alloc_kind {
Some(GlobalAlloc::Memory(alloc)) => {
let data_id = data_id_for_alloc_id(
&mut fx.constants_cx,
fx.module,
ptr.alloc_id,
alloc_id,
alloc.mutability,
);
let local_data_id =
fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
if fx.clif_comments.enabled() {
fx.add_comment(local_data_id, format!("{:?}", ptr.alloc_id));
fx.add_comment(local_data_id, format!("{:?}", alloc_id));
}
fx.bcx.ins().global_value(fx.pointer_type, local_data_id)
}
Expand All @@ -226,10 +227,10 @@ pub(crate) fn codegen_const_value<'tcx>(
}
fx.bcx.ins().global_value(fx.pointer_type, local_data_id)
}
None => bug!("missing allocation {:?}", ptr.alloc_id),
None => bug!("missing allocation {:?}", alloc_id),
};
let val = if ptr.offset.bytes() != 0 {
fx.bcx.ins().iadd_imm(base_addr, i64::try_from(ptr.offset.bytes()).unwrap())
let val = if offset.bytes() != 0 {
fx.bcx.ins().iadd_imm(base_addr, i64::try_from(offset.bytes()).unwrap())
} else {
base_addr
};
Expand Down Expand Up @@ -406,7 +407,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
let bytes = alloc.inspect_with_uninit_and_ptr_outside_interpreter(0..alloc.len()).to_vec();
data_ctx.define(bytes.into_boxed_slice());

for &(offset, (_tag, reloc)) in alloc.relocations().iter() {
for &(offset, alloc_id) in alloc.relocations().iter() {
let addend = {
let endianness = tcx.data_layout.endian;
let offset = offset.bytes() as usize;
Expand All @@ -417,7 +418,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
read_target_uint(endianness, bytes).unwrap()
};

let reloc_target_alloc = tcx.get_global_alloc(reloc).unwrap();
let reloc_target_alloc = tcx.get_global_alloc(alloc_id).unwrap();
let data_id = match reloc_target_alloc {
GlobalAlloc::Function(instance) => {
assert_eq!(addend, 0);
Expand All @@ -427,7 +428,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
continue;
}
GlobalAlloc::Memory(target_alloc) => {
data_id_for_alloc_id(cx, module, reloc, target_alloc.mutability)
data_id_for_alloc_id(cx, module, alloc_id, target_alloc.mutability)
}
GlobalAlloc::Static(def_id) => {
if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::THREAD_LOCAL)
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_codegen_llvm/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,16 +243,17 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
self.const_bitcast(llval, llty)
}
}
Scalar::Ptr(ptr) => {
let (base_addr, base_addr_space) = match self.tcx.global_alloc(ptr.alloc_id) {
Scalar::Ptr(ptr, _size) => {
let (alloc_id, offset) = ptr.into_parts();
let (base_addr, base_addr_space) = match self.tcx.global_alloc(alloc_id) {
GlobalAlloc::Memory(alloc) => {
let init = const_alloc_to_llvm(self, alloc);
let value = match alloc.mutability {
Mutability::Mut => self.static_addr_of_mut(init, alloc.align, None),
_ => self.static_addr_of(init, alloc.align, None),
};
if !self.sess().fewer_names() {
llvm::set_value_name(value, format!("{:?}", ptr.alloc_id).as_bytes());
llvm::set_value_name(value, format!("{:?}", alloc_id).as_bytes());
}
(value, AddressSpace::DATA)
}
Expand All @@ -269,7 +270,7 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
let llval = unsafe {
llvm::LLVMConstInBoundsGEP(
self.const_bitcast(base_addr, self.type_i8p_ext(base_addr_space)),
&self.const_usize(ptr.offset.bytes()),
&self.const_usize(offset.bytes()),
1,
)
};
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_codegen_llvm/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_codegen_ssa::traits::*;
use rustc_hir::def_id::DefId;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
use rustc_middle::mir::interpret::{
read_target_uint, Allocation, ErrorHandled, GlobalAlloc, Pointer,
read_target_uint, Allocation, ErrorHandled, GlobalAlloc, Pointer, Scalar as InterpScalar,
};
use rustc_middle::mir::mono::MonoItem;
use rustc_middle::ty::{self, Instance, Ty};
Expand All @@ -25,7 +25,7 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll
let pointer_size = dl.pointer_size.bytes() as usize;

let mut next_offset = 0;
for &(offset, ((), alloc_id)) in alloc.relocations().iter() {
for &(offset, alloc_id) in alloc.relocations().iter() {
let offset = offset.bytes();
assert_eq!(offset as usize as u64, offset);
let offset = offset as usize;
Expand Down Expand Up @@ -55,7 +55,10 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll
};

llvals.push(cx.scalar_to_backend(
Pointer::new(alloc_id, Size::from_bytes(ptr_offset)).into(),
InterpScalar::from_pointer(
Pointer::new(alloc_id, Size::from_bytes(ptr_offset)),
&cx.tcx,
),
&Scalar { value: Primitive::Pointer, valid_range: 0..=!0 },
cx.type_i8p_ext(address_space),
));
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
Abi::ScalarPair(ref a, _) => a,
_ => bug!("from_const: invalid ScalarPair layout: {:#?}", layout),
};
let a = Scalar::from(Pointer::new(
bx.tcx().create_memory_alloc(data),
Size::from_bytes(start),
));
let a = Scalar::from_pointer(
Pointer::new(bx.tcx().create_memory_alloc(data), Size::from_bytes(start)),
&bx.tcx(),
);
let a_llval = bx.scalar_to_backend(
a,
a_scalar,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
#![feature(iter_zip)]
#![feature(thread_local_const_init)]
#![feature(try_reserve)]
#![feature(nonzero_ops)]
#![recursion_limit = "512"]

#[macro_use]
Expand Down
61 changes: 26 additions & 35 deletions compiler/rustc_middle/src/mir/interpret/allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::ty;
/// module provides higher-level access.
#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, TyEncodable, TyDecodable)]
#[derive(HashStable)]
pub struct Allocation<Tag = (), Extra = ()> {
pub struct Allocation<Tag = AllocId, Extra = ()> {
/// The actual bytes of the allocation.
/// Note that the bytes of a pointer represent the offset of the pointer.
bytes: Vec<u8>,
Expand Down Expand Up @@ -154,25 +154,17 @@ impl<Tag> Allocation<Tag> {
}
}

impl Allocation<()> {
/// Add Tag and Extra fields
pub fn with_tags_and_extra<T, E>(
impl Allocation {
/// Convert Tag and add Extra fields
pub fn with_prov_and_extra<Tag, Extra>(
self,
mut tagger: impl FnMut(AllocId) -> T,
extra: E,
) -> Allocation<T, E> {
mut tagger: impl FnMut(AllocId) -> Tag,
extra: Extra,
) -> Allocation<Tag, Extra> {
Allocation {
bytes: self.bytes,
relocations: Relocations::from_presorted(
self.relocations
.iter()
// The allocations in the relocations (pointers stored *inside* this allocation)
// all get the base pointer tag.
.map(|&(offset, ((), alloc))| {
let tag = tagger(alloc);
(offset, (tag, alloc))
})
.collect(),
self.relocations.iter().map(|&(offset, tag)| (offset, tagger(tag))).collect(),
),
init_mask: self.init_mask,
align: self.align,
Expand Down Expand Up @@ -339,9 +331,9 @@ impl<Tag: Copy, Extra> Allocation<Tag, Extra> {
self.check_relocations(cx, range)?;
} else {
// Maybe a pointer.
if let Some(&(tag, alloc_id)) = self.relocations.get(&range.start) {
let ptr = Pointer::new_with_tag(alloc_id, Size::from_bytes(bits), tag);
return Ok(ScalarMaybeUninit::Scalar(ptr.into()));
if let Some(&prov) = self.relocations.get(&range.start) {
let ptr = Pointer::new(prov, Size::from_bytes(bits));
return Ok(ScalarMaybeUninit::from_pointer(ptr, cx));
}
}
// We don't. Just return the bits.
Expand Down Expand Up @@ -371,18 +363,21 @@ impl<Tag: Copy, Extra> Allocation<Tag, Extra> {
}
};

let bytes = match val.to_bits_or_ptr(range.size, cx) {
Err(val) => u128::from(val.offset.bytes()),
Ok(data) => data,
let (bytes, provenance) = match val.to_bits_or_ptr(range.size) {
Err(val) => {
let (provenance, offset) = val.into_parts();
(u128::from(offset.bytes()), Some(provenance))
}
Ok(data) => (data, None),
};

let endian = cx.data_layout().endian;
let dst = self.get_bytes_mut(cx, range);
write_target_uint(endian, dst, bytes).unwrap();

// See if we have to also write a relocation.
if let Scalar::Ptr(val) = val {
self.relocations.insert(range.start, (val.tag, val.alloc_id));
if let Some(provenance) = provenance {
self.relocations.insert(range.start, provenance);
}

Ok(())
Expand All @@ -392,11 +387,7 @@ impl<Tag: Copy, Extra> Allocation<Tag, Extra> {
/// Relocations.
impl<Tag: Copy, Extra> Allocation<Tag, Extra> {
/// Returns all relocations overlapping with the given pointer-offset pair.
pub fn get_relocations(
&self,
cx: &impl HasDataLayout,
range: AllocRange,
) -> &[(Size, (Tag, AllocId))] {
pub fn get_relocations(&self, cx: &impl HasDataLayout, range: AllocRange) -> &[(Size, Tag)] {
// We have to go back `pointer_size - 1` bytes, as that one would still overlap with
// the beginning of this range.
let start = range.start.bytes().saturating_sub(cx.data_layout().pointer_size.bytes() - 1);
Expand Down Expand Up @@ -582,24 +573,24 @@ impl<Tag, Extra> Allocation<Tag, Extra> {
}
}

/// Relocations.
/// "Relocations" stores the provenance information of pointers stored in memory.
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, TyEncodable, TyDecodable)]
pub struct Relocations<Tag = (), Id = AllocId>(SortedMap<Size, (Tag, Id)>);
pub struct Relocations<Tag = AllocId>(SortedMap<Size, Tag>);

impl<Tag, Id> Relocations<Tag, Id> {
impl<Tag> Relocations<Tag> {
pub fn new() -> Self {
Relocations(SortedMap::new())
}

// The caller must guarantee that the given relocations are already sorted
// by address and contain no duplicates.
pub fn from_presorted(r: Vec<(Size, (Tag, Id))>) -> Self {
pub fn from_presorted(r: Vec<(Size, Tag)>) -> Self {
Relocations(SortedMap::from_presorted_elements(r))
}
}

impl<Tag> Deref for Relocations<Tag> {
type Target = SortedMap<Size, (Tag, AllocId)>;
type Target = SortedMap<Size, Tag>;

fn deref(&self) -> &Self::Target {
&self.0
Expand All @@ -614,7 +605,7 @@ impl<Tag> DerefMut for Relocations<Tag> {

/// A partial, owned list of relocations to transfer into another allocation.
pub struct AllocationRelocations<Tag> {
relative_relocations: Vec<(Size, (Tag, AllocId))>,
relative_relocations: Vec<(Size, Tag)>,
}

impl<Tag: Copy, Extra> Allocation<Tag, Extra> {
Expand Down
28 changes: 15 additions & 13 deletions compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ impl fmt::Display for InvalidProgramInfo<'_> {
/// Details of why a pointer had to be in-bounds.
#[derive(Debug, Copy, Clone, TyEncodable, TyDecodable, HashStable)]
pub enum CheckInAllocMsg {
/// We are dereferencing a pointer (i.e., creating a place).
DerefTest,
/// We are access memory.
MemoryAccessTest,
/// We are doing pointer arithmetic.
Expand All @@ -186,6 +188,7 @@ impl fmt::Display for CheckInAllocMsg {
f,
"{}",
match *self {
CheckInAllocMsg::DerefTest => "dereferencing pointer failed: ",
CheckInAllocMsg::MemoryAccessTest => "memory access failed: ",
CheckInAllocMsg::PointerArithmeticTest => "pointer arithmetic failed: ",
CheckInAllocMsg::InboundsTest => "",
Expand Down Expand Up @@ -238,7 +241,9 @@ pub enum UndefinedBehaviorInfo<'tcx> {
PointerUseAfterFree(AllocId),
/// Used a pointer outside the bounds it is valid for.
PointerOutOfBounds {
ptr: Pointer,
alloc_id: AllocId,
offset: Size,
size: Size,
msg: CheckInAllocMsg,
allocation_size: Size,
},
Expand Down Expand Up @@ -307,19 +312,19 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
InvalidVtableAlignment(msg) => write!(f, "invalid vtable: alignment {}", msg),
UnterminatedCString(p) => write!(
f,
"reading a null-terminated string starting at {} with no null found before end of allocation",
"reading a null-terminated string starting at {:?} with no null found before end of allocation",
p,
),
PointerUseAfterFree(a) => {
write!(f, "pointer to {} was dereferenced after this allocation got freed", a)
}
PointerOutOfBounds { ptr, msg, allocation_size } => write!(
PointerOutOfBounds { alloc_id, offset, size, msg, allocation_size } => write!(
f,
"{}pointer must be in-bounds at offset {}, \
but is outside bounds of {} which has size {}",
"{}pointer must be in-bounds for {} bytes at offset {}, but {} has size {}",
msg,
ptr.offset.bytes(),
ptr.alloc_id,
size.bytes(),
offset.bytes(),
alloc_id,
allocation_size.bytes()
),
DanglingIntPointer(0, CheckInAllocMsg::InboundsTest) => {
Expand Down Expand Up @@ -348,13 +353,13 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
}
InvalidTag(val) => write!(f, "enum value has invalid tag: {}", val),
InvalidFunctionPointer(p) => {
write!(f, "using {} as function pointer but it does not point to a function", p)
write!(f, "using {:?} as function pointer but it does not point to a function", p)
}
InvalidStr(err) => write!(f, "this string is not valid UTF-8: {}", err),
InvalidUninitBytes(Some((alloc, access))) => write!(
f,
"reading {} byte{} of memory starting at {}, \
but {} byte{} {} uninitialized starting at {}, \
"reading {} byte{} of memory starting at {:?}, \
but {} byte{} {} uninitialized starting at {:?}, \
and this operation requires initialized memory",
access.access_size.bytes(),
pluralize!(access.access_size.bytes()),
Expand Down Expand Up @@ -392,8 +397,6 @@ pub enum UnsupportedOpInfo {
//
// The variants below are only reachable from CTFE/const prop, miri will never emit them.
//
/// Encountered raw bytes where we needed a pointer.
ReadBytesAsPointer,
/// Accessing thread local statics
ThreadLocalStatic(DefId),
/// Accessing an unsupported extern static.
Expand All @@ -408,7 +411,6 @@ impl fmt::Display for UnsupportedOpInfo {
ReadExternStatic(did) => write!(f, "cannot read from extern static ({:?})", did),
NoMirFor(did) => write!(f, "no MIR body is available for {:?}", did),
ReadPointerAsBytes => write!(f, "unable to turn pointer into raw bytes",),
ReadBytesAsPointer => write!(f, "unable to turn bytes into a pointer"),
ThreadLocalStatic(did) => write!(f, "cannot access thread local static ({:?})", did),
}
}
Expand Down
Loading