Skip to content

Commit fec1c7a

Browse files
borsbors
committed
Auto merge of #2513 - RalfJung:protected, r=saethlin
slightly improve protector-related error messages I find the current retag messages confusing, since they sound like the item *was* protected, when it still actively *is* protected (and that is, in fact, the issue). Example error message: ``` error: Undefined Behavior: not granting access to tag <3095> because incompatible item [Unique for <3099>] is protected by call 943 --> tests/fail/stacked_borrows/invalidate_against_barrier1.rs:5:25 | 5 | let _val = unsafe { *x }; //~ ERROR: protect | ^^ not granting access to tag <3095> because incompatible item [Unique for <3099>] is protected by call 943 | = 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: <3095> was created by a SharedReadWrite retag at offsets [0x0..0x4] --> tests/fail/stacked_borrows/invalidate_against_barrier1.rs:10:16 | 10 | let xraw = &mut x as *mut _; | ^^^^^^ help: <3095> cannot be used for memory access because that would remove protected tag <3099>, protected by this function call --> tests/fail/stacked_borrows/invalidate_against_barrier1.rs:1:1 | 1 | / fn inner(x: *mut i32, _y: &mut i32) { 2 | | // If `x` and `y` alias, retagging is fine with this... but we really 3 | | // shouldn't be allowed to use `x` at all because `y` was assumed to be 4 | | // unique for the duration of this call. 5 | | let _val = unsafe { *x }; //~ ERROR: protect 6 | | } | |_^ help: <3099> was derived from <3098>, which in turn was created here --> tests/fail/stacked_borrows/invalidate_against_barrier1.rs:12:17 | 12 | inner(xraw, xref); | ^^^^ = note: backtrace: = note: inside `inner` at tests/fail/stacked_borrows/invalidate_against_barrier1.rs:5:25 note: inside `main` at tests/fail/stacked_borrows/invalidate_against_barrier1.rs:12:5 --> tests/fail/stacked_borrows/invalidate_against_barrier1.rs:12:5 | 12 | inner(xraw, xref); | ^^^^^^^^^^^^^^^^^ ``` r? `@saethlin`
2 parents 8866513 + abe890d commit fec1c7a

14 files changed

+86
-82
lines changed

‎src/stacked_borrows/diagnostics.rs‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use smallvec::SmallVec;
22
use std::fmt;
33

44
use rustc_middle::mir::interpret::{alloc_range, AllocId, AllocRange};
5-
use rustc_span::{Span, SpanData};
5+
use rustc_span::{Span, SpanData, DUMMY_SP};
66
use rustc_target::abi::Size;
77

88
use crate::helpers::CurrentSpan;
@@ -91,6 +91,7 @@ impl fmt::Display for InvalidationCause {
9191

9292
#[derive(Clone, Debug)]
9393
struct Protection {
94+
/// The parent tag from which this protected tag was derived.
9495
orig_tag: ProvenanceExtra,
9596
tag: SbTag,
9697
span: Span,
@@ -342,32 +343,39 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir
342343

343344
let protected = protector_tag
344345
.and_then(|protector| {
345-
self.history.protectors.iter().find_map(|protection| {
346-
if protection.tag == protector {
347-
Some((protection.orig_tag, protection.span.data()))
348-
} else {
349-
None
350-
}
346+
self.history.protectors.iter().find(|protection| {
347+
protection.tag == protector
351348
})
352349
})
353-
.and_then(|(tag, call_span)| {
350+
.and_then(|protection| {
354351
self.history.creations.iter().rev().find_map(|event| {
355-
if ProvenanceExtra::Concrete(event.retag.new_tag) == tag {
356-
Some((event.retag.orig_tag, event.span.data(), call_span))
352+
if ProvenanceExtra::Concrete(event.retag.new_tag) == protection.orig_tag {
353+
Some((protection, event))
357354
} else {
358355
None
359356
}
360357
})
361358
})
362-
.map(|(protecting_tag, protecting_tag_span, protection_span)| {
359+
.map(|(protection, protection_parent)| {
360+
let protected_tag = protection.tag;
363361
[
364362
(
365363
format!(
366-
"{tag:?} was protected due to {protecting_tag:?} which was created here"
364+
"{tag:?} cannot be used for memory access because that would remove protected tag {protected_tag:?}, protected by this function call",
367365
),
368-
protecting_tag_span,
366+
protection.span.data(),
369367
),
370-
(format!("this protector is live for this call"), protection_span),
368+
if protection_parent.retag.new_tag == tag {
369+
(format!("{protected_tag:?} was derived from {tag:?}, the tag used for this memory access"), DUMMY_SP.data())
370+
} else {
371+
(
372+
format!(
373+
"{protected_tag:?} was derived from {protected_parent_tag:?}, which in turn was created here",
374+
protected_parent_tag = protection_parent.retag.new_tag,
375+
),
376+
protection_parent.span.data()
377+
)
378+
}
371379
]
372380
});
373381

‎tests/fail/stacked_borrows/aliasing_mut1.stderr‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,12 @@ help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
1111
|
1212
LL | let xraw: *mut i32 = unsafe { mem::transmute(&mut x) };
1313
| ^^^^^^
14-
help: <TAG> was protected due to <TAG> which was created here
15-
--> $DIR/aliasing_mut1.rs:LL:CC
16-
|
17-
LL | let xraw: *mut i32 = unsafe { mem::transmute(&mut x) };
18-
| ^^^^^^
19-
help: this protector is live for this call
14+
help: <TAG> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
2015
--> $DIR/aliasing_mut1.rs:LL:CC
2116
|
2217
LL | pub fn safe(_x: &mut i32, _y: &mut i32) {}
2318
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19+
= help: <TAG> was derived from <TAG>, the tag used for this memory access
2420
= note: backtrace:
2521
= note: inside `safe` at $DIR/aliasing_mut1.rs:LL:CC
2622
note: inside `main` at $DIR/aliasing_mut1.rs:LL:CC

‎tests/fail/stacked_borrows/aliasing_mut2.stderr‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,16 @@ help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
1111
|
1212
LL | let xref = &mut x;
1313
| ^^^^^^
14-
help: <TAG> was protected due to <TAG> which was created here
15-
--> $DIR/aliasing_mut2.rs:LL:CC
16-
|
17-
LL | safe_raw(xshr, xraw);
18-
| ^^^^
19-
help: this protector is live for this call
14+
help: <TAG> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
2015
--> $DIR/aliasing_mut2.rs:LL:CC
2116
|
2217
LL | pub fn safe(_x: &i32, _y: &mut i32) {}
2318
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19+
help: <TAG> was derived from <TAG>, which in turn was created here
20+
--> $DIR/aliasing_mut2.rs:LL:CC
21+
|
22+
LL | safe_raw(xshr, xraw);
23+
| ^^^^
2424
= note: backtrace:
2525
= note: inside `safe` at $DIR/aliasing_mut2.rs:LL:CC
2626
note: inside `main` at $DIR/aliasing_mut2.rs:LL:CC

‎tests/fail/stacked_borrows/aliasing_mut4.stderr‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,16 @@ help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
1111
|
1212
LL | let xref = &mut x;
1313
| ^^^^^^
14-
help: <TAG> was protected due to <TAG> which was created here
15-
--> $DIR/aliasing_mut4.rs:LL:CC
16-
|
17-
LL | safe_raw(xshr, xraw as *mut _);
18-
| ^^^^
19-
help: this protector is live for this call
14+
help: <TAG> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
2015
--> $DIR/aliasing_mut4.rs:LL:CC
2116
|
2217
LL | pub fn safe(_x: &i32, _y: &mut Cell<i32>) {}
2318
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19+
help: <TAG> was derived from <TAG>, which in turn was created here
20+
--> $DIR/aliasing_mut4.rs:LL:CC
21+
|
22+
LL | safe_raw(xshr, xraw as *mut _);
23+
| ^^^^
2424
= note: backtrace:
2525
= note: inside `safe` at $DIR/aliasing_mut4.rs:LL:CC
2626
note: inside `main` at $DIR/aliasing_mut4.rs:LL:CC

‎tests/fail/stacked_borrows/deallocate_against_barrier1.rs‎ renamed to ‎tests/fail/stacked_borrows/deallocate_against_protector1.rs‎

File renamed without changes.

‎tests/fail/stacked_borrows/deallocate_against_barrier1.stderr‎ renamed to ‎tests/fail/stacked_borrows/deallocate_against_protector1.stderr‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,19 @@ LL | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
1212
= note: inside `alloc::alloc::box_free::<i32, std::alloc::Global>` at RUSTLIB/alloc/src/alloc.rs:LL:CC
1313
= note: inside `std::ptr::drop_in_place::<std::boxed::Box<i32>> - shim(Some(std::boxed::Box<i32>))` at RUSTLIB/core/src/ptr/mod.rs:LL:CC
1414
= note: inside `std::mem::drop::<std::boxed::Box<i32>>` at RUSTLIB/core/src/mem/mod.rs:LL:CC
15-
note: inside closure at $DIR/deallocate_against_barrier1.rs:LL:CC
16-
--> $DIR/deallocate_against_barrier1.rs:LL:CC
15+
note: inside closure at $DIR/deallocate_against_protector1.rs:LL:CC
16+
--> $DIR/deallocate_against_protector1.rs:LL:CC
1717
|
1818
LL | drop(unsafe { Box::from_raw(raw) });
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
20-
= 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
21-
note: inside `inner` at $DIR/deallocate_against_barrier1.rs:LL:CC
22-
--> $DIR/deallocate_against_barrier1.rs:LL:CC
20+
= 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
21+
note: inside `inner` at $DIR/deallocate_against_protector1.rs:LL:CC
22+
--> $DIR/deallocate_against_protector1.rs:LL:CC
2323
|
2424
LL | f(x)
2525
| ^^^^
26-
note: inside `main` at $DIR/deallocate_against_barrier1.rs:LL:CC
27-
--> $DIR/deallocate_against_barrier1.rs:LL:CC
26+
note: inside `main` at $DIR/deallocate_against_protector1.rs:LL:CC
27+
--> $DIR/deallocate_against_protector1.rs:LL:CC
2828
|
2929
LL | / inner(Box::leak(Box::new(0)), |x| {
3030
LL | | let raw = x as *mut _;

‎tests/fail/stacked_borrows/deallocate_against_barrier2.rs‎ renamed to ‎tests/fail/stacked_borrows/deallocate_against_protector2.rs‎

File renamed without changes.

‎tests/fail/stacked_borrows/deallocate_against_barrier2.stderr‎ renamed to ‎tests/fail/stacked_borrows/deallocate_against_protector2.stderr‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,19 @@ LL | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
1212
= note: inside `alloc::alloc::box_free::<NotUnpin, std::alloc::Global>` at RUSTLIB/alloc/src/alloc.rs:LL:CC
1313
= note: inside `std::ptr::drop_in_place::<std::boxed::Box<NotUnpin>> - shim(Some(std::boxed::Box<NotUnpin>))` at RUSTLIB/core/src/ptr/mod.rs:LL:CC
1414
= note: inside `std::mem::drop::<std::boxed::Box<NotUnpin>>` at RUSTLIB/core/src/mem/mod.rs:LL:CC
15-
note: inside closure at $DIR/deallocate_against_barrier2.rs:LL:CC
16-
--> $DIR/deallocate_against_barrier2.rs:LL:CC
15+
note: inside closure at $DIR/deallocate_against_protector2.rs:LL:CC
16+
--> $DIR/deallocate_against_protector2.rs:LL:CC
1717
|
1818
LL | drop(unsafe { Box::from_raw(raw) });
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
20-
= 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
21-
note: inside `inner` at $DIR/deallocate_against_barrier2.rs:LL:CC
22-
--> $DIR/deallocate_against_barrier2.rs:LL:CC
20+
= 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
21+
note: inside `inner` at $DIR/deallocate_against_protector2.rs:LL:CC
22+
--> $DIR/deallocate_against_protector2.rs:LL:CC
2323
|
2424
LL | f(x)
2525
| ^^^^
26-
note: inside `main` at $DIR/deallocate_against_barrier2.rs:LL:CC
27-
--> $DIR/deallocate_against_barrier2.rs:LL:CC
26+
note: inside `main` at $DIR/deallocate_against_protector2.rs:LL:CC
27+
--> $DIR/deallocate_against_protector2.rs:LL:CC
2828
|
2929
LL | / inner(Box::leak(Box::new(NotUnpin(0, PhantomPinned))), |x| {
3030
LL | | let raw = x as *mut _;

‎tests/fail/stacked_borrows/illegal_write6.stderr‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,7 @@ help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
1111
|
1212
LL | let p = x as *mut u32;
1313
| ^
14-
help: <TAG> was protected due to <TAG> which was created here
15-
--> $DIR/illegal_write6.rs:LL:CC
16-
|
17-
LL | foo(x, p);
18-
| ^
19-
help: this protector is live for this call
14+
help: <TAG> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
2015
--> $DIR/illegal_write6.rs:LL:CC
2116
|
2217
LL | / fn foo(a: &mut u32, y: *mut u32) -> u32 {
@@ -26,6 +21,11 @@ LL | | unsafe { *y = 2 };
2621
LL | | return *a;
2722
LL | | }
2823
| |_^
24+
help: <TAG> was derived from <TAG>, which in turn was created here
25+
--> $DIR/illegal_write6.rs:LL:CC
26+
|
27+
LL | foo(x, p);
28+
| ^
2929
= note: backtrace:
3030
= note: inside `foo` at $DIR/illegal_write6.rs:LL:CC
3131
note: inside `main` at $DIR/illegal_write6.rs:LL:CC

‎tests/fail/stacked_borrows/invalidate_against_barrier1.rs‎ renamed to ‎tests/fail/stacked_borrows/invalidate_against_protector1.rs‎

File renamed without changes.

0 commit comments

Comments
 (0)