diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs index 5ec8d80fb324..33b777abd9f0 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs @@ -61,12 +61,15 @@ struct Invalidation { #[derive(Clone, Debug)] enum InvalidationCause { Access(AccessKind), - Retag(Permission, RetagCause), + Retag(Permission, RetagInfo), } impl Invalidation { fn generate_diagnostic(&self) -> (String, SpanData) { - let message = if let InvalidationCause::Retag(_, RetagCause::FnEntry) = self.cause { + let message = if matches!( + self.cause, + InvalidationCause::Retag(_, RetagInfo { cause: RetagCause::FnEntry, .. }) + ) { // For a FnEntry retag, our Span points at the caller. // See `DiagnosticCx::log_invalidation`. format!( @@ -87,8 +90,8 @@ impl fmt::Display for InvalidationCause { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { InvalidationCause::Access(kind) => write!(f, "{kind}"), - InvalidationCause::Retag(perm, kind) => - write!(f, "{perm:?} {retag}", retag = kind.summary()), + InvalidationCause::Retag(perm, info) => + write!(f, "{perm:?} {retag}", retag = info.summary()), } } } @@ -129,13 +132,13 @@ impl<'ecx, 'mir, 'tcx> DiagnosticCxBuilder<'ecx, 'mir, 'tcx> { pub fn retag( machine: &'ecx MiriMachine<'mir, 'tcx>, - cause: RetagCause, + info: RetagInfo, new_tag: BorTag, orig_tag: ProvenanceExtra, range: AllocRange, ) -> Self { let operation = - Operation::Retag(RetagOp { cause, new_tag, orig_tag, range, permission: None }); + Operation::Retag(RetagOp { info, new_tag, orig_tag, range, permission: None }); DiagnosticCxBuilder { machine, operation } } @@ -179,13 +182,19 @@ enum Operation { #[derive(Debug, Clone)] struct RetagOp { - cause: RetagCause, + info: RetagInfo, new_tag: BorTag, orig_tag: ProvenanceExtra, range: AllocRange, permission: Option, } +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct RetagInfo { + pub cause: RetagCause, + pub in_field: bool, +} + #[derive(Debug, Clone, Copy, PartialEq)] pub enum RetagCause { Normal, @@ -258,11 +267,11 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { pub fn log_invalidation(&mut self, tag: BorTag) { let mut span = self.machine.current_span(); let (range, cause) = match &self.operation { - Operation::Retag(RetagOp { cause, range, permission, .. }) => { - if *cause == RetagCause::FnEntry { + Operation::Retag(RetagOp { info, range, permission, .. }) => { + if info.cause == RetagCause::FnEntry { span = self.machine.caller_span(); } - (*range, InvalidationCause::Retag(permission.unwrap(), *cause)) + (*range, InvalidationCause::Retag(permission.unwrap(), *info)) } Operation::Access(AccessOp { kind, range, .. }) => (*range, InvalidationCause::Access(*kind)), @@ -374,7 +383,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { ); err_sb_ub( format!("{action}{}", error_cause(stack, op.orig_tag)), - Some(operation_summary(&op.cause.summary(), self.history.id, op.range)), + Some(operation_summary(&op.info.summary(), self.history.id, op.range)), op.orig_tag.and_then(|orig_tag| self.get_logs_relevant_to(orig_tag, None)), ) } @@ -496,14 +505,18 @@ fn error_cause(stack: &Stack, prov_extra: ProvenanceExtra) -> &'static str { } } -impl RetagCause { +impl RetagInfo { fn summary(&self) -> String { - match self { + let mut s = match self.cause { RetagCause::Normal => "retag", RetagCause::FnEntry => "function-entry retag", RetagCause::InPlaceFnPassing => "in-place function argument/return passing protection", RetagCause::TwoPhase => "two-phase retag", } - .to_string() + .to_string(); + if self.in_field { + s.push_str(" (of a reference/box inside this compound value)"); + } + s } } diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 7e89d3a0e8cb..5e1e0d754365 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -5,9 +5,11 @@ pub mod diagnostics; mod item; mod stack; -use log::trace; use std::cmp; use std::fmt::Write; +use std::mem; + +use log::trace; use rustc_data_structures::fx::FxHashSet; use rustc_middle::mir::{Mutability, RetagKind}; @@ -24,7 +26,7 @@ use crate::borrow_tracker::{ }; use crate::*; -use diagnostics::RetagCause; +use diagnostics::{RetagCause, RetagInfo}; pub use item::{Item, Permission}; pub use stack::Stack; @@ -623,7 +625,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' size: Size, new_perm: NewPermission, new_tag: BorTag, - retag_cause: RetagCause, // What caused this retag, for diagnostics only + retag_info: RetagInfo, // diagnostics info about this retag ) -> InterpResult<'tcx, Option> { let this = self.eval_context_mut(); @@ -670,7 +672,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // FIXME: can this be done cleaner? let dcx = DiagnosticCxBuilder::retag( &this.machine, - retag_cause, + retag_info, new_tag, orig_tag, alloc_range(base_offset, size), @@ -761,7 +763,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let global = machine.borrow_tracker.as_ref().unwrap().borrow(); let dcx = DiagnosticCxBuilder::retag( machine, - retag_cause, + retag_info, new_tag, orig_tag, alloc_range(base_offset, size), @@ -804,7 +806,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let global = this.machine.borrow_tracker.as_ref().unwrap().borrow(); let dcx = DiagnosticCxBuilder::retag( &this.machine, - retag_cause, + retag_info, new_tag, orig_tag, alloc_range(base_offset, size), @@ -834,7 +836,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' &mut self, val: &ImmTy<'tcx, Provenance>, new_perm: NewPermission, - cause: RetagCause, // What caused this retag, for diagnostics only + info: RetagInfo, // diagnostics info about this retag ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { let this = self.eval_context_mut(); // We want a place for where the ptr *points to*, so we get one. @@ -852,7 +854,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let new_tag = this.machine.borrow_tracker.as_mut().unwrap().get_mut().new_ptr(); // Reborrow. - let alloc_id = this.sb_reborrow(&place, size, new_perm, new_tag, cause)?; + let alloc_id = this.sb_reborrow(&place, size, new_perm, new_tag, info)?; // Adjust pointer. let new_place = place.map_provenance(|p| { @@ -886,12 +888,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { let this = self.eval_context_mut(); let new_perm = NewPermission::from_ref_ty(val.layout.ty, kind, this); - let retag_cause = match kind { + let cause = match kind { RetagKind::TwoPhase { .. } => RetagCause::TwoPhase, RetagKind::FnEntry => unreachable!(), RetagKind::Raw | RetagKind::Default => RetagCause::Normal, }; - this.sb_retag_reference(val, new_perm, retag_cause) + this.sb_retag_reference(val, new_perm, RetagInfo { cause, in_field: false }) } fn sb_retag_place_contents( @@ -906,7 +908,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { RetagKind::FnEntry => RetagCause::FnEntry, RetagKind::Default => RetagCause::Normal, }; - let mut visitor = RetagVisitor { ecx: this, kind, retag_cause, retag_fields }; + let mut visitor = + RetagVisitor { ecx: this, kind, retag_cause, retag_fields, in_field: false }; return visitor.visit_value(place); // The actual visitor. @@ -915,6 +918,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { kind: RetagKind, retag_cause: RetagCause, retag_fields: RetagFields, + in_field: bool, } impl<'ecx, 'mir, 'tcx> RetagVisitor<'ecx, 'mir, 'tcx> { #[inline(always)] // yes this helps in our benchmarks @@ -922,10 +926,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { &mut self, place: &PlaceTy<'tcx, Provenance>, new_perm: NewPermission, - retag_cause: RetagCause, ) -> InterpResult<'tcx> { let val = self.ecx.read_immediate(&self.ecx.place_to_op(place)?)?; - let val = self.ecx.sb_retag_reference(&val, new_perm, retag_cause)?; + let val = self.ecx.sb_retag_reference( + &val, + new_perm, + RetagInfo { cause: self.retag_cause, in_field: self.in_field }, + )?; self.ecx.write_immediate(*val, place)?; Ok(()) } @@ -943,7 +950,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { // Boxes get a weak protectors, since they may be deallocated. let new_perm = NewPermission::from_box_ty(place.layout.ty, self.kind, self.ecx); - self.retag_ptr_inplace(place, new_perm, self.retag_cause) + self.retag_ptr_inplace(place, new_perm) } fn visit_value(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { @@ -960,7 +967,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ty::Ref(..) => { let new_perm = NewPermission::from_ref_ty(place.layout.ty, self.kind, self.ecx); - self.retag_ptr_inplace(place, new_perm, self.retag_cause)?; + self.retag_ptr_inplace(place, new_perm)?; } ty::RawPtr(..) => { // We do *not* want to recurse into raw pointers -- wide raw pointers have @@ -984,7 +991,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } }; if recurse { + let in_field = mem::replace(&mut self.in_field, true); // remember and restore old value self.walk_value(place)?; + self.in_field = in_field; } } } @@ -1011,7 +1020,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { access: Some(AccessKind::Write), protector: Some(ProtectorKind::StrongProtector), }; - let _new_ptr = this.sb_retag_reference(&ptr, new_perm, RetagCause::InPlaceFnPassing)?; + let _new_ptr = this.sb_retag_reference( + &ptr, + new_perm, + RetagInfo { cause: RetagCause::InPlaceFnPassing, in_field: false }, + )?; // We just throw away `new_ptr`, so nobody can access this memory while it is protected. Ok(()) diff --git a/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr b/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr index 0cb6111ecb9a..b957464f95f6 100644 --- a/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr @@ -8,7 +8,7 @@ LL | | ) | | ^ | | | | |_____________trying to retag from for Unique permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x0..0x10] + | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x0..0x10] | = 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 diff --git a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr index e35d7918c03c..96121f0659fb 100644 --- a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr @@ -5,7 +5,7 @@ LL | foo(some_xref); | ^^^^^^^^^ | | | trying to retag from for SharedReadOnly permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x0..0x4] + | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x0..0x4] | = 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 diff --git a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr index 10c566d06121..0cfaf1235544 100644 --- a/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr @@ -5,7 +5,7 @@ LL | foo(pair_xref); | ^^^^^^^^^ | | | trying to retag from for SharedReadOnly permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x0..0x4] + | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x0..0x4] | = 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 diff --git a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr index f14e8b8532f5..d5b8433568f2 100644 --- a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr @@ -5,7 +5,7 @@ LL | ret | ^^^ | | | trying to retag from for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x4..0x8] + | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x4..0x8] | = 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 diff --git a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr index 9ddaad4d1be3..9ced0da96c43 100644 --- a/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr @@ -5,7 +5,7 @@ LL | ret | ^^^ | | | trying to retag from for SharedReadOnly permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x4..0x8] + | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x4..0x8] | = 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 diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr index ff00c54570cd..89b6cee7d973 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_option.stderr @@ -5,7 +5,7 @@ LL | ret | ^^^ | | | trying to retag from for Unique permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x4..0x8] + | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x4..0x8] | = 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 diff --git a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr index 61d041a8816d..388b00c71467 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr @@ -5,7 +5,7 @@ LL | ret | ^^^ | | | trying to retag from for Unique permission at ALLOC[0x4], but that tag does not exist in the borrow stack for this location - | this error occurs as part of retag at ALLOC[0x4..0x8] + | this error occurs as part of retag (of a reference/box inside this compound value) at ALLOC[0x4..0x8] | = 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