Skip to content
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

interpret: pass MemoryKind to adjust_alloc_base_pointer #124030

Merged
merged 2 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
35 changes: 21 additions & 14 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,28 +288,19 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
}

/// Return the `AllocId` for the given thread-local static in the current thread.
fn thread_local_static_base_pointer(
fn thread_local_static_pointer(
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
def_id: DefId,
) -> InterpResult<'tcx, Pointer<Self::Provenance>> {
throw_unsup!(ThreadLocalStatic(def_id))
}

/// Return the root pointer for the given `extern static`.
fn extern_static_base_pointer(
/// Return the `AllocId` for the given `extern static`.
fn extern_static_pointer(
ecx: &InterpCx<'mir, 'tcx, Self>,
def_id: DefId,
) -> InterpResult<'tcx, Pointer<Self::Provenance>>;

/// Return a "base" pointer for the given allocation: the one that is used for direct
/// accesses to this static/const/fn allocation, or the one returned from the heap allocator.
///
/// Not called on `extern` or thread-local statics (those use the methods above).
fn adjust_alloc_base_pointer(
ecx: &InterpCx<'mir, 'tcx, Self>,
ptr: Pointer,
) -> InterpResult<'tcx, Pointer<Self::Provenance>>;

/// "Int-to-pointer cast"
fn ptr_from_addr_cast(
ecx: &InterpCx<'mir, 'tcx, Self>,
Expand All @@ -336,6 +327,8 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {

/// Called to adjust allocations to the Provenance and AllocExtra of this machine.
///
/// If `alloc` contains pointers, then they are all pointing to globals.
///
/// The way we construct allocations is to always first construct it without extra and then add
/// the extra. This keeps uniform code paths for handling both allocations created by CTFE for
/// globals, and allocations created by Miri during evaluation.
Expand All @@ -354,6 +347,19 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized {
kind: Option<MemoryKind<Self::MemoryKind>>,
) -> InterpResult<'tcx, Cow<'b, Allocation<Self::Provenance, Self::AllocExtra, Self::Bytes>>>;

/// Return a "root" pointer for the given allocation: the one that is used for direct
/// accesses to this static/const/fn allocation, or the one returned from the heap allocator.
///
/// Not called on `extern` or thread-local statics (those use the methods above).
///
/// `kind` is the kind of the allocation the pointer points to; it can be `None` when
/// it's a global and `GLOBAL_KIND` is `None`.
fn adjust_alloc_root_pointer(
ecx: &InterpCx<'mir, 'tcx, Self>,
ptr: Pointer,
kind: Option<MemoryKind<Self::MemoryKind>>,
) -> InterpResult<'tcx, Pointer<Self::Provenance>>;

/// Evaluate the inline assembly.
///
/// This should take care of jumping to the next block (one of `targets`) when asm goto
Expand Down Expand Up @@ -592,7 +598,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
Ok(alloc)
}

fn extern_static_base_pointer(
fn extern_static_pointer(
ecx: &InterpCx<$mir, $tcx, Self>,
def_id: DefId,
) -> InterpResult<$tcx, Pointer> {
Expand All @@ -601,9 +607,10 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
}

#[inline(always)]
fn adjust_alloc_base_pointer(
fn adjust_alloc_root_pointer(
_ecx: &InterpCx<$mir, $tcx, Self>,
ptr: Pointer<CtfeProvenance>,
_kind: Option<MemoryKind<Self::MemoryKind>>,
) -> InterpResult<$tcx, Pointer<CtfeProvenance>> {
Ok(ptr)
}
Expand Down
18 changes: 12 additions & 6 deletions compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
///
/// This function can fail only if `ptr` points to an `extern static`.
#[inline]
pub fn global_base_pointer(
pub fn global_root_pointer(
&self,
ptr: Pointer<CtfeProvenance>,
) -> InterpResult<'tcx, Pointer<M::Provenance>> {
Expand All @@ -178,12 +178,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
bug!("global memory cannot point to thread-local static")
}
Some(GlobalAlloc::Static(def_id)) if self.tcx.is_foreign_item(def_id) => {
return M::extern_static_base_pointer(self, def_id);
return M::extern_static_pointer(self, def_id);
}
None => {
assert!(
self.memory.extra_fn_ptr_map.contains_key(&alloc_id),
"{alloc_id:?} is neither global nor a function pointer"
);
}
_ => {}
}
// And we need to get the provenance.
M::adjust_alloc_base_pointer(self, ptr)
M::adjust_alloc_root_pointer(self, ptr, M::GLOBAL_KIND.map(MemoryKind::Machine))
}

pub fn fn_ptr(&mut self, fn_val: FnVal<'tcx, M::ExtraFnVal>) -> Pointer<M::Provenance> {
Expand All @@ -197,9 +203,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
id
}
};
// Functions are global allocations, so make sure we get the right base pointer.
// Functions are global allocations, so make sure we get the right root pointer.
// We know this is not an `extern static` so this cannot fail.
self.global_base_pointer(Pointer::from(id)).unwrap()
self.global_root_pointer(Pointer::from(id)).unwrap()
}

pub fn allocate_ptr(
Expand Down Expand Up @@ -240,7 +246,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
);
let alloc = M::adjust_allocation(self, id, Cow::Owned(alloc), Some(kind))?;
self.memory.alloc_map.insert(id, (kind, alloc.into_owned()));
M::adjust_alloc_base_pointer(self, Pointer::from(id))
M::adjust_alloc_root_pointer(self, Pointer::from(id), Some(kind))
}

pub fn reallocate_ptr(
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -764,15 +764,15 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Other cases need layout.
let adjust_scalar = |scalar| -> InterpResult<'tcx, _> {
Ok(match scalar {
Scalar::Ptr(ptr, size) => Scalar::Ptr(self.global_base_pointer(ptr)?, size),
Scalar::Ptr(ptr, size) => Scalar::Ptr(self.global_root_pointer(ptr)?, size),
Scalar::Int(int) => Scalar::Int(int),
})
};
let layout = from_known_layout(self.tcx, self.param_env, layout, || self.layout_of(ty))?;
let imm = match val_val {
mir::ConstValue::Indirect { alloc_id, offset } => {
// This is const data, no mutation allowed.
let ptr = self.global_base_pointer(Pointer::new(
let ptr = self.global_root_pointer(Pointer::new(
CtfeProvenance::from(alloc_id).as_immutable(),
offset,
))?;
Expand All @@ -784,7 +784,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// This is const data, no mutation allowed.
let alloc_id = self.tcx.reserve_and_set_memory_alloc(data);
let ptr = Pointer::new(CtfeProvenance::from(alloc_id).as_immutable(), Size::ZERO);
Immediate::new_slice(self.global_base_pointer(ptr)?.into(), meta, self)
Immediate::new_slice(self.global_root_pointer(ptr)?.into(), meta, self)
}
};
Ok(OpTy { op: Operand::Immediate(imm), layout })
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,7 @@ where
) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
// This must be an allocation in `tcx`
let _ = self.tcx.global_alloc(raw.alloc_id);
let ptr = self.global_base_pointer(Pointer::from(raw.alloc_id))?;
let ptr = self.global_root_pointer(Pointer::from(raw.alloc_id))?;
let layout = self.layout_of(raw.ty)?;
Ok(self.ptr_to_mplace(ptr.into(), layout))
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
use rustc_middle::mir::Rvalue::*;
match *rvalue {
ThreadLocalRef(did) => {
let ptr = M::thread_local_static_base_pointer(self, did)?;
let ptr = M::thread_local_static_pointer(self, did)?;
self.write_pointer(ptr, &dest)?;
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
ensure_monomorphic_enough(*self.tcx, poly_trait_ref)?;

let vtable_symbolic_allocation = self.tcx.reserve_and_set_vtable_alloc(ty, poly_trait_ref);
let vtable_ptr = self.global_base_pointer(Pointer::from(vtable_symbolic_allocation))?;
let vtable_ptr = self.global_root_pointer(Pointer::from(vtable_symbolic_allocation))?;
Ok(vtable_ptr.into())
}

Expand Down
15 changes: 10 additions & 5 deletions src/tools/miri/src/alloc_addresses/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,11 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
}

fn addr_from_alloc_id(&self, alloc_id: AllocId) -> InterpResult<'tcx, u64> {
fn addr_from_alloc_id(
&self,
alloc_id: AllocId,
_kind: MemoryKind,
) -> InterpResult<'tcx, u64> {
let ecx = self.eval_context_ref();
let mut global_state = ecx.machine.alloc_addresses.borrow_mut();
let global_state = &mut *global_state;
Expand Down Expand Up @@ -283,16 +287,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}

/// Convert a relative (tcx) pointer to a Miri pointer.
fn ptr_from_rel_ptr(
fn adjust_alloc_root_pointer(
&self,
ptr: Pointer<CtfeProvenance>,
tag: BorTag,
kind: MemoryKind,
) -> InterpResult<'tcx, Pointer<Provenance>> {
let ecx = self.eval_context_ref();

let (prov, offset) = ptr.into_parts(); // offset is relative (AllocId provenance)
let alloc_id = prov.alloc_id();
let base_addr = ecx.addr_from_alloc_id(alloc_id)?;
let base_addr = ecx.addr_from_alloc_id(alloc_id, kind)?;

// Add offset with the right kind of pointer-overflowing arithmetic.
let dl = ecx.data_layout();
Expand All @@ -314,9 +319,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
ecx.alloc_id_from_addr(addr.bytes())?
};

// This cannot fail: since we already have a pointer with that provenance, rel_ptr_to_addr
// This cannot fail: since we already have a pointer with that provenance, adjust_alloc_root_pointer
// must have been called in the past, so we can just look up the address in the map.
let base_addr = ecx.addr_from_alloc_id(alloc_id).unwrap();
let base_addr = *ecx.machine.alloc_addresses.borrow().base_addr.get(&alloc_id).unwrap();

// Wrapping "addr - base_addr"
#[allow(clippy::cast_possible_wrap)] // we want to wrap here
Expand Down
18 changes: 9 additions & 9 deletions src/tools/miri/src/borrow_tracker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ pub struct GlobalStateInner {
borrow_tracker_method: BorrowTrackerMethod,
/// Next unused pointer ID (tag).
next_ptr_tag: BorTag,
/// Table storing the "base" tag for each allocation.
/// The base tag is the one used for the initial pointer.
/// Table storing the "root" tag for each allocation.
/// The root tag is the one used for the initial pointer.
/// We need this in a separate table to handle cyclic statics.
base_ptr_tags: FxHashMap<AllocId, BorTag>,
root_ptr_tags: FxHashMap<AllocId, BorTag>,
/// Next unused call ID (for protectors).
next_call_id: CallId,
/// All currently protected tags.
Expand Down Expand Up @@ -175,7 +175,7 @@ impl GlobalStateInner {
GlobalStateInner {
borrow_tracker_method,
next_ptr_tag: BorTag::one(),
base_ptr_tags: FxHashMap::default(),
root_ptr_tags: FxHashMap::default(),
next_call_id: NonZero::new(1).unwrap(),
protected_tags: FxHashMap::default(),
tracked_pointer_tags,
Expand Down Expand Up @@ -213,8 +213,8 @@ impl GlobalStateInner {
}
}

pub fn base_ptr_tag(&mut self, id: AllocId, machine: &MiriMachine<'_, '_>) -> BorTag {
self.base_ptr_tags.get(&id).copied().unwrap_or_else(|| {
pub fn root_ptr_tag(&mut self, id: AllocId, machine: &MiriMachine<'_, '_>) -> BorTag {
self.root_ptr_tags.get(&id).copied().unwrap_or_else(|| {
let tag = self.new_ptr();
if self.tracked_pointer_tags.contains(&tag) {
machine.emit_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(
Expand All @@ -223,14 +223,14 @@ impl GlobalStateInner {
None,
));
}
trace!("New allocation {:?} has base tag {:?}", id, tag);
self.base_ptr_tags.try_insert(id, tag).unwrap();
trace!("New allocation {:?} has rpot tag {:?}", id, tag);
self.root_ptr_tags.try_insert(id, tag).unwrap();
tag
})
}

pub fn remove_unreachable_allocs(&mut self, allocs: &LiveAllocs<'_, '_, '_>) {
self.base_ptr_tags.retain(|id, _| allocs.is_live(*id));
self.root_ptr_tags.retain(|id, _| allocs.is_live(*id));
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn err_sb_ub<'tcx>(
#[derive(Clone, Debug)]
pub struct AllocHistory {
id: AllocId,
base: (Item, Span),
root: (Item, Span),
creations: smallvec::SmallVec<[Creation; 1]>,
invalidations: smallvec::SmallVec<[Invalidation; 1]>,
protectors: smallvec::SmallVec<[Protection; 1]>,
Expand Down Expand Up @@ -225,7 +225,7 @@ impl AllocHistory {
pub fn new(id: AllocId, item: Item, machine: &MiriMachine<'_, '_>) -> Self {
Self {
id,
base: (item, machine.current_span()),
root: (item, machine.current_span()),
creations: SmallVec::new(),
invalidations: SmallVec::new(),
protectors: SmallVec::new(),
Expand Down Expand Up @@ -342,15 +342,15 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> {
})
})
.or_else(|| {
// If we didn't find a retag that created this tag, it might be the base tag of
// If we didn't find a retag that created this tag, it might be the root tag of
// this allocation.
if self.history.base.0.tag() == tag {
if self.history.root.0.tag() == tag {
Some((
format!(
"{tag:?} was created here, as the base tag for {:?}",
"{tag:?} was created here, as the root tag for {:?}",
self.history.id
),
self.history.base.1.data(),
self.history.root.1.data(),
))
} else {
None
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,9 +518,9 @@ impl Stacks {
// not through a pointer). That is, whenever we directly write to a local, this will pop
// everything else off the stack, invalidating all previous pointers,
// and in particular, *all* raw pointers.
MemoryKind::Stack => (state.base_ptr_tag(id, machine), Permission::Unique),
MemoryKind::Stack => (state.root_ptr_tag(id, machine), Permission::Unique),
// Everything else is shared by default.
_ => (state.base_ptr_tag(id, machine), Permission::SharedReadWrite),
_ => (state.root_ptr_tag(id, machine), Permission::SharedReadWrite),
};
Stacks::new(size, perm, base_tag, id, machine)
}
Expand Down
10 changes: 5 additions & 5 deletions src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl Stack {
let mut first_removed = None;

// We never consider removing the bottom-most tag. For stacks without an unknown
// bottom this preserves the base tag.
// bottom this preserves the root tag.
// Note that the algorithm below is based on considering the tag at read_idx - 1,
// so precisely considering the tag at index 0 for removal when we have an unknown
// bottom would complicate the implementation. The simplification of not considering
Expand Down Expand Up @@ -93,7 +93,7 @@ impl Stack {
self.unique_range = 0..self.len();
}

// Replace any Items which have been collected with the base item, a known-good value.
// Replace any Items which have been collected with the root item, a known-good value.
for i in 0..CACHE_LEN {
if self.cache.idx[i] >= first_removed {
self.cache.items[i] = self.borrows[0];
Expand Down Expand Up @@ -331,7 +331,7 @@ impl<'tcx> Stack {
self.verify_cache_consistency();
}

/// Construct a new `Stack` using the passed `Item` as the base tag.
/// Construct a new `Stack` using the passed `Item` as the root tag.
pub fn new(item: Item) -> Self {
Stack {
borrows: vec![item],
Expand Down Expand Up @@ -438,8 +438,8 @@ impl<'tcx> Stack {
let mut removed = 0;
let mut cursor = 0;
// Remove invalid entries from the cache by rotating them to the end of the cache, then
// keep track of how many invalid elements there are and overwrite them with the base tag.
// The base tag here serves as a harmless default value.
// keep track of how many invalid elements there are and overwrite them with the root tag.
// The root tag here serves as a harmless default value.
for _ in 0..CACHE_LEN - 1 {
if self.cache.idx[cursor] >= start {
self.cache.idx[cursor..CACHE_LEN - removed].rotate_left(1);
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl<'tcx> Tree {
_kind: MemoryKind,
machine: &MiriMachine<'_, 'tcx>,
) -> Self {
let tag = state.base_ptr_tag(id, machine); // Fresh tag for the root
let tag = state.root_ptr_tag(id, machine); // Fresh tag for the root
let span = machine.current_span();
Tree::new(tag, size, span)
}
Expand Down
Loading
Loading