-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Bounds-check with PtrMetadata instead of Len in MIR #133734
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ use rustc_middle::ty::layout::FnAbiOf; | |
use rustc_middle::ty::{self, Instance, Ty}; | ||
use rustc_middle::{bug, mir, span_bug}; | ||
use rustc_span::source_map::Spanned; | ||
use rustc_span::{DesugaringKind, Span}; | ||
use rustc_target::callconv::FnAbi; | ||
use tracing::{info, instrument, trace}; | ||
|
||
|
@@ -80,7 +81,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |
use rustc_middle::mir::StatementKind::*; | ||
|
||
match &stmt.kind { | ||
Assign(box (place, rvalue)) => self.eval_rvalue_into_place(rvalue, *place)?, | ||
Assign(box (place, rvalue)) => { | ||
self.eval_rvalue_into_place(rvalue, *place, stmt.source_info.span)? | ||
} | ||
|
||
SetDiscriminant { place, variant_index } => { | ||
let dest = self.eval_place(**place)?; | ||
|
@@ -159,6 +162,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |
&mut self, | ||
rvalue: &mir::Rvalue<'tcx>, | ||
place: mir::Place<'tcx>, | ||
span: Span, | ||
) -> InterpResult<'tcx> { | ||
let dest = self.eval_place(place)?; | ||
// FIXME: ensure some kind of non-aliasing between LHS and RHS? | ||
|
@@ -250,8 +254,11 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |
let src = self.eval_place(place)?; | ||
let place = self.force_allocation(&src)?; | ||
let mut val = ImmTy::from_immediate(place.to_ref(self), dest.layout); | ||
if !place_base_raw { | ||
if !place_base_raw | ||
&& span.desugaring_kind() != Some(DesugaringKind::IndexBoundsCheckReborrow) | ||
{ | ||
// If this was not already raw, it needs retagging. | ||
// Unless it's the `PtrMetadata(&raw const *_n)` from indexing. | ||
val = M::retag_ptr_value(self, mir::RetagKind::Raw, &val)?; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned before, please add a slice index expression to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh never mind, I forgot that we don't have these retags in MIR any more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah this is kind of a hack but it works, thanks! |
||
self.write_immediate(*val, &dest)?; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -463,6 +463,9 @@ impl<'tcx> Const<'tcx> { | |
let const_val = tcx.valtree_to_const_val((ty, valtree)); | ||
Self::Val(const_val, ty) | ||
} | ||
ty::ConstKind::Unevaluated(ty::UnevaluatedConst { def, args }) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not correct, and there's no justification here? What is this for? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Specifically, there's a distinction between |
||
Self::Unevaluated(UnevaluatedConst { def, args, promoted: None }, ty) | ||
} | ||
_ => Self::Ty(ty, c), | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1163,6 +1163,9 @@ pub enum DesugaringKind { | |
WhileLoop, | ||
/// `async Fn()` bound modifier | ||
BoundModifier, | ||
/// Marks a `&raw const *_1` needed as part of getting the length of a mutable | ||
/// slice for the bounds check, so that MIRI's retag handling can recognize it. | ||
IndexBoundsCheckReborrow, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Span should definitely not be used like this. Span desugarings should only ever be used for diagnostics and things like that -- making miri rely on this behavior seems really bad. If we need to track certain locals for the purposes of some special miri hack, this needs to be recorded in the MIR body separately. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a pretty bad hack, yeah. Medium-term I hope that Miri can entirely stop doing retagging on |
||
} | ||
|
||
impl DesugaringKind { | ||
|
@@ -1179,6 +1182,7 @@ impl DesugaringKind { | |
DesugaringKind::ForLoop => "`for` loop", | ||
DesugaringKind::WhileLoop => "`while` loop", | ||
DesugaringKind::BoundModifier => "trait bound modifier", | ||
DesugaringKind::IndexBoundsCheckReborrow => "slice indexing", | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
// MIR for `index_array` after built | ||
|
||
fn index_array(_1: &[i32; 7], _2: usize) -> &i32 { | ||
debug array => _1; | ||
debug index => _2; | ||
let mut _0: &i32; | ||
let _3: &i32; | ||
let _4: usize; | ||
let mut _5: bool; | ||
|
||
bb0: { | ||
StorageLive(_3); | ||
StorageLive(_4); | ||
_4 = copy _2; | ||
FakeRead(ForIndex, (*_1)); | ||
_5 = Lt(copy _4, const 7_usize); | ||
assert(move _5, "index out of bounds: the length is {} but the index is {}", const 7_usize, copy _4) -> [success: bb1, unwind: bb2]; | ||
} | ||
|
||
bb1: { | ||
_3 = &(*_1)[_4]; | ||
_0 = &(*_3); | ||
StorageDead(_4); | ||
StorageDead(_3); | ||
return; | ||
} | ||
|
||
bb2 (cleanup): { | ||
resume; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
// MIR for `index_const_generic_array` after built | ||
|
||
fn index_const_generic_array(_1: &[i32; N], _2: usize) -> &i32 { | ||
debug array => _1; | ||
debug index => _2; | ||
let mut _0: &i32; | ||
let _3: &i32; | ||
let _4: usize; | ||
let mut _5: bool; | ||
|
||
bb0: { | ||
StorageLive(_3); | ||
StorageLive(_4); | ||
_4 = copy _2; | ||
FakeRead(ForIndex, (*_1)); | ||
_5 = Lt(copy _4, const N); | ||
assert(move _5, "index out of bounds: the length is {} but the index is {}", const N, copy _4) -> [success: bb1, unwind: bb2]; | ||
} | ||
|
||
bb1: { | ||
_3 = &(*_1)[_4]; | ||
_0 = &(*_3); | ||
StorageDead(_4); | ||
StorageDead(_3); | ||
return; | ||
} | ||
|
||
bb2 (cleanup): { | ||
resume; | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.