From ec8cc029c1904bcbbb6ede8fba0251c2aaaf70f8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 21 Nov 2018 15:44:47 +0100 Subject: [PATCH 1/3] on a deref, check that we are not using a mutable ref with a frozen tag --- src/stacked_borrows.rs | 39 +++++-------------- .../static_memory_modification.rs | 3 -- 2 files changed, 9 insertions(+), 33 deletions(-) diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 063a544baa6..16cd3e91643 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -151,6 +151,12 @@ impl<'tcx> Stack { /// Returns the index of the item we matched, `None` if it was the frozen one. /// `kind` indicates which kind of reference is being dereferenced. fn deref(&self, bor: Borrow, kind: RefKind) -> Result, String> { + // Exclude unique ref and frozen tag. + match (kind, bor) { + (RefKind::Unique, Borrow::Shr(Some(_))) => + return Err(format!("Encountered mutable reference with frozen tag")), + _ => {} + } // Checks related to freezing match bor { Borrow::Shr(Some(bor_t)) if kind == RefKind::Frozen => { @@ -490,36 +496,9 @@ fn ptr_dereference( if let Some(mutability) = mutability { format!("{:?}", mutability) } else { format!("raw") }, place.ptr, place.layout.ty); let ptr = place.ptr.to_ptr()?; - // In principle we should not have to do anything here. However, with transmutes involved, - // it can happen that the tag of `ptr` does not actually match `mutability`, and we - // should adjust for that. - // Notably, the compiler can introduce such transmutes by optimizing away `&[mut]*`. - // That can transmute a raw ptr to a (shared/mut) ref, and a mut ref to a shared one. - match (mutability, ptr.tag) { - (None, _) => { - // No further validation on raw accesses. - return Ok(()); - } - (Some(MutMutable), Borrow::Uniq(_)) | - (Some(MutImmutable), Borrow::Shr(_)) => { - // Expected combinations. Nothing to do. - } - (Some(MutMutable), Borrow::Shr(None)) => { - // Raw transmuted to mut ref. This is something real unsafe code does. - // We cannot reborrow here because we do not want to mutate state on a deref. - } - (Some(MutImmutable), Borrow::Uniq(_)) => { - // A mut got transmuted to shr. Can happen even from compiler transformations: - // `&*x` gets optimized to `x` even when `x` is a `&mut`. - } - (Some(MutMutable), Borrow::Shr(Some(_))) => { - // This is just invalid: A shr got transmuted to a mut. - // If we ever allow this, we have to consider what we do when a turn a - // `Raw`-tagged `&mut` into a raw pointer pointing to a frozen location. - // We probably do not want to allow that, but we have to allow - // turning a `Raw`-tagged `&` into a raw ptr to a frozen location. - return err!(MachineError(format!("Encountered mutable reference with frozen tag {:?}", ptr.tag))) - } + if mutability.is_none() { + // No further checks on raw derefs -- only the access itself will be checked. + return Ok(()); } // Get the allocation diff --git a/tests/compile-fail-fullmir/stacked_borrows/static_memory_modification.rs b/tests/compile-fail-fullmir/stacked_borrows/static_memory_modification.rs index 5c605eff678..c092cbfe509 100644 --- a/tests/compile-fail-fullmir/stacked_borrows/static_memory_modification.rs +++ b/tests/compile-fail-fullmir/stacked_borrows/static_memory_modification.rs @@ -1,6 +1,3 @@ -// FIXME still considering whether we are okay with this not being an error -// ignore-test - static X: usize = 5; #[allow(mutable_transmutes)] From 41f89beb3f03cb7cdbfbd754ac2eee9e1f153f18 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 21 Nov 2018 16:01:39 +0100 Subject: [PATCH 2/3] if let --- src/stacked_borrows.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 16cd3e91643..21addbfbf78 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -151,11 +151,9 @@ impl<'tcx> Stack { /// Returns the index of the item we matched, `None` if it was the frozen one. /// `kind` indicates which kind of reference is being dereferenced. fn deref(&self, bor: Borrow, kind: RefKind) -> Result, String> { - // Exclude unique ref and frozen tag. - match (kind, bor) { - (RefKind::Unique, Borrow::Shr(Some(_))) => - return Err(format!("Encountered mutable reference with frozen tag")), - _ => {} + // Exclude unique ref with frozen tag. + if let (RefKind::Unique, Borrow::Shr(Some(_))) = (kind, bor) { + return Err(format!("Encountered mutable reference with frozen tag")); } // Checks related to freezing match bor { From 694d2490f1f10cc46aff0452874ade547e4936b9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 21 Nov 2018 16:02:38 +0100 Subject: [PATCH 3/3] slightly more verbose error msg --- src/stacked_borrows.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 21addbfbf78..f26ef40ea9f 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -153,7 +153,7 @@ impl<'tcx> Stack { fn deref(&self, bor: Borrow, kind: RefKind) -> Result, String> { // Exclude unique ref with frozen tag. if let (RefKind::Unique, Borrow::Shr(Some(_))) = (kind, bor) { - return Err(format!("Encountered mutable reference with frozen tag")); + return Err(format!("Encountered mutable reference with frozen tag ({:?})", bor)); } // Checks related to freezing match bor {