diff options
| author | bors <bors@rust-lang.org> | 2023-05-01 11:13:49 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-05-01 11:13:49 +0000 |
| commit | b7d8c88b64843d05761576aa093a34a8bc176817 (patch) | |
| tree | f1542a0a19b65d4a528e99661a4047efd2c031a0 | |
| parent | 4b87ed9d0f18166a26aadb5289bac004e058b90f (diff) | |
| parent | f4acc67434ce945a5b9e4b8973e5872b05774218 (diff) | |
| download | rust-b7d8c88b64843d05761576aa093a34a8bc176817.tar.gz rust-b7d8c88b64843d05761576aa093a34a8bc176817.zip | |
Auto merge of #111036 - RalfJung:miri, r=RalfJung
update Miri r? `@ghost`
90 files changed, 1436 insertions, 455 deletions
diff --git a/Cargo.lock b/Cargo.lock index f6ee8157b40..d7806b5daa6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5100,11 +5100,12 @@ checksum = "9e79c4d996edb816c91e4308506774452e55e95c3c9de07b6729e17e15a5ef81" [[package]] name = "ui_test" -version = "0.5.0" +version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "54ddb6f31025943e2f9d59237f433711c461a43d9415974c3eb3a4902edc1c1f" +checksum = "3e10f5f88ce8c331a388deda1e6e2bd533c73ca89cc5f539a3df02ed35c8efba" dependencies = [ "bstr 1.3.0", + "cargo-platform", "cargo_metadata 0.15.3", "color-eyre", "colored", @@ -5113,6 +5114,7 @@ dependencies = [ "lazy_static", "regex", "rustc_version", + "rustfix", "serde", "serde_json", "tempfile", diff --git a/src/tools/miri/Cargo.lock b/src/tools/miri/Cargo.lock index 46deebf2cdd..10079ba85b6 100644 --- a/src/tools/miri/Cargo.lock +++ b/src/tools/miri/Cargo.lock @@ -27,6 +27,12 @@ dependencies = [ ] [[package]] +name = "anyhow" +version = "1.0.70" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7de8ce5e0f9f8d88245311066a578d72b7af3e7088f32783804676302df237e4" + +[[package]] name = "atty" version = "0.2.14" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -292,9 +298,9 @@ dependencies = [ [[package]] name = "libffi-sys" -version = "2.2.1" +version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc65067b78c0fc069771e8b9a9e02df71e08858bec92c1f101377c67b9dca7c7" +checksum = "f36115160c57e8529781b4183c2bb51fdc1f6d6d1ed345591d84be7703befb3c" dependencies = [ "cc", ] @@ -571,6 +577,18 @@ dependencies = [ ] [[package]] +name = "rustfix" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ecd2853d9e26988467753bd9912c3a126f642d05d229a4b53f5752ee36c56481" +dependencies = [ + "anyhow", + "log", + "serde", + "serde_json", +] + +[[package]] name = "ryu" version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -744,11 +762,12 @@ dependencies = [ [[package]] name = "ui_test" -version = "0.5.0" +version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "54ddb6f31025943e2f9d59237f433711c461a43d9415974c3eb3a4902edc1c1f" +checksum = "3e10f5f88ce8c331a388deda1e6e2bd533c73ca89cc5f539a3df02ed35c8efba" dependencies = [ "bstr", + "cargo-platform", "cargo_metadata", "color-eyre", "colored", @@ -757,6 +776,7 @@ dependencies = [ "lazy_static", "regex", "rustc_version", + "rustfix", "serde", "serde_json", "tempfile", diff --git a/src/tools/miri/Cargo.toml b/src/tools/miri/Cargo.toml index 6705bf7b007..b962d0c1096 100644 --- a/src/tools/miri/Cargo.toml +++ b/src/tools/miri/Cargo.toml @@ -39,7 +39,7 @@ libloading = "0.7" [dev-dependencies] colored = "2" -ui_test = "0.5" +ui_test = "0.6.2" rustc_version = "0.4" # Features chosen to match those required by env_logger, to avoid rebuilds regex = { version = "1.5.5", default-features = false, features = ["perf", "std"] } diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index 04a1d939d2e..640a953dac9 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -407,7 +407,7 @@ to Miri failing to detect cases of undefined behavior in a program. * `-Zmiri-retag-fields=<all|none|scalar>` controls when Stacked Borrows retagging recurses into fields. `all` means it always recurses (like `-Zmiri-retag-fields`), `none` means it never recurses, `scalar` (the default) means it only recurses for types where we would also emit - `noalias` annotations in the generated LLVM IR (types passed as indivudal scalars or pairs of + `noalias` annotations in the generated LLVM IR (types passed as individual scalars or pairs of scalars). Setting this to `none` is **unsound**. * `-Zmiri-tag-gc=<blocks>` configures how often the pointer tag garbage collector runs. The default is to search for and remove unreachable tags once every `10000` basic blocks. Setting this to diff --git a/src/tools/miri/cargo-miri/src/main.rs b/src/tools/miri/cargo-miri/src/main.rs index 9b5fa7ae873..85c9cdad7df 100644 --- a/src/tools/miri/cargo-miri/src/main.rs +++ b/src/tools/miri/cargo-miri/src/main.rs @@ -81,7 +81,7 @@ fn main() { "miri" => phase_cargo_miri(args), "runner" => phase_runner(args, RunnerPhase::Cargo), arg if arg == env::var("RUSTC").unwrap() => { - // If the first arg is equal to the RUSTC env ariable (which should be set at this + // If the first arg is equal to the RUSTC env variable (which should be set at this // point), then we need to behave as rustc. This is the somewhat counter-intuitive // behavior of having both RUSTC and RUSTC_WRAPPER set // (see https://github.com/rust-lang/cargo/issues/10886). diff --git a/src/tools/miri/miri b/src/tools/miri/miri index 1073ff499ba..4be970b398d 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -121,10 +121,10 @@ rustc-pull) # Update rust-version file. As a separate commit, since making it part of # the merge has confused the heck out of josh in the past. echo "$FETCH_COMMIT" > rust-version - git commit rust-version -m "Preparing for merge from rustc" + git commit rust-version -m "Preparing for merge from rustc" || (echo "FAILED to commit rust-version file, something went wrong"; exit 1) # Fetch given rustc commit and note down which one that was - git fetch http://localhost:8000/rust-lang/rust.git@$FETCH_COMMIT$JOSH_FILTER.git - git merge FETCH_HEAD --no-ff -m "Merge from rustc" + git fetch http://localhost:8000/rust-lang/rust.git@$FETCH_COMMIT$JOSH_FILTER.git || (echo "FAILED to fetch new commits, something went wrong"; exit 1) + git merge FETCH_HEAD --no-ff -m "Merge from rustc" || (echo "FAILED to merge new commits, something went wrong"; exit 1) exit 0 ;; rustc-push) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index f1ed3be2edd..d4bffe5f945 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -d4be8efc6296bace5b1e165f1b34d3c6da76aa8e +eb62877597000ccf8bb99ab131b5977344afdfa3 diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 3aa71bb7e3c..e4ca40570b7 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -120,7 +120,7 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls { #[allow(rustc::potential_query_instability)] // rustc_codegen_ssa (where this code is copied from) also allows this lint fn config(&mut self, config: &mut Config) { if config.opts.prints.is_empty() && self.target_crate { - // Queries overriden here affect the data stored in `rmeta` files of dependencies, + // Queries overridden here affect the data stored in `rmeta` files of dependencies, // which will be used later in non-`MIRI_BE_RUSTC` mode. config.override_queries = Some(|_, local_providers, _| { // `exported_symbols` and `reachable_non_generics` provided by rustc always returns diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index d54adb72887..ffc49eedb5a 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -238,7 +238,7 @@ pub enum BorrowTrackerMethod { } impl BorrowTrackerMethod { - pub fn instanciate_global_state(self, config: &MiriConfig) -> GlobalState { + pub fn instantiate_global_state(self, config: &MiriConfig) -> GlobalState { RefCell::new(GlobalStateInner::new( self, config.tracked_pointer_tags.clone(), 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 2cc8f035466..c9674e0a2fe 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs @@ -292,7 +292,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { .rev() .find_map(|event| { // First, look for a Creation event where the tag and the offset matches. This - // ensrues that we pick the right Creation event when a retag isn't uniform due to + // ensures that we pick the right Creation event when a retag isn't uniform due to // Freeze. let range = event.retag.range; if event.retag.new_tag == tag 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 b766916402e..4d7bbb643b8 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -433,7 +433,7 @@ impl<'tcx> Stack { let (Some(granting_idx), ProvenanceExtra::Concrete(_)) = (granting_idx, derived_from) else { // The parent is a wildcard pointer or matched the unknown bottom. // This is approximate. Nobody knows what happened, so forget everything. - // The new thing is SRW anyway, so we cannot push it "on top of the unkown part" + // The new thing is SRW anyway, so we cannot push it "on top of the unknown part" // (for all we know, it might join an SRW group inside the unknown). trace!("reborrow: forgetting stack entirely due to SharedReadWrite reborrow from wildcard or unknown"); self.set_unknown_bottom(global.next_ptr_tag); @@ -825,7 +825,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' Ok(Some(alloc_id)) } - /// Retags an indidual pointer, returning the retagged version. + /// Retags an individual pointer, returning the retagged version. /// `kind` indicates what kind of reference is being created. fn sb_retag_reference( &mut self, diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs index 1d5cfec3500..064dbe025af 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs @@ -51,7 +51,7 @@ impl Stack { // Note that the algorithm below is based on considering the tag at read_idx - 1, // so precisely considering the tag at index 0 for removal when we have an unknown // bottom would complicate the implementation. The simplification of not considering - // it does not have a significant impact on the degree to which the GC mititages + // it does not have a significant impact on the degree to which the GC mitigates // memory growth. let mut read_idx = 1; let mut write_idx = read_idx; diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs index 97bbdee1d44..2c6d27ced01 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs @@ -1,14 +1,90 @@ -use rustc_data_structures::fx::FxHashMap; - use std::fmt; use std::ops::Range; +use rustc_data_structures::fx::FxHashMap; +use rustc_span::{Span, SpanData}; +use rustc_target::abi::Size; + use crate::borrow_tracker::tree_borrows::{ - err_tb_ub, perms::Permission, tree::LocationState, unimap::UniIndex, + perms::{PermTransition, Permission}, + tree::LocationState, + unimap::UniIndex, }; use crate::borrow_tracker::{AccessKind, ProtectorKind}; use crate::*; +/// Complete data for an event: +/// - `kind` is what happened to the permissions +/// - `access_kind` and `access_range` describe the access that caused the event +/// - `offset` allows filtering only the relevant events for a given memory location +/// (see how we perform the filtering in `History::extract_relevant`. +/// - `span` is the line of code in question +#[derive(Clone, Debug)] +pub struct Event { + pub transition: PermTransition, + pub access_kind: AccessKind, + pub is_foreign: bool, + pub access_range: AllocRange, + pub offset: Size, + pub span: Span, +} + +/// List of all events that affected a tag. +/// NOTE: not all of these events are relevant for a particular location, +/// the events should be filtered before the generation of diagnostics. +/// Available filtering methods include `History::forget` and `History::extract_relevant`. +#[derive(Clone, Debug)] +pub struct History { + pub tag: BorTag, + pub created: (Span, Permission), + pub events: Vec<Event>, +} + +/// History formatted for use by `src/diagnostics.rs`. +/// +/// NOTE: needs to be `Send` because of a bound on `MachineStopType`, hence +/// the use of `SpanData` rather than `Span`. +#[derive(Debug, Clone, Default)] +pub struct HistoryData { + pub events: Vec<(Option<SpanData>, String)>, // includes creation +} + +impl History { + /// Record an additional event to the history. + pub fn push(&mut self, event: Event) { + self.events.push(event); + } +} + +impl HistoryData { + // Format events from `new_history` into those recorded by `self`. + // + // NOTE: also converts `Span` to `SpanData`. + pub fn extend( + &mut self, + new_history: History, + tag_name: &'static str, + show_initial_state: bool, + ) { + let History { tag, created, events } = new_history; + let this = format!("the {tag_name} tag {tag:?}"); + let msg_initial_state = format!(", in the initial state {}", created.1); + let msg_creation = format!( + "{this} was created here{maybe_msg_initial_state}", + maybe_msg_initial_state = if show_initial_state { &msg_initial_state } else { "" }, + ); + + self.events.push((Some(created.0.data()), msg_creation)); + for &Event { transition, access_kind, is_foreign, access_range, span, offset: _ } in &events + { + // NOTE: `offset` is explicitly absent from the error message, it has no significance + // to the user. The meaningful one is `access_range`. + self.events.push((Some(span.data()), format!("{this} then transitioned {transition} due to a {rel} {access_kind} at offsets {access_range:?}", rel = if is_foreign { "foreign" } else { "child" }))); + self.events.push((None, format!("this corresponds to {}", transition.summary()))); + } + } +} + /// Some information that is irrelevant for the algorithm but very /// convenient to know about a tag for debugging and testing. #[derive(Clone, Debug)] @@ -20,18 +96,29 @@ pub struct NodeDebugInfo { /// pointer in the source code. /// Helps match tag numbers to human-readable names. pub name: Option<String>, + /// Notable events in the history of this tag, used for + /// diagnostics. + /// + /// NOTE: by virtue of being part of `NodeDebugInfo`, + /// the history is automatically cleaned up by the GC. + /// NOTE: this is `!Send`, it needs to be converted before displaying + /// the actual diagnostics because `src/diagnostics.rs` requires `Send`. + pub history: History, } + impl NodeDebugInfo { - /// New node info with a name. - pub fn new(tag: BorTag) -> Self { - Self { tag, name: None } + /// Information for a new node. By default it has no + /// name and an empty history. + pub fn new(tag: BorTag, initial: Permission, span: Span) -> Self { + let history = History { tag, created: (span, initial), events: Vec::new() }; + Self { tag, name: None, history } } /// Add a name to the tag. If a same tag is associated to several pointers, /// it can have several names which will be separated by commas. - fn add_name(&mut self, name: &str) { + pub fn add_name(&mut self, name: &str) { if let Some(ref mut prev_name) = &mut self.name { - prev_name.push(','); + prev_name.push_str(", "); prev_name.push_str(name); } else { self.name = Some(String::from(name)); @@ -42,7 +129,7 @@ impl NodeDebugInfo { impl fmt::Display for NodeDebugInfo { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if let Some(ref name) = self.name { - write!(f, "{tag:?} (also named '{name}')", tag = self.tag) + write!(f, "{tag:?} ({name})", tag = self.tag) } else { write!(f, "{tag:?}", tag = self.tag) } @@ -86,7 +173,7 @@ impl<'tcx> Tree { } } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq)] pub(super) enum TransitionError { /// This access is not allowed because some parent tag has insufficient permissions. /// For example, if a tag is `Frozen` and encounters a child write this will @@ -96,63 +183,145 @@ pub(super) enum TransitionError { /// A protector was triggered due to an invalid transition that loses /// too much permissions. /// For example, if a protected tag goes from `Active` to `Frozen` due - /// to a foreign write this will produce a `ProtectedTransition(Active, Frozen)`. + /// to a foreign write this will produce a `ProtectedTransition(PermTransition(Active, Frozen))`. /// This kind of error can only occur on foreign accesses. - ProtectedTransition(Permission, Permission), + ProtectedTransition(PermTransition), /// Cannot deallocate because some tag in the allocation is strongly protected. /// This kind of error can only occur on deallocations. ProtectedDealloc, } +impl History { + /// Keep only the tag and creation + fn forget(&self) -> Self { + History { events: Vec::new(), created: self.created, tag: self.tag } + } + + /// Reconstruct the history relevant to `error_offset` knowing that + /// its permission followed `complete_transition`. + /// + /// Here's how we do this: + /// - we know `full := complete_transition` the transition of the permission from + /// its initialization to the state just before the error was caused, + /// we want to find a chain of events that produces `full` + /// - we decompose `full` into `pre o post` where + /// `pre` is the best applicable transition from recorded events + /// - we select the event that caused `pre` and iterate + /// to find the chain of events that produces `full := post` + /// + /// To find the "best applicable transition" for full: + /// - eliminate events that cannot be applied because their offset is too big + /// - eliminate events that cannot be applied because their starting point is wrong + /// - select the one that happened closest to the range of interest + fn extract_relevant(&self, complete_transition: PermTransition, error_offset: Size) -> Self { + let mut selected_events: Vec<Event> = Vec::new(); + let mut full = complete_transition; + while !full.is_noop() { + let (pre, post) = self + .events + .iter() + .filter(|e| e.offset <= error_offset) + .filter_map(|pre_canditate| { + full.apply_start(pre_canditate.transition) + .map(|post_canditate| (pre_canditate, post_canditate)) + }) + .max_by_key(|(pre_canditate, _post_candidate)| pre_canditate.offset) + .unwrap(); + // If this occurs we will loop infinitely ! + // Make sure to only put non-noop transitions in `History`. + assert!(!pre.transition.is_noop()); + full = post; + selected_events.push(pre.clone()); + } + + History { events: selected_events, created: self.created, tag: self.tag } + } +} + /// Failures that can occur during the execution of Tree Borrows procedures. pub(super) struct TbError<'node> { /// What failure occurred. pub error_kind: TransitionError, + /// The byte at which the conflict occured. + pub error_offset: Size, /// The tag on which the error was triggered. /// On protector violations, this is the tag that was protected. /// On accesses rejected due to insufficient permissions, this is the /// tag that lacked those permissions. - pub faulty_tag: &'node NodeDebugInfo, + pub conflicting_info: &'node NodeDebugInfo, /// Whether this was a Read or Write access. This field is ignored /// when the error was triggered by a deallocation. pub access_kind: AccessKind, /// Which tag the access that caused this error was made through, i.e. /// which tag was used to read/write/deallocate. - pub tag_of_access: &'node NodeDebugInfo, + pub accessed_info: &'node NodeDebugInfo, } impl TbError<'_> { /// Produce a UB error. - pub fn build<'tcx>(self) -> InterpErrorInfo<'tcx> { + pub fn build<'tcx>(self) -> InterpError<'tcx> { use TransitionError::*; - err_tb_ub(match self.error_kind { + let started_as = self.conflicting_info.history.created.1; + let kind = self.access_kind; + let accessed = self.accessed_info; + let conflicting = self.conflicting_info; + let accessed_is_conflicting = accessed.tag == conflicting.tag; + let (pre_error, title, details, conflicting_tag_name) = match self.error_kind { ChildAccessForbidden(perm) => { - format!( - "{kind} through {initial} is forbidden because it is a child of {current} which is {perm}.", - kind=self.access_kind, - initial=self.tag_of_access, - current=self.faulty_tag, - perm=perm, - ) + let conflicting_tag_name = + if accessed_is_conflicting { "accessed" } else { "conflicting" }; + let title = format!("{kind} through {accessed} is forbidden"); + let mut details = Vec::new(); + if !accessed_is_conflicting { + details.push(format!( + "the accessed tag {accessed} is a child of the conflicting tag {conflicting}" + )); + } + details.push(format!( + "the {conflicting_tag_name} tag {conflicting} has state {perm} which forbids child {kind}es" + )); + (perm, title, details, conflicting_tag_name) } - ProtectedTransition(start, end) => { - format!( - "{kind} through {initial} is forbidden because it is a foreign tag for {current}, which would hence change from {start} to {end}, but {current} is protected", - current=self.faulty_tag, - start=start, - end=end, - kind=self.access_kind, - initial=self.tag_of_access, - ) + ProtectedTransition(transition) => { + let conflicting_tag_name = "protected"; + let title = format!("{kind} through {accessed} is forbidden"); + let details = vec![ + format!( + "the accessed tag {accessed} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)" + ), + format!( + "the access would cause the {conflicting_tag_name} tag {conflicting} to transition {transition}" + ), + format!( + "this is {loss}, which is not allowed for protected tags", + loss = transition.summary(), + ), + ]; + (transition.started(), title, details, conflicting_tag_name) } ProtectedDealloc => { - format!( - "the allocation of {initial} also contains {current} which is strongly protected, cannot deallocate", - initial=self.tag_of_access, - current=self.faulty_tag, - ) + let conflicting_tag_name = "strongly protected"; + let title = format!("deallocation through {accessed} is forbidden"); + let details = vec![ + format!( + "the allocation of the accessed tag {accessed} also contains the {conflicting_tag_name} tag {conflicting}" + ), + format!("the {conflicting_tag_name} tag {conflicting} disallows deallocations"), + ]; + (started_as, title, details, conflicting_tag_name) } - }).into() + }; + let pre_transition = PermTransition::from(started_as, pre_error).unwrap(); + let mut history = HistoryData::default(); + if !accessed_is_conflicting { + history.extend(self.accessed_info.history.forget(), "accessed", false); + } + history.extend( + self.conflicting_info.history.extract_relevant(pre_transition, self.error_offset), + conflicting_tag_name, + true, + ); + err_machine_stop!(TerminationInfo::TreeBorrowsUb { title, details, history }) } } diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 2297ceb1259..3361d212f2c 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -14,7 +14,7 @@ use rustc_middle::{ use crate::*; -mod diagnostics; +pub mod diagnostics; mod perms; mod tree; mod unimap; @@ -23,10 +23,6 @@ pub use tree::Tree; pub type AllocState = Tree; -pub fn err_tb_ub<'tcx>(msg: String) -> InterpError<'tcx> { - err_machine_stop!(TerminationInfo::TreeBorrowsUb { msg }) -} - impl<'tcx> Tree { /// Create a new allocation, i.e. a new tree pub fn new_allocation( @@ -37,7 +33,8 @@ impl<'tcx> Tree { machine: &MiriMachine<'_, 'tcx>, ) -> Self { let tag = state.base_ptr_tag(id, machine); // Fresh tag for the root - Tree::new(tag, size) + let span = machine.current_span(); + Tree::new(tag, size, span) } /// Check that an access on the entire range is permitted, and update @@ -64,7 +61,8 @@ impl<'tcx> Tree { ProvenanceExtra::Wildcard => return Ok(()), }; let global = machine.borrow_tracker.as_ref().unwrap(); - self.perform_access(access_kind, tag, range, global) + let span = machine.current_span(); + self.perform_access(access_kind, tag, range, global, span) } /// Check that this pointer has permission to deallocate this range. @@ -82,7 +80,8 @@ impl<'tcx> Tree { ProvenanceExtra::Wildcard => return Ok(()), }; let global = machine.borrow_tracker.as_ref().unwrap(); - self.dealloc(tag, range, global) + let span = machine.current_span(); + self.dealloc(tag, range, global, span) } pub fn expose_tag(&mut self, _tag: BorTag) { @@ -265,6 +264,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' .insert(new_tag, protect); } + let span = this.machine.current_span(); let alloc_extra = this.get_alloc_extra(alloc_id)?; let range = alloc_range(base_offset, ptr_size); let mut tree_borrows = alloc_extra.borrow_tracker_tb().borrow_mut(); @@ -272,18 +272,19 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' if new_perm.perform_read_access { // Count this reborrow as a read access let global = &this.machine.borrow_tracker.as_ref().unwrap(); - tree_borrows.perform_access(AccessKind::Read, orig_tag, range, global)?; + let span = this.machine.current_span(); + tree_borrows.perform_access(AccessKind::Read, orig_tag, range, global, span)?; if let Some(data_race) = alloc_extra.data_race.as_ref() { data_race.read(alloc_id, range, &this.machine)?; } } // Record the parent-child pair in the tree. - tree_borrows.new_child(orig_tag, new_tag, new_perm.initial_state, range)?; + tree_borrows.new_child(orig_tag, new_tag, new_perm.initial_state, range, span)?; Ok(Some((alloc_id, new_tag))) } - /// Retags an indidual pointer, returning the retagged version. + /// Retags an individual pointer, returning the retagged version. fn tb_retag_reference( &mut self, val: &ImmTy<'tcx, Provenance>, diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs index 04b8e1df576..7e3e587db72 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -4,7 +4,7 @@ use std::fmt; use crate::borrow_tracker::tree_borrows::tree::AccessRelatedness; use crate::borrow_tracker::AccessKind; -/// The activation states of a pointer +/// The activation states of a pointer. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum PermissionPriv { /// represents: a local reference that has not yet been written to; @@ -112,47 +112,14 @@ mod transition { } } -impl PermissionPriv { - /// Determines whether a transition that occured is compatible with the presence - /// of a Protector. This is not included in the `transition` functions because - /// it would distract from the few places where the transition is modified - /// because of a protector, but not forbidden. - fn protector_allows_transition(self, new: Self) -> bool { - match (self, new) { - _ if self == new => true, - // It is always a protector violation to not be readable anymore - (_, Disabled) => false, - // In the case of a `Reserved` under a protector, both transitions - // `Reserved => Active` and `Reserved => Frozen` can legitimately occur. - // The first is standard (Child Write), the second is for Foreign Writes - // on protected Reserved where we must ensure that the pointer is not - // written to in the future. - (Reserved { .. }, Active) | (Reserved { .. }, Frozen) => true, - // This pointer should have stayed writeable for the whole function - (Active, Frozen) => false, - _ => unreachable!("Transition from {self:?} to {new:?} should never be possible"), - } - } -} - /// Public interface to the state machine that controls read-write permissions. +/// This is the "private `enum`" pattern. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Permission(PermissionPriv); -impl fmt::Display for Permission { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "{}", - match self.0 { - PermissionPriv::Reserved { .. } => "Reserved", - PermissionPriv::Active => "Active", - PermissionPriv::Frozen => "Frozen", - PermissionPriv::Disabled => "Disabled", - } - ) - } -} +/// Transition from one permission to the next. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct PermTransition(PermissionPriv, PermissionPriv); impl Permission { /// Default initial permission of the root of a new tree. @@ -170,43 +137,148 @@ impl Permission { Self(Frozen) } - /// Pretty-printing. Needs to be here and not in diagnostics.rs - /// because `Self` is private. - pub fn short_name(self) -> &'static str { - // Make sure there are all of the same length as each other - // and also as `diagnostics::DisplayFmtPermission.uninit` otherwise - // alignment will be incorrect. - match self.0 { - Reserved { ty_is_freeze: true } => "Res", - Reserved { ty_is_freeze: false } => "Re*", - Active => "Act", - Frozen => "Frz", - Disabled => "Dis", - } + /// Apply the transition to the inner PermissionPriv. + pub fn perform_access( + kind: AccessKind, + rel_pos: AccessRelatedness, + old_perm: Self, + protected: bool, + ) -> Option<PermTransition> { + let old_state = old_perm.0; + transition::perform_access(kind, rel_pos, old_state, protected) + .map(|new_state| PermTransition(old_state, new_state)) } +} - /// Check that there are no complaints from a possible protector. +impl PermTransition { + /// All transitions created through normal means (using `perform_access`) + /// should be possible, but the same is not guaranteed by construction of + /// transitions inferred by diagnostics. This checks that a transition + /// reconstructed by diagnostics is indeed one that could happen. + fn is_possible(old: PermissionPriv, new: PermissionPriv) -> bool { + old <= new + } + + pub fn from(old: Permission, new: Permission) -> Option<Self> { + Self::is_possible(old.0, new.0).then_some(Self(old.0, new.0)) + } + + pub fn is_noop(self) -> bool { + self.0 == self.1 + } + + /// Extract result of a transition (checks that the starting point matches). + pub fn applied(self, starting_point: Permission) -> Option<Permission> { + (starting_point.0 == self.0).then_some(Permission(self.1)) + } + + /// Extract starting point of a transition + pub fn started(self) -> Permission { + Permission(self.0) + } + + /// Determines whether a transition that occured is compatible with the presence + /// of a Protector. This is not included in the `transition` functions because + /// it would distract from the few places where the transition is modified + /// because of a protector, but not forbidden. /// /// Note: this is not in charge of checking that there *is* a protector, /// it should be used as /// ``` /// let no_protector_error = if is_protected(tag) { - /// old_perm.protector_allows_transition(new_perm) + /// transition.is_allowed_by_protector() /// }; /// ``` - pub fn protector_allows_transition(self, new: Self) -> bool { - self.0.protector_allows_transition(new.0) + pub fn is_allowed_by_protector(&self) -> bool { + let &Self(old, new) = self; + assert!(Self::is_possible(old, new)); + match (old, new) { + _ if old == new => true, + // It is always a protector violation to not be readable anymore + (_, Disabled) => false, + // In the case of a `Reserved` under a protector, both transitions + // `Reserved => Active` and `Reserved => Frozen` can legitimately occur. + // The first is standard (Child Write), the second is for Foreign Writes + // on protected Reserved where we must ensure that the pointer is not + // written to in the future. + (Reserved { .. }, Active) | (Reserved { .. }, Frozen) => true, + // This pointer should have stayed writeable for the whole function + (Active, Frozen) => false, + _ => unreachable!("Transition from {old:?} to {new:?} should never be possible"), + } } - /// Apply the transition to the inner PermissionPriv. - pub fn perform_access( - kind: AccessKind, - rel_pos: AccessRelatedness, - old_perm: Self, - protected: bool, - ) -> Option<Self> { - let old_state = old_perm.0; - transition::perform_access(kind, rel_pos, old_state, protected).map(Self) + /// Composition function: get the transition that can be added after `app` to + /// produce `self`. + pub fn apply_start(self, app: Self) -> Option<Self> { + let new_start = app.applied(Permission(self.0))?; + Self::from(new_start, Permission(self.1)) + } +} + +pub mod diagnostics { + use super::*; + impl fmt::Display for PermissionPriv { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{}", + match self { + PermissionPriv::Reserved { .. } => "Reserved", + PermissionPriv::Active => "Active", + PermissionPriv::Frozen => "Frozen", + PermissionPriv::Disabled => "Disabled", + } + ) + } + } + + impl fmt::Display for PermTransition { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "from {} to {}", self.0, self.1) + } + } + + impl fmt::Display for Permission { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } + } + + impl Permission { + /// Abbreviated name of the permission (uniformly 3 letters for nice alignment). + pub fn short_name(self) -> &'static str { + // Make sure there are all of the same length as each other + // and also as `diagnostics::DisplayFmtPermission.uninit` otherwise + // alignment will be incorrect. + match self.0 { + Reserved { ty_is_freeze: true } => "Res", + Reserved { ty_is_freeze: false } => "Re*", + Active => "Act", + Frozen => "Frz", + Disabled => "Dis", + } + } + } + + impl PermTransition { + /// Readable explanation of the consequences of an event. + /// Fits in the sentence "This accessed caused {trans.summary()}". + /// + /// Important: for the purposes of this explanation, `Reserved` is considered + /// to have write permissions, because that's what the diagnostics care about + /// (otherwise `Reserved -> Frozen` would be considered a noop). + pub fn summary(&self) -> &'static str { + assert!(Self::is_possible(self.0, self.1)); + match (self.0, self.1) { + (_, Active) => "an activation", + (_, Frozen) => "a loss of write permissions", + (Frozen, Disabled) => "a loss of read permissions", + (_, Disabled) => "a loss of read and write permissions", + (old, new) => + unreachable!("Transition from {old:?} to {new:?} should never be possible"), + } + } } } diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs index 86416a0eb1b..6392f5101ad 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -14,10 +14,11 @@ use smallvec::SmallVec; use rustc_const_eval::interpret::InterpResult; use rustc_data_structures::fx::FxHashSet; +use rustc_span::Span; use rustc_target::abi::Size; use crate::borrow_tracker::tree_borrows::{ - diagnostics::{NodeDebugInfo, TbError, TransitionError}, + diagnostics::{self, NodeDebugInfo, TbError, TransitionError}, unimap::{UniEntry, UniIndex, UniKeyMap, UniValMap}, Permission, }; @@ -34,7 +35,7 @@ pub(super) struct LocationState { /// Before initialization we still apply some preemptive transitions on /// `permission` to know what to do in case it ever gets initialized, /// but these can never cause any immediate UB. There can however be UB - /// the moment we attempt to initalize (i.e. child-access) because some + /// the moment we attempt to initialize (i.e. child-access) because some /// foreign access done between the creation and the initialization is /// incompatible with child accesses. initialized: bool, @@ -118,7 +119,7 @@ pub(super) struct Node { /// Data given to the transition function struct NodeAppArgs<'node> { /// Node on which the transition is currently being applied - node: &'node Node, + node: &'node mut Node, /// Mutable access to its permissions perm: UniEntry<'node, LocationState>, /// Relative position of the access @@ -131,14 +132,17 @@ struct ErrHandlerArgs<'node, InErr> { /// Tag that triggered the error (not the tag that was accessed, /// rather the parent tag that had insufficient permissions or the /// non-parent tag that had a protector). - faulty_tag: &'node NodeDebugInfo, + conflicting_info: &'node NodeDebugInfo, + /// Information about the tag that was accessed just before the + /// error was triggered. + accessed_info: &'node NodeDebugInfo, } /// Internal contents of `Tree` with the minimum of mutable access for /// the purposes of the tree traversal functions: the permissions (`perms`) can be /// updated but not the tree structure (`tag_mapping` and `nodes`) struct TreeVisitor<'tree> { tag_mapping: &'tree UniKeyMap<BorTag>, - nodes: &'tree UniValMap<Node>, + nodes: &'tree mut UniValMap<Node>, perms: &'tree mut UniValMap<LocationState>, } @@ -167,6 +171,7 @@ impl<'tree> TreeVisitor<'tree> { ) -> Result<(), OutErr> where { struct TreeVisitAux<NodeApp, ErrHandler> { + accessed_tag: UniIndex, f_propagate: NodeApp, err_builder: ErrHandler, stack: Vec<(UniIndex, AccessRelatedness)>, @@ -190,15 +195,21 @@ where { rel_pos: AccessRelatedness, ) -> Result<(), OutErr> { // 1. apply the propagation function - let node = this.nodes.get(tag).unwrap(); + let node = this.nodes.get_mut(tag).unwrap(); let recurse = (self.f_propagate)(NodeAppArgs { node, perm: this.perms.entry(tag), rel_pos }) .map_err(|error_kind| { (self.err_builder)(ErrHandlerArgs { error_kind, - faulty_tag: &node.debug_info, + conflicting_info: &this.nodes.get(tag).unwrap().debug_info, + accessed_info: &this + .nodes + .get(self.accessed_tag) + .unwrap() + .debug_info, }) })?; + let node = this.nodes.get(tag).unwrap(); // 2. add the children to the stack for future traversal if matches!(recurse, ContinueTraversal::Recurse) { let child_rel = rel_pos.for_child(); @@ -214,7 +225,8 @@ where { } let start_idx = self.tag_mapping.get(&start).unwrap(); - let mut stack = TreeVisitAux { f_propagate, err_builder, stack: Vec::new() }; + let mut stack = + TreeVisitAux { accessed_tag: start_idx, f_propagate, err_builder, stack: Vec::new() }; { let mut path_ascend = Vec::new(); // First climb to the root while recording the path @@ -262,12 +274,15 @@ where { impl Tree { /// Create a new tree, with only a root pointer. - pub fn new(root_tag: BorTag, size: Size) -> Self { + pub fn new(root_tag: BorTag, size: Size, span: Span) -> Self { let root_perm = Permission::new_root(); let mut tag_mapping = UniKeyMap::default(); let root_idx = tag_mapping.insert(root_tag); let nodes = { let mut nodes = UniValMap::<Node>::default(); + let mut debug_info = NodeDebugInfo::new(root_tag, root_perm, span); + // name the root so that all allocations contain one named pointer + debug_info.add_name("root of the allocation"); nodes.insert( root_idx, Node { @@ -275,7 +290,7 @@ impl Tree { parent: None, children: SmallVec::default(), default_initial_perm: root_perm, - debug_info: NodeDebugInfo::new(root_tag), + debug_info, }, ); nodes @@ -297,6 +312,7 @@ impl<'tcx> Tree { new_tag: BorTag, default_initial_perm: Permission, range: AllocRange, + span: Span, ) -> InterpResult<'tcx> { assert!(!self.tag_mapping.contains_key(&new_tag)); let idx = self.tag_mapping.insert(new_tag); @@ -309,7 +325,7 @@ impl<'tcx> Tree { parent: Some(parent_idx), children: SmallVec::default(), default_initial_perm, - debug_info: NodeDebugInfo::new(new_tag), + debug_info: NodeDebugInfo::new(new_tag, default_initial_perm, span), }, ); // Register new_tag as a child of parent_tag @@ -330,11 +346,11 @@ impl<'tcx> Tree { tag: BorTag, range: AllocRange, global: &GlobalState, + span: Span, // diagnostics ) -> InterpResult<'tcx> { - self.perform_access(AccessKind::Write, tag, range, global)?; - let access_info = &self.nodes.get(self.tag_mapping.get(&tag).unwrap()).unwrap().debug_info; - for (_range, perms) in self.rperms.iter_mut(range.start, range.size) { - TreeVisitor { nodes: &self.nodes, tag_mapping: &self.tag_mapping, perms } + self.perform_access(AccessKind::Write, tag, range, global, span)?; + for (offset, perms) in self.rperms.iter_mut(range.start, range.size) { + TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms } .traverse_parents_this_children_others( tag, |args: NodeAppArgs<'_>| -> Result<ContinueTraversal, TransitionError> { @@ -347,13 +363,14 @@ impl<'tcx> Tree { Ok(ContinueTraversal::Recurse) } }, - |args: ErrHandlerArgs<'_, TransitionError>| -> InterpErrorInfo<'tcx> { - let ErrHandlerArgs { error_kind, faulty_tag } = args; + |args: ErrHandlerArgs<'_, TransitionError>| -> InterpError<'tcx> { + let ErrHandlerArgs { error_kind, conflicting_info, accessed_info } = args; TbError { - faulty_tag, + conflicting_info, access_kind: AccessKind::Write, + error_offset: offset, error_kind, - tag_of_access: access_info, + accessed_info, } .build() }, @@ -373,10 +390,10 @@ impl<'tcx> Tree { tag: BorTag, range: AllocRange, global: &GlobalState, + span: Span, // diagnostics ) -> InterpResult<'tcx> { - let access_info = &self.nodes.get(self.tag_mapping.get(&tag).unwrap()).unwrap().debug_info; - for (_range, perms) in self.rperms.iter_mut(range.start, range.size) { - TreeVisitor { nodes: &self.nodes, tag_mapping: &self.tag_mapping, perms } + for (offset, perms) in self.rperms.iter_mut(range.start, range.size) { + TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms } .traverse_parents_this_children_others( tag, |args: NodeAppArgs<'_>| -> Result<ContinueTraversal, TransitionError> { @@ -424,24 +441,42 @@ impl<'tcx> Tree { let old_perm = old_state.permission; let protected = global.borrow().protected_tags.contains_key(&node.tag); - let new_perm = + let transition = Permission::perform_access(access_kind, rel_pos, old_perm, protected) .ok_or(TransitionError::ChildAccessForbidden(old_perm))?; if protected // Can't trigger Protector on uninitialized locations && old_state.initialized - && !old_perm.protector_allows_transition(new_perm) + && !transition.is_allowed_by_protector() { - return Err(TransitionError::ProtectedTransition(old_perm, new_perm)); + return Err(TransitionError::ProtectedTransition(transition)); + } + // Record the event as part of the history + if !transition.is_noop() { + node.debug_info.history.push(diagnostics::Event { + transition, + access_kind, + access_range: range, + is_foreign: rel_pos.is_foreign(), + offset, + span, + }); + old_state.permission = + transition.applied(old_state.permission).unwrap(); } - old_state.permission = new_perm; old_state.initialized |= !rel_pos.is_foreign(); Ok(ContinueTraversal::Recurse) }, - |args: ErrHandlerArgs<'_, TransitionError>| -> InterpErrorInfo<'tcx> { - let ErrHandlerArgs { error_kind, faulty_tag } = args; - TbError { faulty_tag, access_kind, error_kind, tag_of_access: access_info } - .build() + |args: ErrHandlerArgs<'_, TransitionError>| -> InterpError<'tcx> { + let ErrHandlerArgs { error_kind, conflicting_info, accessed_info } = args; + TbError { + conflicting_info, + access_kind, + error_offset: offset, + error_kind, + accessed_info, + } + .build() }, )?; } diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index 1f67b91569c..13306b4809c 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -1199,7 +1199,7 @@ pub struct GlobalState { /// A flag to mark we are currently performing /// a data race free action (such as atomic access) - /// to supress the race detector + /// to suppress the race detector ongoing_action_data_race_free: Cell<bool>, /// Mapping of a vector index to a known set of thread diff --git a/src/tools/miri/src/concurrency/init_once.rs b/src/tools/miri/src/concurrency/init_once.rs index 7a6b8da4082..1f57e8b2b0a 100644 --- a/src/tools/miri/src/concurrency/init_once.rs +++ b/src/tools/miri/src/concurrency/init_once.rs @@ -151,7 +151,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { assert_eq!( init_once.status, InitOnceStatus::Uninitialized, - "begining already begun or complete init once" + "beginning already begun or complete init once" ); init_once.status = InitOnceStatus::Begun; } diff --git a/src/tools/miri/src/concurrency/range_object_map.rs b/src/tools/miri/src/concurrency/range_object_map.rs index dfe2e9f05da..89c009933bb 100644 --- a/src/tools/miri/src/concurrency/range_object_map.rs +++ b/src/tools/miri/src/concurrency/range_object_map.rs @@ -25,9 +25,9 @@ pub struct RangeObjectMap<T> { #[derive(Clone, Debug, PartialEq)] pub enum AccessType { - /// The access perfectly overlaps (same offset and range) with the exsiting allocation + /// The access perfectly overlaps (same offset and range) with the existing allocation PerfectlyOverlapping(Position), - /// The access does not touch any exising allocation + /// The access does not touch any existing allocation Empty(Position), /// The access overlaps with one or more existing allocations ImperfectlyOverlapping(Range<Position>), @@ -115,7 +115,7 @@ impl<T> RangeObjectMap<T> { // want to repeat the binary search on each time, so we ask the caller to supply Position pub fn insert_at_pos(&mut self, pos: Position, range: AllocRange, data: T) { self.v.insert(pos, Elem { range, data }); - // If we aren't the first element, then our start must be greater than the preivous element's end + // If we aren't the first element, then our start must be greater than the previous element's end if pos > 0 { assert!(self.v[pos - 1].range.end() <= range.start); } diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index 662dff62c88..f37a2fd2cd5 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -143,7 +143,7 @@ struct Condvar { waiters: VecDeque<CondvarWaiter>, /// Tracks the happens-before relationship /// between a cond-var signal and a cond-var - /// wait during a non-suprious signal event. + /// wait during a non-spurious signal event. /// Contains the clock of the last thread to /// perform a futex-signal. data_race: VClock, @@ -373,7 +373,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { .expect("invariant violation: lock_count == 0 iff the thread is unlocked"); if mutex.lock_count == 0 { mutex.owner = None; - // The mutex is completely unlocked. Try transfering ownership + // The mutex is completely unlocked. Try transferring ownership // to another thread. if let Some(data_race) = &this.machine.data_race { data_race.validate_lock_release( diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index 5f2bc2ec558..e9bbae4d504 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -272,9 +272,8 @@ impl Time { fn get_wait_time(&self, clock: &Clock) -> Duration { match self { Time::Monotonic(instant) => instant.duration_since(clock.now()), - Time::RealTime(time) => { - time.duration_since(SystemTime::now()).unwrap_or(Duration::new(0, 0)) - } + Time::RealTime(time) => + time.duration_since(SystemTime::now()).unwrap_or(Duration::new(0, 0)), } } } @@ -823,7 +822,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } // Write the current thread-id, switch to the next thread later - // to treat this write operation as occuring on the current thread. + // to treat this write operation as occurring on the current thread. if let Some(thread_info_place) = thread { this.write_scalar( Scalar::from_uint(new_thread_id.to_u32(), thread_info_place.layout.size), @@ -832,7 +831,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } // Finally switch to new thread so that we can push the first stackframe. - // After this all accesses will be treated as occuring in the new thread. + // After this all accesses will be treated as occurring in the new thread. let old_thread_id = this.set_active_thread(new_thread_id); // Perform the function pointer load in the new thread frame. diff --git a/src/tools/miri/src/concurrency/vector_clock.rs b/src/tools/miri/src/concurrency/vector_clock.rs index b36c6be5a72..a6e67ef8699 100644 --- a/src/tools/miri/src/concurrency/vector_clock.rs +++ b/src/tools/miri/src/concurrency/vector_clock.rs @@ -212,16 +212,14 @@ impl PartialOrd for VClock { for (l, r) in iter { match order { Ordering::Equal => order = l.cmp(r), - Ordering::Less => { + Ordering::Less => if l > r { return None; - } - } - Ordering::Greater => { + }, + Ordering::Greater => if l < r { return None; - } - } + }, } } @@ -236,16 +234,18 @@ impl PartialOrd for VClock { Ordering::Equal => Some(order), // Right has at least 1 element > than the implicit 0, // so the only valid values are Ordering::Less or None. - Ordering::Less => match order { - Ordering::Less | Ordering::Equal => Some(Ordering::Less), - Ordering::Greater => None, - }, + Ordering::Less => + match order { + Ordering::Less | Ordering::Equal => Some(Ordering::Less), + Ordering::Greater => None, + }, // Left has at least 1 element > than the implicit 0, // so the only valid values are Ordering::Greater or None. - Ordering::Greater => match order { - Ordering::Greater | Ordering::Equal => Some(Ordering::Greater), - Ordering::Less => None, - }, + Ordering::Greater => + match order { + Ordering::Greater | Ordering::Equal => Some(Ordering::Greater), + Ordering::Less => None, + }, } } diff --git a/src/tools/miri/src/concurrency/weak_memory.rs b/src/tools/miri/src/concurrency/weak_memory.rs index 2a48c9e6cd0..c1395468fee 100644 --- a/src/tools/miri/src/concurrency/weak_memory.rs +++ b/src/tools/miri/src/concurrency/weak_memory.rs @@ -24,7 +24,7 @@ //! However, this model lacks SC accesses and is therefore unusable by Miri (SC accesses are everywhere in library code). //! //! If you find anything that proposes a relaxed memory model that is C++20-consistent, supports all orderings Rust's atomic accesses -//! and fences accept, and is implementable (with operational semanitcs), please open a GitHub issue! +//! and fences accept, and is implementable (with operational semantics), please open a GitHub issue! //! //! One characteristic of this implementation, in contrast to some other notable operational models such as ones proposed in //! Taming Release-Acquire Consistency by Ori Lahav et al. (<https://plv.mpi-sws.org/sra/paper.pdf>) or Promising Semantics noted above, @@ -32,8 +32,8 @@ //! and shared across all threads. This is more memory efficient but does require store elements (representing writes to a location) to record //! information about reads, whereas in the other two models it is the other way round: reads points to the write it got its value from. //! Additionally, writes in our implementation do not have globally unique timestamps attached. In the other two models this timestamp is -//! used to make sure a value in a thread's view is not overwritten by a write that occured earlier than the one in the existing view. -//! In our implementation, this is detected using read information attached to store elements, as there is no data strucutre representing reads. +//! used to make sure a value in a thread's view is not overwritten by a write that occurred earlier than the one in the existing view. +//! In our implementation, this is detected using read information attached to store elements, as there is no data structure representing reads. //! //! The C++ memory model is built around the notion of an 'atomic object', so it would be natural //! to attach store buffers to atomic objects. However, Rust follows LLVM in that it only has @@ -48,7 +48,7 @@ //! One consequence of this difference is that safe/sound Rust allows for more operations on atomic locations //! than the C++20 atomic API was intended to allow, such as non-atomically accessing //! a previously atomically accessed location, or accessing previously atomically accessed locations with a differently sized operation -//! (such as accessing the top 16 bits of an AtomicU32). These senarios are generally undiscussed in formalisations of C++ memory model. +//! (such as accessing the top 16 bits of an AtomicU32). These scenarios are generally undiscussed in formalisations of C++ memory model. //! In Rust, these operations can only be done through a `&mut AtomicFoo` reference or one derived from it, therefore these operations //! can only happen after all previous accesses on the same locations. This implementation is adapted to allow these operations. //! A mixed atomicity read that races with writes, or a write that races with reads or writes will still cause UBs to be thrown. @@ -61,7 +61,7 @@ // // 2. In the operational semantics, each store element keeps the timestamp of a thread when it loads from the store. // If the same thread loads from the same store element multiple times, then the timestamps at all loads are saved in a list of load elements. -// This is not necessary as later loads by the same thread will always have greater timetstamp values, so we only need to record the timestamp of the first +// This is not necessary as later loads by the same thread will always have greater timestamp values, so we only need to record the timestamp of the first // load by each thread. This optimisation is done in tsan11 // (https://github.com/ChrisLidbury/tsan11/blob/ecbd6b81e9b9454e01cba78eb9d88684168132c7/lib/tsan/rtl/tsan_relaxed.h#L35-L37) // and here. @@ -193,7 +193,7 @@ impl StoreBufferAlloc { buffers.remove_pos_range(pos_range); } AccessType::Empty(_) => { - // The range had no weak behaivours attached, do nothing + // The range had no weak behaviours attached, do nothing } } } @@ -336,7 +336,7 @@ impl<'mir, 'tcx: 'mir> StoreBuffer { let mut found_sc = false; // FIXME: we want an inclusive take_while (stops after a false predicate, but // includes the element that gave the false), but such function doesn't yet - // exist in the standard libary https://github.com/rust-lang/rust/issues/62208 + // exist in the standard library https://github.com/rust-lang/rust/issues/62208 // so we have to hack around it with keep_searching let mut keep_searching = true; let candidates = self diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 7a726be00da..aeba0ea5a95 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -7,6 +7,7 @@ use rustc_span::{source_map::DUMMY_SP, SpanData, Symbol}; use rustc_target::abi::{Align, Size}; use crate::borrow_tracker::stacked_borrows::diagnostics::TagHistory; +use crate::borrow_tracker::tree_borrows::diagnostics as tree_diagnostics; use crate::*; /// Details of premature program termination. @@ -23,8 +24,9 @@ pub enum TerminationInfo { history: Option<TagHistory>, }, TreeBorrowsUb { - msg: String, - // FIXME: incomplete + title: String, + details: Vec<String>, + history: tree_diagnostics::HistoryData, }, Int2PtrWithStrictProvenance, Deadlock, @@ -65,7 +67,7 @@ impl fmt::Display for TerminationInfo { "integer-to-pointer casts and `ptr::from_exposed_addr` are not supported with `-Zmiri-strict-provenance`" ), StackedBorrowsUb { msg, .. } => write!(f, "{msg}"), - TreeBorrowsUb { msg } => write!(f, "{msg}"), + TreeBorrowsUb { title, .. } => write!(f, "{title}"), Deadlock => write!(f, "the evaluated program deadlocked"), MultipleSymbolDefinitions { link_name, .. } => write!(f, "multiple definitions of symbol `{link_name}`"), @@ -219,10 +221,16 @@ pub fn report_error<'tcx, 'mir>( } helps }, - TreeBorrowsUb { .. } => { - let helps = vec![ - (None, format!("this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental")), + TreeBorrowsUb { title: _, details, history } => { + let mut helps = vec![ + (None, format!("this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental")) ]; + for m in details { + helps.push((None, m.clone())); + } + for event in history.events.clone() { + helps.push(event); + } helps } MultipleSymbolDefinitions { first, first_crate, second, second_crate, .. } => diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index defd37c3775..43d8f221ce3 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -376,7 +376,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( // Inlining of `DEFAULT` from // https://github.com/rust-lang/rust/blob/master/compiler/rustc_session/src/config/sigpipe.rs. - // Alaways using DEFAULT is okay since we don't support signals in Miri anyway. + // Always using DEFAULT is okay since we don't support signals in Miri anyway. let sigpipe = 2; ecx.call_function( @@ -460,7 +460,7 @@ pub fn eval_entry<'tcx>( return None; } // Check for memory leaks. - info!("Additonal static roots: {:?}", ecx.machine.static_roots); + info!("Additional static roots: {:?}", ecx.machine.static_roots); let leaks = ecx.find_leaked_allocations(&ecx.machine.static_roots); if !leaks.is_empty() { report_leaks(&ecx, leaks); diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 8f6ae729491..a2b49e6f219 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -524,7 +524,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } - // Make sure we visit aggregrates in increasing offset order. + // Make sure we visit aggregates in increasing offset order. fn visit_aggregate( &mut self, place: &MPlaceTy<'tcx, Provenance>, diff --git a/src/tools/miri/src/intptrcast.rs b/src/tools/miri/src/intptrcast.rs index 2ba18293121..4fd0af35304 100644 --- a/src/tools/miri/src/intptrcast.rs +++ b/src/tools/miri/src/intptrcast.rs @@ -77,7 +77,7 @@ impl<'mir, 'tcx> GlobalStateInner { Ok(pos) => Some(global_state.int_to_ptr_map[pos].1), Err(0) => None, Err(pos) => { - // This is the largest of the adresses smaller than `int`, + // This is the largest of the addresses smaller than `int`, // i.e. the greatest lower bound (glb) let (glb, alloc_id) = global_state.int_to_ptr_map[pos - 1]; // This never overflows because `addr >= glb` diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index ecb3e13dd54..21c5a9c1b70 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -32,8 +32,14 @@ use crate::{ *, }; -/// The number of the available real-time signal with the lowest priority. -/// Dummy constant related to epoll, must be between 32 and 64. +/// First real-time signal. +/// `signal(7)` says this must be between 32 and 64 and specifies 34 or 35 +/// as typical values. +pub const SIGRTMIN: i32 = 34; + +/// Last real-time signal. +/// `signal(7)` says it must be between 32 and 64 and specifies +/// `SIGRTMAX` - `SIGRTMIN` >= 8 (which is the value of `_POSIX_RTSIG_MAX`) pub const SIGRTMAX: i32 = 42; /// Extra data stored with each stack frame @@ -495,9 +501,9 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { measureme::Profiler::new(out).expect("Couldn't create `measureme` profiler") }); let rng = StdRng::seed_from_u64(config.seed.unwrap_or(0)); - let borrow_tracker = config.borrow_tracker.map(|bt| bt.instanciate_global_state(config)); + let borrow_tracker = config.borrow_tracker.map(|bt| bt.instantiate_global_state(config)); let data_race = config.data_race_detector.then(|| data_race::GlobalState::new(config)); - // Determinine page size, stack address, and stack size. + // Determine page size, stack address, and stack size. // These values are mostly meaningless, but the stack address is also where we start // allocating physical integer addresses for all allocations. let page_size = if let Some(page_size) = config.page_size { diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index fcee381ff71..44bca3796f9 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -46,7 +46,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // This list should be kept in sync with the one from libstd. let min_align = match this.tcx.sess.target.arch.as_ref() { "x86" | "arm" | "mips" | "powerpc" | "powerpc64" | "asmjs" | "wasm32" => 8, - "x86_64" | "aarch64" | "mips64" | "s390x" | "sparc64" => 16, + "x86_64" | "aarch64" | "mips64" | "s390x" | "sparc64" | "loongarch64" => 16, arch => bug!("unsupported target architecture for malloc: `{}`", arch), }; // Windows always aligns, even small allocations. @@ -744,6 +744,44 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { dest, )?; } + "memcpy" => { + let [ptr_dest, ptr_src, n] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let ptr_dest = this.read_pointer(ptr_dest)?; + let ptr_src = this.read_pointer(ptr_src)?; + let n = this.read_target_usize(n)?; + this.mem_copy( + ptr_src, + Align::ONE, + ptr_dest, + Align::ONE, + Size::from_bytes(n), + true, + )?; + this.write_pointer(ptr_dest, dest)?; + } + "strcpy" => { + let [ptr_dest, ptr_src] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let ptr_dest = this.read_pointer(ptr_dest)?; + let ptr_src = this.read_pointer(ptr_src)?; + + // We use `read_c_str` to determine the amount of data to copy, + // and then use `mem_copy` for the actual copy. This means + // pointer provenance is preserved by this implementation of `strcpy`. + // That is probably overly cautious, but there also is no fundamental + // reason to have `strcpy` destroy pointer provenance. + let n = this.read_c_str(ptr_src)?.len().checked_add(1).unwrap(); + this.mem_copy( + ptr_src, + Align::ONE, + ptr_dest, + Align::ONE, + Size::from_bytes(n), + true, + )?; + this.write_pointer(ptr_dest, dest)?; + } // math functions (note that there are also intrinsics for some other functions) #[rustfmt::skip] diff --git a/src/tools/miri/src/shims/intrinsics/simd.rs b/src/tools/miri/src/shims/intrinsics/simd.rs index f2e16521290..d101f8d3111 100644 --- a/src/tools/miri/src/shims/intrinsics/simd.rs +++ b/src/tools/miri/src/shims/intrinsics/simd.rs @@ -585,9 +585,9 @@ fn simd_element_to_bool(elem: ImmTy<'_, Provenance>) -> InterpResult<'_, bool> { }) } -fn simd_bitmask_index(idx: u32, vec_len: u32, endianess: Endian) -> u32 { +fn simd_bitmask_index(idx: u32, vec_len: u32, endianness: Endian) -> u32 { assert!(idx < vec_len); - match endianess { + match endianness { Endian::Little => idx, #[allow(clippy::integer_arithmetic)] // idx < vec_len Endian::Big => vec_len - 1 - idx, // reverse order of bits diff --git a/src/tools/miri/src/shims/os_str.rs b/src/tools/miri/src/shims/os_str.rs index f010d4251f4..6bc5b8f39d5 100644 --- a/src/tools/miri/src/shims/os_str.rs +++ b/src/tools/miri/src/shims/os_str.rs @@ -329,7 +329,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { match direction { PathConversion::HostToTarget => { // If this start withs a `\`, we add `\\?` so it starts with `\\?\` which is - // some magic path on Windos that *is* considered absolute. + // some magic path on Windows that *is* considered absolute. if converted.get(0).copied() == Some(b'\\') { converted.splice(0..0, b"\\\\?".iter().copied()); } diff --git a/src/tools/miri/src/shims/time.rs b/src/tools/miri/src/shims/time.rs index ef411eb8aa7..2f24c00ce14 100644 --- a/src/tools/miri/src/shims/time.rs +++ b/src/tools/miri/src/shims/time.rs @@ -40,7 +40,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.eval_libc_i32("CLOCK_REALTIME_COARSE"), ]; // The second kind is MONOTONIC clocks for which 0 is an arbitrary time point, but they are - // never allowed to go backwards. We don't need to do any additonal monotonicity + // never allowed to go backwards. We don't need to do any additional monotonicity // enforcement because std::time::Instant already guarantees that it is monotonic. relative_clocks = vec![ this.eval_libc_i32("CLOCK_MONOTONIC"), diff --git a/src/tools/miri/src/shims/tls.rs b/src/tools/miri/src/shims/tls.rs index e9119f9e1ec..685feeaf892 100644 --- a/src/tools/miri/src/shims/tls.rs +++ b/src/tools/miri/src/shims/tls.rs @@ -79,7 +79,7 @@ impl<'tcx> TlsData<'tcx> { trace!("TLS key {} removed", key); Ok(()) } - None => throw_ub_format!("removing a non-existig TLS key: {}", key), + None => throw_ub_format!("removing a nonexistent TLS key: {}", key), } } @@ -175,7 +175,7 @@ impl<'tcx> TlsData<'tcx> { Some(key) => Excluded(key), None => Unbounded, }; - // We interpret the documentaion above (taken from POSIX) as saying that we need to iterate + // We interpret the documentation above (taken from POSIX) as saying that we need to iterate // over all keys and run each destructor at least once before running any destructor a 2nd // time. That's why we have `key` to indicate how far we got in the current iteration. If we // return `None`, `schedule_next_pthread_tls_dtor` will re-try with `ket` set to `None` to diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index de271548217..abec782b4f0 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -11,6 +11,7 @@ use std::time::SystemTime; use log::trace; use rustc_data_structures::fx::FxHashMap; +use rustc_middle::ty::TyCtxt; use rustc_target::abi::{Align, Size}; use crate::shims::os_str::bytes_to_os_str; @@ -31,6 +32,7 @@ pub trait FileDescriptor: std::fmt::Debug + helpers::AsAny { &mut self, _communicate_allowed: bool, _bytes: &mut [u8], + _tcx: TyCtxt<'tcx>, ) -> InterpResult<'tcx, io::Result<usize>> { throw_unsup_format!("cannot read from {}", self.name()); } @@ -39,6 +41,7 @@ pub trait FileDescriptor: std::fmt::Debug + helpers::AsAny { &self, _communicate_allowed: bool, _bytes: &[u8], + _tcx: TyCtxt<'tcx>, ) -> InterpResult<'tcx, io::Result<usize>> { throw_unsup_format!("cannot write to {}", self.name()); } @@ -79,6 +82,7 @@ impl FileDescriptor for FileHandle { &mut self, communicate_allowed: bool, bytes: &mut [u8], + _tcx: TyCtxt<'tcx>, ) -> InterpResult<'tcx, io::Result<usize>> { assert!(communicate_allowed, "isolation should have prevented even opening a file"); Ok(self.file.read(bytes)) @@ -88,6 +92,7 @@ impl FileDescriptor for FileHandle { &self, communicate_allowed: bool, bytes: &[u8], + _tcx: TyCtxt<'tcx>, ) -> InterpResult<'tcx, io::Result<usize>> { assert!(communicate_allowed, "isolation should have prevented even opening a file"); Ok((&mut &self.file).write(bytes)) @@ -153,6 +158,7 @@ impl FileDescriptor for io::Stdin { &mut self, communicate_allowed: bool, bytes: &mut [u8], + _tcx: TyCtxt<'tcx>, ) -> InterpResult<'tcx, io::Result<usize>> { if !communicate_allowed { // We want isolation mode to be deterministic, so we have to disallow all reads, even stdin. @@ -184,6 +190,7 @@ impl FileDescriptor for io::Stdout { &self, _communicate_allowed: bool, bytes: &[u8], + _tcx: TyCtxt<'tcx>, ) -> InterpResult<'tcx, io::Result<usize>> { // We allow writing to stderr even with isolation enabled. let result = Write::write(&mut { self }, bytes); @@ -220,6 +227,7 @@ impl FileDescriptor for io::Stderr { &self, _communicate_allowed: bool, bytes: &[u8], + _tcx: TyCtxt<'tcx>, ) -> InterpResult<'tcx, io::Result<usize>> { // We allow writing to stderr even with isolation enabled. // No need to flush, stderr is not buffered. @@ -252,6 +260,7 @@ impl FileDescriptor for NullOutput { &self, _communicate_allowed: bool, bytes: &[u8], + _tcx: TyCtxt<'tcx>, ) -> InterpResult<'tcx, io::Result<usize>> { // We just don't write anything, but report to the user that we did. Ok(Ok(bytes.len())) @@ -756,8 +765,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let mut bytes = vec![0; usize::try_from(count).unwrap()]; // `File::read` never returns a value larger than `count`, // so this cannot fail. - let result = - file_descriptor.read(communicate, &mut bytes)?.map(|c| i64::try_from(c).unwrap()); + let result = file_descriptor + .read(communicate, &mut bytes, *this.tcx)? + .map(|c| i64::try_from(c).unwrap()); match result { Ok(read_bytes) => { @@ -803,8 +813,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) { let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?; - let result = - file_descriptor.write(communicate, bytes)?.map(|c| i64::try_from(c).unwrap()); + let result = file_descriptor + .write(communicate, bytes, *this.tcx)? + .map(|c| i64::try_from(c).unwrap()); this.try_unwrap_io_result(result) } else { this.handle_not_found() @@ -1015,8 +1026,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let path = this.read_path_from_c_str(pathname_ptr)?.into_owned(); // See <https://github.com/rust-lang/rust/pull/79196> for a discussion of argument sizes. - let at_ampty_path = this.eval_libc_i32("AT_EMPTY_PATH"); - let empty_path_flag = flags & at_ampty_path == at_ampty_path; + let at_empty_path = this.eval_libc_i32("AT_EMPTY_PATH"); + let empty_path_flag = flags & at_empty_path == at_empty_path; // We only support: // * interpreting `path` as an absolute directory, // * interpreting `path` as a path relative to `dirfd` when the latter is `AT_FDCWD`, or @@ -1053,7 +1064,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { return Ok(-1); } - // the `_mask_op` paramter specifies the file information that the caller requested. + // the `_mask_op` parameter specifies the file information that the caller requested. // However `statx` is allowed to return information that was not requested or to not // return information that was requested. This `mask` represents the information we can // actually provide for any target. diff --git a/src/tools/miri/src/shims/unix/linux/fd.rs b/src/tools/miri/src/shims/unix/linux/fd.rs index 3c4a678e598..3c263e4df92 100644 --- a/src/tools/miri/src/shims/unix/linux/fd.rs +++ b/src/tools/miri/src/shims/unix/linux/fd.rs @@ -152,7 +152,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let _maxevents = this.read_scalar(maxevents)?.to_i32()?; let _timeout = this.read_scalar(timeout)?.to_i32()?; - let numevents = 0; if let Some(epfd) = this.machine.file_handler.handles.get_mut(&epfd) { let _epfd = epfd .as_any_mut() @@ -160,7 +159,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { .ok_or_else(|| err_unsup_format!("non-epoll FD passed to `epoll_wait`"))?; // FIXME return number of events ready when scheme for marking events ready exists - Ok(Scalar::from_i32(numevents)) + throw_unsup_format!("returning ready events from epoll_wait is not yet implemented"); } else { Ok(Scalar::from_i32(this.handle_not_found()?)) } diff --git a/src/tools/miri/src/shims/unix/linux/fd/event.rs b/src/tools/miri/src/shims/unix/linux/fd/event.rs index 1db020bb7b6..1f17ffb88c8 100644 --- a/src/tools/miri/src/shims/unix/linux/fd/event.rs +++ b/src/tools/miri/src/shims/unix/linux/fd/event.rs @@ -1,6 +1,8 @@ use crate::shims::unix::fs::FileDescriptor; use rustc_const_eval::interpret::InterpResult; +use rustc_middle::ty::TyCtxt; +use rustc_target::abi::Endian; use std::cell::Cell; use std::io; @@ -36,7 +38,7 @@ impl FileDescriptor for Event { } /// A write call adds the 8-byte integer value supplied in - /// its buffer to the counter. The maximum value that may be + /// its buffer (in native endianess) to the counter. The maximum value that may be /// stored in the counter is the largest unsigned 64-bit value /// minus 1 (i.e., 0xfffffffffffffffe). If the addition would /// cause the counter's value to exceed the maximum, then the @@ -47,17 +49,22 @@ impl FileDescriptor for Event { /// A write fails with the error EINVAL if the size of the /// supplied buffer is less than 8 bytes, or if an attempt is /// made to write the value 0xffffffffffffffff. - /// - /// FIXME: use endianness fn write<'tcx>( &self, _communicate_allowed: bool, bytes: &[u8], + tcx: TyCtxt<'tcx>, ) -> InterpResult<'tcx, io::Result<usize>> { let v1 = self.val.get(); + let bytes: [u8; 8] = bytes.try_into().unwrap(); // FIXME fail gracefully when this has the wrong size + // Convert from target endianess to host endianess. + let num = match tcx.sess.target.endian { + Endian::Little => u64::from_le_bytes(bytes), + Endian::Big => u64::from_be_bytes(bytes), + }; // FIXME handle blocking when addition results in exceeding the max u64 value // or fail with EAGAIN if the file descriptor is nonblocking. - let v2 = v1.checked_add(u64::from_be_bytes(bytes.try_into().unwrap())).unwrap(); + let v2 = v1.checked_add(num).unwrap(); self.val.set(v2); assert_eq!(8, bytes.len()); Ok(Ok(8)) diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index f4e7824d91d..4cb7ee8efca 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -2,6 +2,7 @@ use rustc_span::Symbol; use rustc_target::spec::abi::Abi; use crate::machine::SIGRTMAX; +use crate::machine::SIGRTMIN; use crate::*; use shims::foreign_items::EmulateByNameResult; use shims::unix::fs::EvalContextExt as _; @@ -74,6 +75,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let result = this.socketpair(domain, type_, protocol, sv)?; this.write_scalar(result, dest)?; } + "__libc_current_sigrtmin" => { + let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + + this.write_scalar(Scalar::from_i32(SIGRTMIN), dest)?; + } "__libc_current_sigrtmax" => { let [] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; @@ -163,7 +169,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.linux_statx(&args[1], &args[2], &args[3], &args[4], &args[5])?; this.write_scalar(Scalar::from_target_isize(result.into(), this), dest)?; } - // `futex` is used by some synchonization primitives. + // `futex` is used by some synchronization primitives. id if id == sys_futex => { futex(this, &args[1..], dest)?; } @@ -174,7 +180,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } - // Miscelanneous + // Miscellaneous "getrandom" => { let [ptr, len, flags] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index b3c474dd3c9..05feeac45b5 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -242,7 +242,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // // To distinguish these two cases in already constructed mutexes, we // use the same trick as glibc: for the case when - // `pthread_mutexattr_settype` is caled explicitly, we set the + // `pthread_mutexattr_settype` is called explicitly, we set the // `PTHREAD_MUTEX_NORMAL_FLAG` flag. let normal_kind = kind | PTHREAD_MUTEX_NORMAL_FLAG; // Check that after setting the flag, the kind is distinguishable diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index 665c7ed438f..f72ba5cca7a 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/src/tools/miri/src/shims/windows/foreign_items.rs @@ -96,7 +96,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { if byte_offset != 0 { throw_unsup_format!( - "`NtWriteFile` `ByteOffset` paremeter is non-null, which is unsupported" + "`NtWriteFile` `ByteOffset` parameter is non-null, which is unsupported" ); } diff --git a/src/tools/miri/tests/compiletest.rs b/src/tools/miri/tests/compiletest.rs index 21eaeabe6ad..52deba9108c 100644 --- a/src/tools/miri/tests/compiletest.rs +++ b/src/tools/miri/tests/compiletest.rs @@ -2,7 +2,7 @@ use colored::*; use regex::bytes::Regex; use std::path::{Path, PathBuf}; use std::{env, process::Command}; -use ui_test::{color_eyre::Result, Config, Mode, OutputConflictHandling}; +use ui_test::{color_eyre::Result, Config, Match, Mode, OutputConflictHandling}; fn miri_path() -> PathBuf { PathBuf::from(option_env!("MIRI").unwrap_or(env!("CARGO_BIN_EXE_miri"))) @@ -52,14 +52,13 @@ fn run_tests(mode: Mode, path: &str, target: &str, with_dependencies: bool) -> R mode, program: miri_path(), quiet: false, + edition: Some("2018".into()), ..Config::default() }; let in_rustc_test_suite = option_env!("RUSTC_STAGE").is_some(); // Add some flags we always want. - config.args.push("--edition".into()); - config.args.push("2018".into()); if in_rustc_test_suite { // Less aggressive warnings to make the rustc toolstate management less painful. // (We often get warnings when e.g. a feature gets stabilized or some lint gets added/improved.) @@ -100,12 +99,24 @@ fn run_tests(mode: Mode, path: &str, target: &str, with_dependencies: bool) -> R }; // Handle command-line arguments. + let mut after_dashdash = false; config.path_filter.extend(std::env::args().skip(1).filter(|arg| { + if after_dashdash { + // Just propagate everything. + return true; + } match &**arg { "--quiet" => { config.quiet = true; false } + "--" => { + after_dashdash = true; + false + } + s if s.starts_with('-') => { + panic!("unknown compiletest flag `{s}`"); + } _ => true, } })); @@ -129,8 +140,8 @@ fn run_tests(mode: Mode, path: &str, target: &str, with_dependencies: bool) -> R macro_rules! regexes { ($name:ident: $($regex:expr => $replacement:expr,)*) => {lazy_static::lazy_static! { - static ref $name: Vec<(Regex, &'static [u8])> = vec![ - $((Regex::new($regex).unwrap(), $replacement.as_bytes()),)* + static ref $name: Vec<(Match, &'static [u8])> = vec![ + $((Regex::new($regex).unwrap().into(), $replacement.as_bytes()),)* ]; }}; } diff --git a/src/tools/miri/tests/fail/intrinsics/exact_div1.rs b/src/tools/miri/tests/fail/intrinsics/exact_div1.rs index 3dda9d1090d..e11d8937fe4 100644 --- a/src/tools/miri/tests/fail/intrinsics/exact_div1.rs +++ b/src/tools/miri/tests/fail/intrinsics/exact_div1.rs @@ -1,5 +1,5 @@ #![feature(core_intrinsics)] fn main() { - // divison by 0 + // division by 0 unsafe { std::intrinsics::exact_div(2, 0) }; //~ ERROR: divisor of zero } diff --git a/src/tools/miri/tests/fail/intrinsics/exact_div2.rs b/src/tools/miri/tests/fail/intrinsics/exact_div2.rs index 00064fa0b9c..7914de403a7 100644 --- a/src/tools/miri/tests/fail/intrinsics/exact_div2.rs +++ b/src/tools/miri/tests/fail/intrinsics/exact_div2.rs @@ -1,5 +1,5 @@ #![feature(core_intrinsics)] fn main() { - // divison with a remainder + // division with a remainder unsafe { std::intrinsics::exact_div(2u16, 3) }; //~ ERROR: 2_u16 cannot be divided by 3_u16 without remainder } diff --git a/src/tools/miri/tests/fail/intrinsics/exact_div3.rs b/src/tools/miri/tests/fail/intrinsics/exact_div3.rs index a61abcd137e..50ee5386465 100644 --- a/src/tools/miri/tests/fail/intrinsics/exact_div3.rs +++ b/src/tools/miri/tests/fail/intrinsics/exact_div3.rs @@ -1,5 +1,5 @@ #![feature(core_intrinsics)] fn main() { - // signed divison with a remainder + // signed division with a remainder unsafe { std::intrinsics::exact_div(-19i8, 2) }; //~ ERROR: -19_i8 cannot be divided by 2_i8 without remainder } diff --git a/src/tools/miri/tests/fail/intrinsics/exact_div4.rs b/src/tools/miri/tests/fail/intrinsics/exact_div4.rs index b5b60190b4e..48c55208238 100644 --- a/src/tools/miri/tests/fail/intrinsics/exact_div4.rs +++ b/src/tools/miri/tests/fail/intrinsics/exact_div4.rs @@ -1,5 +1,5 @@ #![feature(core_intrinsics)] fn main() { - // divison of MIN by -1 + // division of MIN by -1 unsafe { std::intrinsics::exact_div(i64::MIN, -1) }; //~ ERROR: overflow in signed remainder (dividing MIN by -1) } diff --git a/src/tools/miri/tests/fail/panic/double_panic.rs b/src/tools/miri/tests/fail/panic/double_panic.rs index 8919d51bb2f..c9501d90b3b 100644 --- a/src/tools/miri/tests/fail/panic/double_panic.rs +++ b/src/tools/miri/tests/fail/panic/double_panic.rs @@ -1,6 +1,8 @@ //@error-pattern: the program aborted //@normalize-stderr-test: "\| +\^+" -> "| ^" //@normalize-stderr-test: "unsafe \{ libc::abort\(\) \}|crate::intrinsics::abort\(\);" -> "ABORT();" +//@normalize-stderr-test: "\n +[0-9]+:[^\n]+" -> "$1" +//@normalize-stderr-test: "\n at [^\n]+" -> "$1" struct Foo; impl Drop for Foo { diff --git a/src/tools/miri/tests/fail/panic/double_panic.stderr b/src/tools/miri/tests/fail/panic/double_panic.stderr index f04dfab36ca..5384f6f6716 100644 --- a/src/tools/miri/tests/fail/panic/double_panic.stderr +++ b/src/tools/miri/tests/fail/panic/double_panic.stderr @@ -2,70 +2,6 @@ thread 'main' panicked at 'first', $DIR/double_panic.rs:LL:CC note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace thread 'main' panicked at 'second', $DIR/double_panic.rs:LL:CC stack backtrace: - 0: std::backtrace_rs::backtrace::miri::trace_unsynchronized - at RUSTLIB/std/src/../../backtrace/src/backtrace/miri.rs:LL:CC - 1: std::backtrace_rs::backtrace::miri::trace - at RUSTLIB/std/src/../../backtrace/src/backtrace/miri.rs:LL:CC - 2: std::backtrace_rs::backtrace::trace_unsynchronized - at RUSTLIB/std/src/../../backtrace/src/backtrace/mod.rs:LL:CC - 3: std::sys_common::backtrace::_print_fmt - at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC - 4: <std::sys_common::backtrace::_print::DisplayBacktrace as std::fmt::Display>::fmt - at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC - 5: core::fmt::rt::Argument::fmt - at RUSTLIB/core/src/fmt/rt.rs:LL:CC - 6: std::fmt::write - at RUSTLIB/core/src/fmt/mod.rs:LL:CC - 7: <std::sys::PLATFORM::stdio::Stderr as std::io::Write>::write_fmt - at RUSTLIB/std/src/io/mod.rs:LL:CC - 8: std::sys_common::backtrace::_print - at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC - 9: std::sys_common::backtrace::print - at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC - 10: std::panicking::default_hook::{closure#1} - at RUSTLIB/std/src/panicking.rs:LL:CC - 11: std::panicking::default_hook - at RUSTLIB/std/src/panicking.rs:LL:CC - 12: std::panicking::rust_panic_with_hook - at RUSTLIB/std/src/panicking.rs:LL:CC - 13: std::rt::begin_panic::{closure#0} - at RUSTLIB/std/src/panicking.rs:LL:CC - 14: std::sys_common::backtrace::__rust_end_short_backtrace - at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC - 15: std::rt::begin_panic - at RUSTLIB/std/src/panicking.rs:LL:CC - 16: <Foo as std::ops::Drop>::drop - at $DIR/double_panic.rs:LL:CC - 17: std::ptr::drop_in_place - shim(Some(Foo)) - at RUSTLIB/core/src/ptr/mod.rs:LL:CC - 18: main - at $DIR/double_panic.rs:LL:CC - 19: <fn() as std::ops::FnOnce<()>>::call_once - shim(fn()) - at RUSTLIB/core/src/ops/function.rs:LL:CC - 20: std::sys_common::backtrace::__rust_begin_short_backtrace - at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC - 21: std::rt::lang_start::{closure#0} - at RUSTLIB/std/src/rt.rs:LL:CC - 22: std::ops::function::impls::call_once - at RUSTLIB/core/src/ops/function.rs:LL:CC - 23: std::panicking::r#try::do_call - at RUSTLIB/std/src/panicking.rs:LL:CC - 24: std::panicking::r#try - at RUSTLIB/std/src/panicking.rs:LL:CC - 25: std::panic::catch_unwind - at RUSTLIB/std/src/panic.rs:LL:CC - 26: std::rt::lang_start_internal::{closure#2} - at RUSTLIB/std/src/rt.rs:LL:CC - 27: std::panicking::r#try::do_call - at RUSTLIB/std/src/panicking.rs:LL:CC - 28: std::panicking::r#try - at RUSTLIB/std/src/panicking.rs:LL:CC - 29: std::panic::catch_unwind - at RUSTLIB/std/src/panic.rs:LL:CC - 30: std::rt::lang_start_internal - at RUSTLIB/std/src/rt.rs:LL:CC - 31: std::rt::lang_start - at RUSTLIB/std/src/rt.rs:LL:CC thread panicked while panicking. aborting. error: abnormal termination: the program aborted execution --> RUSTLIB/std/src/sys/PLATFORM/mod.rs:LL:CC diff --git a/src/tools/miri/tests/fail/stacked_borrows/illegal_read3.rs b/src/tools/miri/tests/fail/stacked_borrows/illegal_read3.rs index 43ea0a0e84d..9c0c974223b 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/illegal_read3.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/illegal_read3.rs @@ -1,5 +1,5 @@ // A callee may not read the destination of our `&mut` without us noticing. -// Thise code got carefully checked to not introduce any reborrows +// This code got carefully checked to not introduce any reborrows // that are not explicit in the source. Let's hope the compiler does not break this later! use std::mem; diff --git a/src/tools/miri/tests/fail/terminate-terminator.rs b/src/tools/miri/tests/fail/terminate-terminator.rs index f4931659fc8..22ffa1b2711 100644 --- a/src/tools/miri/tests/fail/terminate-terminator.rs +++ b/src/tools/miri/tests/fail/terminate-terminator.rs @@ -12,14 +12,13 @@ impl Drop for Foo { #[inline(always)] fn has_cleanup() { - //~^ ERROR: panic in a function that cannot unwind - // FIXME(nbdd0121): The error should be reported at the call site. let _f = Foo; panic!(); } extern "C" fn panic_abort() { has_cleanup(); + //~^ ERROR: panic in a function that cannot unwind } fn main() { diff --git a/src/tools/miri/tests/fail/terminate-terminator.stderr b/src/tools/miri/tests/fail/terminate-terminator.stderr index c046678f73f..8ce4bb7cbb5 100644 --- a/src/tools/miri/tests/fail/terminate-terminator.stderr +++ b/src/tools/miri/tests/fail/terminate-terminator.stderr @@ -6,8 +6,6 @@ error: abnormal termination: panic in a function that cannot unwind --> $DIR/terminate-terminator.rs:LL:CC | LL | / fn has_cleanup() { -LL | | -LL | | // FIXME(nbdd0121): The error should be reported at the call site. LL | | let _f = Foo; LL | | panic!(); LL | | } diff --git a/src/tools/miri/tests/pass-dep/tokio/sleep.rs b/src/tools/miri/tests/fail/tokio/sleep.rs index 00cc68eba3e..6fdfbc9913a 100644 --- a/src/tools/miri/tests/pass-dep/tokio/sleep.rs +++ b/src/tools/miri/tests/fail/tokio/sleep.rs @@ -1,5 +1,7 @@ //@compile-flags: -Zmiri-permissive-provenance -Zmiri-backtrace=full //@only-target-x86_64-unknown-linux: support for tokio only on linux and x86 +//@error-pattern: returning ready events from epoll_wait is not yet implemented +//@normalize-stderr-test: " += note:.*\n" -> "" use tokio::time::{sleep, Duration, Instant}; diff --git a/src/tools/miri/tests/fail/tokio/sleep.stderr b/src/tools/miri/tests/fail/tokio/sleep.stderr new file mode 100644 index 00000000000..ac2a984ed51 --- /dev/null +++ b/src/tools/miri/tests/fail/tokio/sleep.stderr @@ -0,0 +1,15 @@ +error: unsupported operation: returning ready events from epoll_wait is not yet implemented + --> CARGO_REGISTRY/.../epoll.rs:LL:CC + | +LL | / syscall!(epoll_wait( +LL | | self.ep, +LL | | events.as_mut_ptr(), +LL | | events.capacity() as i32, +LL | | timeout, +LL | | )) + | |__________^ returning ready events from epoll_wait is not yet implemented + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/tree-borrows/alternate-read-write.stderr b/src/tools/miri/tests/fail/tree-borrows/alternate-read-write.stderr index 7c5bcd4e7b0..bb601fc8835 100644 --- a/src/tools/miri/tests/fail/tree-borrows/alternate-read-write.stderr +++ b/src/tools/miri/tests/fail/tree-borrows/alternate-read-write.stderr @@ -1,11 +1,35 @@ -error: Undefined Behavior: write access through <TAG> is forbidden because it is a child of <TAG> which is Frozen. +error: Undefined Behavior: write access through <TAG> is forbidden --> $DIR/alternate-read-write.rs:LL:CC | LL | *y += 1; // Failure - | ^^^^^^^ write access through <TAG> is forbidden because it is a child of <TAG> which is Frozen. + | ^^^^^^^ write access through <TAG> is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = note: BACKTRACE: + = help: the accessed tag <TAG> is a child of the conflicting tag <TAG> + = help: the conflicting tag <TAG> has state Frozen which forbids child write accesses +help: the accessed tag <TAG> was created here + --> $DIR/alternate-read-write.rs:LL:CC + | +LL | let y = unsafe { &mut *(x as *mut u8) }; + | ^^^^^^^^^^^^^^^^^^^^ +help: the conflicting tag <TAG> was created here, in the initial state Reserved + --> $DIR/alternate-read-write.rs:LL:CC + | +LL | let y = unsafe { &mut *(x as *mut u8) }; + | ^^^^^^^^^^^^^^^^^^^^ +help: the conflicting tag <TAG> then transitioned from Reserved to Active due to a child write access at offsets [0x0..0x1] + --> $DIR/alternate-read-write.rs:LL:CC + | +LL | *y += 1; // Success + | ^^^^^^^ + = help: this corresponds to an activation +help: the conflicting tag <TAG> then transitioned from Active to Frozen due to a foreign read access at offsets [0x0..0x1] + --> $DIR/alternate-read-write.rs:LL:CC + | +LL | let _val = *x; + | ^^ + = help: this corresponds to a loss of write permissions + = note: BACKTRACE (of the first span): = note: inside `main` at $DIR/alternate-read-write.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/tree-borrows/error-range.rs b/src/tools/miri/tests/fail/tree-borrows/error-range.rs new file mode 100644 index 00000000000..d9980e75d60 --- /dev/null +++ b/src/tools/miri/tests/fail/tree-borrows/error-range.rs @@ -0,0 +1,27 @@ +//@compile-flags: -Zmiri-tree-borrows + +use core::ptr::addr_of_mut; + +// Check that the diagnostics correctly report the exact range at fault +// and trim irrelevant events. +fn main() { + unsafe { + let data = &mut [0u8; 16]; + + // Create and activate a few bytes + let rmut = &mut *addr_of_mut!(data[0..6]); + rmut[3] += 1; + rmut[4] += 1; + rmut[5] += 1; + + // Now make them lose some perms + let _v = data[3]; + let _v = data[4]; + let _v = data[5]; + data[4] = 1; + data[5] = 1; + + // Final test + rmut[5] += 1; //~ ERROR: /read access through .* is forbidden/ + } +} diff --git a/src/tools/miri/tests/fail/tree-borrows/error-range.stderr b/src/tools/miri/tests/fail/tree-borrows/error-range.stderr new file mode 100644 index 00000000000..bc829fd86d3 --- /dev/null +++ b/src/tools/miri/tests/fail/tree-borrows/error-range.stderr @@ -0,0 +1,44 @@ +error: Undefined Behavior: read access through <TAG> is forbidden + --> $DIR/error-range.rs:LL:CC + | +LL | rmut[5] += 1; + | ^^^^^^^^^^^^ read access through <TAG> is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag <TAG> is a child of the conflicting tag <TAG> + = help: the conflicting tag <TAG> has state Disabled which forbids child read accesses +help: the accessed tag <TAG> was created here + --> $DIR/error-range.rs:LL:CC + | +LL | let rmut = &mut *addr_of_mut!(data[0..6]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: the conflicting tag <TAG> was created here, in the initial state Reserved + --> $DIR/error-range.rs:LL:CC + | +LL | let rmut = &mut *addr_of_mut!(data[0..6]); + | ^^^^ +help: the conflicting tag <TAG> then transitioned from Reserved to Active due to a child write access at offsets [0x5..0x6] + --> $DIR/error-range.rs:LL:CC + | +LL | rmut[5] += 1; + | ^^^^^^^^^^^^ + = help: this corresponds to an activation +help: the conflicting tag <TAG> then transitioned from Active to Frozen due to a foreign read access at offsets [0x5..0x6] + --> $DIR/error-range.rs:LL:CC + | +LL | let _v = data[5]; + | ^^^^^^^ + = help: this corresponds to a loss of write permissions +help: the conflicting tag <TAG> then transitioned from Frozen to Disabled due to a foreign write access at offsets [0x5..0x6] + --> $DIR/error-range.rs:LL:CC + | +LL | data[5] = 1; + | ^^^^^^^^^^^ + = help: this corresponds to a loss of read permissions + = note: BACKTRACE (of the first span): + = note: inside `main` at $DIR/error-range.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/tree-borrows/fragile-data-race.stderr b/src/tools/miri/tests/fail/tree-borrows/fragile-data-race.stderr index a078d18d3b0..79744964a8b 100644 --- a/src/tools/miri/tests/fail/tree-borrows/fragile-data-race.stderr +++ b/src/tools/miri/tests/fail/tree-borrows/fragile-data-race.stderr @@ -1,11 +1,29 @@ -error: Undefined Behavior: write access through <TAG> is forbidden because it is a child of <TAG> which is Frozen. +error: Undefined Behavior: write access through <TAG> is forbidden --> $DIR/fragile-data-race.rs:LL:CC | LL | unsafe { *p = 1 }; - | ^^^^^^ write access through <TAG> is forbidden because it is a child of <TAG> which is Frozen. + | ^^^^^^ write access through <TAG> is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = note: BACKTRACE: + = help: the accessed tag <TAG> is a child of the conflicting tag <TAG> + = help: the conflicting tag <TAG> has state Frozen which forbids child write accesses +help: the accessed tag <TAG> was created here + --> $DIR/fragile-data-race.rs:LL:CC + | +LL | fn thread_1(x: &mut u8) -> SendPtr { + | ^ +help: the conflicting tag <TAG> was created here, in the initial state Reserved + --> RUSTLIB/std/src/panic.rs:LL:CC + | +LL | pub fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> { + | ^ +help: the conflicting tag <TAG> then transitioned from Reserved to Frozen due to a foreign read access at offsets [0x0..0x1] + --> RUSTLIB/core/src/ptr/mod.rs:LL:CC + | +LL | crate::intrinsics::read_via_copy(src) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: this corresponds to a loss of write permissions + = note: BACKTRACE (of the first span): = note: inside `main` at $DIR/fragile-data-race.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/tree-borrows/outside-range.stderr b/src/tools/miri/tests/fail/tree-borrows/outside-range.stderr index 4396c63679e..14696c704fc 100644 --- a/src/tools/miri/tests/fail/tree-borrows/outside-range.stderr +++ b/src/tools/miri/tests/fail/tree-borrows/outside-range.stderr @@ -1,11 +1,24 @@ -error: Undefined Behavior: write access through <TAG> is forbidden because it is a foreign tag for <TAG>, which would hence change from Reserved to Disabled, but <TAG> is protected +error: Undefined Behavior: write access through <TAG> is forbidden --> $DIR/outside-range.rs:LL:CC | LL | *y.add(3) = 42; - | ^^^^^^^^^^^^^^ write access through <TAG> is forbidden because it is a foreign tag for <TAG>, which would hence change from Reserved to Disabled, but <TAG> is protected + | ^^^^^^^^^^^^^^ write access through <TAG> is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = note: BACKTRACE: + = help: the accessed tag <TAG> is foreign to the protected tag <TAG> (i.e., it is not a child) + = help: the access would cause the protected tag <TAG> to transition from Reserved to Disabled + = help: this is a loss of read and write permissions, which is not allowed for protected tags +help: the accessed tag <TAG> was created here + --> $DIR/outside-range.rs:LL:CC + | +LL | let raw = data.as_mut_ptr(); + | ^^^^^^^^^^^^^^^^^ +help: the protected tag <TAG> was created here, in the initial state Reserved + --> $DIR/outside-range.rs:LL:CC + | +LL | unsafe fn stuff(x: &mut u8, y: *mut u8) { + | ^ + = note: BACKTRACE (of the first span): = note: inside `stuff` at $DIR/outside-range.rs:LL:CC note: inside `main` --> $DIR/outside-range.rs:LL:CC diff --git a/src/tools/miri/tests/fail/tree-borrows/read-to-local.stderr b/src/tools/miri/tests/fail/tree-borrows/read-to-local.stderr index 7d9367c87d0..8c9c2f8f965 100644 --- a/src/tools/miri/tests/fail/tree-borrows/read-to-local.stderr +++ b/src/tools/miri/tests/fail/tree-borrows/read-to-local.stderr @@ -1,12 +1,31 @@ -error: Undefined Behavior: write access through <TAG> is forbidden because it is a child of <TAG> which is Frozen. +error: Undefined Behavior: write access through <TAG> is forbidden --> $DIR/read-to-local.rs:LL:CC | LL | *ptr = 0; - | ^^^^^^^^ write access through <TAG> is forbidden because it is a child of <TAG> which is Frozen. + | ^^^^^^^^ write access through <TAG> is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = note: BACKTRACE: + = help: the accessed tag <TAG> has state Frozen which forbids child write accesses +help: the accessed tag <TAG> was created here, in the initial state Reserved + --> $DIR/read-to-local.rs:LL:CC + | +LL | let mref = &mut root; + | ^^^^^^^^^ +help: the accessed tag <TAG> then transitioned from Reserved to Active due to a child write access at offsets [0x0..0x1] + --> $DIR/read-to-local.rs:LL:CC + | +LL | *ptr = 0; // Write + | ^^^^^^^^ + = help: this corresponds to an activation +help: the accessed tag <TAG> then transitioned from Active to Frozen due to a foreign read access at offsets [0x0..0x1] + --> $DIR/read-to-local.rs:LL:CC + | +LL | assert_eq!(root, 0); // Parent Read + | ^^^^^^^^^^^^^^^^^^^ + = help: this corresponds to a loss of write permissions + = note: BACKTRACE (of the first span): = note: inside `main` at $DIR/read-to-local.rs:LL:CC + = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info) note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/tree-borrows/reserved/cell-protected-write.stderr b/src/tools/miri/tests/fail/tree-borrows/reserved/cell-protected-write.stderr index 8ae1c09470a..b85793ff063 100644 --- a/src/tools/miri/tests/fail/tree-borrows/reserved/cell-protected-write.stderr +++ b/src/tools/miri/tests/fail/tree-borrows/reserved/cell-protected-write.stderr @@ -1,20 +1,34 @@ ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Re*| └─┬──<TAG=base> -| Re*| ├─┬──<TAG=x> -| Re*| │ └─┬──<TAG=caller:x> -| Re*| │ └────<TAG=callee:x> Strongly protected -| Re*| └────<TAG=y,callee:y,caller:y> +| Act| └─┬──<TAG=root of the allocation> +| Re*| └─┬──<TAG=base> +| Re*| ├─┬──<TAG=x> +| Re*| │ └─┬──<TAG=caller:x> +| Re*| │ └────<TAG=callee:x> Strongly protected +| Re*| └────<TAG=y, callee:y, caller:y> ────────────────────────────────────────────────────────────────────── -error: Undefined Behavior: write access through <TAG> (also named 'y,callee:y,caller:y') is forbidden because it is a foreign tag for <TAG> (also named 'callee:x'), which would hence change from Reserved to Disabled, but <TAG> (also named 'callee:x') is protected +error: Undefined Behavior: write access through <TAG> (y, callee:y, caller:y) is forbidden --> $DIR/cell-protected-write.rs:LL:CC | LL | *y = 1; - | ^^^^^^ write access through <TAG> (also named 'y,callee:y,caller:y') is forbidden because it is a foreign tag for <TAG> (also named 'callee:x'), which would hence change from Reserved to Disabled, but <TAG> (also named 'callee:x') is protected + | ^^^^^^ write access through <TAG> (y, callee:y, caller:y) is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = note: BACKTRACE: + = help: the accessed tag <TAG> (y, callee:y, caller:y) is foreign to the protected tag <TAG> (callee:x) (i.e., it is not a child) + = help: the access would cause the protected tag <TAG> (callee:x) to transition from Reserved to Disabled + = help: this is a loss of read and write permissions, which is not allowed for protected tags +help: the accessed tag <TAG> was created here + --> $DIR/cell-protected-write.rs:LL:CC + | +LL | let y = (&mut *n).get(); + | ^^^^^^^^^ +help: the protected tag <TAG> was created here, in the initial state Reserved + --> $DIR/cell-protected-write.rs:LL:CC + | +LL | unsafe fn write_second(x: &mut UnsafeCell<u8>, y: *mut u8) { + | ^ + = note: BACKTRACE (of the first span): = note: inside `main::write_second` at $DIR/cell-protected-write.rs:LL:CC note: inside `main` --> $DIR/cell-protected-write.rs:LL:CC diff --git a/src/tools/miri/tests/fail/tree-borrows/reserved/int-protected-write.stderr b/src/tools/miri/tests/fail/tree-borrows/reserved/int-protected-write.stderr index a199fc0f0da..5de7dc0c7c5 100644 --- a/src/tools/miri/tests/fail/tree-borrows/reserved/int-protected-write.stderr +++ b/src/tools/miri/tests/fail/tree-borrows/reserved/int-protected-write.stderr @@ -1,20 +1,34 @@ ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Res| └─┬──<TAG=n> -| Res| ├─┬──<TAG=x> -| Res| │ └─┬──<TAG=caller:x> -| Res| │ └────<TAG=callee:x> Strongly protected -| Res| └────<TAG=y,callee:y,caller:y> +| Act| └─┬──<TAG=root of the allocation> +| Res| └─┬──<TAG=n> +| Res| ├─┬──<TAG=x> +| Res| │ └─┬──<TAG=caller:x> +| Res| │ └────<TAG=callee:x> Strongly protected +| Res| └────<TAG=y, callee:y, caller:y> ────────────────────────────────────────────────────────────────────── -error: Undefined Behavior: write access through <TAG> (also named 'y,callee:y,caller:y') is forbidden because it is a foreign tag for <TAG> (also named 'callee:x'), which would hence change from Reserved to Disabled, but <TAG> (also named 'callee:x') is protected +error: Undefined Behavior: write access through <TAG> (y, callee:y, caller:y) is forbidden --> $DIR/int-protected-write.rs:LL:CC | LL | *y = 0; - | ^^^^^^ write access through <TAG> (also named 'y,callee:y,caller:y') is forbidden because it is a foreign tag for <TAG> (also named 'callee:x'), which would hence change from Reserved to Disabled, but <TAG> (also named 'callee:x') is protected + | ^^^^^^ write access through <TAG> (y, callee:y, caller:y) is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = note: BACKTRACE: + = help: the accessed tag <TAG> (y, callee:y, caller:y) is foreign to the protected tag <TAG> (callee:x) (i.e., it is not a child) + = help: the access would cause the protected tag <TAG> (callee:x) to transition from Reserved to Disabled + = help: this is a loss of read and write permissions, which is not allowed for protected tags +help: the accessed tag <TAG> was created here + --> $DIR/int-protected-write.rs:LL:CC + | +LL | let y = (&mut *n) as *mut _; + | ^^^^^^^^^ +help: the protected tag <TAG> was created here, in the initial state Reserved + --> $DIR/int-protected-write.rs:LL:CC + | +LL | unsafe fn write_second(x: &mut u8, y: *mut u8) { + | ^ + = note: BACKTRACE (of the first span): = note: inside `main::write_second` at $DIR/int-protected-write.rs:LL:CC note: inside `main` --> $DIR/int-protected-write.rs:LL:CC diff --git a/src/tools/miri/tests/fail/tree-borrows/strongly-protected.rs b/src/tools/miri/tests/fail/tree-borrows/strongly-protected.rs new file mode 100644 index 00000000000..a68efea890c --- /dev/null +++ b/src/tools/miri/tests/fail/tree-borrows/strongly-protected.rs @@ -0,0 +1,14 @@ +//@compile-flags: -Zmiri-tree-borrows +//@error-pattern: /deallocation through .* is forbidden/ + +fn inner(x: &mut i32, f: fn(&mut i32)) { + // `f` may mutate, but it may not deallocate! + f(x) +} + +fn main() { + inner(Box::leak(Box::new(0)), |x| { + let raw = x as *mut _; + drop(unsafe { Box::from_raw(raw) }); + }); +} diff --git a/src/tools/miri/tests/fail/tree-borrows/strongly-protected.stderr b/src/tools/miri/tests/fail/tree-borrows/strongly-protected.stderr new file mode 100644 index 00000000000..97088d5854c --- /dev/null +++ b/src/tools/miri/tests/fail/tree-borrows/strongly-protected.stderr @@ -0,0 +1,49 @@ +error: Undefined Behavior: deallocation through <TAG> is forbidden + --> RUSTLIB/alloc/src/alloc.rs:LL:CC + | +LL | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ deallocation through <TAG> is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the allocation of the accessed tag <TAG> also contains the strongly protected tag <TAG> + = help: the strongly protected tag <TAG> disallows deallocations +help: the accessed tag <TAG> was created here + --> $DIR/strongly-protected.rs:LL:CC + | +LL | drop(unsafe { Box::from_raw(raw) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: the strongly protected tag <TAG> was created here, in the initial state Reserved + --> $DIR/strongly-protected.rs:LL:CC + | +LL | fn inner(x: &mut i32, f: fn(&mut i32)) { + | ^ + = note: BACKTRACE (of the first span): + = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC + = note: inside `<std::alloc::Global as std::alloc::Allocator>::deallocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC + = note: inside `alloc::alloc::box_free::<i32, std::alloc::Global>` at RUSTLIB/alloc/src/alloc.rs:LL:CC + = 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 + = note: inside `std::mem::drop::<std::boxed::Box<i32>>` at RUSTLIB/core/src/mem/mod.rs:LL:CC +note: inside closure + --> $DIR/strongly-protected.rs:LL:CC + | +LL | drop(unsafe { Box::from_raw(raw) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: inside `<[closure@$DIR/strongly-protected.rs:LL:CC] as std::ops::FnOnce<(&mut i32,)>>::call_once - shim` at RUSTLIB/core/src/ops/function.rs:LL:CC +note: inside `inner` + --> $DIR/strongly-protected.rs:LL:CC + | +LL | f(x) + | ^^^^ +note: inside `main` + --> $DIR/strongly-protected.rs:LL:CC + | +LL | / inner(Box::leak(Box::new(0)), |x| { +LL | | let raw = x as *mut _; +LL | | drop(unsafe { Box::from_raw(raw) }); +LL | | }); + | |______^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/tree-borrows/write-during-2phase.stderr b/src/tools/miri/tests/fail/tree-borrows/write-during-2phase.stderr index e511eb9cf8f..f6285bdcf16 100644 --- a/src/tools/miri/tests/fail/tree-borrows/write-during-2phase.stderr +++ b/src/tools/miri/tests/fail/tree-borrows/write-during-2phase.stderr @@ -1,11 +1,30 @@ -error: Undefined Behavior: read access through <TAG> is forbidden because it is a child of <TAG> which is Disabled. +error: Undefined Behavior: read access through <TAG> is forbidden --> $DIR/write-during-2phase.rs:LL:CC | LL | fn add(&mut self, n: u64) -> u64 { - | ^^^^^^^^^ read access through <TAG> is forbidden because it is a child of <TAG> which is Disabled. + | ^^^^^^^^^ read access through <TAG> is forbidden | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = note: BACKTRACE: + = help: the accessed tag <TAG> has state Disabled which forbids child read accesses +help: the accessed tag <TAG> was created here, in the initial state Reserved + --> $DIR/write-during-2phase.rs:LL:CC + | +LL | let _res = f.add(unsafe { + | ________________^ +LL | | let n = f.0; +LL | | // This is the access at fault, but it's not immediately apparent because +LL | | // the reference that got invalidated is not under a Protector. +LL | | *inner = 42; +LL | | n +LL | | }); + | |______^ +help: the accessed tag <TAG> then transitioned from Reserved to Disabled due to a foreign write access at offsets [0x0..0x8] + --> $DIR/write-during-2phase.rs:LL:CC + | +LL | *inner = 42; + | ^^^^^^^^^^^ + = help: this corresponds to a loss of read and write permissions + = note: BACKTRACE (of the first span): = note: inside `Foo::add` at $DIR/write-during-2phase.rs:LL:CC note: inside `main` --> $DIR/write-during-2phase.rs:LL:CC diff --git a/src/tools/miri/tests/fail/unaligned_pointers/intptrcast_alignment_check.rs b/src/tools/miri/tests/fail/unaligned_pointers/intptrcast_alignment_check.rs index c1041ee32a4..ed43e552506 100644 --- a/src/tools/miri/tests/fail/unaligned_pointers/intptrcast_alignment_check.rs +++ b/src/tools/miri/tests/fail/unaligned_pointers/intptrcast_alignment_check.rs @@ -1,7 +1,7 @@ //@compile-flags: -Zmiri-symbolic-alignment-check -Zmiri-permissive-provenance -Cdebug-assertions=no // With the symbolic alignment check, even with intptrcast and without // validation, we want to be *sure* to catch bugs that arise from pointers being -// insufficiently aligned. The only way to achieve that is not not let programs +// insufficiently aligned. The only way to achieve that is not to let programs // exploit integer information for alignment, so here we test that this is // indeed the case. // diff --git a/src/tools/miri/tests/panic/oob_subslice.rs b/src/tools/miri/tests/panic/oob_subslice.rs new file mode 100644 index 00000000000..4e79b6a99e9 --- /dev/null +++ b/src/tools/miri/tests/panic/oob_subslice.rs @@ -0,0 +1,7 @@ +// This once failed with "unwinding past a stack frame that does not allow unwinding", +// fixed by https://github.com/rust-lang/rust/issues/110233. + +fn main() { + let x = [1, 2, 3, 4]; + let _val = &x[..=4]; +} diff --git a/src/tools/miri/tests/panic/oob_subslice.stderr b/src/tools/miri/tests/panic/oob_subslice.stderr new file mode 100644 index 00000000000..4c6deaeccf6 --- /dev/null +++ b/src/tools/miri/tests/panic/oob_subslice.stderr @@ -0,0 +1,2 @@ +thread 'main' panicked at 'range end index 5 out of range for slice of length 4', $DIR/oob_subslice.rs:LL:CC +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/src/tools/miri/tests/pass-dep/concurrency/tls_pthread_drop_order.rs b/src/tools/miri/tests/pass-dep/concurrency/tls_pthread_drop_order.rs index 6516396ac54..ae874740f2b 100644 --- a/src/tools/miri/tests/pass-dep/concurrency/tls_pthread_drop_order.rs +++ b/src/tools/miri/tests/pass-dep/concurrency/tls_pthread_drop_order.rs @@ -14,7 +14,7 @@ static mut RECORD: usize = 0; static mut KEYS: [Key; 2] = [0; 2]; static mut GLOBALS: [u64; 2] = [1, 0]; -static mut CANNARY: *mut u64 = ptr::null_mut(); // this serves as a cannary: if TLS dtors are not run properly, this will not get deallocated, making the test fail. +static mut CANARY: *mut u64 = ptr::null_mut(); // this serves as a canary: if TLS dtors are not run properly, this will not get deallocated, making the test fail. pub unsafe fn create(dtor: Option<unsafe extern "C" fn(*mut u8)>) -> Key { let mut key = 0; @@ -33,7 +33,7 @@ pub fn record(r: usize) { } unsafe extern "C" fn dtor(ptr: *mut u64) { - assert!(CANNARY != ptr::null_mut()); // make sure we do not get run too often + assert!(CANARY != ptr::null_mut()); // make sure we do not get run too often let val = *ptr; let which_key = @@ -45,15 +45,15 @@ unsafe extern "C" fn dtor(ptr: *mut u64) { set(KEYS[which_key], ptr as *mut _); } - // Check if the records matches what we expect. If yes, clear the cannary. - // If the record is wrong, the cannary will never get cleared, leading to a leak -> test fails. + // Check if the records matches what we expect. If yes, clear the canary. + // If the record is wrong, the canary will never get cleared, leading to a leak -> test fails. // If the record is incomplete (i.e., more dtor calls happen), the check at the beginning of this function will fail -> test fails. // The correct sequence is: First key 0, then key 1, then key 0. // Note that this relies on dtor order, which is not specified by POSIX, but seems to be // consistent between Miri and Linux currently (as of Aug 2022). if RECORD == 0_1_0 { - drop(Box::from_raw(CANNARY)); - CANNARY = ptr::null_mut(); + drop(Box::from_raw(CANARY)); + CANARY = ptr::null_mut(); } } @@ -67,7 +67,7 @@ fn main() { set(*key, global as *mut _ as *mut u8); } - // Initialize cannary - CANNARY = Box::into_raw(Box::new(0u64)); + // Initialize canary + CANARY = Box::into_raw(Box::new(0u64)); } } diff --git a/src/tools/miri/tests/pass-dep/shims/libc-fs.rs b/src/tools/miri/tests/pass-dep/shims/libc-fs.rs index cd071a7f32a..fbdf27688a9 100644 --- a/src/tools/miri/tests/pass-dep/shims/libc-fs.rs +++ b/src/tools/miri/tests/pass-dep/shims/libc-fs.rs @@ -130,7 +130,7 @@ fn test_readlink() { let mut large_buf = vec![0xFF; expected_path.len() + 1]; let res = unsafe { libc::readlink(symlink_c_ptr, large_buf.as_mut_ptr().cast(), large_buf.len()) }; - // Check that the resovled path was properly written into the buf. + // Check that the resolved path was properly written into the buf. assert_eq!(&large_buf[..(large_buf.len() - 1)], expected_path); assert_eq!(large_buf.last(), Some(&0xFF)); assert_eq!(res, large_buf.len() as isize - 1); diff --git a/src/tools/miri/tests/pass-dep/shims/libc-misc.rs b/src/tools/miri/tests/pass-dep/shims/libc-misc.rs index 98e1c3a0adb..82ef59427ae 100644 --- a/src/tools/miri/tests/pass-dep/shims/libc-misc.rs +++ b/src/tools/miri/tests/pass-dep/shims/libc-misc.rs @@ -90,7 +90,7 @@ fn test_posix_realpath_errors() { use std::ffi::CString; use std::io::ErrorKind; - // Test non-existent path returns an error. + // Test nonexistent path returns an error. let c_path = CString::new("./nothing_to_see_here").expect("CString::new failed"); let r = unsafe { libc::realpath(c_path.as_ptr(), std::ptr::null_mut()) }; assert!(r.is_null()); @@ -302,6 +302,101 @@ fn test_posix_mkstemp() { } } +fn test_memcpy() { + unsafe { + let src = [1i8, 2, 3]; + let dest = libc::calloc(3, 1); + libc::memcpy(dest, src.as_ptr() as *const libc::c_void, 3); + let slc = std::slice::from_raw_parts(dest as *const i8, 3); + assert_eq!(*slc, [1i8, 2, 3]); + libc::free(dest); + } + + unsafe { + let src = [1i8, 2, 3]; + let dest = libc::calloc(4, 1); + libc::memcpy(dest, src.as_ptr() as *const libc::c_void, 3); + let slc = std::slice::from_raw_parts(dest as *const i8, 4); + assert_eq!(*slc, [1i8, 2, 3, 0]); + libc::free(dest); + } + + unsafe { + let src = 123_i32; + let mut dest = 0_i32; + libc::memcpy( + &mut dest as *mut i32 as *mut libc::c_void, + &src as *const i32 as *const libc::c_void, + std::mem::size_of::<i32>(), + ); + assert_eq!(dest, src); + } + + unsafe { + let src = Some(123); + let mut dest: Option<i32> = None; + libc::memcpy( + &mut dest as *mut Option<i32> as *mut libc::c_void, + &src as *const Option<i32> as *const libc::c_void, + std::mem::size_of::<Option<i32>>(), + ); + assert_eq!(dest, src); + } + + unsafe { + let src = &123; + let mut dest = &42; + libc::memcpy( + &mut dest as *mut &'static i32 as *mut libc::c_void, + &src as *const &'static i32 as *const libc::c_void, + std::mem::size_of::<&'static i32>(), + ); + assert_eq!(*dest, 123); + } +} + +fn test_strcpy() { + use std::ffi::{CStr, CString}; + + // case: src_size equals dest_size + unsafe { + let src = CString::new("rust").unwrap(); + let size = src.as_bytes_with_nul().len(); + let dest = libc::malloc(size); + libc::strcpy(dest as *mut libc::c_char, src.as_ptr()); + assert_eq!(CStr::from_ptr(dest as *const libc::c_char), src.as_ref()); + libc::free(dest); + } + + // case: src_size is less than dest_size + unsafe { + let src = CString::new("rust").unwrap(); + let size = src.as_bytes_with_nul().len(); + let dest = libc::malloc(size + 1); + libc::strcpy(dest as *mut libc::c_char, src.as_ptr()); + assert_eq!(CStr::from_ptr(dest as *const libc::c_char), src.as_ref()); + libc::free(dest); + } +} + +#[cfg(target_os = "linux")] +fn test_sigrt() { + let min = libc::SIGRTMIN(); + let max = libc::SIGRTMAX(); + + // "The Linux kernel supports a range of 33 different real-time + // signals, numbered 32 to 64" + assert!(min >= 32); + assert!(max >= 32); + assert!(min <= 64); + assert!(max <= 64); + + // "POSIX.1-2001 requires that an implementation support at least + // _POSIX_RTSIG_MAX (8) real-time signals." + assert!(min < max); + assert!(max - min >= 8) +} + fn main() { test_posix_gettimeofday(); test_posix_mkstemp(); @@ -315,9 +410,13 @@ fn main() { test_isatty(); test_clocks(); + test_memcpy(); + test_strcpy(); + #[cfg(target_os = "linux")] { test_posix_fadvise(); test_sync_file_range(); + test_sigrt(); } } diff --git a/src/tools/miri/tests/pass-dep/tokio/tokio_mvp.rs b/src/tools/miri/tests/pass-dep/tokio/tokio_mvp.rs index 0ed2a941bc4..769a7a7d384 100644 --- a/src/tools/miri/tests/pass-dep/tokio/tokio_mvp.rs +++ b/src/tools/miri/tests/pass-dep/tokio/tokio_mvp.rs @@ -1,5 +1,5 @@ // Need to disable preemption to stay on the supported MVP codepath in mio. -//@compile-flags: -Zmiri-permissive-provenance +//@compile-flags: -Zmiri-permissive-provenance -Zmiri-preemption-rate=0 //@only-target-x86_64-unknown-linux: support for tokio exists only on linux and x86 #[tokio::main] diff --git a/src/tools/miri/tests/pass/0weak_memory_consistency.rs b/src/tools/miri/tests/pass/0weak_memory_consistency.rs index f3820bd660d..3a531eede67 100644 --- a/src/tools/miri/tests/pass/0weak_memory_consistency.rs +++ b/src/tools/miri/tests/pass/0weak_memory_consistency.rs @@ -10,7 +10,7 @@ // the RNG and never observed in our tests. // // To mitigate this, each test is ran enough times such that the chance -// of spurious success is very low. These tests never supriously fail. +// of spurious success is very low. These tests never spuriously fail. // Test cases and their consistent outcomes are from // http://svr-pes20-cppmem.cl.cam.ac.uk/cppmem/ diff --git a/src/tools/miri/tests/pass/dyn-arbitrary-self.rs b/src/tools/miri/tests/pass/dyn-arbitrary-self.rs index fc58775a195..6be13b155f4 100644 --- a/src/tools/miri/tests/pass/dyn-arbitrary-self.rs +++ b/src/tools/miri/tests/pass/dyn-arbitrary-self.rs @@ -93,7 +93,7 @@ fn pointers_and_wrappers() { trait Trait { // This method isn't object-safe yet. Unsized by-value `self` is object-safe (but not callable - // without unsized_locals), but wrappers arond `Self` currently are not. + // without unsized_locals), but wrappers around `Self` currently are not. // FIXME (mikeyhew) uncomment this when unsized rvalues object-safety is implemented // fn wrapper(self: Wrapper<Self>) -> i32; fn ptr_wrapper(self: Ptr<Wrapper<Self>>) -> i32; diff --git a/src/tools/miri/tests/pass/global_allocator.rs b/src/tools/miri/tests/pass/global_allocator.rs index 24a56c663f0..9a40c322b36 100644 --- a/src/tools/miri/tests/pass/global_allocator.rs +++ b/src/tools/miri/tests/pass/global_allocator.rs @@ -19,7 +19,7 @@ unsafe impl GlobalAlloc for Allocator { unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { if layout.size() == 123 { - println!("Dellocated!") + println!("Deallocated!") } System.dealloc(ptr, layout) diff --git a/src/tools/miri/tests/pass/global_allocator.stdout b/src/tools/miri/tests/pass/global_allocator.stdout index 411a4cdd146..30c61946407 100644 --- a/src/tools/miri/tests/pass/global_allocator.stdout +++ b/src/tools/miri/tests/pass/global_allocator.stdout @@ -1,2 +1,2 @@ Allocated! -Dellocated! +Deallocated! diff --git a/src/tools/miri/tests/pass/issues/issue-29746.rs b/src/tools/miri/tests/pass/issues/issue-29746.rs index 43bed4464b9..6b971c5217b 100644 --- a/src/tools/miri/tests/pass/issues/issue-29746.rs +++ b/src/tools/miri/tests/pass/issues/issue-29746.rs @@ -7,7 +7,7 @@ macro_rules! zip { }; // Intermediate steps to build the zipped expression, the match pattern, and - // and the output tuple of the closure, using macro hygene to repeatedly + // and the output tuple of the closure, using macro hygiene to repeatedly // introduce new variables named 'x'. ([$a:expr, $($rest:expr),*], $zip:expr, $pat:pat, [$($flat:expr),*]) => { zip!([$($rest),*], $zip.zip($a), ($pat,x), [$($flat),*, x]) diff --git a/src/tools/miri/tests/pass/packed_struct.rs b/src/tools/miri/tests/pass/packed_struct.rs index 85acab858aa..0b06167aec2 100644 --- a/src/tools/miri/tests/pass/packed_struct.rs +++ b/src/tools/miri/tests/pass/packed_struct.rs @@ -36,7 +36,7 @@ fn test_basic() { let b = x.b; assert_eq!(a, 42); assert_eq!(b, 99); - assert_eq!(&x.fill, &0); // `fill` just requirs 1-byte-align, so this is fine + assert_eq!(&x.fill, &0); // `fill` just requires 1-byte-align, so this is fine // can't do `assert_eq!(x.a, 42)`, because `assert_eq!` takes a reference assert_eq!({ x.a }, 42); assert_eq!({ x.b }, 99); diff --git a/src/tools/miri/tests/pass/ptr_offset.rs b/src/tools/miri/tests/pass/ptr_offset.rs index 95eac8522fb..92b275b0032 100644 --- a/src/tools/miri/tests/pass/ptr_offset.rs +++ b/src/tools/miri/tests/pass/ptr_offset.rs @@ -63,7 +63,7 @@ fn ptr_arith_offset_overflow() { let v = [1i16, 2]; let x = &mut ptr::null(); // going through memory as there are more sanity checks along that path *x = v.as_ptr().wrapping_offset(1); // ptr to the 2nd element - // Adding 2*isize::max and then 1 is like substracting 1 + // Adding 2*isize::max and then 1 is like subtracting 1 *x = x.wrapping_offset(isize::MAX); *x = x.wrapping_offset(isize::MAX); *x = x.wrapping_offset(1); diff --git a/src/tools/miri/tests/pass/rfc1623.rs b/src/tools/miri/tests/pass/rfc1623.rs index 76e2c01e745..8f1ef1b75cf 100644 --- a/src/tools/miri/tests/pass/rfc1623.rs +++ b/src/tools/miri/tests/pass/rfc1623.rs @@ -58,7 +58,7 @@ fn main() { STATIC_SIMPLE_FN(x); CONST_SIMPLE_FN(x); - STATIC_BAZ(BYTES); // neees static lifetime + STATIC_BAZ(BYTES); // needs static lifetime CONST_BAZ(BYTES); // make sure this works with different lifetimes diff --git a/src/tools/miri/tests/pass/shims/fs.rs b/src/tools/miri/tests/pass/shims/fs.rs index e379288de01..af245aa89aa 100644 --- a/src/tools/miri/tests/pass/shims/fs.rs +++ b/src/tools/miri/tests/pass/shims/fs.rs @@ -365,7 +365,7 @@ fn test_directory() { // Deleting the directory should succeed. remove_dir(&dir_path).unwrap(); - // Reading the metadata of a non-existent directory should fail with a "not found" error. + // Reading the metadata of a nonexistent directory should fail with a "not found" error. assert_eq!(ErrorKind::NotFound, check_metadata(&[], &dir_path).unwrap_err().kind()); // To test remove_dir_all, re-create the directory with a file and a directory in it. diff --git a/src/tools/miri/tests/pass/stacked-borrows/int-to-ptr.rs b/src/tools/miri/tests/pass/stacked-borrows/int-to-ptr.rs index c3e30627a7c..9e604f9abb8 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/int-to-ptr.rs +++ b/src/tools/miri/tests/pass/stacked-borrows/int-to-ptr.rs @@ -23,17 +23,17 @@ fn example(variant: bool) { let mut c = 42u32; let x_unique1 = &mut c; - // [..., Unique(1)] + // stack: [..., Unique(1)] let x_raw2 = x_unique1 as *mut u32; let x_raw2_addr = x_raw2.expose_addr(); - // [..., Unique(1), SharedRW(2)] + // stack: [..., Unique(1), SharedRW(2)] let x_unique3 = &mut *x_raw2; - // [.., Unique(1), SharedRW(2), Unique(3)] + // stack: [.., Unique(1), SharedRW(2), Unique(3)] assert_eq!(not_so_innocent(x_unique3), x_raw2_addr); - // [.., Unique(1), SharedRW(2), Unique(3), ..., SharedRW(4)] + // stack: [.., Unique(1), SharedRW(2), Unique(3), ..., SharedRW(4)] // Do an int2ptr cast. This can pick tag 2 or 4 (the two previously exposed tags). // 4 is the "obvious" choice (topmost tag, what we used to do with untagged pointers). diff --git a/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs b/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs index 8e78efa73c7..d7d7d1f97d6 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs +++ b/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs @@ -90,7 +90,7 @@ fn mut_raw_mut() { assert_eq!(unsafe { *xraw }, 4); assert_eq!(*xref1, 4); assert_eq!(unsafe { *xraw }, 4); - // we cannot use xref2; see `compile-fail/stacked-borows/illegal_read4.rs` + // we cannot use xref2; see `compile-fail/stacked-borrows/illegal_read4.rs` } assert_eq!(x, 4); } @@ -104,7 +104,7 @@ fn partially_invalidate_mut() { assert_eq!(*data, (1, 1)); } -// Make sure that we can handle the situation where a loaction is frozen when being dropped. +// Make sure that we can handle the situation where a location is frozen when being dropped. fn drop_after_sharing() { let x = String::from("hello!"); let _len = x.len(); @@ -224,7 +224,7 @@ fn wide_raw_ptr_in_tuple() { fn not_unpin_not_protected() { // `&mut !Unpin`, at least for now, does not get `noalias` nor `dereferenceable`, so we also // don't add protectors. (We could, but until we have a better idea for where we want to go with - // the self-referntial-generator situation, it does not seem worth the potential trouble.) + // the self-referential-generator situation, it does not seem worth the potential trouble.) use std::marker::PhantomPinned; pub struct NotUnpin(i32, PhantomPinned); diff --git a/src/tools/miri/tests/pass/strings.rs b/src/tools/miri/tests/pass/strings.rs index 5e2d2e9b5b5..72cdbe7ed5d 100644 --- a/src/tools/miri/tests/pass/strings.rs +++ b/src/tools/miri/tests/pass/strings.rs @@ -29,7 +29,7 @@ fn unique_aliasing() { // This is a regression test for the aliasing rules of a `Unique<T>` pointer. // At the time of writing this test case, Miri does not treat `Unique<T>` // pointers as a special case, these are treated like any other raw pointer. - // However, there are existing Github issues which may lead to `Unique<T>` + // However, there are existing GitHub issues which may lead to `Unique<T>` // becoming a special case through asserting unique ownership over the pointee: // - https://github.com/rust-lang/unsafe-code-guidelines/issues/258 // - https://github.com/rust-lang/unsafe-code-guidelines/issues/262 diff --git a/src/tools/miri/tests/pass/tree-borrows/cell-alternate-writes.stderr b/src/tools/miri/tests/pass/tree-borrows/cell-alternate-writes.stderr index d4bc822b4bb..1eab4685a35 100644 --- a/src/tools/miri/tests/pass/tree-borrows/cell-alternate-writes.stderr +++ b/src/tools/miri/tests/pass/tree-borrows/cell-alternate-writes.stderr @@ -1,10 +1,12 @@ ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Re*| └────<TAG=data,x,y> +| Act| └─┬──<TAG=root of the allocation> +| Re*| └────<TAG=data, x, y> ────────────────────────────────────────────────────────────────────── ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Act| └────<TAG=data,x,y> +| Act| └─┬──<TAG=root of the allocation> +| Act| └────<TAG=data, x, y> ────────────────────────────────────────────────────────────────────── diff --git a/src/tools/miri/tests/pass/tree-borrows/end-of-protector.stderr b/src/tools/miri/tests/pass/tree-borrows/end-of-protector.stderr index d08d6948320..c20da1a593f 100644 --- a/src/tools/miri/tests/pass/tree-borrows/end-of-protector.stderr +++ b/src/tools/miri/tests/pass/tree-borrows/end-of-protector.stderr @@ -1,32 +1,36 @@ ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Res| └─┬──<TAG=data> -| Res| └────<TAG=x> +| Act| └─┬──<TAG=root of the allocation> +| Res| └─┬──<TAG=data> +| Res| └────<TAG=x> ────────────────────────────────────────────────────────────────────── ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Res| └─┬──<TAG=data> -| Res| └─┬──<TAG=x> -| Res| └─┬──<TAG=caller:x> -| Res| └────<TAG=callee:x> Strongly protected +| Act| └─┬──<TAG=root of the allocation> +| Res| └─┬──<TAG=data> +| Res| └─┬──<TAG=x> +| Res| └─┬──<TAG=caller:x> +| Res| └────<TAG=callee:x> Strongly protected ────────────────────────────────────────────────────────────────────── ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Res| └─┬──<TAG=data> -| Res| ├─┬──<TAG=x> -| Res| │ └─┬──<TAG=caller:x> -| Res| │ └────<TAG=callee:x> -| Res| └────<TAG=y> +| Act| └─┬──<TAG=root of the allocation> +| Res| └─┬──<TAG=data> +| Res| ├─┬──<TAG=x> +| Res| │ └─┬──<TAG=caller:x> +| Res| │ └────<TAG=callee:x> +| Res| └────<TAG=y> ────────────────────────────────────────────────────────────────────── ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Act| └─┬──<TAG=data> -| Dis| ├─┬──<TAG=x> -| Dis| │ └─┬──<TAG=caller:x> -| Dis| │ └────<TAG=callee:x> -| Act| └────<TAG=y> +| Act| └─┬──<TAG=root of the allocation> +| Act| └─┬──<TAG=data> +| Dis| ├─┬──<TAG=x> +| Dis| │ └─┬──<TAG=caller:x> +| Dis| │ └────<TAG=callee:x> +| Act| └────<TAG=y> ────────────────────────────────────────────────────────────────────── diff --git a/src/tools/miri/tests/pass/tree-borrows/formatting.stderr b/src/tools/miri/tests/pass/tree-borrows/formatting.stderr index a59775cf21f..8c3779fe1f7 100644 --- a/src/tools/miri/tests/pass/tree-borrows/formatting.stderr +++ b/src/tools/miri/tests/pass/tree-borrows/formatting.stderr @@ -1,29 +1,31 @@ -───────────────────────────────────────────────────────────────────────────── +─────────────────────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1.. 2.. 10.. 11..100..101..1000..1001..1024 -| Res| Act| Res| Act| Res| Act| Res| Act| Res| └─┬──<TAG=data> -|----| Act|----|?Dis|----|?Dis| ----| ?Dis| ----| ├────<TAG=data[1]> -|----|----|----| Act|----|?Dis| ----| ?Dis| ----| ├────<TAG=data[10]> -|----|----|----|----|----| Frz| ----| ?Dis| ----| ├────<TAG=data[100]> -|----|----|----|----|----|----| ----| Act| ----| └────<TAG=data[1000]> -───────────────────────────────────────────────────────────────────────────── +| Act| Act| Act| Act| Act| Act| Act| Act| Act| └─┬──<TAG=root of the allocation> +| Res| Act| Res| Act| Res| Act| Res| Act| Res| └─┬──<TAG=data> +|----| Act|----|?Dis|----|?Dis| ----| ?Dis| ----| ├────<TAG=data[1]> +|----|----|----| Act|----|?Dis| ----| ?Dis| ----| ├────<TAG=data[10]> +|----|----|----|----|----| Frz| ----| ?Dis| ----| ├────<TAG=data[100]> +|----|----|----|----|----|----| ----| Act| ----| └────<TAG=data[1000]> +─────────────────────────────────────────────────────────────────────────────────────── ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Frz| └─┬──<TAG=x> -| Frz| ├─┬──<TAG=xa> -| Frz| │ ├────<TAG=xaa> -| Frz| │ └────<TAG=xab> -| Frz| ├─┬──<TAG=xb> -| Frz| │ └─┬──<TAG=xba> -| Frz| │ └─┬──<TAG=xbaa> -| Frz| │ └─┬──<TAG=xbaaa> -| Frz| │ └────<TAG=xbaaaa> -| Frz| └─┬──<TAG=xc> -| Frz| ├─┬──<TAG=xca> -| Frz| │ ├────<TAG=xcaa> -| Frz| │ └────<TAG=xcab> -| Frz| └─┬──<TAG=xcb> -| Frz| ├────<TAG=xcba> -| Frz| └────<TAG=xcbb> +| Act| └─┬──<TAG=root of the allocation> +| Frz| └─┬──<TAG=x> +| Frz| ├─┬──<TAG=xa> +| Frz| │ ├────<TAG=xaa> +| Frz| │ └────<TAG=xab> +| Frz| ├─┬──<TAG=xb> +| Frz| │ └─┬──<TAG=xba> +| Frz| │ └─┬──<TAG=xbaa> +| Frz| │ └─┬──<TAG=xbaaa> +| Frz| │ └────<TAG=xbaaaa> +| Frz| └─┬──<TAG=xc> +| Frz| ├─┬──<TAG=xca> +| Frz| │ ├────<TAG=xcaa> +| Frz| │ └────<TAG=xcab> +| Frz| └─┬──<TAG=xcb> +| Frz| ├────<TAG=xcba> +| Frz| └────<TAG=xcbb> ────────────────────────────────────────────────────────────────────── diff --git a/src/tools/miri/tests/pass/tree-borrows/read-only-from-mut.rs b/src/tools/miri/tests/pass/tree-borrows/read-only-from-mut.rs deleted file mode 100644 index 4daf06c777e..00000000000 --- a/src/tools/miri/tests/pass/tree-borrows/read-only-from-mut.rs +++ /dev/null @@ -1,14 +0,0 @@ -//@compile-flags: -Zmiri-tree-borrows - -// Tree Borrows has no issue with several mutable references existing -// at the same time, as long as they are used only immutably. -// I.e. multiple Reserved can coexist. -pub fn main() { - unsafe { - let base = &mut 42u64; - let r1 = &mut *(base as *mut u64); - let r2 = &mut *(base as *mut u64); - let _l = *r1; - let _l = *r2; - } -} diff --git a/src/tools/miri/tests/pass/tree-borrows/reborrow-is-read.stderr b/src/tools/miri/tests/pass/tree-borrows/reborrow-is-read.stderr index b9c52c20640..8c4323b2f7f 100644 --- a/src/tools/miri/tests/pass/tree-borrows/reborrow-is-read.stderr +++ b/src/tools/miri/tests/pass/tree-borrows/reborrow-is-read.stderr @@ -1,13 +1,15 @@ ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Act| └─┬──<TAG=parent> -| Act| └────<TAG=x> +| Act| └─┬──<TAG=root of the allocation> +| Act| └─┬──<TAG=parent> +| Act| └────<TAG=x> ────────────────────────────────────────────────────────────────────── ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Act| └─┬──<TAG=parent> -| Frz| ├────<TAG=x> -| Res| └────<TAG=y> +| Act| └─┬──<TAG=root of the allocation> +| Act| └─┬──<TAG=parent> +| Frz| ├────<TAG=x> +| Res| └────<TAG=y> ────────────────────────────────────────────────────────────────────── diff --git a/src/tools/miri/tests/pass/tree-borrows/reserved.stderr b/src/tools/miri/tests/pass/tree-borrows/reserved.stderr index d76ee0f8266..afb91700222 100644 --- a/src/tools/miri/tests/pass/tree-borrows/reserved.stderr +++ b/src/tools/miri/tests/pass/tree-borrows/reserved.stderr @@ -2,51 +2,57 @@ ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Re*| └─┬──<TAG=base> -| Re*| ├─┬──<TAG=x> -| Re*| │ └─┬──<TAG=caller:x> -| Frz| │ └────<TAG=callee:x> -| Re*| └────<TAG=y,caller:y,callee:y> +| Act| └─┬──<TAG=root of the allocation> +| Re*| └─┬──<TAG=base> +| Re*| ├─┬──<TAG=x> +| Re*| │ └─┬──<TAG=caller:x> +| Frz| │ └────<TAG=callee:x> +| Re*| └────<TAG=y, caller:y, callee:y> ────────────────────────────────────────────────────────────────────── [interior mut] Foreign Read: Re* -> Re* ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 8 -| Re*| └─┬──<TAG=base> -| Re*| ├────<TAG=x> -| Re*| └────<TAG=y> +| Act| └─┬──<TAG=root of the allocation> +| Re*| └─┬──<TAG=base> +| Re*| ├────<TAG=x> +| Re*| └────<TAG=y> ────────────────────────────────────────────────────────────────────── [interior mut] Foreign Write: Re* -> Re* ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 8 -| Act| └─┬──<TAG=base> -| Re*| ├────<TAG=x> -| Act| └────<TAG=y> +| Act| └─┬──<TAG=root of the allocation> +| Act| └─┬──<TAG=base> +| Re*| ├────<TAG=x> +| Act| └────<TAG=y> ────────────────────────────────────────────────────────────────────── [protected] Foreign Read: Res -> Frz ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Res| └─┬──<TAG=base> -| Res| ├─┬──<TAG=x> -| Res| │ └─┬──<TAG=caller:x> -| Frz| │ └────<TAG=callee:x> -| Res| └────<TAG=y,caller:y,callee:y> +| Act| └─┬──<TAG=root of the allocation> +| Res| └─┬──<TAG=base> +| Res| ├─┬──<TAG=x> +| Res| │ └─┬──<TAG=caller:x> +| Frz| │ └────<TAG=callee:x> +| Res| └────<TAG=y, caller:y, callee:y> ────────────────────────────────────────────────────────────────────── [] Foreign Read: Res -> Res ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Res| └─┬──<TAG=base> -| Res| ├────<TAG=x> -| Res| └────<TAG=y> +| Act| └─┬──<TAG=root of the allocation> +| Res| └─┬──<TAG=base> +| Res| ├────<TAG=x> +| Res| └────<TAG=y> ────────────────────────────────────────────────────────────────────── [] Foreign Write: Res -> Dis ────────────────────────────────────────────────────────────────────── Warning: this tree is indicative only. Some tags may have been hidden. 0.. 1 -| Act| └─┬──<TAG=base> -| Dis| ├────<TAG=x> -| Act| └────<TAG=y> +| Act| └─┬──<TAG=root of the allocation> +| Act| └─┬──<TAG=base> +| Dis| ├────<TAG=x> +| Act| └────<TAG=y> ────────────────────────────────────────────────────────────────────── diff --git a/src/tools/miri/tests/pass/tree-borrows/tree-borrows.rs b/src/tools/miri/tests/pass/tree-borrows/tree-borrows.rs new file mode 100644 index 00000000000..aa6f7078890 --- /dev/null +++ b/src/tools/miri/tests/pass/tree-borrows/tree-borrows.rs @@ -0,0 +1,280 @@ +//@compile-flags: -Zmiri-tree-borrows +#![feature(allocator_api)] + +use std::mem; +use std::ptr; + +fn main() { + aliasing_read_only_mutable_refs(); + string_as_mut_ptr(); + + // Stacked Borrows tests + read_does_not_invalidate1(); + read_does_not_invalidate2(); + mut_raw_then_mut_shr(); + mut_shr_then_mut_raw(); + mut_raw_mut(); + partially_invalidate_mut(); + drop_after_sharing(); + direct_mut_to_const_raw(); + two_raw(); + shr_and_raw(); + disjoint_mutable_subborrows(); + raw_ref_to_part(); + array_casts(); + mut_below_shr(); + wide_raw_ptr_in_tuple(); + not_unpin_not_protected(); +} + +// Tree Borrows has no issue with several mutable references existing +// at the same time, as long as they are used only immutably. +// I.e. multiple Reserved can coexist. +pub fn aliasing_read_only_mutable_refs() { + unsafe { + let base = &mut 42u64; + let r1 = &mut *(base as *mut u64); + let r2 = &mut *(base as *mut u64); + let _l = *r1; + let _l = *r2; + } +} + +pub fn string_as_mut_ptr() { + // This errors in Stacked Borrows since as_mut_ptr restricts the provenance, + // but with Tree Borrows it should work. + unsafe { + let mut s = String::from("hello"); + s.reserve(1); // make the `str` that `s` derefs to not cover the entire `s`. + + // Prevent automatically dropping the String's data + let mut s = mem::ManuallyDrop::new(s); + + let ptr = s.as_mut_ptr(); + let len = s.len(); + let capacity = s.capacity(); + + let s = String::from_raw_parts(ptr, len, capacity); + + assert_eq!(String::from("hello"), s); + } +} + +// ----- The tests below were taken from Stacked Borrows ---- + +// Make sure that reading from an `&mut` does, like reborrowing to `&`, +// NOT invalidate other reborrows. +fn read_does_not_invalidate1() { + fn foo(x: &mut (i32, i32)) -> &i32 { + let xraw = x as *mut (i32, i32); + let ret = unsafe { &(*xraw).1 }; + let _val = x.1; // we just read, this does NOT invalidate the reborrows. + ret + } + assert_eq!(*foo(&mut (1, 2)), 2); +} +// Same as above, but this time we first create a raw, then read from `&mut` +// and then freeze from the raw. +fn read_does_not_invalidate2() { + fn foo(x: &mut (i32, i32)) -> &i32 { + let xraw = x as *mut (i32, i32); + let _val = x.1; // we just read, this does NOT invalidate the raw reborrow. + let ret = unsafe { &(*xraw).1 }; + ret + } + assert_eq!(*foo(&mut (1, 2)), 2); +} + +// Escape a mut to raw, then share the same mut and use the share, then the raw. +// That should work. +fn mut_raw_then_mut_shr() { + let mut x = 2; + let xref = &mut x; + let xraw = &mut *xref as *mut _; + let xshr = &*xref; + assert_eq!(*xshr, 2); + unsafe { + *xraw = 4; + } + assert_eq!(x, 4); +} + +// Create first a shared reference and then a raw pointer from a `&mut` +// should permit mutation through that raw pointer. +fn mut_shr_then_mut_raw() { + let xref = &mut 2; + let _xshr = &*xref; + let xraw = xref as *mut _; + unsafe { + *xraw = 3; + } + assert_eq!(*xref, 3); +} + +// Ensure that if we derive from a mut a raw, and then from that a mut, +// and then read through the original mut, that does not invalidate the raw. +// This shows that the read-exception for `&mut` applies even if the `Shr` item +// on the stack is not at the top. +fn mut_raw_mut() { + let mut x = 2; + { + let xref1 = &mut x; + let xraw = xref1 as *mut _; + let _xref2 = unsafe { &mut *xraw }; + let _val = *xref1; + unsafe { + *xraw = 4; + } + // we can now use both xraw and xref1, for reading + assert_eq!(*xref1, 4); + assert_eq!(unsafe { *xraw }, 4); + assert_eq!(*xref1, 4); + assert_eq!(unsafe { *xraw }, 4); + // we cannot use xref2; see `compile-fail/stacked-borrows/illegal_read4.rs` + } + assert_eq!(x, 4); +} + +fn partially_invalidate_mut() { + let data = &mut (0u8, 0u8); + let reborrow = &mut *data as *mut (u8, u8); + let shard = unsafe { &mut (*reborrow).0 }; + data.1 += 1; // the deref overlaps with `shard`, but that is ok; the access does not overlap. + *shard += 1; // so we can still use `shard`. + assert_eq!(*data, (1, 1)); +} + +// Make sure that we can handle the situation where a location is frozen when being dropped. +fn drop_after_sharing() { + let x = String::from("hello!"); + let _len = x.len(); +} + +// Make sure that coercing &mut T to *const T produces a writeable pointer. +fn direct_mut_to_const_raw() { + // TODO: This is currently disabled, waiting on a decision on <https://github.com/rust-lang/rust/issues/56604> + /*let x = &mut 0; + let y: *const i32 = x; + unsafe { *(y as *mut i32) = 1; } + assert_eq!(*x, 1); + */ +} + +// Make sure that we can create two raw pointers from a mutable reference and use them both. +fn two_raw() { + unsafe { + let x = &mut 0; + let y1 = x as *mut _; + let y2 = x as *mut _; + *y1 += 2; + *y2 += 1; + } +} + +// Make sure that creating a *mut does not invalidate existing shared references. +fn shr_and_raw() { + unsafe { + let x = &mut 0; + let y1: &i32 = mem::transmute(&*x); // launder lifetimes + let y2 = x as *mut _; + let _val = *y1; + *y2 += 1; + } +} + +fn disjoint_mutable_subborrows() { + struct Foo { + a: String, + b: Vec<u32>, + } + + unsafe fn borrow_field_a<'a>(this: *mut Foo) -> &'a mut String { + &mut (*this).a + } + + unsafe fn borrow_field_b<'a>(this: *mut Foo) -> &'a mut Vec<u32> { + &mut (*this).b + } + + let mut foo = Foo { a: "hello".into(), b: vec![0, 1, 2] }; + + let ptr = &mut foo as *mut Foo; + + let a = unsafe { borrow_field_a(ptr) }; + let b = unsafe { borrow_field_b(ptr) }; + b.push(4); + a.push_str(" world"); + assert_eq!(format!("{:?} {:?}", a, b), r#""hello world" [0, 1, 2, 4]"#); +} + +fn raw_ref_to_part() { + struct Part { + _lame: i32, + } + + #[repr(C)] + struct Whole { + part: Part, + extra: i32, + } + + let it = Box::new(Whole { part: Part { _lame: 0 }, extra: 42 }); + let whole = ptr::addr_of_mut!(*Box::leak(it)); + let part = unsafe { ptr::addr_of_mut!((*whole).part) }; + let typed = unsafe { &mut *(part as *mut Whole) }; + assert!(typed.extra == 42); + drop(unsafe { Box::from_raw(whole) }); +} + +/// When casting an array reference to a raw element ptr, that should cover the whole array. +fn array_casts() { + let mut x: [usize; 2] = [0, 0]; + let p = &mut x as *mut usize; + unsafe { + *p.add(1) = 1; + } + + let x: [usize; 2] = [0, 1]; + let p = &x as *const usize; + assert_eq!(unsafe { *p.add(1) }, 1); +} + +/// Transmuting &&i32 to &&mut i32 is fine. +fn mut_below_shr() { + let x = 0; + let y = &x; + let p = unsafe { core::mem::transmute::<&&i32, &&mut i32>(&y) }; + let r = &**p; + let _val = *r; +} + +fn wide_raw_ptr_in_tuple() { + let mut x: Box<dyn std::any::Any> = Box::new("ouch"); + let r = &mut *x as *mut dyn std::any::Any; + // This triggers the visitor-based recursive retagging. It is *not* supposed to retag raw + // pointers, but then the visitor might recurse into the "fields" of a wide raw pointer and + // finds a reference (to a vtable) there that it wants to retag... and that would be Wrong. + let pair = (r, &0); + let r = unsafe { &mut *pair.0 }; + // Make sure the fn ptr part of the vtable is still fine. + r.type_id(); +} + +fn not_unpin_not_protected() { + // `&mut !Unpin`, at least for now, does not get `noalias` nor `dereferenceable`, so we also + // don't add protectors. (We could, but until we have a better idea for where we want to go with + // the self-referential-generator situation, it does not seem worth the potential trouble.) + use std::marker::PhantomPinned; + + pub struct NotUnpin(i32, PhantomPinned); + + fn inner(x: &mut NotUnpin, f: fn(&mut NotUnpin)) { + // `f` may mutate, but it may not deallocate! + f(x) + } + + inner(Box::leak(Box::new(NotUnpin(0, PhantomPinned))), |x| { + let raw = x as *mut _; + drop(unsafe { Box::from_raw(raw) }); + }); +} diff --git a/src/tools/miri/tests/pass/weak_memory/extra_cpp.rs b/src/tools/miri/tests/pass/weak_memory/extra_cpp.rs index 07cbb4a803f..94df7308080 100644 --- a/src/tools/miri/tests/pass/weak_memory/extra_cpp.rs +++ b/src/tools/miri/tests/pass/weak_memory/extra_cpp.rs @@ -1,6 +1,6 @@ //@compile-flags: -Zmiri-ignore-leaks -// Tests operations not perfomable through C++'s atomic API +// Tests operations not performable through C++'s atomic API // but doable in safe (at least sound) Rust. #![feature(atomic_from_mut)] diff --git a/src/tools/miri/tests/pass/weak_memory/extra_cpp_unsafe.rs b/src/tools/miri/tests/pass/weak_memory/extra_cpp_unsafe.rs index f7e2748408f..48b15191b38 100644 --- a/src/tools/miri/tests/pass/weak_memory/extra_cpp_unsafe.rs +++ b/src/tools/miri/tests/pass/weak_memory/extra_cpp_unsafe.rs @@ -1,6 +1,6 @@ //@compile-flags: -Zmiri-ignore-leaks -// Tests operations not perfomable through C++'s atomic API +// Tests operations not performable through C++'s atomic API // but doable in unsafe Rust which we think *should* be fine. // Nonetheless they may be determined as inconsistent with the // memory model in the future. |