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

Remove Allocation.runtime_mutability #50193

Closed
wants to merge 1 commit into from
Closed
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
1 change: 0 additions & 1 deletion src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,6 @@ impl<'a> HashStable<StableHashingContext<'a>> for mir::interpret::Allocation {
}
self.undef_mask.hash_stable(hcx, hasher);
self.align.hash_stable(hcx, hasher);
self.runtime_mutability.hash_stable(hcx, hasher);
}
}

Expand Down
6 changes: 0 additions & 6 deletions src/librustc/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use ty::{self, TyCtxt};
use ty::layout::{self, Align, HasDataLayout};
use middle::region;
use std::iter;
use syntax::ast::Mutability;
use rustc_serialize::{Encoder, Decoder, Decodable, Encodable};

#[derive(Clone, Debug, PartialEq)]
Expand Down Expand Up @@ -247,10 +246,6 @@ pub struct Allocation {
pub undef_mask: UndefMask,
/// The alignment of the allocation to detect unaligned reads.
pub align: Align,
/// Whether the allocation (of a static) should be put into mutable memory when translating
///
/// Only happens for `static mut` or `static` with interior mutability
pub runtime_mutability: Mutability,
}

impl Allocation {
Expand All @@ -262,7 +257,6 @@ impl Allocation {
relocations: BTreeMap::new(),
undef_mask,
align: Align::from_bytes(1, 1).unwrap(),
runtime_mutability: Mutability::Immutable,
}
}
}
Expand Down
10 changes: 1 addition & 9 deletions src/librustc_mir/interpret/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,7 @@ fn eval_body_and_ecx<'a, 'mir, 'tcx>(
layout.align,
None,
)?;
let internally_mutable = !layout.ty.is_freeze(tcx, param_env, mir.span);
let mutability = tcx.is_static(cid.instance.def_id());
let mutability = if mutability == Some(hir::Mutability::MutMutable) || internally_mutable {
Mutability::Mutable
} else {
Mutability::Immutable
};
let cleanup = StackPopCleanup::MarkStatic(mutability);
let cleanup = StackPopCleanup::MarkStatic;
let name = ty::tls::with(|tcx| tcx.item_path_str(cid.instance.def_id()));
let prom = cid.promoted.map_or(String::new(), |p| format!("::promoted[{:?}]", p));
trace!("const_eval: pushing stack frame for global: {}{}", name, prom);
Expand Down Expand Up @@ -315,7 +308,6 @@ impl<'mir, 'tcx> super::Machine<'mir, 'tcx> for CompileTimeEvaluator {
fn mark_static_initialized<'a>(
_mem: &mut Memory<'a, 'mir, 'tcx, Self>,
_id: AllocId,
_mutability: Mutability,
) -> EvalResult<'tcx, bool> {
Ok(false)
}
Expand Down
6 changes: 2 additions & 4 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use rustc::ty::maps::TyCtxtAt;
use rustc_data_structures::indexed_vec::{IndexVec, Idx};
use rustc::middle::const_val::FrameInfo;
use syntax::codemap::{self, Span};
use syntax::ast::Mutability;
use rustc::mir::interpret::{
GlobalId, Value, Pointer, PrimVal, PrimValKind,
EvalError, EvalResult, EvalErrorKind, MemoryPointer,
Expand Down Expand Up @@ -96,7 +95,7 @@ pub enum StackPopCleanup {
/// isn't modifyable afterwards in case of constants.
/// In case of `static mut`, mark the memory to ensure it's never marked as immutable through
/// references or deallocated
MarkStatic(Mutability),
MarkStatic,
/// A regular stackframe added due to a function call will need to get forwarded to the next
/// block
Goto(mir::BasicBlock),
Expand Down Expand Up @@ -440,12 +439,11 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
self.memory.cur_frame = self.cur_frame();
}
match frame.return_to_block {
StackPopCleanup::MarkStatic(mutable) => {
StackPopCleanup::MarkStatic => {
if let Place::Ptr { ptr, .. } = frame.return_place {
// FIXME: to_ptr()? might be too extreme here, static zsts might reach this under certain conditions
self.memory.mark_static_initialized(
ptr.to_ptr()?.alloc_id,
mutable,
)?
} else {
bug!("StackPopCleanup::MarkStatic on: {:?}", frame.return_place);
Expand Down
1 change: 0 additions & 1 deletion src/librustc_mir/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ pub trait Machine<'mir, 'tcx>: Sized {
fn mark_static_initialized<'a>(
_mem: &mut Memory<'a, 'mir, 'tcx, Self>,
_id: AllocId,
_mutability: Mutability,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This argument needs to stay for miri the tool

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the tool use it for?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Knowing whether the memory can be changed. The tool actually mutates statics.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tool shouldn't use mark_static_initialized at all though, since that only applies to compile time execution?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to do this because otherwise we deallocate the memory of the 42 in static FOO: &u32 = &42; after the computation is done.

) -> EvalResult<'tcx, bool>;

/// Called when requiring a pointer to a static. Non const eval can
Expand Down
17 changes: 5 additions & 12 deletions src/librustc_mir/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use std::{ptr, io};
use rustc::ty::Instance;
use rustc::ty::maps::TyCtxtAt;
use rustc::ty::layout::{self, Align, TargetDataLayout};
use syntax::ast::Mutability;

use rustc_data_structures::fx::{FxHashSet, FxHashMap};
use rustc::mir::interpret::{MemoryPointer, AllocId, Allocation, AccessKind, UndefMask, Value, Pointer,
Expand Down Expand Up @@ -91,7 +90,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
relocations: BTreeMap::new(),
undef_mask: UndefMask::new(size),
align,
runtime_mutability: Mutability::Immutable,
};
let id = self.tcx.interpret_interner.reserve();
M::add_lock(self, id);
Expand Down Expand Up @@ -499,29 +497,26 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
fn mark_inner_allocation_initialized(
&mut self,
alloc: AllocId,
mutability: Mutability,
) -> EvalResult<'tcx> {
match self.alloc_kind.get(&alloc) {
// do not go into statics
None => Ok(()),
// just locals and machine allocs
Some(_) => self.mark_static_initialized(alloc, mutability),
Some(_) => self.mark_static_initialized(alloc),
}
}

/// mark an allocation as static and initialized, either mutable or not
pub fn mark_static_initialized(
&mut self,
alloc_id: AllocId,
mutability: Mutability,
) -> EvalResult<'tcx> {
trace!(
"mark_static_initialized {:?}, mutability: {:?}",
"mark_static_initialized {:?}",
alloc_id,
mutability
);
// The machine handled it
if M::mark_static_initialized(self, alloc_id, mutability)? {
if M::mark_static_initialized(self, alloc_id)? {
return Ok(())
}
let alloc = self.alloc_map.remove(&alloc_id);
Expand All @@ -531,14 +526,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
Some(MemoryKind::Stack) => {},
}
let uninit = self.uninitialized_statics.remove(&alloc_id);
if let Some(mut alloc) = alloc.or(uninit) {
// ensure llvm knows not to put this into immutable memroy
alloc.runtime_mutability = mutability;
if let Some(alloc) = alloc.or(uninit) {
let alloc = self.tcx.intern_const_alloc(alloc);
self.tcx.interpret_interner.intern_at_reserved(alloc_id, alloc);
// recurse into inner allocations
for &alloc in alloc.relocations.values() {
self.mark_inner_allocation_initialized(alloc, mutability)?;
self.mark_inner_allocation_initialized(alloc)?;
}
} else {
bug!("no allocation found for {:?}", alloc_id);
Expand Down
2 changes: 0 additions & 2 deletions src/librustc_mir/interpret/traits.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use rustc::ty::{self, Ty};
use rustc::ty::layout::{Size, Align, LayoutOf};
use syntax::ast::Mutability;

use rustc::mir::interpret::{PrimVal, Value, MemoryPointer, EvalResult};
use super::{EvalContext, Machine};
Expand Down Expand Up @@ -53,7 +52,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {

self.memory.mark_static_initialized(
vtable.alloc_id,
Mutability::Immutable,
)?;

Ok(vtable)
Expand Down
7 changes: 1 addition & 6 deletions src/librustc_trans/mir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use common::{C_bytes, C_struct, C_uint_big, C_undef, C_usize};
use consts;
use type_of::LayoutLlvmExt;
use type_::Type;
use syntax::ast::Mutability;

use super::super::callee;
use super::FunctionCx;
Expand Down Expand Up @@ -57,11 +56,7 @@ pub fn primval_to_llvm(cx: &CodegenCx,
} else if let Some(alloc) = cx.tcx.interpret_interner
.get_alloc(ptr.alloc_id) {
let init = global_initializer(cx, alloc);
if alloc.runtime_mutability == Mutability::Mutable {
consts::addr_of_mut(cx, init, alloc.align, "byte_str")
} else {
consts::addr_of(cx, init, alloc.align, "byte_str")
}
consts::addr_of(cx, init, alloc.align, "byte_str")
} else {
bug!("missing allocation {:?}", ptr.alloc_id);
};
Expand Down