Skip to content

Commit da45adc

Browse files
borsbors
committed
Auto merge of #2519 - saethlin:rustup, r=RalfJung
Use the better FnEntry spans in protector errors Example error, from `tests/fail/stacked_borrows/invalidate_against_protector1.rs`: ``` error: Undefined Behavior: not granting access to tag <3095> because that would remove [Unique for <3099>] which is protected because it is an argument of call 943 --> tests/fail/stacked_borrows/invalidate_against_protector1.rs:5:25 | 5 | let _val = unsafe { *x }; //~ ERROR: protect | ^^ not granting access to tag <3095> because that would remove [Unique for <3099>] which is protected because it is an argument of 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_protector1.rs:10:16 | 10 | let xraw = &mut x as *mut _; | ^^^^^^ help: <3099> is this argument --> tests/fail/stacked_borrows/invalidate_against_protector1.rs:1:23 | 1 | fn inner(x: *mut i32, _y: &mut i32) { | ^^ = note: backtrace: = note: inside `inner` at tests/fail/stacked_borrows/invalidate_against_protector1.rs:5:25 note: inside `main` at tests/fail/stacked_borrows/invalidate_against_protector1.rs:12:5 --> tests/fail/stacked_borrows/invalidate_against_protector1.rs:12:5 | 12 | inner(xraw, xref); | ^^^^^^^^^^^^^^^^^ ``` Benchmarks report no change, within noise.
2 parents 284b59c + da0d482 commit da45adc

15 files changed

+80
-139
lines changed

‎rust-version

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
4065b89b1e7287047d7d6c65e7abd7b8ee70bcf0
1+
94b2b15e63c5d2b2a6a0910e3dae554ce9415bf9

‎src/diagnostics.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,8 @@ pub fn report_error<'tcx, 'mir>(
183183
if let Some((msg, span)) = invalidated {
184184
helps.push((Some(span), msg));
185185
}
186-
if let Some([(protector_msg, protector_span), (protection_msg, protection_span)]) = protected {
186+
if let Some((protector_msg, protector_span)) = protected {
187187
helps.push((Some(protector_span), protector_msg));
188-
helps.push((Some(protection_span), protection_msg));
189188
}
190189
}
191190
helps

‎src/machine.rs

+9-4
Original file line numberDiff line numberDiff line change
@@ -743,10 +743,15 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
743743
}
744744

745745
let alloc = alloc.into_owned();
746-
let stacks =
747-
ecx.machine.stacked_borrows.as_ref().map(|stacked_borrows| {
748-
Stacks::new_allocation(id, alloc.size(), stacked_borrows, kind)
749-
});
746+
let stacks = ecx.machine.stacked_borrows.as_ref().map(|stacked_borrows| {
747+
Stacks::new_allocation(
748+
id,
749+
alloc.size(),
750+
stacked_borrows,
751+
kind,
752+
ecx.machine.current_span(*ecx.tcx),
753+
)
754+
});
750755
let race_alloc = ecx.machine.data_race.as_ref().map(|data_race| {
751756
data_race::AllocExtra::new_allocation(
752757
data_race,

‎src/stacked_borrows/diagnostics.rs

+21-43
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, DUMMY_SP};
5+
use rustc_span::{Span, SpanData};
66
use rustc_target::abi::Size;
77

88
use crate::helpers::CurrentSpan;
@@ -14,6 +14,7 @@ use rustc_middle::mir::interpret::InterpError;
1414
#[derive(Clone, Debug)]
1515
pub struct AllocHistory {
1616
id: AllocId,
17+
base: (Item, Span),
1718
creations: smallvec::SmallVec<[Creation; 1]>,
1819
invalidations: smallvec::SmallVec<[Invalidation; 1]>,
1920
protectors: smallvec::SmallVec<[Protection; 1]>,
@@ -91,8 +92,6 @@ impl fmt::Display for InvalidationCause {
9192

9293
#[derive(Clone, Debug)]
9394
struct Protection {
94-
/// The parent tag from which this protected tag was derived.
95-
orig_tag: ProvenanceExtra,
9695
tag: SbTag,
9796
span: Span,
9897
}
@@ -101,7 +100,7 @@ struct Protection {
101100
pub struct TagHistory {
102101
pub created: (String, SpanData),
103102
pub invalidated: Option<(String, SpanData)>,
104-
pub protected: Option<([(String, SpanData); 2])>,
103+
pub protected: Option<(String, SpanData)>,
105104
}
106105

107106
pub struct DiagnosticCxBuilder<'span, 'ecx, 'mir, 'tcx> {
@@ -228,9 +227,10 @@ struct DeallocOp {
228227
}
229228

230229
impl AllocHistory {
231-
pub fn new(id: AllocId) -> Self {
230+
pub fn new(id: AllocId, item: Item, current_span: &mut CurrentSpan<'_, '_, '_>) -> Self {
232231
Self {
233232
id,
233+
base: (item, current_span.get()),
234234
creations: SmallVec::new(),
235235
invalidations: SmallVec::new(),
236236
protectors: SmallVec::new(),
@@ -290,11 +290,7 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir
290290
let Operation::Retag(op) = &self.operation else {
291291
unreachable!("Protectors can only be created during a retag")
292292
};
293-
self.history.protectors.push(Protection {
294-
orig_tag: op.orig_tag,
295-
tag: op.new_tag,
296-
span: self.current_span.get(),
297-
});
293+
self.history.protectors.push(Protection { tag: op.new_tag, span: self.current_span.get() });
298294
}
299295

300296
pub fn get_logs_relevant_to(
@@ -331,6 +327,17 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir
331327
None
332328
}
333329
})
330+
}).or_else(|| {
331+
// If we didn't find a retag that created this tag, it might be the base tag of
332+
// this allocation.
333+
if self.history.base.0.tag() == tag {
334+
Some((
335+
format!("{:?} was created here, as a base tag for {:?}", tag, self.history.id),
336+
self.history.base.1.data()
337+
))
338+
} else {
339+
None
340+
}
334341
}) else {
335342
// But if we don't have a creation event, this is related to a wildcard, and there
336343
// is really nothing we can do to help.
@@ -343,40 +350,11 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir
343350

344351
let protected = protector_tag
345352
.and_then(|protector| {
346-
self.history.protectors.iter().find(|protection| {
347-
protection.tag == protector
348-
})
353+
self.history.protectors.iter().find(|protection| protection.tag == protector)
349354
})
350-
.and_then(|protection| {
351-
self.history.creations.iter().rev().find_map(|event| {
352-
if ProvenanceExtra::Concrete(event.retag.new_tag) == protection.orig_tag {
353-
Some((protection, event))
354-
} else {
355-
None
356-
}
357-
})
358-
})
359-
.map(|(protection, protection_parent)| {
355+
.map(|protection| {
360356
let protected_tag = protection.tag;
361-
[
362-
(
363-
format!(
364-
"{tag:?} cannot be used for memory access because that would remove protected tag {protected_tag:?}, protected by this function call",
365-
),
366-
protection.span.data(),
367-
),
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-
}
379-
]
357+
(format!("{protected_tag:?} is this argument"), protection.span.data())
380358
});
381359

382360
Some(TagHistory { created, invalidated, protected })
@@ -448,7 +426,7 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir
448426
| Operation::Access(AccessOp { tag, .. }) =>
449427
err_sb_ub(
450428
format!(
451-
"not granting access to tag {:?} because incompatible item {:?} is protected by call {:?}",
429+
"not granting access to tag {:?} because that would remove {:?} which is protected because it is an argument of call {:?}",
452430
tag, item, call_id
453431
),
454432
None,

‎src/stacked_borrows/mod.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -500,13 +500,19 @@ impl<'tcx> Stack {
500500
impl<'tcx> Stacks {
501501
/// Creates a new stack with an initial tag. For diagnostic purposes, we also need to know
502502
/// the [`AllocId`] of the allocation this is associated with.
503-
fn new(size: Size, perm: Permission, tag: SbTag, id: AllocId) -> Self {
503+
fn new(
504+
size: Size,
505+
perm: Permission,
506+
tag: SbTag,
507+
id: AllocId,
508+
current_span: &mut CurrentSpan<'_, '_, '_>,
509+
) -> Self {
504510
let item = Item::new(tag, perm, false);
505511
let stack = Stack::new(item);
506512

507513
Stacks {
508514
stacks: RangeMap::new(size, stack),
509-
history: AllocHistory::new(id),
515+
history: AllocHistory::new(id, item, current_span),
510516
exposed_tags: FxHashSet::default(),
511517
}
512518
}
@@ -538,6 +544,7 @@ impl Stacks {
538544
size: Size,
539545
state: &GlobalState,
540546
kind: MemoryKind<MiriMemoryKind>,
547+
mut current_span: CurrentSpan<'_, '_, '_>,
541548
) -> Self {
542549
let mut extra = state.borrow_mut();
543550
let (base_tag, perm) = match kind {
@@ -550,7 +557,7 @@ impl Stacks {
550557
// Everything else is shared by default.
551558
_ => (extra.base_ptr_tag(id), Permission::SharedReadWrite),
552559
};
553-
Stacks::new(size, perm, base_tag, id)
560+
Stacks::new(size, perm, base_tag, id, &mut current_span)
554561
}
555562

556563
#[inline(always)]

‎tests/fail/stacked_borrows/aliasing_mut1.stderr

+4-5
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: not granting access to tag <TAG> because incompatible item [Unique for <TAG>] is protected by call ID
1+
error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is protected because it is an argument of call ID
22
--> $DIR/aliasing_mut1.rs:LL:CC
33
|
44
LL | pub fn safe(_x: &mut i32, _y: &mut i32) {}
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <TAG> because incompatible item [Unique for <TAG>] is protected by call ID
5+
| ^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is protected because it is an argument of call ID
66
|
77
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
88
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
@@ -11,12 +11,11 @@ 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> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
14+
help: <TAG> is this argument
1515
--> $DIR/aliasing_mut1.rs:LL:CC
1616
|
1717
LL | pub fn safe(_x: &mut i32, _y: &mut i32) {}
18-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19-
= help: <TAG> was derived from <TAG>, the tag used for this memory access
18+
| ^^
2019
= note: backtrace:
2120
= note: inside `safe` at $DIR/aliasing_mut1.rs:LL:CC
2221
note: inside `main` at $DIR/aliasing_mut1.rs:LL:CC

‎tests/fail/stacked_borrows/aliasing_mut2.stderr

+4-9
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: not granting access to tag <TAG> because incompatible item [SharedReadOnly for <TAG>] is protected by call ID
1+
error: Undefined Behavior: not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is protected because it is an argument of call ID
22
--> $DIR/aliasing_mut2.rs:LL:CC
33
|
44
LL | pub fn safe(_x: &i32, _y: &mut i32) {}
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <TAG> because incompatible item [SharedReadOnly for <TAG>] is protected by call ID
5+
| ^^ not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is protected because it is an argument of call ID
66
|
77
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
88
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
@@ -11,16 +11,11 @@ help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
1111
|
1212
LL | let xref = &mut x;
1313
| ^^^^^^
14-
help: <TAG> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
14+
help: <TAG> is this argument
1515
--> $DIR/aliasing_mut2.rs:LL:CC
1616
|
1717
LL | pub fn safe(_x: &i32, _y: &mut i32) {}
18-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
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-
| ^^^^
18+
| ^^
2419
= note: backtrace:
2520
= note: inside `safe` at $DIR/aliasing_mut2.rs:LL:CC
2621
note: inside `main` at $DIR/aliasing_mut2.rs:LL:CC

‎tests/fail/stacked_borrows/aliasing_mut3.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ error: Undefined Behavior: trying to retag from <TAG> for SharedReadOnly permiss
22
--> $DIR/aliasing_mut3.rs:LL:CC
33
|
44
LL | pub fn safe(_x: &mut i32, _y: &i32) {}
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6-
| |
7-
| trying to retag from <TAG> for SharedReadOnly permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location
8-
| this error occurs as part of FnEntry retag at ALLOC[0x0..0x4]
5+
| ^^
6+
| |
7+
| trying to retag from <TAG> for SharedReadOnly permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location
8+
| this error occurs as part of FnEntry retag at ALLOC[0x0..0x4]
99
|
1010
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
1111
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information

‎tests/fail/stacked_borrows/aliasing_mut4.stderr

+4-9
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: not granting access to tag <TAG> because incompatible item [SharedReadOnly for <TAG>] is protected by call ID
1+
error: Undefined Behavior: not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is protected because it is an argument of call ID
22
--> $DIR/aliasing_mut4.rs:LL:CC
33
|
44
LL | pub fn safe(_x: &i32, _y: &mut Cell<i32>) {}
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <TAG> because incompatible item [SharedReadOnly for <TAG>] is protected by call ID
5+
| ^^ not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is protected because it is an argument of call ID
66
|
77
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
88
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
@@ -11,16 +11,11 @@ help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
1111
|
1212
LL | let xref = &mut x;
1313
| ^^^^^^
14-
help: <TAG> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
14+
help: <TAG> is this argument
1515
--> $DIR/aliasing_mut4.rs:LL:CC
1616
|
1717
LL | pub fn safe(_x: &i32, _y: &mut Cell<i32>) {}
18-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
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-
| ^^^^
18+
| ^^
2419
= note: backtrace:
2520
= note: inside `safe` at $DIR/aliasing_mut4.rs:LL:CC
2621
note: inside `main` at $DIR/aliasing_mut4.rs:LL:CC

‎tests/fail/stacked_borrows/illegal_write6.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ fn main() {
77
fn foo(a: &mut u32, y: *mut u32) -> u32 {
88
*a = 1;
99
let _b = &*a;
10-
unsafe { *y = 2 }; //~ ERROR: /not granting access .* because incompatible item .* is protected/
10+
unsafe { *y = 2 }; //~ ERROR: /not granting access .* because that would remove .* which is protected/
1111
return *a;
1212
}

‎tests/fail/stacked_borrows/illegal_write6.stderr

+5-15
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: not granting access to tag <TAG> because incompatible item [Unique for <TAG>] is protected by call ID
1+
error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is protected because it is an argument of call ID
22
--> $DIR/illegal_write6.rs:LL:CC
33
|
44
LL | unsafe { *y = 2 };
5-
| ^^^^^^ not granting access to tag <TAG> because incompatible item [Unique for <TAG>] is protected by call ID
5+
| ^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is protected because it is an argument of call ID
66
|
77
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
88
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
@@ -11,21 +11,11 @@ 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> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
14+
help: <TAG> is this argument
1515
--> $DIR/illegal_write6.rs:LL:CC
1616
|
17-
LL | / fn foo(a: &mut u32, y: *mut u32) -> u32 {
18-
LL | | *a = 1;
19-
LL | | let _b = &*a;
20-
LL | | unsafe { *y = 2 };
21-
LL | | return *a;
22-
LL | | }
23-
| |_^
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-
| ^
17+
LL | fn foo(a: &mut u32, y: *mut u32) -> u32 {
18+
| ^
2919
= note: backtrace:
3020
= note: inside `foo` at $DIR/illegal_write6.rs:LL:CC
3121
note: inside `main` at $DIR/illegal_write6.rs:LL:CC

‎tests/fail/stacked_borrows/invalidate_against_protector1.stderr

+5-15
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: not granting access to tag <TAG> because incompatible item [Unique for <TAG>] is protected by call ID
1+
error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is protected because it is an argument of call ID
22
--> $DIR/invalidate_against_protector1.rs:LL:CC
33
|
44
LL | let _val = unsafe { *x };
5-
| ^^ not granting access to tag <TAG> because incompatible item [Unique for <TAG>] is protected by call ID
5+
| ^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is protected because it is an argument of call ID
66
|
77
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
88
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
@@ -11,21 +11,11 @@ help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
1111
|
1212
LL | let xraw = &mut x as *mut _;
1313
| ^^^^^^
14-
help: <TAG> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
14+
help: <TAG> is this argument
1515
--> $DIR/invalidate_against_protector1.rs:LL:CC
1616
|
17-
LL | / fn inner(x: *mut i32, _y: &mut i32) {
18-
LL | | // If `x` and `y` alias, retagging is fine with this... but we really
19-
LL | | // shouldn't be allowed to use `x` at all because `y` was assumed to be
20-
LL | | // unique for the duration of this call.
21-
LL | | let _val = unsafe { *x };
22-
LL | | }
23-
| |_^
24-
help: <TAG> was derived from <TAG>, which in turn was created here
25-
--> $DIR/invalidate_against_protector1.rs:LL:CC
26-
|
27-
LL | inner(xraw, xref);
28-
| ^^^^
17+
LL | fn inner(x: *mut i32, _y: &mut i32) {
18+
| ^^
2919
= note: backtrace:
3020
= note: inside `inner` at $DIR/invalidate_against_protector1.rs:LL:CC
3121
note: inside `main` at $DIR/invalidate_against_protector1.rs:LL:CC

0 commit comments

Comments
 (0)