From abe890d2ce8e6fa08bb5c2ccc82e45cdfbd35481 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 27 Aug 2022 11:19:35 -0400 Subject: [PATCH] slightly improve protector-related error messages also rename some tests that still used outdated "barrier" terminology --- src/stacked_borrows/diagnostics.rs | 36 +++++++++++-------- .../fail/stacked_borrows/aliasing_mut1.stderr | 8 ++--- .../fail/stacked_borrows/aliasing_mut2.stderr | 12 +++---- .../fail/stacked_borrows/aliasing_mut4.stderr | 12 +++---- ...r1.rs => deallocate_against_protector1.rs} | 0 ...r => deallocate_against_protector1.stderr} | 14 ++++---- ...r2.rs => deallocate_against_protector2.rs} | 0 ...r => deallocate_against_protector2.stderr} | 14 ++++---- .../stacked_borrows/illegal_write6.stderr | 12 +++---- ...r1.rs => invalidate_against_protector1.rs} | 0 ...r => invalidate_against_protector1.stderr} | 24 ++++++------- ...r2.rs => invalidate_against_protector2.rs} | 0 ...r => invalidate_against_protector2.stderr} | 24 ++++++------- .../stacked_borrows/newtype_retagging.stderr | 12 +++---- 14 files changed, 86 insertions(+), 82 deletions(-) rename tests/fail/stacked_borrows/{deallocate_against_barrier1.rs => deallocate_against_protector1.rs} (100%) rename tests/fail/stacked_borrows/{deallocate_against_barrier1.stderr => deallocate_against_protector1.stderr} (75%) rename tests/fail/stacked_borrows/{deallocate_against_barrier2.rs => deallocate_against_protector2.rs} (100%) rename tests/fail/stacked_borrows/{deallocate_against_barrier2.stderr => deallocate_against_protector2.stderr} (76%) rename tests/fail/stacked_borrows/{invalidate_against_barrier1.rs => invalidate_against_protector1.rs} (100%) rename tests/fail/stacked_borrows/{invalidate_against_barrier1.stderr => invalidate_against_protector1.stderr} (69%) rename tests/fail/stacked_borrows/{invalidate_against_barrier2.rs => invalidate_against_protector2.rs} (100%) rename tests/fail/stacked_borrows/{invalidate_against_barrier2.stderr => invalidate_against_protector2.stderr} (69%) diff --git a/src/stacked_borrows/diagnostics.rs b/src/stacked_borrows/diagnostics.rs index 741a3d363dd3..87f0ce741918 100644 --- a/src/stacked_borrows/diagnostics.rs +++ b/src/stacked_borrows/diagnostics.rs @@ -2,7 +2,7 @@ use smallvec::SmallVec; use std::fmt; use rustc_middle::mir::interpret::{alloc_range, AllocId, AllocRange}; -use rustc_span::{Span, SpanData}; +use rustc_span::{Span, SpanData, DUMMY_SP}; use rustc_target::abi::Size; use crate::helpers::CurrentSpan; @@ -91,6 +91,7 @@ impl fmt::Display for InvalidationCause { #[derive(Clone, Debug)] struct Protection { + /// The parent tag from which this protected tag was derived. orig_tag: ProvenanceExtra, tag: SbTag, span: Span, @@ -342,32 +343,39 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir let protected = protector_tag .and_then(|protector| { - self.history.protectors.iter().find_map(|protection| { - if protection.tag == protector { - Some((protection.orig_tag, protection.span.data())) - } else { - None - } + self.history.protectors.iter().find(|protection| { + protection.tag == protector }) }) - .and_then(|(tag, call_span)| { + .and_then(|protection| { self.history.creations.iter().rev().find_map(|event| { - if ProvenanceExtra::Concrete(event.retag.new_tag) == tag { - Some((event.retag.orig_tag, event.span.data(), call_span)) + if ProvenanceExtra::Concrete(event.retag.new_tag) == protection.orig_tag { + Some((protection, event)) } else { None } }) }) - .map(|(protecting_tag, protecting_tag_span, protection_span)| { + .map(|(protection, protection_parent)| { + let protected_tag = protection.tag; [ ( format!( - "{tag:?} was protected due to {protecting_tag:?} which was created here" + "{tag:?} cannot be used for memory access because that would remove protected tag {protected_tag:?}, protected by this function call", ), - protecting_tag_span, + protection.span.data(), ), - (format!("this protector is live for this call"), protection_span), + if protection_parent.retag.new_tag == tag { + (format!("{protected_tag:?} was derived from {tag:?}, the tag used for this memory access"), DUMMY_SP.data()) + } else { + ( + format!( + "{protected_tag:?} was derived from {protected_parent_tag:?}, which in turn was created here", + protected_parent_tag = protection_parent.retag.new_tag, + ), + protection_parent.span.data() + ) + } ] }); diff --git a/tests/fail/stacked_borrows/aliasing_mut1.stderr b/tests/fail/stacked_borrows/aliasing_mut1.stderr index e3f05f3350a5..e5be3061b32c 100644 --- a/tests/fail/stacked_borrows/aliasing_mut1.stderr +++ b/tests/fail/stacked_borrows/aliasing_mut1.stderr @@ -11,16 +11,12 @@ help: was created by a Unique retag at offsets [0x0..0x4] | LL | let xraw: *mut i32 = unsafe { mem::transmute(&mut x) }; | ^^^^^^ -help: was protected due to which was created here - --> $DIR/aliasing_mut1.rs:LL:CC - | -LL | let xraw: *mut i32 = unsafe { mem::transmute(&mut x) }; - | ^^^^^^ -help: this protector is live for this call +help: cannot be used for memory access because that would remove protected tag , protected by this function call --> $DIR/aliasing_mut1.rs:LL:CC | LL | pub fn safe(_x: &mut i32, _y: &mut i32) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: was derived from , the tag used for this memory access = note: backtrace: = note: inside `safe` at $DIR/aliasing_mut1.rs:LL:CC note: inside `main` at $DIR/aliasing_mut1.rs:LL:CC diff --git a/tests/fail/stacked_borrows/aliasing_mut2.stderr b/tests/fail/stacked_borrows/aliasing_mut2.stderr index 94c2ffa07f71..c3dd3a893c07 100644 --- a/tests/fail/stacked_borrows/aliasing_mut2.stderr +++ b/tests/fail/stacked_borrows/aliasing_mut2.stderr @@ -11,16 +11,16 @@ help: was created by a Unique retag at offsets [0x0..0x4] | LL | let xref = &mut x; | ^^^^^^ -help: was protected due to which was created here - --> $DIR/aliasing_mut2.rs:LL:CC - | -LL | safe_raw(xshr, xraw); - | ^^^^ -help: this protector is live for this call +help: cannot be used for memory access because that would remove protected tag , protected by this function call --> $DIR/aliasing_mut2.rs:LL:CC | LL | pub fn safe(_x: &i32, _y: &mut i32) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: was derived from , which in turn was created here + --> $DIR/aliasing_mut2.rs:LL:CC + | +LL | safe_raw(xshr, xraw); + | ^^^^ = note: backtrace: = note: inside `safe` at $DIR/aliasing_mut2.rs:LL:CC note: inside `main` at $DIR/aliasing_mut2.rs:LL:CC diff --git a/tests/fail/stacked_borrows/aliasing_mut4.stderr b/tests/fail/stacked_borrows/aliasing_mut4.stderr index f48d39b2e49f..601422ece302 100644 --- a/tests/fail/stacked_borrows/aliasing_mut4.stderr +++ b/tests/fail/stacked_borrows/aliasing_mut4.stderr @@ -11,16 +11,16 @@ help: was created by a Unique retag at offsets [0x0..0x4] | LL | let xref = &mut x; | ^^^^^^ -help: was protected due to which was created here - --> $DIR/aliasing_mut4.rs:LL:CC - | -LL | safe_raw(xshr, xraw as *mut _); - | ^^^^ -help: this protector is live for this call +help: cannot be used for memory access because that would remove protected tag , protected by this function call --> $DIR/aliasing_mut4.rs:LL:CC | LL | pub fn safe(_x: &i32, _y: &mut Cell) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: was derived from , which in turn was created here + --> $DIR/aliasing_mut4.rs:LL:CC + | +LL | safe_raw(xshr, xraw as *mut _); + | ^^^^ = note: backtrace: = note: inside `safe` at $DIR/aliasing_mut4.rs:LL:CC note: inside `main` at $DIR/aliasing_mut4.rs:LL:CC diff --git a/tests/fail/stacked_borrows/deallocate_against_barrier1.rs b/tests/fail/stacked_borrows/deallocate_against_protector1.rs similarity index 100% rename from tests/fail/stacked_borrows/deallocate_against_barrier1.rs rename to tests/fail/stacked_borrows/deallocate_against_protector1.rs diff --git a/tests/fail/stacked_borrows/deallocate_against_barrier1.stderr b/tests/fail/stacked_borrows/deallocate_against_protector1.stderr similarity index 75% rename from tests/fail/stacked_borrows/deallocate_against_barrier1.stderr rename to tests/fail/stacked_borrows/deallocate_against_protector1.stderr index 689c0a5deae6..2ead0c6a9dda 100644 --- a/tests/fail/stacked_borrows/deallocate_against_barrier1.stderr +++ b/tests/fail/stacked_borrows/deallocate_against_protector1.stderr @@ -12,19 +12,19 @@ LL | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) } = note: inside `alloc::alloc::box_free::` at RUSTLIB/alloc/src/alloc.rs:LL:CC = note: inside `std::ptr::drop_in_place::> - shim(Some(std::boxed::Box))` at RUSTLIB/core/src/ptr/mod.rs:LL:CC = note: inside `std::mem::drop::>` at RUSTLIB/core/src/mem/mod.rs:LL:CC -note: inside closure at $DIR/deallocate_against_barrier1.rs:LL:CC - --> $DIR/deallocate_against_barrier1.rs:LL:CC +note: inside closure at $DIR/deallocate_against_protector1.rs:LL:CC + --> $DIR/deallocate_against_protector1.rs:LL:CC | LL | drop(unsafe { Box::from_raw(raw) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: inside `<[closure@$DIR/deallocate_against_barrier1.rs:LL:CC] as std::ops::FnOnce<(&mut i32,)>>::call_once - shim` at RUSTLIB/core/src/ops/function.rs:LL:CC -note: inside `inner` at $DIR/deallocate_against_barrier1.rs:LL:CC - --> $DIR/deallocate_against_barrier1.rs:LL:CC + = note: inside `<[closure@$DIR/deallocate_against_protector1.rs:LL:CC] as std::ops::FnOnce<(&mut i32,)>>::call_once - shim` at RUSTLIB/core/src/ops/function.rs:LL:CC +note: inside `inner` at $DIR/deallocate_against_protector1.rs:LL:CC + --> $DIR/deallocate_against_protector1.rs:LL:CC | LL | f(x) | ^^^^ -note: inside `main` at $DIR/deallocate_against_barrier1.rs:LL:CC - --> $DIR/deallocate_against_barrier1.rs:LL:CC +note: inside `main` at $DIR/deallocate_against_protector1.rs:LL:CC + --> $DIR/deallocate_against_protector1.rs:LL:CC | LL | / inner(Box::leak(Box::new(0)), |x| { LL | | let raw = x as *mut _; diff --git a/tests/fail/stacked_borrows/deallocate_against_barrier2.rs b/tests/fail/stacked_borrows/deallocate_against_protector2.rs similarity index 100% rename from tests/fail/stacked_borrows/deallocate_against_barrier2.rs rename to tests/fail/stacked_borrows/deallocate_against_protector2.rs diff --git a/tests/fail/stacked_borrows/deallocate_against_barrier2.stderr b/tests/fail/stacked_borrows/deallocate_against_protector2.stderr similarity index 76% rename from tests/fail/stacked_borrows/deallocate_against_barrier2.stderr rename to tests/fail/stacked_borrows/deallocate_against_protector2.stderr index a1a7ce0c6bb6..60be936bd7e4 100644 --- a/tests/fail/stacked_borrows/deallocate_against_barrier2.stderr +++ b/tests/fail/stacked_borrows/deallocate_against_protector2.stderr @@ -12,19 +12,19 @@ LL | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) } = note: inside `alloc::alloc::box_free::` at RUSTLIB/alloc/src/alloc.rs:LL:CC = note: inside `std::ptr::drop_in_place::> - shim(Some(std::boxed::Box))` at RUSTLIB/core/src/ptr/mod.rs:LL:CC = note: inside `std::mem::drop::>` at RUSTLIB/core/src/mem/mod.rs:LL:CC -note: inside closure at $DIR/deallocate_against_barrier2.rs:LL:CC - --> $DIR/deallocate_against_barrier2.rs:LL:CC +note: inside closure at $DIR/deallocate_against_protector2.rs:LL:CC + --> $DIR/deallocate_against_protector2.rs:LL:CC | LL | drop(unsafe { Box::from_raw(raw) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: inside `<[closure@$DIR/deallocate_against_barrier2.rs:LL:CC] as std::ops::FnOnce<(&mut NotUnpin,)>>::call_once - shim` at RUSTLIB/core/src/ops/function.rs:LL:CC -note: inside `inner` at $DIR/deallocate_against_barrier2.rs:LL:CC - --> $DIR/deallocate_against_barrier2.rs:LL:CC + = note: inside `<[closure@$DIR/deallocate_against_protector2.rs:LL:CC] as std::ops::FnOnce<(&mut NotUnpin,)>>::call_once - shim` at RUSTLIB/core/src/ops/function.rs:LL:CC +note: inside `inner` at $DIR/deallocate_against_protector2.rs:LL:CC + --> $DIR/deallocate_against_protector2.rs:LL:CC | LL | f(x) | ^^^^ -note: inside `main` at $DIR/deallocate_against_barrier2.rs:LL:CC - --> $DIR/deallocate_against_barrier2.rs:LL:CC +note: inside `main` at $DIR/deallocate_against_protector2.rs:LL:CC + --> $DIR/deallocate_against_protector2.rs:LL:CC | LL | / inner(Box::leak(Box::new(NotUnpin(0, PhantomPinned))), |x| { LL | | let raw = x as *mut _; diff --git a/tests/fail/stacked_borrows/illegal_write6.stderr b/tests/fail/stacked_borrows/illegal_write6.stderr index 56576b1ff39b..fd3b19adcf5e 100644 --- a/tests/fail/stacked_borrows/illegal_write6.stderr +++ b/tests/fail/stacked_borrows/illegal_write6.stderr @@ -11,12 +11,7 @@ help: was created by a SharedReadWrite retag at offsets [0x0..0x4] | LL | let p = x as *mut u32; | ^ -help: was protected due to which was created here - --> $DIR/illegal_write6.rs:LL:CC - | -LL | foo(x, p); - | ^ -help: this protector is live for this call +help: cannot be used for memory access because that would remove protected tag , protected by this function call --> $DIR/illegal_write6.rs:LL:CC | LL | / fn foo(a: &mut u32, y: *mut u32) -> u32 { @@ -26,6 +21,11 @@ LL | | unsafe { *y = 2 }; LL | | return *a; LL | | } | |_^ +help: was derived from , which in turn was created here + --> $DIR/illegal_write6.rs:LL:CC + | +LL | foo(x, p); + | ^ = note: backtrace: = note: inside `foo` at $DIR/illegal_write6.rs:LL:CC note: inside `main` at $DIR/illegal_write6.rs:LL:CC diff --git a/tests/fail/stacked_borrows/invalidate_against_barrier1.rs b/tests/fail/stacked_borrows/invalidate_against_protector1.rs similarity index 100% rename from tests/fail/stacked_borrows/invalidate_against_barrier1.rs rename to tests/fail/stacked_borrows/invalidate_against_protector1.rs diff --git a/tests/fail/stacked_borrows/invalidate_against_barrier1.stderr b/tests/fail/stacked_borrows/invalidate_against_protector1.stderr similarity index 69% rename from tests/fail/stacked_borrows/invalidate_against_barrier1.stderr rename to tests/fail/stacked_borrows/invalidate_against_protector1.stderr index 378f7b7d1ef5..18236adec882 100644 --- a/tests/fail/stacked_borrows/invalidate_against_barrier1.stderr +++ b/tests/fail/stacked_borrows/invalidate_against_protector1.stderr @@ -1,5 +1,5 @@ error: Undefined Behavior: not granting access to tag because incompatible item [Unique for ] is protected by call ID - --> $DIR/invalidate_against_barrier1.rs:LL:CC + --> $DIR/invalidate_against_protector1.rs:LL:CC | LL | let _val = unsafe { *x }; | ^^ not granting access to tag because incompatible item [Unique for ] is protected by call ID @@ -7,17 +7,12 @@ LL | let _val = unsafe { *x }; = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information help: was created by a SharedReadWrite retag at offsets [0x0..0x4] - --> $DIR/invalidate_against_barrier1.rs:LL:CC + --> $DIR/invalidate_against_protector1.rs:LL:CC | LL | let xraw = &mut x as *mut _; | ^^^^^^ -help: was protected due to which was created here - --> $DIR/invalidate_against_barrier1.rs:LL:CC - | -LL | inner(xraw, xref); - | ^^^^ -help: this protector is live for this call - --> $DIR/invalidate_against_barrier1.rs:LL:CC +help: cannot be used for memory access because that would remove protected tag , protected by this function call + --> $DIR/invalidate_against_protector1.rs:LL:CC | LL | / fn inner(x: *mut i32, _y: &mut i32) { LL | | // If `x` and `y` alias, retagging is fine with this... but we really @@ -26,10 +21,15 @@ LL | | // unique for the duration of this call. LL | | let _val = unsafe { *x }; LL | | } | |_^ +help: was derived from , which in turn was created here + --> $DIR/invalidate_against_protector1.rs:LL:CC + | +LL | inner(xraw, xref); + | ^^^^ = note: backtrace: - = note: inside `inner` at $DIR/invalidate_against_barrier1.rs:LL:CC -note: inside `main` at $DIR/invalidate_against_barrier1.rs:LL:CC - --> $DIR/invalidate_against_barrier1.rs:LL:CC + = note: inside `inner` at $DIR/invalidate_against_protector1.rs:LL:CC +note: inside `main` at $DIR/invalidate_against_protector1.rs:LL:CC + --> $DIR/invalidate_against_protector1.rs:LL:CC | LL | inner(xraw, xref); | ^^^^^^^^^^^^^^^^^ diff --git a/tests/fail/stacked_borrows/invalidate_against_barrier2.rs b/tests/fail/stacked_borrows/invalidate_against_protector2.rs similarity index 100% rename from tests/fail/stacked_borrows/invalidate_against_barrier2.rs rename to tests/fail/stacked_borrows/invalidate_against_protector2.rs diff --git a/tests/fail/stacked_borrows/invalidate_against_barrier2.stderr b/tests/fail/stacked_borrows/invalidate_against_protector2.stderr similarity index 69% rename from tests/fail/stacked_borrows/invalidate_against_barrier2.stderr rename to tests/fail/stacked_borrows/invalidate_against_protector2.stderr index 0352b671823e..20b9d47bbef3 100644 --- a/tests/fail/stacked_borrows/invalidate_against_barrier2.stderr +++ b/tests/fail/stacked_borrows/invalidate_against_protector2.stderr @@ -1,5 +1,5 @@ error: Undefined Behavior: not granting access to tag because incompatible item [SharedReadOnly for ] is protected by call ID - --> $DIR/invalidate_against_barrier2.rs:LL:CC + --> $DIR/invalidate_against_protector2.rs:LL:CC | LL | unsafe { *x = 0 }; | ^^^^^^ not granting access to tag because incompatible item [SharedReadOnly for ] is protected by call ID @@ -7,17 +7,12 @@ LL | unsafe { *x = 0 }; = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information help: was created by a SharedReadWrite retag at offsets [0x0..0x4] - --> $DIR/invalidate_against_barrier2.rs:LL:CC + --> $DIR/invalidate_against_protector2.rs:LL:CC | LL | let xraw = &mut x as *mut _; | ^^^^^^ -help: was protected due to which was created here - --> $DIR/invalidate_against_barrier2.rs:LL:CC - | -LL | inner(xraw, xref); - | ^^^^ -help: this protector is live for this call - --> $DIR/invalidate_against_barrier2.rs:LL:CC +help: cannot be used for memory access because that would remove protected tag , protected by this function call + --> $DIR/invalidate_against_protector2.rs:LL:CC | LL | / fn inner(x: *mut i32, _y: &i32) { LL | | // If `x` and `y` alias, retagging is fine with this... but we really @@ -26,10 +21,15 @@ LL | | // immutable for the duration of this call. LL | | unsafe { *x = 0 }; LL | | } | |_^ +help: was derived from , which in turn was created here + --> $DIR/invalidate_against_protector2.rs:LL:CC + | +LL | inner(xraw, xref); + | ^^^^ = note: backtrace: - = note: inside `inner` at $DIR/invalidate_against_barrier2.rs:LL:CC -note: inside `main` at $DIR/invalidate_against_barrier2.rs:LL:CC - --> $DIR/invalidate_against_barrier2.rs:LL:CC + = note: inside `inner` at $DIR/invalidate_against_protector2.rs:LL:CC +note: inside `main` at $DIR/invalidate_against_protector2.rs:LL:CC + --> $DIR/invalidate_against_protector2.rs:LL:CC | LL | inner(xraw, xref); | ^^^^^^^^^^^^^^^^^ diff --git a/tests/fail/stacked_borrows/newtype_retagging.stderr b/tests/fail/stacked_borrows/newtype_retagging.stderr index 2d26787231dc..e75502d36105 100644 --- a/tests/fail/stacked_borrows/newtype_retagging.stderr +++ b/tests/fail/stacked_borrows/newtype_retagging.stderr @@ -11,18 +11,18 @@ help: was created by a SharedReadWrite retag at offsets [0x0..0x4] | LL | let ptr = Box::into_raw(Box::new(0i32)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -help: was protected due to which was created here - --> $DIR/newtype_retagging.rs:LL:CC - | -LL | Newtype(&mut *ptr), - | ^^^^^^^^^^^^^^^^^^ -help: this protector is live for this call +help: cannot be used for memory access because that would remove protected tag , protected by this function call --> $DIR/newtype_retagging.rs:LL:CC | LL | / fn dealloc_while_running(_n: Newtype<'_>, dealloc: impl FnOnce()) { LL | | dealloc(); LL | | } | |_^ +help: was derived from , which in turn was created here + --> $DIR/newtype_retagging.rs:LL:CC + | +LL | Newtype(&mut *ptr), + | ^^^^^^^^^^^^^^^^^^ = note: backtrace: = note: inside `std::boxed::Box::::from_raw_in` at RUSTLIB/alloc/src/boxed.rs:LL:CC = note: inside `std::boxed::Box::::from_raw` at RUSTLIB/alloc/src/boxed.rs:LL:CC