Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 4e88b73

Browse files
committedOct 29, 2018
Auto merge of #55270 - RalfJung:stacked-borrows-ng, r=oli-obk
miri engine: Stacked Borrows NG For more refined tracking in miri, we do return untagged pointers from the memory abstraction after allocations and let the caller decide how to tag these. Also refactor the `tag_(de)reference` hooks so they can be more easily called in the ref-to-place and place-to-ref methods, and reorder things in validation: validation calls ref-to-place which (when running in miri) triggers some checks, so we want to run it rather late and catch other problems first. We also do not need to redundantly check the ref to be allocated any more, the checks miri does anyway imply thath. r? @oli-obk
2 parents bcb05a0 + 95b19bb commit 4e88b73

File tree

9 files changed

+174
-130
lines changed

9 files changed

+174
-130
lines changed
 

‎src/librustc_mir/const_eval.rs

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ use rustc::hir::{self, def_id::DefId};
2020
use rustc::hir::def::Def;
2121
use rustc::mir::interpret::{ConstEvalErr, ErrorHandled};
2222
use rustc::mir;
23-
use rustc::ty::{self, Ty, TyCtxt, Instance, query::TyCtxtAt};
24-
use rustc::ty::layout::{self, Size, LayoutOf, TyLayout};
23+
use rustc::ty::{self, TyCtxt, Instance, query::TyCtxtAt};
24+
use rustc::ty::layout::{self, LayoutOf, TyLayout};
2525
use rustc::ty::subst::Subst;
2626
use rustc::traits::Reveal;
2727
use rustc_data_structures::indexed_vec::IndexVec;
@@ -32,7 +32,7 @@ use syntax::ast::Mutability;
3232
use syntax::source_map::{Span, DUMMY_SP};
3333

3434
use interpret::{self,
35-
PlaceTy, MemPlace, OpTy, Operand, Value, Pointer, Scalar, ConstValue,
35+
PlaceTy, MemPlace, OpTy, Operand, Value, Scalar, ConstValue, Pointer,
3636
EvalResult, EvalError, EvalErrorKind, GlobalId, EvalContext, StackPopCleanup,
3737
Allocation, AllocId, MemoryKind,
3838
snapshot, RefTracking,
@@ -426,7 +426,7 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx>
426426
}
427427

428428
#[inline(always)]
429-
fn static_with_default_tag(
429+
fn adjust_static_allocation(
430430
alloc: &'_ Allocation
431431
) -> Cow<'_, Allocation<Self::PointerTag>> {
432432
// We do not use a tag so we can just cheaply forward the reference
@@ -467,23 +467,12 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx>
467467
}
468468

469469
#[inline(always)]
470-
fn tag_reference(
470+
fn tag_new_allocation(
471471
_ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,
472-
_ptr: Pointer<Self::PointerTag>,
473-
_pointee_ty: Ty<'tcx>,
474-
_pointee_size: Size,
475-
_borrow_kind: Option<mir::BorrowKind>,
476-
) -> EvalResult<'tcx, Self::PointerTag> {
477-
Ok(())
478-
}
479-
480-
#[inline(always)]
481-
fn tag_dereference(
482-
_ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
483-
_ptr: Pointer<Self::PointerTag>,
484-
_ptr_ty: Ty<'tcx>,
485-
) -> EvalResult<'tcx, Self::PointerTag> {
486-
Ok(())
472+
ptr: Pointer,
473+
_kind: MemoryKind<Self::MemoryKinds>,
474+
) -> EvalResult<'tcx, Pointer> {
475+
Ok(ptr)
487476
}
488477
}
489478

‎src/librustc_mir/interpret/cast.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
110110
def_id,
111111
substs,
112112
).ok_or_else(|| EvalErrorKind::TooGeneric.into());
113-
let fn_ptr = self.memory.create_fn_alloc(instance?);
113+
let fn_ptr = self.memory.create_fn_alloc(instance?).with_default_tag();
114114
self.write_scalar(Scalar::Ptr(fn_ptr.into()), dest)?;
115115
}
116116
ref other => bug!("reify fn pointer on {:?}", other),
@@ -143,7 +143,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
143143
substs,
144144
ty::ClosureKind::FnOnce,
145145
);
146-
let fn_ptr = self.memory.create_fn_alloc(instance);
146+
let fn_ptr = self.memory.create_fn_alloc(instance).with_default_tag();
147147
let val = Value::Scalar(Scalar::Ptr(fn_ptr.into()).into());
148148
self.write_value(val, dest)?;
149149
}

‎src/librustc_mir/interpret/eval_context.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ pub struct EvalContext<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'a, 'mir, 'tcx>> {
4747
pub(crate) param_env: ty::ParamEnv<'tcx>,
4848

4949
/// The virtual memory system.
50-
pub memory: Memory<'a, 'mir, 'tcx, M>,
50+
pub(crate) memory: Memory<'a, 'mir, 'tcx, M>,
5151

5252
/// The virtual call stack.
5353
pub(crate) stack: Vec<Frame<'mir, 'tcx, M::PointerTag>>,
@@ -334,7 +334,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
334334
}
335335

336336
pub fn str_to_value(&mut self, s: &str) -> EvalResult<'tcx, Value<M::PointerTag>> {
337-
let ptr = self.memory.allocate_static_bytes(s.as_bytes());
337+
let ptr = self.memory.allocate_static_bytes(s.as_bytes()).with_default_tag();
338338
Ok(Value::new_slice(Scalar::Ptr(ptr), s.len() as u64, self.tcx.tcx))
339339
}
340340

‎src/librustc_mir/interpret/machine.rs

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@
1515
use std::borrow::{Borrow, Cow};
1616
use std::hash::Hash;
1717

18-
use rustc::hir::def_id::DefId;
18+
use rustc::hir::{self, def_id::DefId};
1919
use rustc::mir;
2020
use rustc::ty::{self, Ty, layout::{Size, TyLayout}, query::TyCtxtAt};
2121

2222
use super::{
2323
Allocation, AllocId, EvalResult, Scalar,
24-
EvalContext, PlaceTy, OpTy, Pointer, MemoryKind,
24+
EvalContext, PlaceTy, OpTy, Pointer, MemPlace, MemoryKind,
2525
};
2626

2727
/// Classifying memory accesses
@@ -81,6 +81,7 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized {
8181

8282
/// Tag tracked alongside every pointer. This is used to implement "Stacked Borrows"
8383
/// <https://www.ralfj.de/blog/2018/08/07/stacked-borrows.html>.
84+
/// The `default()` is used for pointers to consts, statics, vtables and functions.
8485
type PointerTag: ::std::fmt::Debug + Default + Copy + Eq + Hash + 'static;
8586

8687
/// Extra data stored in every allocation.
@@ -151,13 +152,13 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized {
151152
) -> EvalResult<'tcx, Cow<'tcx, Allocation<Self::PointerTag, Self::AllocExtra>>>;
152153

153154
/// Called to turn an allocation obtained from the `tcx` into one that has
154-
/// the appropriate tags on each pointer.
155+
/// the right type for this machine.
155156
///
156157
/// This should avoid copying if no work has to be done! If this returns an owned
157-
/// allocation (because a copy had to be done to add the tags), machine memory will
158+
/// allocation (because a copy had to be done to add tags or metadata), machine memory will
158159
/// cache the result. (This relies on `AllocMap::get_or` being able to add the
159160
/// owned allocation to the map even when the map is shared.)
160-
fn static_with_default_tag(
161+
fn adjust_static_allocation(
161162
alloc: &'_ Allocation
162163
) -> Cow<'_, Allocation<Self::PointerTag, Self::AllocExtra>>;
163164

@@ -204,24 +205,40 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized {
204205
Ok(())
205206
}
206207

208+
/// Add the tag for a newly allocated pointer.
209+
fn tag_new_allocation(
210+
ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,
211+
ptr: Pointer,
212+
kind: MemoryKind<Self::MemoryKinds>,
213+
) -> EvalResult<'tcx, Pointer<Self::PointerTag>>;
214+
207215
/// Executed when evaluating the `&` operator: Creating a new reference.
208-
/// This has the chance to adjust the tag.
209-
/// `borrow_kind` can be `None` in case a raw ptr is being created.
216+
/// This has the chance to adjust the tag. It should not change anything else!
217+
/// `mutability` can be `None` in case a raw ptr is being created.
218+
#[inline]
210219
fn tag_reference(
211-
ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,
212-
ptr: Pointer<Self::PointerTag>,
213-
pointee_ty: Ty<'tcx>,
214-
pointee_size: Size,
215-
borrow_kind: Option<mir::BorrowKind>,
216-
) -> EvalResult<'tcx, Self::PointerTag>;
220+
_ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,
221+
place: MemPlace<Self::PointerTag>,
222+
_ty: Ty<'tcx>,
223+
_size: Size,
224+
_mutability: Option<hir::Mutability>,
225+
) -> EvalResult<'tcx, MemPlace<Self::PointerTag>> {
226+
Ok(place)
227+
}
217228

218229
/// Executed when evaluating the `*` operator: Following a reference.
219-
/// This has the change to adjust the tag.
230+
/// This has the change to adjust the tag. It should not change anything else!
231+
/// `mutability` can be `None` in case a raw ptr is being dereferenced.
232+
#[inline]
220233
fn tag_dereference(
221-
ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
222-
ptr: Pointer<Self::PointerTag>,
223-
ptr_ty: Ty<'tcx>,
224-
) -> EvalResult<'tcx, Self::PointerTag>;
234+
_ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
235+
place: MemPlace<Self::PointerTag>,
236+
_ty: Ty<'tcx>,
237+
_size: Size,
238+
_mutability: Option<hir::Mutability>,
239+
) -> EvalResult<'tcx, MemPlace<Self::PointerTag>> {
240+
Ok(place)
241+
}
225242

226243
/// Execute a validation operation
227244
#[inline]

‎src/librustc_mir/interpret/memory.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
117117
}
118118
}
119119

120-
pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer<M::PointerTag> {
121-
Pointer::from(self.tcx.alloc_map.lock().create_fn_alloc(instance)).with_default_tag()
120+
pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer {
121+
Pointer::from(self.tcx.alloc_map.lock().create_fn_alloc(instance))
122122
}
123123

124-
pub fn allocate_static_bytes(&mut self, bytes: &[u8]) -> Pointer<M::PointerTag> {
125-
Pointer::from(self.tcx.allocate_bytes(bytes)).with_default_tag()
124+
pub fn allocate_static_bytes(&mut self, bytes: &[u8]) -> Pointer {
125+
Pointer::from(self.tcx.allocate_bytes(bytes))
126126
}
127127

128128
pub fn allocate_with(
@@ -140,9 +140,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
140140
size: Size,
141141
align: Align,
142142
kind: MemoryKind<M::MemoryKinds>,
143-
) -> EvalResult<'tcx, Pointer<M::PointerTag>> {
144-
let ptr = Pointer::from(self.allocate_with(Allocation::undef(size, align), kind)?);
145-
Ok(ptr.with_default_tag())
143+
) -> EvalResult<'tcx, Pointer> {
144+
Ok(Pointer::from(self.allocate_with(Allocation::undef(size, align), kind)?))
146145
}
147146

148147
pub fn reallocate(
@@ -153,17 +152,18 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
153152
new_size: Size,
154153
new_align: Align,
155154
kind: MemoryKind<M::MemoryKinds>,
156-
) -> EvalResult<'tcx, Pointer<M::PointerTag>> {
155+
) -> EvalResult<'tcx, Pointer> {
157156
if ptr.offset.bytes() != 0 {
158157
return err!(ReallocateNonBasePtr);
159158
}
160159

161-
// For simplicities' sake, we implement reallocate as "alloc, copy, dealloc"
160+
// For simplicities' sake, we implement reallocate as "alloc, copy, dealloc".
161+
// This happens so rarely, the perf advantage is outweighed by the maintenance cost.
162162
let new_ptr = self.allocate(new_size, new_align, kind)?;
163163
self.copy(
164164
ptr.into(),
165165
old_align,
166-
new_ptr.into(),
166+
new_ptr.with_default_tag().into(),
167167
new_align,
168168
old_size.min(new_size),
169169
/*nonoverlapping*/ true,
@@ -347,7 +347,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
347347
Some(AllocType::Memory(mem)) => {
348348
// We got tcx memory. Let the machine figure out whether and how to
349349
// turn that into memory with the right pointer tag.
350-
return Ok(M::static_with_default_tag(mem))
350+
return Ok(M::adjust_static_allocation(mem))
351351
}
352352
Some(AllocType::Function(..)) => {
353353
return err!(DerefFunctionPointer)
@@ -381,7 +381,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
381381
if let ConstValue::ByRef(_, allocation, _) = const_val.val {
382382
// We got tcx memory. Let the machine figure out whether and how to
383383
// turn that into memory with the right pointer tag.
384-
M::static_with_default_tag(allocation)
384+
M::adjust_static_allocation(allocation)
385385
} else {
386386
bug!("Matching on non-ByRef static")
387387
}

‎src/librustc_mir/interpret/operand.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,16 @@ impl<'tcx, Tag> Value<Tag> {
217217
Value::ScalarPair(ptr, _) => ptr.not_undef(),
218218
}
219219
}
220+
221+
/// Convert the value into its metadata.
222+
/// Throws away the first half of a ScalarPair!
223+
#[inline]
224+
pub fn to_meta(self) -> EvalResult<'tcx, Option<Scalar<Tag>>> {
225+
Ok(match self {
226+
Value::Scalar(_) => None,
227+
Value::ScalarPair(_, meta) => Some(meta.not_undef()?),
228+
})
229+
}
220230
}
221231

222232
// ScalarPair needs a type to interpret, so we often have a value and a type together

‎src/librustc_mir/interpret/place.rs

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use std::convert::TryFrom;
1616
use std::hash::Hash;
1717

18+
use rustc::hir;
1819
use rustc::mir;
1920
use rustc::ty::{self, Ty};
2021
use rustc::ty::layout::{self, Size, Align, LayoutOf, TyLayout, HasDataLayout};
@@ -270,24 +271,28 @@ where
270271
&self,
271272
val: ValTy<'tcx, M::PointerTag>,
272273
) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
273-
let ptr = match val.to_scalar_ptr()? {
274-
Scalar::Ptr(ptr) if M::ENABLE_PTR_TRACKING_HOOKS => {
275-
// Machine might want to track the `*` operator
276-
let tag = M::tag_dereference(self, ptr, val.layout.ty)?;
277-
Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag))
278-
}
279-
other => other,
280-
};
281-
282274
let pointee_type = val.layout.ty.builtin_deref(true).unwrap().ty;
283275
let layout = self.layout_of(pointee_type)?;
284-
let align = layout.align;
285276

286-
let mplace = match *val {
287-
Value::Scalar(_) =>
288-
MemPlace { ptr, align, meta: None },
289-
Value::ScalarPair(_, meta) =>
290-
MemPlace { ptr, align, meta: Some(meta.not_undef()?) },
277+
let align = layout.align;
278+
let meta = val.to_meta()?;
279+
let ptr = val.to_scalar_ptr()?;
280+
let mplace = MemPlace { ptr, align, meta };
281+
// Pointer tag tracking might want to adjust the tag.
282+
let mplace = if M::ENABLE_PTR_TRACKING_HOOKS {
283+
let (size, _) = self.size_and_align_of(meta, layout)?
284+
// for extern types, just cover what we can
285+
.unwrap_or_else(|| layout.size_and_align());
286+
let mutbl = match val.layout.ty.sty {
287+
// `builtin_deref` considers boxes immutable, that's useless for our purposes
288+
ty::Ref(_, _, mutbl) => Some(mutbl),
289+
ty::Adt(def, _) if def.is_box() => Some(hir::MutMutable),
290+
ty::RawPtr(_) => None,
291+
_ => bug!("Unexpected pointer type {}", val.layout.ty.sty),
292+
};
293+
M::tag_dereference(self, mplace, pointee_type, size, mutbl)?
294+
} else {
295+
mplace
291296
};
292297
Ok(MPlaceTy { mplace, layout })
293298
}
@@ -299,19 +304,25 @@ where
299304
place: MPlaceTy<'tcx, M::PointerTag>,
300305
borrow_kind: Option<mir::BorrowKind>,
301306
) -> EvalResult<'tcx, Value<M::PointerTag>> {
302-
let ptr = match place.ptr {
303-
Scalar::Ptr(ptr) if M::ENABLE_PTR_TRACKING_HOOKS => {
304-
// Machine might want to track the `&` operator
305-
let (size, _) = self.size_and_align_of_mplace(place)?
306-
.expect("create_ref cannot determine size");
307-
let tag = M::tag_reference(self, ptr, place.layout.ty, size, borrow_kind)?;
308-
Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag))
309-
},
310-
other => other,
307+
// Pointer tag tracking might want to adjust the tag
308+
let place = if M::ENABLE_PTR_TRACKING_HOOKS {
309+
let (size, _) = self.size_and_align_of_mplace(place)?
310+
// for extern types, just cover what we can
311+
.unwrap_or_else(|| place.layout.size_and_align());
312+
let mutbl = match borrow_kind {
313+
Some(mir::BorrowKind::Mut { .. }) |
314+
Some(mir::BorrowKind::Unique) =>
315+
Some(hir::MutMutable),
316+
Some(_) => Some(hir::MutImmutable),
317+
None => None,
318+
};
319+
M::tag_reference(self, *place, place.layout.ty, size, mutbl)?
320+
} else {
321+
*place
311322
};
312323
Ok(match place.meta {
313-
None => Value::Scalar(ptr.into()),
314-
Some(meta) => Value::ScalarPair(ptr.into(), meta.into()),
324+
None => Value::Scalar(place.ptr.into()),
325+
Some(meta) => Value::ScalarPair(place.ptr.into(), meta.into()),
315326
})
316327
}
317328

@@ -845,6 +856,8 @@ where
845856
}
846857

847858
/// Make sure that a place is in memory, and return where it is.
859+
/// If the place currently refers to a local that doesn't yet have a matching allocation,
860+
/// create such an allocation.
848861
/// This is essentially `force_to_memplace`.
849862
pub fn force_allocation(
850863
&mut self,
@@ -888,10 +901,11 @@ where
888901
) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> {
889902
if layout.is_unsized() {
890903
assert!(self.tcx.features().unsized_locals, "cannot alloc memory for unsized type");
891-
// FIXME: What should we do here?
904+
// FIXME: What should we do here? We should definitely also tag!
892905
Ok(MPlaceTy::dangling(layout, &self))
893906
} else {
894907
let ptr = self.memory.allocate(layout.size, layout.align, kind)?;
908+
let ptr = M::tag_new_allocation(self, ptr, kind)?;
895909
Ok(MPlaceTy::from_aligned_ptr(ptr, layout))
896910
}
897911
}

‎src/librustc_mir/interpret/traits.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
5454
ptr_size * (3 + methods.len() as u64),
5555
ptr_align,
5656
MemoryKind::Vtable,
57-
)?;
57+
)?.with_default_tag();
5858

5959
let drop = ::monomorphize::resolve_drop_in_place(*self.tcx, ty);
60-
let drop = self.memory.create_fn_alloc(drop);
60+
let drop = self.memory.create_fn_alloc(drop).with_default_tag();
6161
self.memory.write_ptr_sized(vtable, ptr_align, Scalar::Ptr(drop).into())?;
6262

6363
let size_ptr = vtable.offset(ptr_size, &self)?;
@@ -69,7 +69,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
6969
for (i, method) in methods.iter().enumerate() {
7070
if let Some((def_id, substs)) = *method {
7171
let instance = self.resolve(def_id, substs)?;
72-
let fn_ptr = self.memory.create_fn_alloc(instance);
72+
let fn_ptr = self.memory.create_fn_alloc(instance).with_default_tag();
7373
let method_ptr = vtable.offset(ptr_size * (3 + i as u64), &self)?;
7474
self.memory.write_ptr_sized(method_ptr, ptr_align, Scalar::Ptr(fn_ptr).into())?;
7575
}

‎src/librustc_mir/interpret/validity.rs

Lines changed: 60 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use std::fmt::Write;
1212
use std::hash::Hash;
1313

1414
use syntax_pos::symbol::Symbol;
15-
use rustc::ty::layout::{self, Size, Align, TyLayout};
15+
use rustc::ty::layout::{self, Size, Align, TyLayout, LayoutOf};
1616
use rustc::ty;
1717
use rustc_data_structures::fx::FxHashSet;
1818
use rustc::mir::interpret::{
@@ -176,19 +176,27 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
176176
// undef. We should fix that, but let's start low.
177177
}
178178
}
179-
_ if ty.is_box() || ty.is_region_ptr() || ty.is_unsafe_ptr() => {
180-
// Handle fat pointers. We also check fat raw pointers,
181-
// their metadata must be valid!
182-
// This also checks that the ptr itself is initialized, which
183-
// seems reasonable even for raw pointers.
184-
let place = try_validation!(self.ref_to_mplace(value),
185-
"undefined data in pointer", path);
179+
ty::RawPtr(..) => {
180+
// No undef allowed here. Eventually this should be consistent with
181+
// the integer types.
182+
let _ptr = try_validation!(value.to_scalar_ptr(),
183+
"undefined address in pointer", path);
184+
let _meta = try_validation!(value.to_meta(),
185+
"uninitialized data in fat pointer metadata", path);
186+
}
187+
_ if ty.is_box() || ty.is_region_ptr() => {
188+
// Handle fat pointers.
186189
// Check metadata early, for better diagnostics
187-
if place.layout.is_unsized() {
188-
let tail = self.tcx.struct_tail(place.layout.ty);
190+
let ptr = try_validation!(value.to_scalar_ptr(),
191+
"undefined address in pointer", path);
192+
let meta = try_validation!(value.to_meta(),
193+
"uninitialized data in fat pointer metadata", path);
194+
let layout = self.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?;
195+
if layout.is_unsized() {
196+
let tail = self.tcx.struct_tail(layout.ty);
189197
match tail.sty {
190198
ty::Dynamic(..) => {
191-
let vtable = try_validation!(place.meta.unwrap().to_ptr(),
199+
let vtable = try_validation!(meta.unwrap().to_ptr(),
192200
"non-pointer vtable in fat pointer", path);
193201
try_validation!(self.read_drop_type_from_vtable(vtable),
194202
"invalid drop fn in vtable", path);
@@ -197,7 +205,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
197205
// FIXME: More checks for the vtable.
198206
}
199207
ty::Slice(..) | ty::Str => {
200-
try_validation!(place.meta.unwrap().to_usize(self),
208+
try_validation!(meta.unwrap().to_usize(self),
201209
"non-integer slice length in fat pointer", path);
202210
}
203211
ty::Foreign(..) => {
@@ -207,59 +215,65 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
207215
bug!("Unexpected unsized type tail: {:?}", tail),
208216
}
209217
}
210-
// for safe ptrs, also check the ptr values itself
211-
if !ty.is_unsafe_ptr() {
212-
// Make sure this is non-NULL and aligned
213-
let (size, align) = self.size_and_align_of(place.meta, place.layout)?
214-
// for the purpose of validity, consider foreign types to have
215-
// alignment and size determined by the layout (size will be 0,
216-
// alignment should take attributes into account).
217-
.unwrap_or_else(|| place.layout.size_and_align());
218-
match self.memory.check_align(place.ptr, align) {
219-
Ok(_) => {},
220-
Err(err) => match err.kind {
218+
// Make sure this is non-NULL and aligned
219+
let (size, align) = self.size_and_align_of(meta, layout)?
220+
// for the purpose of validity, consider foreign types to have
221+
// alignment and size determined by the layout (size will be 0,
222+
// alignment should take attributes into account).
223+
.unwrap_or_else(|| layout.size_and_align());
224+
match self.memory.check_align(ptr, align) {
225+
Ok(_) => {},
226+
Err(err) => {
227+
error!("{:?} is not aligned to {:?}", ptr, align);
228+
match err.kind {
221229
EvalErrorKind::InvalidNullPointerUsage =>
222230
return validation_failure!("NULL reference", path),
223231
EvalErrorKind::AlignmentCheckFailed { .. } =>
224232
return validation_failure!("unaligned reference", path),
225233
_ =>
226234
return validation_failure!(
227235
"dangling (out-of-bounds) reference (might be NULL at \
228-
run-time)",
236+
run-time)",
229237
path
230238
),
231239
}
232240
}
233-
// non-ZST also have to be dereferenceable
241+
}
242+
// Turn ptr into place.
243+
// `ref_to_mplace` also calls the machine hook for (re)activating the tag,
244+
// which in turn will (in full miri) check if the pointer is dereferencable.
245+
let place = self.ref_to_mplace(value)?;
246+
// Recursive checking
247+
if let Some(ref_tracking) = ref_tracking {
248+
assert!(const_mode, "We should only do recursie checking in const mode");
234249
if size != Size::ZERO {
250+
// Non-ZST also have to be dereferencable
235251
let ptr = try_validation!(place.ptr.to_ptr(),
236252
"integer pointer in non-ZST reference", path);
237-
if const_mode {
238-
// Skip validation entirely for some external statics
239-
let alloc_kind = self.tcx.alloc_map.lock().get(ptr.alloc_id);
240-
if let Some(AllocType::Static(did)) = alloc_kind {
241-
// `extern static` cannot be validated as they have no body.
242-
// FIXME: Statics from other crates are also skipped.
243-
// They might be checked at a different type, but for now we
244-
// want to avoid recursing too deeply. This is not sound!
245-
if !did.is_local() || self.tcx.is_foreign_item(did) {
246-
return Ok(());
247-
}
253+
// Skip validation entirely for some external statics
254+
let alloc_kind = self.tcx.alloc_map.lock().get(ptr.alloc_id);
255+
if let Some(AllocType::Static(did)) = alloc_kind {
256+
// `extern static` cannot be validated as they have no body.
257+
// FIXME: Statics from other crates are also skipped.
258+
// They might be checked at a different type, but for now we
259+
// want to avoid recursing too deeply. This is not sound!
260+
if !did.is_local() || self.tcx.is_foreign_item(did) {
261+
return Ok(());
248262
}
249263
}
264+
// Maintain the invariant that the place we are checking is
265+
// already verified to be in-bounds.
250266
try_validation!(self.memory.check_bounds(ptr, size, false),
251267
"dangling (not entirely in bounds) reference", path);
252268
}
253-
if let Some(ref_tracking) = ref_tracking {
254-
// Check if we have encountered this pointer+layout combination
255-
// before. Proceed recursively even for integer pointers, no
256-
// reason to skip them! They are (recursively) valid for some ZST,
257-
// but not for others (e.g. `!` is a ZST).
258-
let op = place.into();
259-
if ref_tracking.seen.insert(op) {
260-
trace!("Recursing below ptr {:#?}", *op);
261-
ref_tracking.todo.push((op, path_clone_and_deref(path)));
262-
}
269+
// Check if we have encountered this pointer+layout combination
270+
// before. Proceed recursively even for integer pointers, no
271+
// reason to skip them! They are (recursively) valid for some ZST,
272+
// but not for others (e.g. `!` is a ZST).
273+
let op = place.into();
274+
if ref_tracking.seen.insert(op) {
275+
trace!("Recursing below ptr {:#?}", *op);
276+
ref_tracking.todo.push((op, path_clone_and_deref(path)));
263277
}
264278
}
265279
}

0 commit comments

Comments
 (0)
Please sign in to comment.