From 8bbb0404f8c49c0d703492303e766d0703017bb8 Mon Sep 17 00:00:00 2001 From: Neven Villani Date: Thu, 16 Mar 2023 14:47:44 +0100 Subject: [PATCH] TB: integration --- src/tools/miri/src/bin/miri.rs | 4 ++- src/tools/miri/src/diagnostics.rs | 14 +++++++- src/tools/miri/src/eval.rs | 4 +-- src/tools/miri/src/lib.rs | 1 + src/tools/miri/src/machine.rs | 6 ++-- src/tools/miri/src/shims/foreign_items.rs | 41 ++++++++++++++++++----- 6 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index a2caeb97297..6fe3fa7fb1b 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -32,7 +32,7 @@ }; use rustc_session::{config::CrateType, search_paths::PathKind, CtfeBacktrace}; -use miri::{BacktraceStyle, ProvenanceMode, RetagFields}; +use miri::{BacktraceStyle, BorrowTrackerMethod, ProvenanceMode, RetagFields}; struct MiriCompilerCalls { miri_config: miri::MiriConfig, @@ -317,6 +317,8 @@ fn main() { miri_config.validate = false; } else if arg == "-Zmiri-disable-stacked-borrows" { miri_config.borrow_tracker = None; + } else if arg == "-Zmiri-tree-borrows" { + miri_config.borrow_tracker = Some(BorrowTrackerMethod::TreeBorrows); } else if arg == "-Zmiri-disable-data-race-detector" { miri_config.data_race_detector = false; miri_config.weak_memory_emulation = false; diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 035c0e64233..3c13118122c 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -22,6 +22,10 @@ pub enum TerminationInfo { help: Option, history: Option, }, + TreeBorrowsUb { + msg: String, + // FIXME: incomplete + }, Int2PtrWithStrictProvenance, Deadlock, MultipleSymbolDefinitions { @@ -61,6 +65,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { "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}"), Deadlock => write!(f, "the evaluated program deadlocked"), MultipleSymbolDefinitions { link_name, .. } => write!(f, "multiple definitions of symbol `{link_name}`"), @@ -184,7 +189,8 @@ pub fn report_error<'tcx, 'mir>( Abort(_) => Some("abnormal termination"), UnsupportedInIsolation(_) | Int2PtrWithStrictProvenance => Some("unsupported operation"), - StackedBorrowsUb { .. } | DataRace { .. } => Some("Undefined Behavior"), + StackedBorrowsUb { .. } | TreeBorrowsUb { .. } | DataRace { .. } => + Some("Undefined Behavior"), Deadlock => Some("deadlock"), MultipleSymbolDefinitions { .. } | SymbolShimClashing { .. } => None, }; @@ -212,6 +218,12 @@ 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")), + ]; + helps } MultipleSymbolDefinitions { first, first_crate, second, second_crate, .. } => vec![ diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index 8443e907938..a32b18595b5 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -87,7 +87,7 @@ pub struct MiriConfig { pub env: Vec<(OsString, OsString)>, /// Determine if validity checking is enabled. pub validate: bool, - /// Determines if Stacked Borrows is enabled. + /// Determines if Stacked Borrows or Tree Borrows is enabled. pub borrow_tracker: Option, /// Controls alignment checking. pub check_alignment: AlignmentCheck, @@ -134,7 +134,7 @@ pub struct MiriConfig { pub preemption_rate: f64, /// Report the current instruction being executed every N basic blocks. pub report_progress: Option, - /// Whether Stacked Borrows retagging should recurse into fields of datatypes. + /// Whether Stacked Borrows and Tree Borrows retagging should recurse into fields of datatypes. pub retag_fields: RetagFields, /// The location of a shared object file to load when calling external functions /// FIXME! consider allowing users to specify paths to multiple SO files, or to a directory diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 5412507ecfa..01d0f01d319 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -95,6 +95,7 @@ pub use crate::borrow_tracker::stacked_borrows::{ EvalContextExt as _, Item, Permission, Stack, Stacks, }; +pub use crate::borrow_tracker::tree_borrows::{EvalContextExt as _, Tree}; pub use crate::borrow_tracker::{ BorTag, BorrowTrackerMethod, CallId, EvalContextExt as _, RetagFields, }; diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 969c81f7e32..eaa441d22ec 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -38,7 +38,7 @@ /// Extra data stored with each stack frame pub struct FrameExtra<'tcx> { - /// Extra data for Stacked Borrows. + /// Extra data for the Borrow Tracker. pub borrow_tracker: Option, /// If this is Some(), then this is a special "catch unwind" frame (the frame of `try_fn` @@ -146,7 +146,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { pub enum Provenance { Concrete { alloc_id: AllocId, - /// Stacked Borrows tag. + /// Borrow Tracker tag. tag: BorTag, }, Wildcard, @@ -195,7 +195,7 @@ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { } else { write!(f, "[{alloc_id:?}]")?; } - // Print Stacked Borrows tag. + // Print Borrow Tracker tag. write!(f, "{tag:?}")?; } Provenance::Wildcard => { diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 03275ed4ed1..73439133af2 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -232,6 +232,19 @@ fn lookup_exported_symbol( } } + /// Read bytes from a `(ptr, len)` argument + fn read_byte_slice<'i>(&'i self, bytes: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, &'i [u8]> + where + 'mir: 'i, + { + let this = self.eval_context_ref(); + let (ptr, len) = this.read_immediate(bytes)?.to_scalar_pair(); + let ptr = ptr.to_pointer(this)?; + let len = len.to_target_usize(this)?; + let bytes = this.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?; + Ok(bytes) + } + /// Emulates calling a foreign item, failing if the item is not supported. /// This function will handle `goto_block` if needed. /// Returns Ok(None) if the foreign item was completely handled @@ -427,13 +440,27 @@ fn emulate_foreign_item_by_name( })?; this.write_scalar(Scalar::from_u64(alloc_id.0.get()), dest)?; } - "miri_print_borrow_stacks" => { - let [id] = this.check_shim(abi, Abi::Rust, link_name, args)?; + "miri_print_borrow_state" => { + let [id, show_unnamed] = this.check_shim(abi, Abi::Rust, link_name, args)?; let id = this.read_scalar(id)?.to_u64()?; + let show_unnamed = this.read_scalar(show_unnamed)?.to_bool()?; if let Some(id) = std::num::NonZeroU64::new(id) { - this.print_stacks(AllocId(id))?; + this.print_borrow_state(AllocId(id), show_unnamed)?; } } + "miri_pointer_name" => { + // This associates a name to a tag. Very useful for debugging, and also makes + // tests more strict. + let [ptr, nth_parent, name] = this.check_shim(abi, Abi::Rust, link_name, args)?; + let ptr = this.read_pointer(ptr)?; + let nth_parent = this.read_scalar(nth_parent)?.to_u8()?; + let name = this.read_byte_slice(name)?; + // We must make `name` owned because we need to + // end the shared borrow from `read_byte_slice` before we can + // start the mutable borrow for `give_pointer_debug_name`. + let name = String::from_utf8_lossy(name).into_owned(); + this.give_pointer_debug_name(ptr, nth_parent, &name)?; + } "miri_static_root" => { let [ptr] = this.check_shim(abi, Abi::Rust, link_name, args)?; let ptr = this.read_pointer(ptr)?; @@ -487,12 +514,8 @@ fn emulate_foreign_item_by_name( // Writes some bytes to the interpreter's stdout/stderr. See the // README for details. "miri_write_to_stdout" | "miri_write_to_stderr" => { - let [bytes] = this.check_shim(abi, Abi::Rust, link_name, args)?; - let (ptr, len) = this.read_immediate(bytes)?.to_scalar_pair(); - let ptr = ptr.to_pointer(this)?; - let len = len.to_target_usize(this)?; - let msg = this.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(len))?; - + let [msg] = this.check_shim(abi, Abi::Rust, link_name, args)?; + let msg = this.read_byte_slice(msg)?; // Note: we're ignoring errors writing to host stdout/stderr. let _ignore = match link_name.as_str() { "miri_write_to_stdout" => std::io::stdout().write_all(msg),