From 64457a19689169dab464ebc37b2f3619f39442dd Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 2 Oct 2019 14:41:42 -0700 Subject: [PATCH 1/6] Check type of place base (not projection) for `!Freeze` This prevents unsafe code from taking a reference to a field that is `Freeze` and using pointer arithmetic to change it to a reference to a field that is not `Freeze` without causing a local to become marked as indirectly mutable. Instead, taking a reference to the `Freeze` field will cause the whole struct to be marked as indirectly mutable. --- src/librustc_mir/dataflow/impls/indirect_mutation.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/indirect_mutation.rs b/src/librustc_mir/dataflow/impls/indirect_mutation.rs index 990425c3252e0..8819e50772020 100644 --- a/src/librustc_mir/dataflow/impls/indirect_mutation.rs +++ b/src/librustc_mir/dataflow/impls/indirect_mutation.rs @@ -104,15 +104,17 @@ impl<'tcx> TransferFunction<'_, '_, 'tcx> { kind: mir::BorrowKind, borrowed_place: &mir::Place<'tcx>, ) -> bool { - let borrowed_ty = borrowed_place.ty(self.body, self.tcx).ty; + let base_ty = borrowed_place.base.ty(self.body).ty; // Zero-sized types cannot be mutated, since there is nothing inside to mutate. // // FIXME: For now, we only exempt arrays of length zero. We need to carefully // consider the effects before extending this to all ZSTs. - if let ty::Array(_, len) = borrowed_ty.kind { - if len.try_eval_usize(self.tcx, self.param_env) == Some(0) { - return false; + if borrowed_place.projection.is_empty() { + if let ty::Array(_, len) = base_ty.kind { + if len.try_eval_usize(self.tcx, self.param_env) == Some(0) { + return false; + } } } @@ -122,7 +124,7 @@ impl<'tcx> TransferFunction<'_, '_, 'tcx> { | mir::BorrowKind::Shared | mir::BorrowKind::Shallow | mir::BorrowKind::Unique - => !borrowed_ty.is_freeze(self.tcx, self.param_env, DUMMY_SP), + => !base_ty.is_freeze(self.tcx, self.param_env, DUMMY_SP), } } } From da0969f27fb56faaf83d0d10783fdd46cc51d3b5 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 2 Oct 2019 16:20:42 -0700 Subject: [PATCH 2/6] Update `IndirectlyMutableLocals` test with correct behavior --- src/test/ui/mir-dataflow/indirect-mutation-offset.rs | 10 +++++++--- .../ui/mir-dataflow/indirect-mutation-offset.stderr | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/test/ui/mir-dataflow/indirect-mutation-offset.rs b/src/test/ui/mir-dataflow/indirect-mutation-offset.rs index 804b70d26527a..26fa57baaafbc 100644 --- a/src/test/ui/mir-dataflow/indirect-mutation-offset.rs +++ b/src/test/ui/mir-dataflow/indirect-mutation-offset.rs @@ -19,7 +19,11 @@ const BOO: i32 = { cell: UnsafeCell::new(0), }; - let p_zst: *const _ = &x.zst ; // Doesn't cause `x` to get marked as indirectly mutable. + unsafe { rustc_peek(x) }; //~ ERROR rustc_peek: bit not set + + let p_zst: *const _ = &x.zst ; + + unsafe { rustc_peek(x) }; let rmut_cell = unsafe { // Take advantage of the fact that `zst` and `cell` are at the same location in memory. @@ -30,9 +34,9 @@ const BOO: i32 = { &mut *pmut_cell }; - *rmut_cell = 42; // Mutates `x` indirectly even though `x` is not marked indirectly mutable!!! + *rmut_cell = 42; let val = *rmut_cell; - unsafe { rustc_peek(x) }; //~ ERROR rustc_peek: bit not set + unsafe { rustc_peek(x) }; val }; diff --git a/src/test/ui/mir-dataflow/indirect-mutation-offset.stderr b/src/test/ui/mir-dataflow/indirect-mutation-offset.stderr index 16bd17813134a..65c31197b762e 100644 --- a/src/test/ui/mir-dataflow/indirect-mutation-offset.stderr +++ b/src/test/ui/mir-dataflow/indirect-mutation-offset.stderr @@ -1,5 +1,5 @@ error: rustc_peek: bit not set - --> $DIR/indirect-mutation-offset.rs:35:14 + --> $DIR/indirect-mutation-offset.rs:21:14 | LL | unsafe { rustc_peek(x) }; | ^^^^^^^^^^^^^ From 6487ce0bf4e8ff1a97cd9d12a4c40a23f8560f7c Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 2 Oct 2019 16:21:01 -0700 Subject: [PATCH 3/6] Add more peek tests for `IndirectlyMutableLocals` --- .../ui/mir-dataflow/indirect-mutation-1.rs | 33 +++++++++++++++++++ .../mir-dataflow/indirect-mutation-1.stderr | 16 +++++++++ .../ui/mir-dataflow/indirect-mutation-2.rs | 30 +++++++++++++++++ .../mir-dataflow/indirect-mutation-2.stderr | 22 +++++++++++++ 4 files changed, 101 insertions(+) create mode 100644 src/test/ui/mir-dataflow/indirect-mutation-1.rs create mode 100644 src/test/ui/mir-dataflow/indirect-mutation-1.stderr create mode 100644 src/test/ui/mir-dataflow/indirect-mutation-2.rs create mode 100644 src/test/ui/mir-dataflow/indirect-mutation-2.stderr diff --git a/src/test/ui/mir-dataflow/indirect-mutation-1.rs b/src/test/ui/mir-dataflow/indirect-mutation-1.rs new file mode 100644 index 0000000000000..3d0f891e5a2d1 --- /dev/null +++ b/src/test/ui/mir-dataflow/indirect-mutation-1.rs @@ -0,0 +1,33 @@ +#![feature(core_intrinsics, rustc_attrs, const_raw_ptr_deref)] + +use std::cell::Cell; +use std::intrinsics::rustc_peek; + +#[rustc_mir(rustc_peek_indirectly_mutable, stop_after_dataflow)] +pub fn mut_ref(flag: bool) -> i32 { + let mut i = 0; + let cell = Cell::new(0); + + if flag { + let p = &mut i; + unsafe { rustc_peek(i) }; + *p = 1; + + let p = &cell; + unsafe { rustc_peek(cell) }; + p.set(2); + } else { + unsafe { rustc_peek(i) }; //~ ERROR rustc_peek: bit not set + unsafe { rustc_peek(cell) }; //~ ERROR rustc_peek: bit not set + + let p = &mut cell; + unsafe { rustc_peek(cell) }; + *p = Cell::new(3); + } + + unsafe { rustc_peek(i) }; + unsafe { rustc_peek(cell) }; + i + cell.get() +} + +fn main() {} diff --git a/src/test/ui/mir-dataflow/indirect-mutation-1.stderr b/src/test/ui/mir-dataflow/indirect-mutation-1.stderr new file mode 100644 index 0000000000000..e09e3feedaa4b --- /dev/null +++ b/src/test/ui/mir-dataflow/indirect-mutation-1.stderr @@ -0,0 +1,16 @@ +error: rustc_peek: bit not set + --> $DIR/indirect-mutation-1.rs:20:18 + | +LL | unsafe { rustc_peek(i) }; + | ^^^^^^^^^^^^^ + +error: rustc_peek: bit not set + --> $DIR/indirect-mutation-1.rs:21:18 + | +LL | unsafe { rustc_peek(cell) }; + | ^^^^^^^^^^^^^^^^ + +error: stop_after_dataflow ended compilation + +error: aborting due to 3 previous errors + diff --git a/src/test/ui/mir-dataflow/indirect-mutation-2.rs b/src/test/ui/mir-dataflow/indirect-mutation-2.rs new file mode 100644 index 0000000000000..2953aaa1135c4 --- /dev/null +++ b/src/test/ui/mir-dataflow/indirect-mutation-2.rs @@ -0,0 +1,30 @@ +#![feature(core_intrinsics, rustc_attrs, const_raw_ptr_deref)] + +use std::cell::Cell; +use std::intrinsics::rustc_peek; + +struct Arr { + inner: [Cell; 0], + _peek: (), +} + +// Zero-sized arrays are never mutable. +#[rustc_mir(rustc_peek_indirectly_mutable, stop_after_dataflow)] +pub fn zst(flag: bool) { + { + let arr: [i32; 0] = []; + let s: &mut [i32] = &mut arr; + unsafe { rustc_peek(arr) }; //~ ERROR rustc_peek: bit not set + } + + { + let arr: [Cell; 0] = []; + let s: &[Cell] = &arr; + unsafe { rustc_peek(arr) }; //~ ERROR rustc_peek: bit not set + + let ss = &s; + unsafe { rustc_peek(arr) }; //~ ERROR rustc_peek: bit not set + } +} + +fn main() {} diff --git a/src/test/ui/mir-dataflow/indirect-mutation-2.stderr b/src/test/ui/mir-dataflow/indirect-mutation-2.stderr new file mode 100644 index 0000000000000..f8ef668951fe6 --- /dev/null +++ b/src/test/ui/mir-dataflow/indirect-mutation-2.stderr @@ -0,0 +1,22 @@ +error: rustc_peek: bit not set + --> $DIR/indirect-mutation-2.rs:17:18 + | +LL | unsafe { rustc_peek(arr) }; + | ^^^^^^^^^^^^^^^ + +error: rustc_peek: bit not set + --> $DIR/indirect-mutation-2.rs:23:18 + | +LL | unsafe { rustc_peek(arr) }; + | ^^^^^^^^^^^^^^^ + +error: rustc_peek: bit not set + --> $DIR/indirect-mutation-2.rs:26:18 + | +LL | unsafe { rustc_peek(arr) }; + | ^^^^^^^^^^^^^^^ + +error: stop_after_dataflow ended compilation + +error: aborting due to 4 previous errors + From 2f894298a19f6c91967ec08d7dc73f388de6998f Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 2 Oct 2019 15:35:59 -0700 Subject: [PATCH 4/6] Remove `borrowck_graphviz_postflow` from test --- src/test/ui/mir-dataflow/indirect-mutation-offset.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/ui/mir-dataflow/indirect-mutation-offset.rs b/src/test/ui/mir-dataflow/indirect-mutation-offset.rs index 26fa57baaafbc..da56ff3616dc9 100644 --- a/src/test/ui/mir-dataflow/indirect-mutation-offset.rs +++ b/src/test/ui/mir-dataflow/indirect-mutation-offset.rs @@ -12,7 +12,6 @@ struct PartialInteriorMut { } #[rustc_mir(rustc_peek_indirectly_mutable,stop_after_dataflow)] -#[rustc_mir(borrowck_graphviz_postflow="indirect.dot")] const BOO: i32 = { let x = PartialInteriorMut { zst: [], From 895f9c4a9de55bdbcedb39f50ac4a3070a212af0 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 2 Oct 2019 16:19:32 -0700 Subject: [PATCH 5/6] Downgrade unnecessary `warn` to `trace` --- src/librustc_mir/transform/rustc_peek.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index 6edd28a4259a5..299f0082d0604 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -276,7 +276,7 @@ impl<'tcx> RustcPeekAt<'tcx> for IndirectlyMutableLocals<'_, 'tcx> { flow_state: &BitSet, call: PeekCall, ) { - warn!("peek_at: place={:?}", place); + trace!("peek_at: place={:?}", place); let local = match place { mir::Place { base: mir::PlaceBase::Local(l), projection: box [] } => *l, _ => { From 91c77e9b4c3a2e8eff1196b929e4386170b4b1da Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 3 Oct 2019 09:59:30 -0700 Subject: [PATCH 6/6] Enusure tests would build outside of peek --- src/test/ui/mir-dataflow/indirect-mutation-1.rs | 4 ++-- src/test/ui/mir-dataflow/indirect-mutation-2.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/ui/mir-dataflow/indirect-mutation-1.rs b/src/test/ui/mir-dataflow/indirect-mutation-1.rs index 3d0f891e5a2d1..03c9034bd130f 100644 --- a/src/test/ui/mir-dataflow/indirect-mutation-1.rs +++ b/src/test/ui/mir-dataflow/indirect-mutation-1.rs @@ -1,4 +1,4 @@ -#![feature(core_intrinsics, rustc_attrs, const_raw_ptr_deref)] +#![feature(core_intrinsics, rustc_attrs)] use std::cell::Cell; use std::intrinsics::rustc_peek; @@ -6,7 +6,7 @@ use std::intrinsics::rustc_peek; #[rustc_mir(rustc_peek_indirectly_mutable, stop_after_dataflow)] pub fn mut_ref(flag: bool) -> i32 { let mut i = 0; - let cell = Cell::new(0); + let mut cell = Cell::new(0); if flag { let p = &mut i; diff --git a/src/test/ui/mir-dataflow/indirect-mutation-2.rs b/src/test/ui/mir-dataflow/indirect-mutation-2.rs index 2953aaa1135c4..98b0c150e26d7 100644 --- a/src/test/ui/mir-dataflow/indirect-mutation-2.rs +++ b/src/test/ui/mir-dataflow/indirect-mutation-2.rs @@ -1,4 +1,4 @@ -#![feature(core_intrinsics, rustc_attrs, const_raw_ptr_deref)] +#![feature(core_intrinsics, rustc_attrs)] use std::cell::Cell; use std::intrinsics::rustc_peek; @@ -12,7 +12,7 @@ struct Arr { #[rustc_mir(rustc_peek_indirectly_mutable, stop_after_dataflow)] pub fn zst(flag: bool) { { - let arr: [i32; 0] = []; + let mut arr: [i32; 0] = []; let s: &mut [i32] = &mut arr; unsafe { rustc_peek(arr) }; //~ ERROR rustc_peek: bit not set }