From ae3b96174cf9a0ef77e62217f8d2911356e74a2b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 21 Jul 2023 16:28:29 +0200 Subject: [PATCH] ask people to reach out if we declare too much UB --- .../stacked_borrows/diagnostics.rs | 26 ++++++++++++++----- src/borrow_tracker/stacked_borrows/mod.rs | 11 +------- src/diagnostics.rs | 5 ++-- .../buggy_split_at_mut.stack.stderr | 5 ++-- .../pass_invalid_shr_option.stack.stderr | 1 + .../pass_invalid_shr_tuple.stack.stderr | 1 + .../return_invalid_shr_option.stack.stderr | 1 + .../return_invalid_shr_tuple.stack.stderr | 1 + .../return_invalid_mut_option.stderr | 1 + .../return_invalid_mut_tuple.stderr | 1 + 10 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/borrow_tracker/stacked_borrows/diagnostics.rs b/src/borrow_tracker/stacked_borrows/diagnostics.rs index 33b777abd9..9b0f13dd62 100644 --- a/src/borrow_tracker/stacked_borrows/diagnostics.rs +++ b/src/borrow_tracker/stacked_borrows/diagnostics.rs @@ -6,11 +6,19 @@ use rustc_span::{Span, SpanData}; use rustc_target::abi::Size; use crate::borrow_tracker::{ - stacked_borrows::{err_sb_ub, Permission}, - AccessKind, GlobalStateInner, ProtectorKind, + stacked_borrows::Permission, AccessKind, GlobalStateInner, ProtectorKind, }; use crate::*; +/// Error reporting +fn err_sb_ub<'tcx>( + msg: String, + help: Vec, + history: Option, +) -> InterpError<'tcx> { + err_machine_stop!(TerminationInfo::StackedBorrowsUb { msg, help, history }) +} + #[derive(Clone, Debug)] pub struct AllocHistory { id: AllocId, @@ -381,9 +389,13 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { self.history.id, self.offset.bytes(), ); + let mut helps = vec![operation_summary(&op.info.summary(), self.history.id, op.range)]; + if op.info.in_field { + helps.push(format!("errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling")); + } err_sb_ub( format!("{action}{}", error_cause(stack, op.orig_tag)), - Some(operation_summary(&op.info.summary(), self.history.id, op.range)), + helps, op.orig_tag.and_then(|orig_tag| self.get_logs_relevant_to(orig_tag, None)), ) } @@ -406,7 +418,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { ); err_sb_ub( format!("{action}{}", error_cause(stack, op.tag)), - Some(operation_summary("an access", self.history.id, op.range)), + vec![operation_summary("an access", self.history.id, op.range)], op.tag.and_then(|tag| self.get_logs_relevant_to(tag, None)), ) } @@ -432,7 +444,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { Operation::Dealloc(_) => err_sb_ub( format!("deallocating while item {item:?} is {protected} by call {call_id:?}",), - None, + vec![], None, ), Operation::Retag(RetagOp { orig_tag: tag, .. }) @@ -441,7 +453,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { format!( "not granting access to tag {tag:?} because that would remove {item:?} which is {protected} because it is an argument of call {call_id:?}", ), - None, + vec![], tag.and_then(|tag| self.get_logs_relevant_to(tag, Some(item.tag()))), ), } @@ -459,7 +471,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { alloc_id = self.history.id, cause = error_cause(stack, op.tag), ), - None, + vec![], op.tag.and_then(|tag| self.get_logs_relevant_to(tag, None)), ) } diff --git a/src/borrow_tracker/stacked_borrows/mod.rs b/src/borrow_tracker/stacked_borrows/mod.rs index 5e1e0d7543..1aed436e88 100644 --- a/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/borrow_tracker/stacked_borrows/mod.rs @@ -21,7 +21,7 @@ use rustc_middle::ty::{ use rustc_target::abi::{Abi, Size}; use crate::borrow_tracker::{ - stacked_borrows::diagnostics::{AllocHistory, DiagnosticCx, DiagnosticCxBuilder, TagHistory}, + stacked_borrows::diagnostics::{AllocHistory, DiagnosticCx, DiagnosticCxBuilder}, AccessKind, GlobalStateInner, ProtectorKind, RetagFields, }; use crate::*; @@ -170,15 +170,6 @@ impl NewPermission { } } -/// Error reporting -pub fn err_sb_ub<'tcx>( - msg: String, - help: Option, - history: Option, -) -> InterpError<'tcx> { - err_machine_stop!(TerminationInfo::StackedBorrowsUb { msg, help, history }) -} - // # Stacked Borrows Core Begin /// We need to make at least the following things true: diff --git a/src/diagnostics.rs b/src/diagnostics.rs index 2a06bd871e..8d9901807e 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -22,7 +22,7 @@ pub enum TerminationInfo { UnsupportedInIsolation(String), StackedBorrowsUb { msg: String, - help: Option, + help: Vec, history: Option, }, TreeBorrowsUb { @@ -224,11 +224,10 @@ pub fn report_error<'tcx, 'mir>( (None, format!("or pass `-Zmiri-isolation-error=warn` to configure Miri to return an error code from isolated operations (if supported for that operation) and continue with a warning")), ], StackedBorrowsUb { help, history, .. } => { - let url = "https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md"; msg.extend(help.clone()); let mut helps = vec![ (None, format!("this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental")), - (None, format!("see {url} for further information")), + (None, format!("see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information")), ]; if let Some(TagHistory {created, invalidated, protected}) = history.clone() { helps.push((Some(created.1), created.0)); diff --git a/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr b/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr index b957464f95..daa4339225 100644 --- a/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr +++ b/tests/fail/both_borrows/buggy_split_at_mut.stack.stderr @@ -7,8 +7,9 @@ LL | | from_raw_parts_mut(ptr.offset(mid as isize), len - mid), 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 (of a reference/box inside this compound value) at ALLOC[0x0..0x10] + | | 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 (of a reference/box inside this compound value) at ALLOC[0x0..0x10] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr b/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr index 96121f0659..26d9f38f23 100644 --- a/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr +++ b/tests/fail/both_borrows/pass_invalid_shr_option.stack.stderr @@ -6,6 +6,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 (of a reference/box inside this compound value) at ALLOC[0x0..0x4] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr b/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr index 0cfaf12355..5f0fbf1275 100644 --- a/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr +++ b/tests/fail/both_borrows/pass_invalid_shr_tuple.stack.stderr @@ -6,6 +6,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 (of a reference/box inside this compound value) at ALLOC[0x0..0x4] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr b/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr index d5b8433568..7a9f061228 100644 --- a/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr +++ b/tests/fail/both_borrows/return_invalid_shr_option.stack.stderr @@ -6,6 +6,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 (of a reference/box inside this compound value) at ALLOC[0x4..0x8] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr b/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr index 9ced0da96c..6a98c9121e 100644 --- a/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr +++ b/tests/fail/both_borrows/return_invalid_shr_tuple.stack.stderr @@ -6,6 +6,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 (of a reference/box inside this compound value) at ALLOC[0x4..0x8] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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/tests/fail/stacked_borrows/return_invalid_mut_option.stderr b/tests/fail/stacked_borrows/return_invalid_mut_option.stderr index 89b6cee7d9..d357ab9639 100644 --- a/tests/fail/stacked_borrows/return_invalid_mut_option.stderr +++ b/tests/fail/stacked_borrows/return_invalid_mut_option.stderr @@ -6,6 +6,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 (of a reference/box inside this compound value) at ALLOC[0x4..0x8] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr b/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr index 388b00c714..d346e6fa89 100644 --- a/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr +++ b/tests/fail/stacked_borrows/return_invalid_mut_tuple.stderr @@ -6,6 +6,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 (of a reference/box inside this compound value) at ALLOC[0x4..0x8] + | errors for retagging in fields are fairly new; please reach out to us (e.g. at ) if you find this error troubling | = 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