Skip to content

Commit 5f8bd0a

Browse files
borsbors
committed
Auto merge of #55549 - RalfJung:miri-visitor, r=<try>
Value visitors for miri Generalize the traversal part of validation to a `ValueVisitor`. ~~This includes #55316, [click here](RalfJung/rust@retagging...RalfJung:miri-visitor) for just the new commits.~~ This PR does not change the interface to miri, so I use the opportunity to update miri.
2 parents e53a5ff + 7f93557 commit 5f8bd0a

25 files changed

+728
-408
lines changed

‎src/librustc/mir/interpret/error.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use ty::{Ty, layout};
1515
use ty::layout::{Size, Align, LayoutError};
1616
use rustc_target::spec::abi::Abi;
1717

18-
use super::Pointer;
18+
use super::{Pointer, Scalar};
1919

2020
use backtrace::Backtrace;
2121

@@ -240,7 +240,7 @@ pub enum EvalErrorKind<'tcx, O> {
240240
InvalidMemoryAccess,
241241
InvalidFunctionPointer,
242242
InvalidBool,
243-
InvalidDiscriminant(u128),
243+
InvalidDiscriminant(Scalar),
244244
PointerOutOfBounds {
245245
ptr: Pointer,
246246
access: bool,

‎src/librustc/mir/interpret/value.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
#![allow(unknown_lints)]
11+
use std::fmt;
1212

1313
use ty::layout::{HasDataLayout, Size};
1414
use ty::subst::Substs;
@@ -99,6 +99,15 @@ pub enum Scalar<Tag=(), Id=AllocId> {
9999
Ptr(Pointer<Tag, Id>),
100100
}
101101

102+
impl<Tag> fmt::Display for Scalar<Tag> {
103+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
104+
match self {
105+
Scalar::Ptr(_) => write!(f, "a pointer"),
106+
Scalar::Bits { bits, .. } => write!(f, "{}", bits),
107+
}
108+
}
109+
}
110+
102111
impl<'tcx> Scalar<()> {
103112
#[inline]
104113
pub fn with_default_tag<Tag>(self) -> Scalar<Tag>

‎src/librustc_mir/const_eval.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -539,10 +539,10 @@ fn validate_const<'a, 'tcx>(
539539
let val = (|| {
540540
let op = ecx.const_to_op(constant)?;
541541
let mut ref_tracking = RefTracking::new(op);
542-
while let Some((op, mut path)) = ref_tracking.todo.pop() {
542+
while let Some((op, path)) = ref_tracking.todo.pop() {
543543
ecx.validate_operand(
544544
op,
545-
&mut path,
545+
path,
546546
Some(&mut ref_tracking),
547547
/* const_mode */ true,
548548
)?;

‎src/librustc_mir/interpret/eval_context.rs

+21-12
Original file line numberDiff line numberDiff line change
@@ -139,25 +139,34 @@ impl<'tcx, Tag> LocalValue<Tag> {
139139
}
140140
}
141141

142-
impl<'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout
143-
for &'b EvalContext<'a, 'mir, 'tcx, M>
142+
impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout
143+
for &EvalContext<'a, 'mir, 'tcx, M>
144144
{
145145
#[inline]
146146
fn data_layout(&self) -> &layout::TargetDataLayout {
147147
&self.tcx.data_layout
148148
}
149149
}
150150

151-
impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout
152-
for &'c &'b mut EvalContext<'a, 'mir, 'tcx, M>
151+
impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout
152+
for &&EvalContext<'a, 'mir, 'tcx, M>
153153
{
154154
#[inline]
155155
fn data_layout(&self) -> &layout::TargetDataLayout {
156156
&self.tcx.data_layout
157157
}
158158
}
159159

160-
impl<'b, 'a, 'mir, 'tcx, M> layout::HasTyCtxt<'tcx> for &'b EvalContext<'a, 'mir, 'tcx, M>
160+
impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout
161+
for & &mut EvalContext<'a, 'mir, 'tcx, M>
162+
{
163+
#[inline]
164+
fn data_layout(&self) -> &layout::TargetDataLayout {
165+
&self.tcx.data_layout
166+
}
167+
}
168+
169+
impl<'a, 'mir, 'tcx, M> layout::HasTyCtxt<'tcx> for &EvalContext<'a, 'mir, 'tcx, M>
161170
where M: Machine<'a, 'mir, 'tcx>
162171
{
163172
#[inline]
@@ -166,17 +175,17 @@ impl<'b, 'a, 'mir, 'tcx, M> layout::HasTyCtxt<'tcx> for &'b EvalContext<'a, 'mir
166175
}
167176
}
168177

169-
impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> layout::HasTyCtxt<'tcx>
170-
for &'c &'b mut EvalContext<'a, 'mir, 'tcx, M>
178+
impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> layout::HasTyCtxt<'tcx>
179+
for & &mut EvalContext<'a, 'mir, 'tcx, M>
171180
{
172181
#[inline]
173182
fn tcx<'d>(&'d self) -> TyCtxt<'d, 'tcx, 'tcx> {
174183
*self.tcx
175184
}
176185
}
177186

178-
impl<'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf
179-
for &'b EvalContext<'a, 'mir, 'tcx, M>
187+
impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf
188+
for &EvalContext<'a, 'mir, 'tcx, M>
180189
{
181190
type Ty = Ty<'tcx>;
182191
type TyLayout = EvalResult<'tcx, TyLayout<'tcx>>;
@@ -188,8 +197,8 @@ impl<'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf
188197
}
189198
}
190199

191-
impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf
192-
for &'c &'b mut EvalContext<'a, 'mir, 'tcx, M>
200+
impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf
201+
for & &mut EvalContext<'a, 'mir, 'tcx, M>
193202
{
194203
type Ty = Ty<'tcx>;
195204
type TyLayout = EvalResult<'tcx, TyLayout<'tcx>>;
@@ -551,7 +560,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
551560
// return place is always a local and then this cannot happen.
552561
self.validate_operand(
553562
self.place_to_op(return_place)?,
554-
&mut vec![],
563+
vec![],
555564
None,
556565
/*const_mode*/false,
557566
)?;

‎src/librustc_mir/interpret/machine.rs

+9-13
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ use std::hash::Hash;
1717

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

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

2727
/// Whether this kind of memory is allowed to leak
@@ -217,26 +217,22 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized {
217217
#[inline]
218218
fn tag_reference(
219219
_ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>,
220-
place: MemPlace<Self::PointerTag>,
221-
_ty: Ty<'tcx>,
222-
_size: Size,
220+
place: MPlaceTy<'tcx, Self::PointerTag>,
223221
_mutability: Option<hir::Mutability>,
224-
) -> EvalResult<'tcx, MemPlace<Self::PointerTag>> {
225-
Ok(place)
222+
) -> EvalResult<'tcx, Scalar<Self::PointerTag>> {
223+
Ok(place.ptr)
226224
}
227225

228226
/// Executed when evaluating the `*` operator: Following a reference.
229-
/// This has the change to adjust the tag. It should not change anything else!
227+
/// This has the chance to adjust the tag. It should not change anything else!
230228
/// `mutability` can be `None` in case a raw ptr is being dereferenced.
231229
#[inline]
232230
fn tag_dereference(
233231
_ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
234-
place: MemPlace<Self::PointerTag>,
235-
_ty: Ty<'tcx>,
236-
_size: Size,
232+
place: MPlaceTy<'tcx, Self::PointerTag>,
237233
_mutability: Option<hir::Mutability>,
238-
) -> EvalResult<'tcx, MemPlace<Self::PointerTag>> {
239-
Ok(place)
234+
) -> EvalResult<'tcx, Scalar<Self::PointerTag>> {
235+
Ok(place.ptr)
240236
}
241237

242238
/// Execute a validation operation

‎src/librustc_mir/interpret/mod.rs

+3
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ mod terminator;
2323
mod traits;
2424
mod validity;
2525
mod intrinsics;
26+
mod visitor;
2627

2728
pub use rustc::mir::interpret::*; // have all the `interpret` symbols in one place: here
2829

@@ -38,4 +39,6 @@ pub use self::machine::{Machine, AllocMap, MayLeak};
3839

3940
pub use self::operand::{ScalarMaybeUndef, Value, ValTy, Operand, OpTy};
4041

42+
pub use self::visitor::ValueVisitor;
43+
4144
pub use self::validity::RefTracking;

‎src/librustc_mir/interpret/operand.rs

+17-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
//! All high-level functions to read from memory work on operands as sources.
1313
1414
use std::convert::TryInto;
15+
use std::fmt;
1516

1617
use rustc::{mir, ty};
1718
use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt};
@@ -36,6 +37,15 @@ impl<Tag> From<Scalar<Tag>> for ScalarMaybeUndef<Tag> {
3637
}
3738
}
3839

40+
impl<Tag> fmt::Display for ScalarMaybeUndef<Tag> {
41+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
42+
match self {
43+
ScalarMaybeUndef::Undef => write!(f, "uninitialized bytes"),
44+
ScalarMaybeUndef::Scalar(s) => write!(f, "{}", s),
45+
}
46+
}
47+
}
48+
3949
impl<'tcx> ScalarMaybeUndef<()> {
4050
#[inline]
4151
pub fn with_default_tag<Tag>(self) -> ScalarMaybeUndef<Tag>
@@ -729,8 +739,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
729739
Ok(match rval.layout.variants {
730740
layout::Variants::Single { .. } => bug!(),
731741
layout::Variants::Tagged { .. } => {
742+
let bits_discr = match raw_discr.to_bits(discr_val.layout.size) {
743+
Ok(raw_discr) => raw_discr,
744+
Err(_) => return err!(InvalidDiscriminant(raw_discr.erase_tag())),
745+
};
732746
let real_discr = if discr_val.layout.ty.is_signed() {
733-
let i = raw_discr.to_bits(discr_val.layout.size)? as i128;
747+
let i = bits_discr as i128;
734748
// going from layout tag type to typeck discriminant type
735749
// requires first sign extending with the layout discriminant
736750
let shift = 128 - discr_val.layout.size.bits();
@@ -745,15 +759,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
745759
let truncatee = sexted as u128;
746760
(truncatee << shift) >> shift
747761
} else {
748-
raw_discr.to_bits(discr_val.layout.size)?
762+
bits_discr
749763
};
750764
// Make sure we catch invalid discriminants
751765
let index = rval.layout.ty
752766
.ty_adt_def()
753767
.expect("tagged layout for non adt")
754768
.discriminants(self.tcx.tcx)
755769
.position(|var| var.val == real_discr)
756-
.ok_or_else(|| EvalErrorKind::InvalidDiscriminant(real_discr))?;
770+
.ok_or_else(|| EvalErrorKind::InvalidDiscriminant(raw_discr.erase_tag()))?;
757771
(real_discr, index)
758772
},
759773
layout::Variants::NicheFilling {

‎src/librustc_mir/interpret/place.rs

+17-23
Original file line numberDiff line numberDiff line change
@@ -278,42 +278,34 @@ where
278278
let meta = val.to_meta()?;
279279
let ptr = val.to_scalar_ptr()?;
280280
let mplace = MemPlace { ptr, align, meta };
281+
let mut mplace = MPlaceTy { mplace, layout };
281282
// 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());
283+
if M::ENABLE_PTR_TRACKING_HOOKS {
286284
let mutbl = match val.layout.ty.sty {
287285
// `builtin_deref` considers boxes immutable, that's useless for our purposes
288286
ty::Ref(_, _, mutbl) => Some(mutbl),
289287
ty::Adt(def, _) if def.is_box() => Some(hir::MutMutable),
290288
ty::RawPtr(_) => None,
291289
_ => bug!("Unexpected pointer type {}", val.layout.ty.sty),
292290
};
293-
M::tag_dereference(self, mplace, pointee_type, size, mutbl)?
294-
} else {
295-
mplace
296-
};
297-
Ok(MPlaceTy { mplace, layout })
291+
mplace.mplace.ptr = M::tag_dereference(self, mplace, mutbl)?;
292+
}
293+
// Done
294+
Ok(mplace)
298295
}
299296

300297
/// Turn a mplace into a (thin or fat) pointer, as a reference, pointing to the same space.
301298
/// This is the inverse of `ref_to_mplace`.
302299
/// `mutbl` indicates whether we are create a shared or mutable ref, or a raw pointer (`None`).
303300
pub fn create_ref(
304301
&mut self,
305-
place: MPlaceTy<'tcx, M::PointerTag>,
302+
mut place: MPlaceTy<'tcx, M::PointerTag>,
306303
mutbl: Option<hir::Mutability>,
307304
) -> EvalResult<'tcx, Value<M::PointerTag>> {
308305
// Pointer tag tracking might want to adjust the tag
309-
let place = if M::ENABLE_PTR_TRACKING_HOOKS {
310-
let (size, _) = self.size_and_align_of_mplace(place)?
311-
// for extern types, just cover what we can
312-
.unwrap_or_else(|| place.layout.size_and_align());
313-
M::tag_reference(self, *place, place.layout.ty, size, mutbl)?
314-
} else {
315-
*place
316-
};
306+
if M::ENABLE_PTR_TRACKING_HOOKS {
307+
place.mplace.ptr = M::tag_reference(self, place, mutbl)?
308+
}
317309
Ok(match place.meta {
318310
None => Value::Scalar(place.ptr.into()),
319311
Some(meta) => Value::ScalarPair(place.ptr.into(), meta.into()),
@@ -489,6 +481,8 @@ where
489481

490482
/// Get the place of a field inside the place, and also the field's type.
491483
/// Just a convenience function, but used quite a bit.
484+
/// This is the only projection that might have a side-effect: We cannot project
485+
/// into the field of a local `ScalarPair`, we have to first allocate it.
492486
pub fn place_field(
493487
&mut self,
494488
base: PlaceTy<'tcx, M::PointerTag>,
@@ -501,7 +495,7 @@ where
501495
}
502496

503497
pub fn place_downcast(
504-
&mut self,
498+
&self,
505499
base: PlaceTy<'tcx, M::PointerTag>,
506500
variant: usize,
507501
) -> EvalResult<'tcx, PlaceTy<'tcx, M::PointerTag>> {
@@ -510,7 +504,7 @@ where
510504
Place::Ptr(mplace) =>
511505
self.mplace_downcast(MPlaceTy { mplace, layout: base.layout }, variant)?.into(),
512506
Place::Local { .. } => {
513-
let layout = base.layout.for_variant(&self, variant);
507+
let layout = base.layout.for_variant(self, variant);
514508
PlaceTy { layout, ..base }
515509
}
516510
})
@@ -643,7 +637,7 @@ where
643637

644638
if M::enforce_validity(self) {
645639
// Data got changed, better make sure it matches the type!
646-
self.validate_operand(self.place_to_op(dest)?, &mut vec![], None, /*const_mode*/false)?;
640+
self.validate_operand(self.place_to_op(dest)?, vec![], None, /*const_mode*/false)?;
647641
}
648642

649643
Ok(())
@@ -765,7 +759,7 @@ where
765759

766760
if M::enforce_validity(self) {
767761
// Data got changed, better make sure it matches the type!
768-
self.validate_operand(self.place_to_op(dest)?, &mut vec![], None, /*const_mode*/false)?;
762+
self.validate_operand(self.place_to_op(dest)?, vec![], None, /*const_mode*/false)?;
769763
}
770764

771765
Ok(())
@@ -843,7 +837,7 @@ where
843837

844838
if M::enforce_validity(self) {
845839
// Data got changed, better make sure it matches the type!
846-
self.validate_operand(dest.into(), &mut vec![], None, /*const_mode*/false)?;
840+
self.validate_operand(dest.into(), vec![], None, /*const_mode*/false)?;
847841
}
848842

849843
Ok(())

0 commit comments

Comments
 (0)