-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Lower index bounds checking to PtrMetadata
, this time with the right fake borrow semantics 😸
#135748
base: master
Are you sure you want to change the base?
Lower index bounds checking to PtrMetadata
, this time with the right fake borrow semantics 😸
#135748
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
…ake borrow
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -9,7 +9,6 @@ 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}; | ||||||||
|
||||||||
|
@@ -81,9 +80,7 @@ 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, stmt.source_info.span)? | ||||||||
} | ||||||||
Assign(box (place, rvalue)) => self.eval_rvalue_into_place(rvalue, *place)?, | ||||||||
|
||||||||
SetDiscriminant { place, variant_index } => { | ||||||||
let dest = self.eval_place(**place)?; | ||||||||
|
@@ -162,7 +159,6 @@ 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? | ||||||||
|
@@ -241,7 +237,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |||||||
self.write_immediate(*val, &dest)?; | ||||||||
} | ||||||||
|
||||||||
RawPtr(_, place) => { | ||||||||
RawPtr(kind, place) => { | ||||||||
// Figure out whether this is an addr_of of an already raw place. | ||||||||
let place_base_raw = if place.is_indirect_first_projection() { | ||||||||
let ty = self.frame().body.local_decls[place.local].ty; | ||||||||
|
@@ -254,13 +250,8 @@ 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 | ||||||||
&& span.desugaring_kind() != Some(DesugaringKind::IndexBoundsCheckReborrow) | ||||||||
{ | ||||||||
if !place_base_raw && !kind.is_fake() { | ||||||||
// If this was not already raw, it needs retagging. | ||||||||
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.
Suggested change
|
||||||||
// As a special hack, we exclude the desugared `PtrMetadata(&raw const *_n)` | ||||||||
// from indexing. (Really we should not do any retag on `&raw` but that does not | ||||||||
// currently work with Stacked Borrows.) | ||||||||
val = M::retag_ptr_value(self, mir::RetagKind::Raw, &val)?; | ||||||||
} | ||||||||
self.write_immediate(*val, &dest)?; | ||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -180,6 +180,51 @@ pub enum BorrowKind { | |
Mut { kind: MutBorrowKind }, | ||
} | ||
|
||
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, TyEncodable, TyDecodable)] | ||
#[derive(Hash, HashStable)] | ||
pub enum RawPtrKind { | ||
Mut, | ||
Not, | ||
FakeForPtrMetadata, | ||
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. Please add a doc comment explaining this variant. |
||
} | ||
|
||
impl From<Mutability> for RawPtrKind { | ||
fn from(other: Mutability) -> Self { | ||
match other { | ||
Mutability::Mut => RawPtrKind::Mut, | ||
Mutability::Not => RawPtrKind::Not, | ||
} | ||
} | ||
} | ||
|
||
impl RawPtrKind { | ||
pub fn is_fake(self) -> bool { | ||
match self { | ||
RawPtrKind::Mut | RawPtrKind::Not => false, | ||
RawPtrKind::FakeForPtrMetadata => true, | ||
} | ||
} | ||
|
||
pub fn to_mutbl_lossy(self) -> Mutability { | ||
match self { | ||
RawPtrKind::Mut => Mutability::Mut, | ||
RawPtrKind::Not => Mutability::Not, | ||
|
||
// We have no type corresponding to a shallow borrow, so use | ||
// `&` as an approximation. | ||
Comment on lines
+213
to
+214
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 comment looks like it still needs updating. |
||
RawPtrKind::FakeForPtrMetadata => Mutability::Not, | ||
} | ||
} | ||
|
||
pub fn ptr_str(self) -> &'static str { | ||
match self { | ||
RawPtrKind::Mut => "mut", | ||
RawPtrKind::Not => "const", | ||
RawPtrKind::FakeForPtrMetadata => "const (fake)", | ||
} | ||
} | ||
} | ||
|
||
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, TyEncodable, TyDecodable)] | ||
#[derive(Hash, HashStable)] | ||
pub enum MutBorrowKind { | ||
|
@@ -1349,7 +1394,7 @@ pub enum Rvalue<'tcx> { | |
/// | ||
/// Like with references, the semantics of this operation are heavily dependent on the aliasing | ||
/// model. | ||
RawPtr(Mutability, Place<'tcx>), | ||
RawPtr(RawPtrKind, Place<'tcx>), | ||
|
||
/// Yields the length of the place, as a `usize`. | ||
/// | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh, this is dangerous. What is the justification for allowing all these raw borrows in const? It seems to be this relies on a non-local invariant about where and how
FakeForPtrMetadata
can occur. I am not a fan if such invariants; I'd prefer the const checker to be correct at least for all MIR that passes the MIR validator.