Skip to content

Commit

Permalink
Ignore mut borrow from drop terminator in const-eval
Browse files Browse the repository at this point in the history
  • Loading branch information
ecstatic-morse committed Feb 13, 2020
1 parent 15a5382 commit 0984639
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 12 deletions.
45 changes: 33 additions & 12 deletions src/librustc_mir/dataflow/impls/borrowed_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@ pub type MaybeMutBorrowedLocals<'mir, 'tcx> = MaybeBorrowedLocals<MutBorrow<'mir
/// function call or inline assembly.
pub struct MaybeBorrowedLocals<K = AnyBorrow> {
kind: K,
ignore_borrow_on_drop: bool,
}

impl MaybeBorrowedLocals {
/// A dataflow analysis that records whether a pointer or reference exists that may alias the
/// given local.
pub fn all_borrows() -> Self {
MaybeBorrowedLocals { kind: AnyBorrow }
MaybeBorrowedLocals { kind: AnyBorrow, ignore_borrow_on_drop: false }
}
}

Expand All @@ -43,13 +44,37 @@ impl MaybeMutBorrowedLocals<'mir, 'tcx> {
body: &'mir mir::Body<'tcx>,
param_env: ParamEnv<'tcx>,
) -> Self {
MaybeBorrowedLocals { kind: MutBorrow { body, tcx, param_env } }
MaybeBorrowedLocals {
kind: MutBorrow { body, tcx, param_env },
ignore_borrow_on_drop: false,
}
}
}

impl<K> MaybeBorrowedLocals<K> {
/// During dataflow analysis, ignore the borrow that may occur when a place is dropped.
///
/// Drop terminators may call custom drop glue (`Drop::drop`), which takes `&mut self` as a
/// parameter. In the general case, a drop impl could launder that reference into the
/// surrounding environment through a raw pointer, thus creating a valid `*mut` pointing to the
/// dropped local. We are not yet willing to declare this particular case UB, so we must treat
/// all dropped locals as mutably borrowed for now. See discussion on [#61069].
///
/// In some contexts, we know that this borrow will never occur. For example, during
/// const-eval, custom drop glue cannot be run. Code that calls this should document the
/// assumptions that justify `Drop` terminators in this way.
///
/// [#61069]: https://github.com/rust-lang/rust/pull/61069
pub fn unsound_ignore_borrow_on_drop(self) -> Self {
MaybeBorrowedLocals { ignore_borrow_on_drop: true, ..self }
}

fn transfer_function<'a, T>(&'a self, trans: &'a mut T) -> TransferFunction<'a, T, K> {
TransferFunction { kind: &self.kind, trans }
TransferFunction {
kind: &self.kind,
trans,
ignore_borrow_on_drop: self.ignore_borrow_on_drop,
}
}
}

Expand Down Expand Up @@ -112,6 +137,7 @@ impl<K> BottomValue for MaybeBorrowedLocals<K> {
struct TransferFunction<'a, T, K> {
trans: &'a mut T,
kind: &'a K,
ignore_borrow_on_drop: bool,
}

impl<T, K> Visitor<'tcx> for TransferFunction<'a, T, K>
Expand Down Expand Up @@ -162,17 +188,12 @@ where
self.super_terminator(terminator, location);

match terminator.kind {
// Drop terminators may call custom drop glue (`Drop::drop`), which takes `&mut self`
// as a parameter. Hypothetically, a drop impl could launder that reference into the
// surrounding environment through a raw pointer, thus creating a valid `*mut` pointing
// to the dropped local. We are not yet willing to declare this particular case UB, so
// we must treat all dropped locals as mutably borrowed for now. See discussion on
// [#61069].
//
// [#61069]: https://github.com/rust-lang/rust/pull/61069
mir::TerminatorKind::Drop { location: dropped_place, .. }
| mir::TerminatorKind::DropAndReplace { location: dropped_place, .. } => {
self.trans.gen(dropped_place.local);
// See documentation for `unsound_ignore_borrow_on_drop` for an explanation.
if !self.ignore_borrow_on_drop {
self.trans.gen(dropped_place.local);
}
}

TerminatorKind::Abort
Expand Down
6 changes: 6 additions & 0 deletions src/librustc_mir/transform/check_consts/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,13 @@ impl Validator<'a, 'mir, 'tcx> {
let needs_drop = QualifCursor::new(NeedsDrop, item);
let has_mut_interior = QualifCursor::new(HasMutInterior, item);

// We can use `unsound_ignore_borrow_on_drop` here because custom drop impls are not
// allowed in a const.
//
// FIXME(ecstaticmorse): Someday we want to allow custom drop impls. How do we do this
// without breaking stable code?
let indirectly_mutable = MaybeMutBorrowedLocals::mut_borrows_only(tcx, *body, param_env)
.unsound_ignore_borrow_on_drop()
.into_engine(tcx, *body, def_id)
.iterate_to_fixpoint()
.into_results_cursor(*body);
Expand Down

0 comments on commit 0984639

Please sign in to comment.