From 7225524744ba94e32baf7540f4e064db01252458 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 20 Nov 2022 18:07:51 +0100 Subject: [PATCH 01/31] add a weak form of protection that justifies Box noalias --- .../miri/src/stacked_borrows/diagnostics.rs | 26 +++--- src/tools/miri/src/stacked_borrows/mod.rs | 87 +++++++++++++------ .../fail/stacked_borrows/aliasing_mut1.stderr | 4 +- .../fail/stacked_borrows/aliasing_mut2.stderr | 4 +- .../fail/stacked_borrows/aliasing_mut4.stderr | 4 +- .../stacked_borrows/box_noalias_violation.rs | 14 +++ .../box_noalias_violation.stderr | 30 +++++++ .../deallocate_against_protector1.rs | 2 +- .../deallocate_against_protector1.stderr | 4 +- .../deallocate_against_protector2.rs | 2 +- .../deallocate_against_protector2.stderr | 4 +- .../fail/stacked_borrows/illegal_write6.rs | 2 +- .../stacked_borrows/illegal_write6.stderr | 4 +- .../invalidate_against_protector1.stderr | 4 +- .../invalidate_against_protector2.stderr | 4 +- .../invalidate_against_protector3.stderr | 4 +- .../stacked_borrows/newtype_pair_retagging.rs | 2 +- .../newtype_pair_retagging.stderr | 4 +- .../fail/stacked_borrows/newtype_retagging.rs | 2 +- .../stacked_borrows/newtype_retagging.stderr | 4 +- 20 files changed, 146 insertions(+), 65 deletions(-) create mode 100644 src/tools/miri/tests/fail/stacked_borrows/box_noalias_violation.rs create mode 100644 src/tools/miri/tests/fail/stacked_borrows/box_noalias_violation.stderr diff --git a/src/tools/miri/src/stacked_borrows/diagnostics.rs b/src/tools/miri/src/stacked_borrows/diagnostics.rs index d3843b030347f..323ec3d75f9f8 100644 --- a/src/tools/miri/src/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/stacked_borrows/diagnostics.rs @@ -6,7 +6,7 @@ use rustc_span::{Span, SpanData}; use rustc_target::abi::Size; use crate::helpers::CurrentSpan; -use crate::stacked_borrows::{err_sb_ub, AccessKind, GlobalStateInner, Permission}; +use crate::stacked_borrows::{err_sb_ub, AccessKind, GlobalStateInner, Permission, ProtectorKind}; use crate::*; use rustc_middle::mir::interpret::InterpError; @@ -288,7 +288,11 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir } Operation::Access(AccessOp { kind, range, .. }) => (*range, InvalidationCause::Access(*kind)), - _ => unreachable!("Tags can only be invalidated during a retag or access"), + _ => { + // This can be reached, but never be relevant later since the entire allocation is + // gone now. + return; + } }; self.history.invalidations.push(Invalidation { tag, range, span, cause }); } @@ -369,7 +373,7 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir /// Report a descriptive error when `new` could not be granted from `derived_from`. #[inline(never)] // This is only called on fatal code paths - pub fn grant_error(&self, perm: Permission, stack: &Stack) -> InterpError<'tcx> { + pub(super) fn grant_error(&self, perm: Permission, stack: &Stack) -> InterpError<'tcx> { let Operation::Retag(op) = &self.operation else { unreachable!("grant_error should only be called during a retag") }; @@ -389,7 +393,7 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir /// Report a descriptive error when `access` is not permitted based on `tag`. #[inline(never)] // This is only called on fatal code paths - pub fn access_error(&self, stack: &Stack) -> InterpError<'tcx> { + pub(super) fn access_error(&self, stack: &Stack) -> InterpError<'tcx> { let Operation::Access(op) = &self.operation else { unreachable!("access_error should only be called during an access") }; @@ -408,7 +412,11 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir } #[inline(never)] // This is only called on fatal code paths - pub fn protector_error(&self, item: &Item) -> InterpError<'tcx> { + pub(super) fn protector_error(&self, item: &Item, kind: ProtectorKind) -> InterpError<'tcx> { + let protected = match kind { + ProtectorKind::WeakProtector => "weakly protected", + ProtectorKind::StrongProtector => "strongly protected", + }; let call_id = self .threads .all_stacks() @@ -422,10 +430,7 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir match self.operation { Operation::Dealloc(_) => err_sb_ub( - format!( - "deallocating while item {:?} is protected by call {:?}", - item, call_id - ), + format!("deallocating while item {item:?} is {protected} by call {call_id:?}",), None, None, ), @@ -433,8 +438,7 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir | Operation::Access(AccessOp { tag, .. }) => err_sb_ub( format!( - "not granting access to tag {:?} because that would remove {:?} which is protected because it is an argument of call {:?}", - tag, item, call_id + "not granting access to tag {tag:?} because that would remove {item:?} which is {protected} because it is an argument of call {call_id:?}", ), None, tag.and_then(|tag| self.get_logs_relevant_to(tag, Some(item.tag()))), diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/stacked_borrows/mod.rs index f2e2df5ad0800..648797211a77a 100644 --- a/src/tools/miri/src/stacked_borrows/mod.rs +++ b/src/tools/miri/src/stacked_borrows/mod.rs @@ -91,6 +91,26 @@ pub struct Stacks { modified_since_last_gc: bool, } +/// The flavor of the protector. +#[derive(Copy, Clone, Debug)] +enum ProtectorKind { + /// Protected against aliasing violations from other pointers. + /// + /// Items protected like this cause UB when they are invalidated, *but* the pointer itself may + /// still be used to issue a deallocation. + /// + /// This is required for LLVM IR pointers that are `noalias` but *not* `dereferenceable`. + WeakProtector, + + /// Protected against any kind of invalidation. + /// + /// Items protected like this cause UB when they are invalidated or the memory is deallocated. + /// This is strictly stronger protection than `WeakProtector`. + /// + /// This is required for LLVM IR pointers that are `dereferenceable` (and also allows `noalias`). + StrongProtector, +} + /// Extra global state, available to the memory access hooks. #[derive(Debug)] pub struct GlobalStateInner { @@ -102,12 +122,12 @@ pub struct GlobalStateInner { base_ptr_tags: FxHashMap, /// Next unused call ID (for protectors). next_call_id: CallId, - /// All currently protected tags. + /// All currently protected tags, and the status of their protection. /// An item is protected if its tag is in this set, *and* it has the "protected" bit set. /// We add tags to this when they are created with a protector in `reborrow`, and /// we remove tags from this when the call which is protecting them returns, in /// `GlobalStateInner::end_call`. See `Stack::item_popped` for more details. - protected_tags: FxHashSet, + protected_tags: FxHashMap, /// The pointer ids to trace tracked_pointer_tags: FxHashSet, /// The call ids to trace @@ -189,7 +209,7 @@ impl GlobalStateInner { next_ptr_tag: SbTag(NonZeroU64::new(1).unwrap()), base_ptr_tags: FxHashMap::default(), next_call_id: NonZeroU64::new(1).unwrap(), - protected_tags: FxHashSet::default(), + protected_tags: FxHashMap::default(), tracked_pointer_tags, tracked_call_ids, retag_fields, @@ -314,6 +334,7 @@ impl<'tcx> Stack { item: &Item, global: &GlobalStateInner, dcx: &mut DiagnosticCx<'_, '_, '_, '_, 'tcx>, + deallocation: bool, ) -> InterpResult<'tcx> { if !global.tracked_pointer_tags.is_empty() { dcx.check_tracked_tag_popped(item, global); @@ -336,8 +357,11 @@ impl<'tcx> Stack { // 2. Most frames protect only one or two tags. So this duplicative global turns a search // which ends up about linear in the number of protected tags in the program into a // constant time check (and a slow linear, because the tags in the frames aren't contiguous). - if global.protected_tags.contains(&item.tag()) { - return Err(dcx.protector_error(item).into()); + if let Some(&protector_kind) = global.protected_tags.get(&item.tag()) { + let allowed = deallocation && matches!(protector_kind, ProtectorKind::WeakProtector); + if !allowed { + return Err(dcx.protector_error(item, protector_kind).into()); + } } Ok(()) } @@ -350,7 +374,7 @@ impl<'tcx> Stack { &mut self, access: AccessKind, tag: ProvenanceExtra, - global: &mut GlobalStateInner, + global: &GlobalStateInner, dcx: &mut DiagnosticCx<'_, '_, '_, '_, 'tcx>, exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { @@ -377,7 +401,7 @@ impl<'tcx> Stack { 0 }; self.pop_items_after(first_incompatible_idx, |item| { - Stack::item_popped(&item, global, dcx)?; + Stack::item_popped(&item, global, dcx, /* deallocation */ false)?; dcx.log_invalidation(item.tag()); Ok(()) })?; @@ -398,7 +422,7 @@ impl<'tcx> Stack { 0 }; self.disable_uniques_starting_at(first_incompatible_idx, |item| { - Stack::item_popped(&item, global, dcx)?; + Stack::item_popped(&item, global, dcx, /* deallocation */ false)?; dcx.log_invalidation(item.tag()); Ok(()) })?; @@ -440,14 +464,15 @@ impl<'tcx> Stack { dcx: &mut DiagnosticCx<'_, '_, '_, '_, 'tcx>, exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { - // Step 1: Make sure there is a granting item. - self.find_granting(AccessKind::Write, tag, exposed_tags) + // Step 1: Make a write access. + // As part of this we do regular protector checking, i.e. even weakly protected items cause UB when popped. + self.access(AccessKind::Write, tag, global, dcx, exposed_tags) .map_err(|_| dcx.dealloc_error())?; - // Step 2: Consider all items removed. This checks for protectors. + // Step 2: Pretend we remove the remaining items, checking if any are strongly protected. for idx in (0..self.len()).rev() { let item = self.get(idx).unwrap(); - Stack::item_popped(&item, global, dcx)?; + Stack::item_popped(&item, global, dcx, /* deallocation */ true)?; } Ok(()) @@ -698,7 +723,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' kind: RefKind, retag_cause: RetagCause, // What caused this retag, for diagnostics only new_tag: SbTag, - protect: bool, + protect: Option, ) -> InterpResult<'tcx, Option> { let this = self.eval_context_mut(); @@ -761,7 +786,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' ); let mut dcx = dcx.build(&mut stacked_borrows.history, base_offset); dcx.log_creation(); - if protect { + if protect.is_some() { dcx.log_protector(); } } @@ -821,10 +846,16 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' size.bytes() ); - if protect { + if let Some(protect) = protect { // See comment in `Stack::item_popped` for why we store the tag twice. this.frame_mut().extra.stacked_borrows.as_mut().unwrap().protected_tags.push(new_tag); - this.machine.stacked_borrows.as_mut().unwrap().get_mut().protected_tags.insert(new_tag); + this.machine + .stacked_borrows + .as_mut() + .unwrap() + .get_mut() + .protected_tags + .insert(new_tag, protect); } // Update the stacks. @@ -866,7 +897,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' Permission::SharedReadWrite }; let protected = if frozen { - protect + protect.is_some() } else { // We do not protect inside UnsafeCell. // This fixes https://github.com/rust-lang/rust/issues/55005. @@ -899,7 +930,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' .as_mut() .expect("we should have Stacked Borrows data") .borrow_mut(); - let item = Item::new(new_tag, perm, protect); + let item = Item::new(new_tag, perm, protect.is_some()); let range = alloc_range(base_offset, size); let mut global = machine.stacked_borrows.as_ref().unwrap().borrow_mut(); // FIXME: can't share this with the current_span inside log_creation @@ -926,7 +957,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' val: &ImmTy<'tcx, Provenance>, kind: RefKind, retag_cause: RetagCause, // What caused this retag, for diagnostics only - protect: bool, + protect: Option, ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { let this = self.eval_context_mut(); // We want a place for where the ptr *points to*, so we get one. @@ -996,7 +1027,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { place: &PlaceTy<'tcx, Provenance>, ref_kind: RefKind, retag_cause: RetagCause, - protector: bool, + protector: Option, ) -> InterpResult<'tcx> { let val = self.ecx.read_immediate(&self.ecx.place_to_op(place)?)?; let val = self.ecx.retag_reference(&val, ref_kind, retag_cause, protector)?; @@ -1015,13 +1046,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { - // Boxes do not get a protector: protectors reflect that references outlive the call - // they were passed in to; that's just not the case for boxes. + // Boxes get a weak protectors, since they may be deallocated. self.retag_place( place, RefKind::Unique { two_phase: false }, self.retag_cause, - /*protector*/ false, + /*protector*/ + (self.kind == RetagKind::FnEntry).then_some(ProtectorKind::WeakProtector), ) } @@ -1046,7 +1077,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { place, ref_kind, self.retag_cause, - /*protector*/ self.kind == RetagKind::FnEntry, + /*protector*/ + (self.kind == RetagKind::FnEntry) + .then_some(ProtectorKind::StrongProtector), )?; } ty::RawPtr(tym) => { @@ -1059,7 +1092,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { place, RefKind::Raw { mutable: tym.mutbl == Mutability::Mut }, self.retag_cause, - /*protector*/ false, + /*protector*/ None, )?; } } @@ -1110,12 +1143,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // (The pointer type does not matter, so we use a raw pointer.) let ptr_layout = this.layout_of(this.tcx.mk_mut_ptr(return_place.layout.ty))?; let val = ImmTy::from_immediate(return_place.to_ref(this), ptr_layout); - // Reborrow it. + // Reborrow it. With protection! That is part of the point. let val = this.retag_reference( &val, RefKind::Unique { two_phase: false }, RetagCause::FnReturn, - /*protector*/ true, + /*protector*/ Some(ProtectorKind::StrongProtector), )?; // And use reborrowed pointer for return place. let return_place = this.ref_to_mplace(&val)?; diff --git a/src/tools/miri/tests/fail/stacked_borrows/aliasing_mut1.stderr b/src/tools/miri/tests/fail/stacked_borrows/aliasing_mut1.stderr index 5d4679b13ad1f..268d253ad5b06 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/aliasing_mut1.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/aliasing_mut1.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID --> $DIR/aliasing_mut1.rs:LL:CC | LL | pub fn safe(_x: &mut i32, _y: &mut i32) {} - | ^^ not granting access to tag because that would remove [Unique for ] which is protected because it is an argument of call ID + | ^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/stacked_borrows/aliasing_mut2.stderr b/src/tools/miri/tests/fail/stacked_borrows/aliasing_mut2.stderr index c8408c150e779..77a542f45a256 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/aliasing_mut2.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/aliasing_mut2.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID --> $DIR/aliasing_mut2.rs:LL:CC | LL | pub fn safe(_x: &i32, _y: &mut i32) {} - | ^^ not granting access to tag because that would remove [SharedReadOnly for ] which is protected because it is an argument of call ID + | ^^ not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/stacked_borrows/aliasing_mut4.stderr b/src/tools/miri/tests/fail/stacked_borrows/aliasing_mut4.stderr index c53fe70f6dd33..e592b154a7326 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/aliasing_mut4.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/aliasing_mut4.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID --> $DIR/aliasing_mut4.rs:LL:CC | LL | pub fn safe(_x: &i32, _y: &mut Cell) {} - | ^^ not granting access to tag because that would remove [SharedReadOnly for ] which is protected because it is an argument of call ID + | ^^ not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/stacked_borrows/box_noalias_violation.rs b/src/tools/miri/tests/fail/stacked_borrows/box_noalias_violation.rs new file mode 100644 index 0000000000000..2067149b6c87c --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/box_noalias_violation.rs @@ -0,0 +1,14 @@ +unsafe fn test(mut x: Box, y: *const i32) -> i32 { + // We will call this in a way that x and y alias. + *x = 5; + std::mem::forget(x); + *y //~ERROR: weakly protected +} + +fn main() { + unsafe { + let mut v = 42; + let ptr = &mut v as *mut i32; + test(Box::from_raw(ptr), ptr); + } +} diff --git a/src/tools/miri/tests/fail/stacked_borrows/box_noalias_violation.stderr b/src/tools/miri/tests/fail/stacked_borrows/box_noalias_violation.stderr new file mode 100644 index 0000000000000..3c84cbcfd5182 --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/box_noalias_violation.stderr @@ -0,0 +1,30 @@ +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is weakly protected because it is an argument of call ID + --> $DIR/box_noalias_violation.rs:LL:CC + | +LL | *y + | ^^ not granting access to tag because that would remove [Unique for ] which is weakly protected because it is an argument of call ID + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental + = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information +help: was created by a SharedReadWrite retag at offsets [0x0..0x4] + --> $DIR/box_noalias_violation.rs:LL:CC + | +LL | let ptr = &mut v as *mut i32; + | ^^^^^^ +help: is this argument + --> $DIR/box_noalias_violation.rs:LL:CC + | +LL | unsafe fn test(mut x: Box, y: *const i32) -> i32 { + | ^^^^^ + = note: BACKTRACE: + = note: inside `test` at $DIR/box_noalias_violation.rs:LL:CC +note: inside `main` at $DIR/box_noalias_violation.rs:LL:CC + --> $DIR/box_noalias_violation.rs:LL:CC + | +LL | test(Box::from_raw(ptr), ptr); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +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/stacked_borrows/deallocate_against_protector1.rs b/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector1.rs index 9b710424c55c4..4036dce5bebab 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector1.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector1.rs @@ -1,4 +1,4 @@ -//@error-pattern: /deallocating while item \[Unique for .*\] is protected/ +//@error-pattern: /deallocating while item \[Unique for .*\] is strongly protected/ fn inner(x: &mut i32, f: fn(&mut i32)) { // `f` may mutate, but it may not deallocate! diff --git a/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector1.stderr b/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector1.stderr index a5db4a00c69e7..bb3eaec1e85c0 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector1.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector1.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: deallocating while item [Unique for ] is protected by call ID +error: Undefined Behavior: deallocating while item [Unique for ] is strongly protected by call ID --> RUSTLIB/alloc/src/alloc.rs:LL:CC | LL | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ deallocating while item [Unique for ] is protected by call ID + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ deallocating while item [Unique for ] is strongly protected by call ID | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.rs b/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.rs index 36e133e383650..fd67dccd14df6 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.rs @@ -1,4 +1,4 @@ -//@error-pattern: /deallocating while item \[SharedReadWrite for .*\] is protected/ +//@error-pattern: /deallocating while item \[SharedReadWrite for .*\] is strongly protected/ use std::marker::PhantomPinned; pub struct NotUnpin(i32, PhantomPinned); diff --git a/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.stderr b/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.stderr index 99c6ee6eb0743..25bab1aa564a6 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/deallocate_against_protector2.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: deallocating while item [SharedReadWrite for ] is protected by call ID +error: Undefined Behavior: deallocating while item [SharedReadWrite for ] is strongly protected by call ID --> RUSTLIB/alloc/src/alloc.rs:LL:CC | LL | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ deallocating while item [SharedReadWrite for ] is protected by call ID + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ deallocating while item [SharedReadWrite for ] is strongly protected by call ID | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/stacked_borrows/illegal_write6.rs b/src/tools/miri/tests/fail/stacked_borrows/illegal_write6.rs index 448f1493367af..7983b30ea1822 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/illegal_write6.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/illegal_write6.rs @@ -7,6 +7,6 @@ fn main() { fn foo(a: &mut u32, y: *mut u32) -> u32 { *a = 1; let _b = &*a; - unsafe { *y = 2 }; //~ ERROR: /not granting access .* because that would remove .* which is protected/ + unsafe { *y = 2 }; //~ ERROR: /not granting access .* because that would remove .* which is strongly protected/ return *a; } diff --git a/src/tools/miri/tests/fail/stacked_borrows/illegal_write6.stderr b/src/tools/miri/tests/fail/stacked_borrows/illegal_write6.stderr index 331faa89f8604..1a627b8a88300 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/illegal_write6.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/illegal_write6.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID --> $DIR/illegal_write6.rs:LL:CC | LL | unsafe { *y = 2 }; - | ^^^^^^ not granting access to tag because that would remove [Unique for ] which is protected because it is an argument of call ID + | ^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector1.stderr b/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector1.stderr index f87bd2319abd7..1ef36b7ac10fc 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector1.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector1.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID --> $DIR/invalidate_against_protector1.rs:LL:CC | LL | let _val = unsafe { *x }; - | ^^ not granting access to tag because that would remove [Unique for ] which is protected because it is an argument of call ID + | ^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector2.stderr b/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector2.stderr index 07c51a39b825b..941b936e5d724 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector2.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector2.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID --> $DIR/invalidate_against_protector2.rs:LL:CC | LL | unsafe { *x = 0 }; - | ^^^^^^ not granting access to tag because that would remove [SharedReadOnly for ] which is protected because it is an argument of call ID + | ^^^^^^ not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector3.stderr b/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector3.stderr index afda15ea160e2..176a859ee8af7 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector3.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/invalidate_against_protector3.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID --> $DIR/invalidate_against_protector3.rs:LL:CC | LL | unsafe { *x = 0 }; - | ^^^^^^ not granting access to tag because that would remove [SharedReadOnly for ] which is protected because it is an argument of call ID + | ^^^^^^ not granting access to tag because that would remove [SharedReadOnly for ] which is strongly protected because it is an argument of call ID | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/stacked_borrows/newtype_pair_retagging.rs b/src/tools/miri/tests/fail/stacked_borrows/newtype_pair_retagging.rs index cc774500a3c69..c19bcb99cc1ce 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/newtype_pair_retagging.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/newtype_pair_retagging.rs @@ -1,4 +1,4 @@ -//@error-pattern: which is protected +//@error-pattern: which is strongly protected struct Newtype<'a>(&'a mut i32, i32); fn dealloc_while_running(_n: Newtype<'_>, dealloc: impl FnOnce()) { diff --git a/src/tools/miri/tests/fail/stacked_borrows/newtype_pair_retagging.stderr b/src/tools/miri/tests/fail/stacked_borrows/newtype_pair_retagging.stderr index 60a8b2a6260ba..70186dd3a53f3 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/newtype_pair_retagging.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/newtype_pair_retagging.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID --> RUSTLIB/alloc/src/boxed.rs:LL:CC | LL | Box(unsafe { Unique::new_unchecked(raw) }, alloc) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is protected because it is an argument of call ID + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information diff --git a/src/tools/miri/tests/fail/stacked_borrows/newtype_retagging.rs b/src/tools/miri/tests/fail/stacked_borrows/newtype_retagging.rs index 1aa6e240e30f1..2bbe7122ec747 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/newtype_retagging.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/newtype_retagging.rs @@ -1,4 +1,4 @@ -//@error-pattern: which is protected +//@error-pattern: which is strongly protected struct Newtype<'a>(&'a mut i32); fn dealloc_while_running(_n: Newtype<'_>, dealloc: impl FnOnce()) { diff --git a/src/tools/miri/tests/fail/stacked_borrows/newtype_retagging.stderr b/src/tools/miri/tests/fail/stacked_borrows/newtype_retagging.stderr index 06a9b86c6f45a..69fa27c9c096f 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/newtype_retagging.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/newtype_retagging.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is protected because it is an argument of call ID +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID --> RUSTLIB/alloc/src/boxed.rs:LL:CC | LL | Box(unsafe { Unique::new_unchecked(raw) }, alloc) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is protected because it is an argument of call ID + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID | = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information From bf9e73f6d4cca9fb01d74e09bde1fc8be3981d4e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 20 Nov 2022 18:11:30 +0100 Subject: [PATCH 02/31] some things don't need to be mutable --- src/tools/miri/src/stacked_borrows/mod.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/stacked_borrows/mod.rs index 648797211a77a..9ea61ae56236e 100644 --- a/src/tools/miri/src/stacked_borrows/mod.rs +++ b/src/tools/miri/src/stacked_borrows/mod.rs @@ -488,7 +488,7 @@ impl<'tcx> Stack { &mut self, derived_from: ProvenanceExtra, new: Item, - global: &mut GlobalStateInner, + global: &GlobalStateInner, dcx: &mut DiagnosticCx<'_, '_, '_, '_, 'tcx>, exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { @@ -658,9 +658,9 @@ impl Stacks { range.size.bytes() ); let dcx = DiagnosticCxBuilder::read(&mut current_span, threads, tag, range); - let mut state = state.borrow_mut(); + let state = state.borrow(); self.for_each(range, dcx, |stack, dcx, exposed_tags| { - stack.access(AccessKind::Read, tag, &mut state, dcx, exposed_tags) + stack.access(AccessKind::Read, tag, &state, dcx, exposed_tags) }) } @@ -681,9 +681,9 @@ impl Stacks { range.size.bytes() ); let dcx = DiagnosticCxBuilder::write(&mut current_span, threads, tag, range); - let mut state = state.borrow_mut(); + let state = state.borrow(); self.for_each(range, dcx, |stack, dcx, exposed_tags| { - stack.access(AccessKind::Write, tag, &mut state, dcx, exposed_tags) + stack.access(AccessKind::Write, tag, &state, dcx, exposed_tags) }) } @@ -904,7 +904,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' false }; let item = Item::new(new_tag, perm, protected); - let mut global = this.machine.stacked_borrows.as_ref().unwrap().borrow_mut(); + let global = this.machine.stacked_borrows.as_ref().unwrap().borrow(); let dcx = DiagnosticCxBuilder::retag( &mut current_span, // FIXME avoid this `clone` &this.machine.threads, @@ -914,7 +914,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' alloc_range(base_offset, size), ); stacked_borrows.for_each(range, dcx, |stack, dcx, exposed_tags| { - stack.grant(orig_tag, item, &mut global, dcx, exposed_tags) + stack.grant(orig_tag, item, &global, dcx, exposed_tags) }) })?; return Ok(Some(alloc_id)); @@ -932,7 +932,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' .borrow_mut(); let item = Item::new(new_tag, perm, protect.is_some()); let range = alloc_range(base_offset, size); - let mut global = machine.stacked_borrows.as_ref().unwrap().borrow_mut(); + let global = machine.stacked_borrows.as_ref().unwrap().borrow(); // FIXME: can't share this with the current_span inside log_creation let current_span = &mut machine.current_span(); let dcx = DiagnosticCxBuilder::retag( @@ -944,7 +944,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' alloc_range(base_offset, size), ); stacked_borrows.for_each(range, dcx, |stack, dcx, exposed_tags| { - stack.grant(orig_tag, item, &mut global, dcx, exposed_tags) + stack.grant(orig_tag, item, &global, dcx, exposed_tags) })?; Ok(Some(alloc_id)) From 32e9d00585e29d291f9fa7866a00de6d230da8b2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 20 Nov 2022 22:22:38 +0100 Subject: [PATCH 03/31] tweaks --- .../miri/src/stacked_borrows/diagnostics.rs | 2 +- src/tools/miri/src/stacked_borrows/mod.rs | 28 +++++++++++++------ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/tools/miri/src/stacked_borrows/diagnostics.rs b/src/tools/miri/src/stacked_borrows/diagnostics.rs index 323ec3d75f9f8..f307bf11edd36 100644 --- a/src/tools/miri/src/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/stacked_borrows/diagnostics.rs @@ -288,7 +288,7 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir } Operation::Access(AccessOp { kind, range, .. }) => (*range, InvalidationCause::Access(*kind)), - _ => { + Operation::Dealloc(_) => { // This can be reached, but never be relevant later since the entire allocation is // gone now. return; diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/stacked_borrows/mod.rs index 9ea61ae56236e..bc52428910aa0 100644 --- a/src/tools/miri/src/stacked_borrows/mod.rs +++ b/src/tools/miri/src/stacked_borrows/mod.rs @@ -65,7 +65,7 @@ pub struct FrameExtra { /// incremental updates of the global list of protected tags stored in the /// `stacked_borrows::GlobalState` upon function return, and if we attempt to pop a protected /// tag, to identify which call is responsible for protecting the tag. - /// See `Stack::item_popped` for more explanation. + /// See `Stack::item_invalidated` for more explanation. /// /// This will contain one tag per reference passed to the function, so /// a size of 2 is enough for the vast majority of functions. @@ -126,7 +126,7 @@ pub struct GlobalStateInner { /// An item is protected if its tag is in this set, *and* it has the "protected" bit set. /// We add tags to this when they are created with a protector in `reborrow`, and /// we remove tags from this when the call which is protecting them returns, in - /// `GlobalStateInner::end_call`. See `Stack::item_popped` for more details. + /// `GlobalStateInner::end_call`. See `Stack::item_invalidated` for more details. protected_tags: FxHashMap, /// The pointer ids to trace tracked_pointer_tags: FxHashSet, @@ -292,6 +292,13 @@ impl Permission { } } +/// Determines whether an item was invalidated by a conflicting access, or by deallocation. +#[derive(Copy, Clone, Debug)] +enum ItemInvalidationCause { + Conflict, + Dealloc, +} + /// Core per-location operations: access, dealloc, reborrow. impl<'tcx> Stack { /// Find the first write-incompatible item above the given one -- @@ -330,11 +337,11 @@ impl<'tcx> Stack { /// Within `provoking_access, the `AllocRange` refers the entire operation, and /// the `Size` refers to the specific location in the `AllocRange` that we are /// currently checking. - fn item_popped( + fn item_invalidated( item: &Item, global: &GlobalStateInner, dcx: &mut DiagnosticCx<'_, '_, '_, '_, 'tcx>, - deallocation: bool, + cause: ItemInvalidationCause, ) -> InterpResult<'tcx> { if !global.tracked_pointer_tags.is_empty() { dcx.check_tracked_tag_popped(item, global); @@ -358,7 +365,10 @@ impl<'tcx> Stack { // which ends up about linear in the number of protected tags in the program into a // constant time check (and a slow linear, because the tags in the frames aren't contiguous). if let Some(&protector_kind) = global.protected_tags.get(&item.tag()) { - let allowed = deallocation && matches!(protector_kind, ProtectorKind::WeakProtector); + // The only way this is okay is if the protector is weak and we are deallocating with + // the right pointer. + let allowed = matches!(cause, ItemInvalidationCause::Dealloc) + && matches!(protector_kind, ProtectorKind::WeakProtector); if !allowed { return Err(dcx.protector_error(item, protector_kind).into()); } @@ -401,7 +411,7 @@ impl<'tcx> Stack { 0 }; self.pop_items_after(first_incompatible_idx, |item| { - Stack::item_popped(&item, global, dcx, /* deallocation */ false)?; + Stack::item_invalidated(&item, global, dcx, ItemInvalidationCause::Conflict)?; dcx.log_invalidation(item.tag()); Ok(()) })?; @@ -422,7 +432,7 @@ impl<'tcx> Stack { 0 }; self.disable_uniques_starting_at(first_incompatible_idx, |item| { - Stack::item_popped(&item, global, dcx, /* deallocation */ false)?; + Stack::item_invalidated(&item, global, dcx, ItemInvalidationCause::Conflict)?; dcx.log_invalidation(item.tag()); Ok(()) })?; @@ -472,7 +482,7 @@ impl<'tcx> Stack { // Step 2: Pretend we remove the remaining items, checking if any are strongly protected. for idx in (0..self.len()).rev() { let item = self.get(idx).unwrap(); - Stack::item_popped(&item, global, dcx, /* deallocation */ true)?; + Stack::item_invalidated(&item, global, dcx, ItemInvalidationCause::Dealloc)?; } Ok(()) @@ -847,7 +857,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' ); if let Some(protect) = protect { - // See comment in `Stack::item_popped` for why we store the tag twice. + // See comment in `Stack::item_invalidated` for why we store the tag twice. this.frame_mut().extra.stacked_borrows.as_mut().unwrap().protected_tags.push(new_tag); this.machine .stacked_borrows From 23270ae8d39ae15020476f92ec9f4e05960f5de0 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Thu, 3 Nov 2022 00:09:00 -0400 Subject: [PATCH 04/31] Incrementally track which frame to use for diagnostics --- src/tools/miri/src/concurrency/thread.rs | 53 +++++++++++- src/tools/miri/src/helpers.rs | 74 +++++------------ src/tools/miri/src/lib.rs | 2 +- src/tools/miri/src/machine.rs | 41 +++++---- .../miri/src/stacked_borrows/diagnostics.rs | 83 +++++++------------ src/tools/miri/src/stacked_borrows/mod.rs | 55 +++++------- 6 files changed, 148 insertions(+), 160 deletions(-) diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index 8fbee9a352294..96d988eb9a3bb 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -118,6 +118,13 @@ pub struct Thread<'mir, 'tcx> { /// The virtual call stack. stack: Vec>>, + /// The index of the topmost user-relevant frame in `stack`. This field must contain + /// the value produced by `get_top_user_relevant_frame`. + /// The `None` state here represents + /// This field is a cache to reduce how often we call that method. The cache is manually + /// maintained inside `MiriMachine::after_stack_push` and `MiriMachine::after_stack_pop`. + top_user_relevant_frame: Option, + /// The join status. join_status: ThreadJoinStatus, @@ -147,6 +154,38 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> { fn thread_name(&self) -> &[u8] { if let Some(ref thread_name) = self.thread_name { thread_name } else { b"" } } + + /// Return the top user-relevant frame, if there is one. + /// Note that the choice to return `None` here when there is no user-relevant frame is part of + /// justifying the optimization that only pushes of user-relevant frames require updating the + /// `top_user_relevant_frame` field. + fn compute_top_user_relevant_frame(&self) -> Option { + self.stack + .iter() + .enumerate() + .rev() + .find_map(|(idx, frame)| if frame.extra.is_user_relevant { Some(idx) } else { None }) + } + + /// Re-compute the top user-relevant frame from scratch. + pub fn recompute_top_user_relevant_frame(&mut self) { + self.top_user_relevant_frame = self.compute_top_user_relevant_frame(); + } + + /// Set the top user-relevant frame to the given value. Must be equal to what + /// `get_top_user_relevant_frame` would return! + pub fn set_top_user_relevant_frame(&mut self, frame_idx: usize) { + debug_assert_eq!(Some(frame_idx), self.compute_top_user_relevant_frame()); + self.top_user_relevant_frame = Some(frame_idx); + } + + pub fn top_user_relevant_frame(&self) -> usize { + debug_assert_eq!(self.top_user_relevant_frame, self.compute_top_user_relevant_frame()); + // This can be called upon creation of an allocation. We create allocations while setting up + // parts of the Rust runtime when we do not have any stack frames yet, so we need to handle + // empty stacks. + self.top_user_relevant_frame.unwrap_or_else(|| self.stack.len().saturating_sub(1)) + } } impl<'mir, 'tcx> std::fmt::Debug for Thread<'mir, 'tcx> { @@ -167,6 +206,7 @@ impl<'mir, 'tcx> Default for Thread<'mir, 'tcx> { state: ThreadState::Enabled, thread_name: None, stack: Vec::new(), + top_user_relevant_frame: None, join_status: ThreadJoinStatus::Joinable, panic_payload: None, last_error: None, @@ -184,8 +224,15 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> { impl VisitTags for Thread<'_, '_> { fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { - let Thread { panic_payload, last_error, stack, state: _, thread_name: _, join_status: _ } = - self; + let Thread { + panic_payload, + last_error, + stack, + top_user_relevant_frame: _, + state: _, + thread_name: _, + join_status: _, + } = self; panic_payload.visit_tags(visit); last_error.visit_tags(visit); @@ -414,7 +461,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> { } /// Get a shared borrow of the currently active thread. - fn active_thread_ref(&self) -> &Thread<'mir, 'tcx> { + pub fn active_thread_ref(&self) -> &Thread<'mir, 'tcx> { &self.threads[self.active_thread] } diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index cd5e989b43482..958ed50496b1f 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -936,31 +936,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { - pub fn current_span(&self) -> CurrentSpan<'_, 'mir, 'tcx> { - CurrentSpan { current_frame_idx: None, machine: self } - } -} - -/// A `CurrentSpan` should be created infrequently (ideally once) per interpreter step. It does -/// nothing on creation, but when `CurrentSpan::get` is called, searches the current stack for the -/// topmost frame which corresponds to a local crate, and returns the current span in that frame. -/// The result of that search is cached so that later calls are approximately free. -#[derive(Clone)] -pub struct CurrentSpan<'a, 'mir, 'tcx> { - current_frame_idx: Option, - machine: &'a MiriMachine<'mir, 'tcx>, -} - -impl<'a, 'mir: 'a, 'tcx: 'a + 'mir> CurrentSpan<'a, 'mir, 'tcx> { - pub fn machine(&self) -> &'a MiriMachine<'mir, 'tcx> { - self.machine - } - - /// Get the current span, skipping non-local frames. + /// Get the current span in the topmost function which is workspace-local and not + /// `#[track_caller]`. /// This function is backed by a cache, and can be assumed to be very fast. - pub fn get(&mut self) -> Span { - let idx = self.current_frame_idx(); - self.stack().get(idx).map(Frame::current_span).unwrap_or(rustc_span::DUMMY_SP) + pub fn current_span(&self) -> Span { + self.stack() + .get(self.top_user_relevant_frame()) + .map(Frame::current_span) + .unwrap_or(rustc_span::DUMMY_SP) } /// Returns the span of the *caller* of the current operation, again @@ -968,46 +951,27 @@ impl<'a, 'mir: 'a, 'tcx: 'a + 'mir> CurrentSpan<'a, 'mir, 'tcx> { /// current operation is not in a local crate. /// This is useful when we are processing something which occurs on function-entry and we want /// to point at the call to the function, not the function definition generally. - pub fn get_caller(&mut self) -> Span { + pub fn caller_span(&self) -> Span { // We need to go down at least to the caller (len - 2), or however - // far we have to go to find a frame in a local crate. - let local_frame_idx = self.current_frame_idx(); + // far we have to go to find a frame in a local crate which is also not #[track_caller]. + let frame_idx = self.top_user_relevant_frame(); let stack = self.stack(); - let idx = cmp::min(local_frame_idx, stack.len().saturating_sub(2)); - stack.get(idx).map(Frame::current_span).unwrap_or(rustc_span::DUMMY_SP) + let frame_idx = cmp::min(frame_idx, stack.len().saturating_sub(2)); + stack.get(frame_idx).map(Frame::current_span).unwrap_or(rustc_span::DUMMY_SP) } fn stack(&self) -> &[Frame<'mir, 'tcx, Provenance, machine::FrameData<'tcx>>] { - self.machine.threads.active_thread_stack() + self.threads.active_thread_stack() } - fn current_frame_idx(&mut self) -> usize { - *self - .current_frame_idx - .get_or_insert_with(|| Self::compute_current_frame_index(self.machine)) + fn top_user_relevant_frame(&self) -> usize { + self.threads.active_thread_ref().top_user_relevant_frame() } - // Find the position of the inner-most frame which is part of the crate being - // compiled/executed, part of the Cargo workspace, and is also not #[track_caller]. - #[inline(never)] - fn compute_current_frame_index(machine: &MiriMachine<'_, '_>) -> usize { - machine - .threads - .active_thread_stack() - .iter() - .enumerate() - .rev() - .find_map(|(idx, frame)| { - let def_id = frame.instance.def_id(); - if (def_id.is_local() || machine.local_crates.contains(&def_id.krate)) - && !frame.instance.def.requires_caller_location(machine.tcx) - { - Some(idx) - } else { - None - } - }) - .unwrap_or(0) + pub fn is_user_relevant(&self, frame: &Frame<'mir, 'tcx, Provenance>) -> bool { + let def_id = frame.instance.def_id(); + (def_id.is_local() || self.local_crates.contains(&def_id.krate)) + && !frame.instance.def.requires_caller_location(self.tcx) } } diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 66df0d737c087..8913f8aa10fcd 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -97,7 +97,7 @@ pub use crate::diagnostics::{ pub use crate::eval::{ create_ecx, eval_entry, AlignmentCheck, BacktraceStyle, IsolatedOp, MiriConfig, RejectOpWith, }; -pub use crate::helpers::{CurrentSpan, EvalContextExt as _}; +pub use crate::helpers::EvalContextExt as _; pub use crate::intptrcast::ProvenanceMode; pub use crate::machine::{ AllocExtra, FrameData, MiriInterpCx, MiriInterpCxExt, MiriMachine, MiriMemoryKind, diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 8243ccd90a366..f5fa8f5f09b46 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -50,12 +50,18 @@ pub struct FrameData<'tcx> { /// for the start of this frame. When we finish executing this frame, /// we use this to register a completed event with `measureme`. pub timing: Option, + + /// Indicates whether a `Frame` is part of a workspace-local crate and is also not + /// `#[track_caller]`. We compute this once on creation and store the result, as an + /// optimization. + /// This is used by `MiriMachine::current_span` and `MiriMachine::caller_span` + pub is_user_relevant: bool, } impl<'tcx> std::fmt::Debug for FrameData<'tcx> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { // Omitting `timing`, it does not support `Debug`. - let FrameData { stacked_borrows, catch_unwind, timing: _ } = self; + let FrameData { stacked_borrows, catch_unwind, timing: _, is_user_relevant: _ } = self; f.debug_struct("FrameData") .field("stacked_borrows", stacked_borrows) .field("catch_unwind", catch_unwind) @@ -65,7 +71,7 @@ impl<'tcx> std::fmt::Debug for FrameData<'tcx> { impl VisitTags for FrameData<'_> { fn visit_tags(&self, visit: &mut dyn FnMut(SbTag)) { - let FrameData { catch_unwind, stacked_borrows, timing: _ } = self; + let FrameData { catch_unwind, stacked_borrows, timing: _, is_user_relevant: _ } = self; catch_unwind.visit_tags(visit); stacked_borrows.visit_tags(visit); @@ -895,13 +901,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { let alloc = alloc.into_owned(); let stacks = ecx.machine.stacked_borrows.as_ref().map(|stacked_borrows| { - Stacks::new_allocation( - id, - alloc.size(), - stacked_borrows, - kind, - ecx.machine.current_span(), - ) + Stacks::new_allocation(id, alloc.size(), stacked_borrows, kind, &ecx.machine) }); let race_alloc = ecx.machine.data_race.as_ref().map(|data_race| { data_race::AllocExtra::new_allocation( @@ -1016,8 +1016,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { prov_extra, range, machine.stacked_borrows.as_ref().unwrap(), - machine.current_span(), - &machine.threads, + machine, )?; } if let Some(weak_memory) = &alloc_extra.weak_memory { @@ -1048,8 +1047,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { prov_extra, range, machine.stacked_borrows.as_ref().unwrap(), - machine.current_span(), - &machine.threads, + machine, )?; } if let Some(weak_memory) = &alloc_extra.weak_memory { @@ -1083,8 +1081,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { prove_extra, range, machine.stacked_borrows.as_ref().unwrap(), - machine.current_span(), - &machine.threads, + machine, ) } else { Ok(()) @@ -1126,7 +1123,9 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { stacked_borrows: stacked_borrows.map(|sb| sb.borrow_mut().new_frame(&ecx.machine)), catch_unwind: None, timing, + is_user_relevant: ecx.machine.is_user_relevant(&frame), }; + Ok(frame.with_extra(extra)) } @@ -1174,6 +1173,13 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { #[inline(always)] fn after_stack_push(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { + if ecx.frame().extra.is_user_relevant { + // We just pushed a local frame, so we know that the topmost local frame is the topmost + // frame. If we push a non-local frame, there's no need to do anything. + let stack_len = ecx.active_thread_stack().len(); + ecx.active_thread_mut().set_top_user_relevant_frame(stack_len - 1); + } + if ecx.machine.stacked_borrows.is_some() { ecx.retag_return_place() } else { Ok(()) } } @@ -1183,6 +1189,11 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { mut frame: Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>, unwinding: bool, ) -> InterpResult<'tcx, StackPopJump> { + if frame.extra.is_user_relevant { + // All that we store is whether or not the frame we just removed is local, so now we + // have no idea where the next topmost local frame is. So we recompute it. + ecx.active_thread_mut().recompute_top_user_relevant_frame(); + } let timing = frame.extra.timing.take(); if let Some(stacked_borrows) = &ecx.machine.stacked_borrows { stacked_borrows.borrow_mut().end_call(&frame.extra); diff --git a/src/tools/miri/src/stacked_borrows/diagnostics.rs b/src/tools/miri/src/stacked_borrows/diagnostics.rs index f307bf11edd36..662d8ada735eb 100644 --- a/src/tools/miri/src/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/stacked_borrows/diagnostics.rs @@ -5,7 +5,6 @@ use rustc_middle::mir::interpret::{alloc_range, AllocId, AllocRange}; use rustc_span::{Span, SpanData}; use rustc_target::abi::Size; -use crate::helpers::CurrentSpan; use crate::stacked_borrows::{err_sb_ub, AccessKind, GlobalStateInner, Permission, ProtectorKind}; use crate::*; @@ -110,42 +109,29 @@ pub struct TagHistory { pub protected: Option<(String, SpanData)>, } -pub struct DiagnosticCxBuilder<'span, 'ecx, 'mir, 'tcx> { +pub struct DiagnosticCxBuilder<'ecx, 'mir, 'tcx> { operation: Operation, - // 'span cannot be merged with any other lifetime since they appear invariantly, under the - // mutable ref. - current_span: &'span mut CurrentSpan<'ecx, 'mir, 'tcx>, - threads: &'ecx ThreadManager<'mir, 'tcx>, + machine: &'ecx MiriMachine<'mir, 'tcx>, } -pub struct DiagnosticCx<'span, 'history, 'ecx, 'mir, 'tcx> { +pub struct DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { operation: Operation, - // 'span and 'history cannot be merged, since when we call `unbuild` we need - // to return the exact 'span that was used when calling `build`. - current_span: &'span mut CurrentSpan<'ecx, 'mir, 'tcx>, - threads: &'ecx ThreadManager<'mir, 'tcx>, + machine: &'ecx MiriMachine<'mir, 'tcx>, history: &'history mut AllocHistory, offset: Size, } -impl<'span, 'ecx, 'mir, 'tcx> DiagnosticCxBuilder<'span, 'ecx, 'mir, 'tcx> { +impl<'ecx, 'mir, 'tcx> DiagnosticCxBuilder<'ecx, 'mir, 'tcx> { pub fn build<'history>( self, history: &'history mut AllocHistory, offset: Size, - ) -> DiagnosticCx<'span, 'history, 'ecx, 'mir, 'tcx> { - DiagnosticCx { - operation: self.operation, - current_span: self.current_span, - threads: self.threads, - history, - offset, - } + ) -> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { + DiagnosticCx { operation: self.operation, machine: self.machine, history, offset } } pub fn retag( - current_span: &'span mut CurrentSpan<'ecx, 'mir, 'tcx>, - threads: &'ecx ThreadManager<'mir, 'tcx>, + machine: &'ecx MiriMachine<'mir, 'tcx>, cause: RetagCause, new_tag: SbTag, orig_tag: ProvenanceExtra, @@ -154,46 +140,36 @@ impl<'span, 'ecx, 'mir, 'tcx> DiagnosticCxBuilder<'span, 'ecx, 'mir, 'tcx> { let operation = Operation::Retag(RetagOp { cause, new_tag, orig_tag, range, permission: None }); - DiagnosticCxBuilder { current_span, threads, operation } + DiagnosticCxBuilder { machine, operation } } pub fn read( - current_span: &'span mut CurrentSpan<'ecx, 'mir, 'tcx>, - threads: &'ecx ThreadManager<'mir, 'tcx>, + machine: &'ecx MiriMachine<'mir, 'tcx>, tag: ProvenanceExtra, range: AllocRange, ) -> Self { let operation = Operation::Access(AccessOp { kind: AccessKind::Read, tag, range }); - DiagnosticCxBuilder { current_span, threads, operation } + DiagnosticCxBuilder { machine, operation } } pub fn write( - current_span: &'span mut CurrentSpan<'ecx, 'mir, 'tcx>, - threads: &'ecx ThreadManager<'mir, 'tcx>, + machine: &'ecx MiriMachine<'mir, 'tcx>, tag: ProvenanceExtra, range: AllocRange, ) -> Self { let operation = Operation::Access(AccessOp { kind: AccessKind::Write, tag, range }); - DiagnosticCxBuilder { current_span, threads, operation } + DiagnosticCxBuilder { machine, operation } } - pub fn dealloc( - current_span: &'span mut CurrentSpan<'ecx, 'mir, 'tcx>, - threads: &'ecx ThreadManager<'mir, 'tcx>, - tag: ProvenanceExtra, - ) -> Self { + pub fn dealloc(machine: &'ecx MiriMachine<'mir, 'tcx>, tag: ProvenanceExtra) -> Self { let operation = Operation::Dealloc(DeallocOp { tag }); - DiagnosticCxBuilder { current_span, threads, operation } + DiagnosticCxBuilder { machine, operation } } } -impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir, 'tcx> { - pub fn unbuild(self) -> DiagnosticCxBuilder<'span, 'ecx, 'mir, 'tcx> { - DiagnosticCxBuilder { - operation: self.operation, - current_span: self.current_span, - threads: self.threads, - } +impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { + pub fn unbuild(self) -> DiagnosticCxBuilder<'ecx, 'mir, 'tcx> { + DiagnosticCxBuilder { machine: self.machine, operation: self.operation } } } @@ -234,10 +210,10 @@ struct DeallocOp { } impl AllocHistory { - pub fn new(id: AllocId, item: Item, current_span: &mut CurrentSpan<'_, '_, '_>) -> Self { + pub fn new(id: AllocId, item: Item, machine: &MiriMachine<'_, '_>) -> Self { Self { id, - base: (item, current_span.get()), + base: (item, machine.current_span()), creations: SmallVec::new(), invalidations: SmallVec::new(), protectors: SmallVec::new(), @@ -245,7 +221,7 @@ impl AllocHistory { } } -impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir, 'tcx> { +impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { pub fn start_grant(&mut self, perm: Permission) { let Operation::Retag(op) = &mut self.operation else { unreachable!("start_grant must only be called during a retag, this is: {:?}", self.operation) @@ -274,15 +250,17 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir let Operation::Retag(op) = &self.operation else { unreachable!("log_creation must only be called during a retag") }; - self.history.creations.push(Creation { retag: op.clone(), span: self.current_span.get() }); + self.history + .creations + .push(Creation { retag: op.clone(), span: self.machine.current_span() }); } pub fn log_invalidation(&mut self, tag: SbTag) { - let mut span = self.current_span.get(); + let mut span = self.machine.current_span(); let (range, cause) = match &self.operation { Operation::Retag(RetagOp { cause, range, permission, .. }) => { if *cause == RetagCause::FnEntry { - span = self.current_span.get_caller(); + span = self.machine.caller_span(); } (*range, InvalidationCause::Retag(permission.unwrap(), *cause)) } @@ -301,7 +279,9 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir let Operation::Retag(op) = &self.operation else { unreachable!("Protectors can only be created during a retag") }; - self.history.protectors.push(Protection { tag: op.new_tag, span: self.current_span.get() }); + self.history + .protectors + .push(Protection { tag: op.new_tag, span: self.machine.current_span() }); } pub fn get_logs_relevant_to( @@ -418,6 +398,7 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir ProtectorKind::StrongProtector => "strongly protected", }; let call_id = self + .machine .threads .all_stacks() .flatten() @@ -482,9 +463,7 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir Some((orig_tag, kind)) } }; - self.current_span - .machine() - .emit_diagnostic(NonHaltingDiagnostic::PoppedPointerTag(*item, summary)); + self.machine.emit_diagnostic(NonHaltingDiagnostic::PoppedPointerTag(*item, summary)); } } diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/stacked_borrows/mod.rs index bc52428910aa0..e49b3e4f7246c 100644 --- a/src/tools/miri/src/stacked_borrows/mod.rs +++ b/src/tools/miri/src/stacked_borrows/mod.rs @@ -340,7 +340,7 @@ impl<'tcx> Stack { fn item_invalidated( item: &Item, global: &GlobalStateInner, - dcx: &mut DiagnosticCx<'_, '_, '_, '_, 'tcx>, + dcx: &mut DiagnosticCx<'_, '_, '_, 'tcx>, cause: ItemInvalidationCause, ) -> InterpResult<'tcx> { if !global.tracked_pointer_tags.is_empty() { @@ -385,7 +385,7 @@ impl<'tcx> Stack { access: AccessKind, tag: ProvenanceExtra, global: &GlobalStateInner, - dcx: &mut DiagnosticCx<'_, '_, '_, '_, 'tcx>, + dcx: &mut DiagnosticCx<'_, '_, '_, 'tcx>, exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { // Two main steps: Find granting item, remove incompatible items above. @@ -471,7 +471,7 @@ impl<'tcx> Stack { &mut self, tag: ProvenanceExtra, global: &GlobalStateInner, - dcx: &mut DiagnosticCx<'_, '_, '_, '_, 'tcx>, + dcx: &mut DiagnosticCx<'_, '_, '_, 'tcx>, exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { // Step 1: Make a write access. @@ -499,7 +499,7 @@ impl<'tcx> Stack { derived_from: ProvenanceExtra, new: Item, global: &GlobalStateInner, - dcx: &mut DiagnosticCx<'_, '_, '_, '_, 'tcx>, + dcx: &mut DiagnosticCx<'_, '_, '_, 'tcx>, exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { dcx.start_grant(new.perm()); @@ -590,14 +590,14 @@ impl<'tcx> Stacks { perm: Permission, tag: SbTag, id: AllocId, - current_span: &mut CurrentSpan<'_, '_, '_>, + machine: &MiriMachine<'_, '_>, ) -> Self { let item = Item::new(tag, perm, false); let stack = Stack::new(item); Stacks { stacks: RangeMap::new(size, stack), - history: AllocHistory::new(id, item, current_span), + history: AllocHistory::new(id, item, machine), exposed_tags: FxHashSet::default(), modified_since_last_gc: false, } @@ -607,10 +607,10 @@ impl<'tcx> Stacks { fn for_each( &mut self, range: AllocRange, - mut dcx_builder: DiagnosticCxBuilder<'_, '_, '_, 'tcx>, + mut dcx_builder: DiagnosticCxBuilder<'_, '_, 'tcx>, mut f: impl FnMut( &mut Stack, - &mut DiagnosticCx<'_, '_, '_, '_, 'tcx>, + &mut DiagnosticCx<'_, '_, '_, 'tcx>, &mut FxHashSet, ) -> InterpResult<'tcx>, ) -> InterpResult<'tcx> { @@ -631,7 +631,7 @@ impl Stacks { size: Size, state: &GlobalState, kind: MemoryKind, - mut current_span: CurrentSpan<'_, '_, '_>, + machine: &MiriMachine<'_, '_>, ) -> Self { let mut extra = state.borrow_mut(); let (base_tag, perm) = match kind { @@ -640,12 +640,11 @@ impl Stacks { // not through a pointer). That is, whenever we directly write to a local, this will pop // everything else off the stack, invalidating all previous pointers, // and in particular, *all* raw pointers. - MemoryKind::Stack => - (extra.base_ptr_tag(id, current_span.machine()), Permission::Unique), + MemoryKind::Stack => (extra.base_ptr_tag(id, machine), Permission::Unique), // Everything else is shared by default. - _ => (extra.base_ptr_tag(id, current_span.machine()), Permission::SharedReadWrite), + _ => (extra.base_ptr_tag(id, machine), Permission::SharedReadWrite), }; - Stacks::new(size, perm, base_tag, id, &mut current_span) + Stacks::new(size, perm, base_tag, id, machine) } #[inline(always)] @@ -655,8 +654,7 @@ impl Stacks { tag: ProvenanceExtra, range: AllocRange, state: &GlobalState, - mut current_span: CurrentSpan<'ecx, 'mir, 'tcx>, - threads: &'ecx ThreadManager<'mir, 'tcx>, + machine: &'ecx MiriMachine<'mir, 'tcx>, ) -> InterpResult<'tcx> where 'tcx: 'ecx, @@ -667,7 +665,7 @@ impl Stacks { Pointer::new(alloc_id, range.start), range.size.bytes() ); - let dcx = DiagnosticCxBuilder::read(&mut current_span, threads, tag, range); + let dcx = DiagnosticCxBuilder::read(machine, tag, range); let state = state.borrow(); self.for_each(range, dcx, |stack, dcx, exposed_tags| { stack.access(AccessKind::Read, tag, &state, dcx, exposed_tags) @@ -681,8 +679,7 @@ impl Stacks { tag: ProvenanceExtra, range: AllocRange, state: &GlobalState, - mut current_span: CurrentSpan<'ecx, 'mir, 'tcx>, - threads: &'ecx ThreadManager<'mir, 'tcx>, + machine: &'ecx MiriMachine<'mir, 'tcx>, ) -> InterpResult<'tcx> { trace!( "write access with tag {:?}: {:?}, size {}", @@ -690,7 +687,7 @@ impl Stacks { Pointer::new(alloc_id, range.start), range.size.bytes() ); - let dcx = DiagnosticCxBuilder::write(&mut current_span, threads, tag, range); + let dcx = DiagnosticCxBuilder::write(machine, tag, range); let state = state.borrow(); self.for_each(range, dcx, |stack, dcx, exposed_tags| { stack.access(AccessKind::Write, tag, &state, dcx, exposed_tags) @@ -704,11 +701,10 @@ impl Stacks { tag: ProvenanceExtra, range: AllocRange, state: &GlobalState, - mut current_span: CurrentSpan<'ecx, 'mir, 'tcx>, - threads: &'ecx ThreadManager<'mir, 'tcx>, + machine: &'ecx MiriMachine<'mir, 'tcx>, ) -> InterpResult<'tcx> { trace!("deallocation with tag {:?}: {:?}, size {}", tag, alloc_id, range.size.bytes()); - let dcx = DiagnosticCxBuilder::dealloc(&mut current_span, threads, tag); + let dcx = DiagnosticCxBuilder::dealloc(machine, tag); let state = state.borrow(); self.for_each(range, dcx, |stack, dcx, exposed_tags| { stack.dealloc(tag, &state, dcx, exposed_tags) @@ -773,7 +769,6 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let (_size, _align, alloc_kind) = this.get_alloc_info(alloc_id); match alloc_kind { AllocKind::LiveData => { - let current_span = &mut this.machine.current_span(); // This should have alloc_extra data, but `get_alloc_extra` can still fail // if converting this alloc_id from a global to a local one // uncovers a non-supported `extern static`. @@ -783,12 +778,10 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' .as_ref() .expect("we should have Stacked Borrows data") .borrow_mut(); - let threads = &this.machine.threads; // Note that we create a *second* `DiagnosticCxBuilder` below for the actual retag. // FIXME: can this be done cleaner? let dcx = DiagnosticCxBuilder::retag( - current_span, - threads, + &this.machine, retag_cause, new_tag, orig_tag, @@ -895,8 +888,6 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' .as_ref() .expect("we should have Stacked Borrows data") .borrow_mut(); - // FIXME: can't share this with the current_span inside log_creation - let mut current_span = this.machine.current_span(); this.visit_freeze_sensitive(place, size, |mut range, frozen| { // Adjust range. range.start += base_offset; @@ -916,8 +907,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let item = Item::new(new_tag, perm, protected); let global = this.machine.stacked_borrows.as_ref().unwrap().borrow(); let dcx = DiagnosticCxBuilder::retag( - &mut current_span, // FIXME avoid this `clone` - &this.machine.threads, + &this.machine, retag_cause, new_tag, orig_tag, @@ -943,11 +933,8 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let item = Item::new(new_tag, perm, protect.is_some()); let range = alloc_range(base_offset, size); let global = machine.stacked_borrows.as_ref().unwrap().borrow(); - // FIXME: can't share this with the current_span inside log_creation - let current_span = &mut machine.current_span(); let dcx = DiagnosticCxBuilder::retag( - current_span, - &machine.threads, + machine, retag_cause, new_tag, orig_tag, From 859310a3708b9e7708f1b01fed42e8834917bfa7 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 22 Nov 2022 11:52:35 +0100 Subject: [PATCH 05/31] ensure current getrandom works with strict provenance --- src/tools/miri/test_dependencies/Cargo.toml | 2 +- src/tools/miri/tests/pass-dep/getrandom_1.rs | 8 ++++++++ src/tools/miri/tests/pass-dep/random.rs | 8 +++----- ...t-isolation.rs => libc-getrandom-without-isolation.rs} | 0 .../shims/{linux-getrandom.rs => libc-getrandom.rs} | 0 5 files changed, 12 insertions(+), 6 deletions(-) create mode 100644 src/tools/miri/tests/pass-dep/getrandom_1.rs rename src/tools/miri/tests/pass-dep/shims/{linux-getrandom-without-isolation.rs => libc-getrandom-without-isolation.rs} (100%) rename src/tools/miri/tests/pass-dep/shims/{linux-getrandom.rs => libc-getrandom.rs} (100%) diff --git a/src/tools/miri/test_dependencies/Cargo.toml b/src/tools/miri/test_dependencies/Cargo.toml index 58f731f91d0f4..d8c70935fb144 100644 --- a/src/tools/miri/test_dependencies/Cargo.toml +++ b/src/tools/miri/test_dependencies/Cargo.toml @@ -15,7 +15,7 @@ page_size = "0.4.1" num_cpus = "1.10.1" getrandom_1 = { package = "getrandom", version = "0.1" } -getrandom_2 = { package = "getrandom", version = "0.2" } +getrandom = { version = "0.2" } rand = { version = "0.8", features = ["small_rng"] } [workspace] diff --git a/src/tools/miri/tests/pass-dep/getrandom_1.rs b/src/tools/miri/tests/pass-dep/getrandom_1.rs new file mode 100644 index 0000000000000..2c7bd93fbdb38 --- /dev/null +++ b/src/tools/miri/tests/pass-dep/getrandom_1.rs @@ -0,0 +1,8 @@ +// mac-os `getrandom_1` does some pointer shenanigans +//@compile-flags: -Zmiri-permissive-provenance + +/// Test old version of `getrandom`. +fn main() { + let mut data = vec![0; 16]; + getrandom_1::getrandom(&mut data).unwrap(); +} diff --git a/src/tools/miri/tests/pass-dep/random.rs b/src/tools/miri/tests/pass-dep/random.rs index 5eccf3b0ea115..0cd8b06d63d8a 100644 --- a/src/tools/miri/tests/pass-dep/random.rs +++ b/src/tools/miri/tests/pass-dep/random.rs @@ -1,12 +1,10 @@ -// mac-os `getrandom_1` does some pointer shenanigans -//@compile-flags: -Zmiri-permissive-provenance +//@compile-flags: -Zmiri-strict-provenance use rand::{rngs::SmallRng, Rng, SeedableRng}; fn main() { - // Test `getrandom` directly (in multiple different versions). + // Test `getrandom` directly. let mut data = vec![0; 16]; - getrandom_1::getrandom(&mut data).unwrap(); - getrandom_2::getrandom(&mut data).unwrap(); + getrandom::getrandom(&mut data).unwrap(); // Try seeding with "real" entropy. let mut rng = SmallRng::from_entropy(); diff --git a/src/tools/miri/tests/pass-dep/shims/linux-getrandom-without-isolation.rs b/src/tools/miri/tests/pass-dep/shims/libc-getrandom-without-isolation.rs similarity index 100% rename from src/tools/miri/tests/pass-dep/shims/linux-getrandom-without-isolation.rs rename to src/tools/miri/tests/pass-dep/shims/libc-getrandom-without-isolation.rs diff --git a/src/tools/miri/tests/pass-dep/shims/linux-getrandom.rs b/src/tools/miri/tests/pass-dep/shims/libc-getrandom.rs similarity index 100% rename from src/tools/miri/tests/pass-dep/shims/linux-getrandom.rs rename to src/tools/miri/tests/pass-dep/shims/libc-getrandom.rs From 8061e2fd53b29391e47347d928fb35f31625c3b0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 22 Nov 2022 11:54:04 +0100 Subject: [PATCH 06/31] update test_dependencies --- src/tools/miri/test_dependencies/Cargo.lock | 94 +++++++++++-------- src/tools/miri/test_dependencies/Cargo.toml | 2 +- .../miri/tests/fail/crates/tokio_mvp.stderr | 4 +- 3 files changed, 57 insertions(+), 43 deletions(-) diff --git a/src/tools/miri/test_dependencies/Cargo.lock b/src/tools/miri/test_dependencies/Cargo.lock index 78cf9a8d51bcb..c728e7c0778db 100644 --- a/src/tools/miri/test_dependencies/Cargo.lock +++ b/src/tools/miri/test_dependencies/Cargo.lock @@ -16,9 +16,9 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bytes" -version = "1.2.1" +version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec8a7b6a70fde80372154c65702f00a0f56f3e1c36abbc6c440484be248856db" +checksum = "dfb24e866b15a1af2a1b663f10c6b6b8f397a84aadb828f12e5b289ec23a3a3c" [[package]] name = "cfg-if" @@ -39,9 +39,9 @@ dependencies = [ [[package]] name = "getrandom" -version = "0.2.7" +version = "0.2.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4eb1a864a501629691edf6c15a593b7a51eebaa1e8468e9ddc623de7c9b58ec6" +checksum = "c05aeb6a22b8f62540c194aac980f2115af067bfe15a0734d7277a768d396b31" dependencies = [ "cfg-if", "libc", @@ -59,9 +59,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.133" +version = "0.2.137" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c0f80d65747a3e43d1596c7c5492d95d5edddaabd45a7fcdb02b95f644164966" +checksum = "fc7fcc620a3bff7cdd7a365be3376c97191aeaccc2a603e600951e452615bf89" [[package]] name = "lock_api" @@ -90,9 +90,9 @@ checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" [[package]] name = "mio" -version = "0.8.4" +version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57ee1c23c7c63b0c9250c339ffdc69255f110b298b901b9f6c82547b7b87caaf" +checksum = "e5d732bc30207a6423068df043e3d02e0735b155ad7ce1a6f76fe2baa5b158de" dependencies = [ "libc", "log", @@ -105,7 +105,7 @@ name = "miri-test-deps" version = "0.1.0" dependencies = [ "getrandom 0.1.16", - "getrandom 0.2.7", + "getrandom 0.2.8", "libc", "num_cpus", "page_size", @@ -115,9 +115,9 @@ dependencies = [ [[package]] name = "num_cpus" -version = "1.13.1" +version = "1.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "19e64526ebdee182341572e50e9ad03965aa510cd94427a4549448f285e957a1" +checksum = "f6058e64324c71e02bc2b150e4f3bc8286db6c83092132ffa3f6b1eab0f9def5" dependencies = [ "hermit-abi", "libc", @@ -125,9 +125,9 @@ dependencies = [ [[package]] name = "page_size" -version = "0.4.2" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eebde548fbbf1ea81a99b128872779c437752fb99f217c45245e1a61dcd9edcd" +checksum = "1b7663cbd190cfd818d08efa8497f6cd383076688c49a391ef7c0d03cd12b561" dependencies = [ "libc", "winapi", @@ -145,9 +145,9 @@ dependencies = [ [[package]] name = "parking_lot_core" -version = "0.9.3" +version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "09a279cbf25cb0757810394fbc1e359949b59e348145c643a939a525692e6929" +checksum = "4dc9e0dc2adc1c69d09143aff38d3d30c5c3f0df0dad82e6d25547af174ebec0" dependencies = [ "cfg-if", "libc", @@ -164,15 +164,15 @@ checksum = "e0a7ae3ac2f1173085d398531c705756c94a4c56843785df85a60c1a0afac116" [[package]] name = "ppv-lite86" -version = "0.2.16" +version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eb9f9e6e233e5c4a35559a617bf40a4ec447db2e84c20b55a6f83167b7e57872" +checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" [[package]] name = "proc-macro2" -version = "1.0.45" +version = "1.0.47" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3edcd08cf4fea98d1ae6c9ddd3b8ccb1acac7c3693d62625969a7daa04a2ae36" +checksum = "5ea3d908b0e36316caf9e9e2c4625cdde190a7e6f440d794667ed17a1855e725" dependencies = [ "unicode-ident", ] @@ -213,7 +213,7 @@ version = "0.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" dependencies = [ - "getrandom 0.2.7", + "getrandom 0.2.8", ] [[package]] @@ -242,9 +242,9 @@ dependencies = [ [[package]] name = "smallvec" -version = "1.9.0" +version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2fd0db749597d91ff862fd1d55ea87f7855a744a8425a64695b6fca237d1dad1" +checksum = "a507befe795404456341dfab10cef66ead4c041f62b8b11bbb92bffe5d0953e0" [[package]] name = "socket2" @@ -258,9 +258,9 @@ dependencies = [ [[package]] name = "syn" -version = "1.0.101" +version = "1.0.103" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e90cde112c4b9690b8cbe810cba9ddd8bc1d7472e2cae317b69e9438c1cba7d2" +checksum = "a864042229133ada95abf3b54fdc62ef5ccabe9515b64717bcb9a1919e59445d" dependencies = [ "proc-macro2", "quote", @@ -269,9 +269,9 @@ dependencies = [ [[package]] name = "tokio" -version = "1.21.2" +version = "1.22.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9e03c497dc955702ba729190dc4aac6f2a0ce97f913e5b1b5912fc5039d9099" +checksum = "d76ce4a75fb488c605c54bf610f221cea8b0dafb53333c1a67e8ee199dcd2ae3" dependencies = [ "autocfg", "bytes", @@ -300,9 +300,9 @@ dependencies = [ [[package]] name = "unicode-ident" -version = "1.0.4" +version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dcc811dc4066ac62f84f11307873c4850cb653bfa9b1719cee2bd2204a4bc5dd" +checksum = "6ceab39d59e4c9499d4e5a8ee0e2735b891bb7308ac83dfb4e80cad195c9f6f3" [[package]] name = "wasi" @@ -340,43 +340,57 @@ checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" [[package]] name = "windows-sys" -version = "0.36.1" +version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ea04155a16a59f9eab786fe12a4a450e75cdb175f9e0d80da1e17db09f55b8d2" +checksum = "5a3e1820f08b8513f676f7ab6c1f99ff312fb97b553d30ff4dd86f9f15728aa7" dependencies = [ + "windows_aarch64_gnullvm", "windows_aarch64_msvc", "windows_i686_gnu", "windows_i686_msvc", "windows_x86_64_gnu", + "windows_x86_64_gnullvm", "windows_x86_64_msvc", ] +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41d2aa71f6f0cbe00ae5167d90ef3cfe66527d6f613ca78ac8024c3ccab9a19e" + [[package]] name = "windows_aarch64_msvc" -version = "0.36.1" +version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9bb8c3fd39ade2d67e9874ac4f3db21f0d710bee00fe7cab16949ec184eeaa47" +checksum = "dd0f252f5a35cac83d6311b2e795981f5ee6e67eb1f9a7f64eb4500fbc4dcdb4" [[package]] name = "windows_i686_gnu" -version = "0.36.1" +version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "180e6ccf01daf4c426b846dfc66db1fc518f074baa793aa7d9b9aaeffad6a3b6" +checksum = "fbeae19f6716841636c28d695375df17562ca208b2b7d0dc47635a50ae6c5de7" [[package]] name = "windows_i686_msvc" -version = "0.36.1" +version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2e7917148b2812d1eeafaeb22a97e4813dfa60a3f8f78ebe204bcc88f12f024" +checksum = "84c12f65daa39dd2babe6e442988fc329d6243fdce47d7d2d155b8d874862246" [[package]] name = "windows_x86_64_gnu" -version = "0.36.1" +version = "0.42.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf7b1b21b5362cbc318f686150e5bcea75ecedc74dd157d874d754a2ca44b0ed" + +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4dcd171b8776c41b97521e5da127a2d86ad280114807d0b2ab1e462bc764d9e1" +checksum = "09d525d2ba30eeb3297665bd434a54297e4170c7f1a44cad4ef58095b4cd2028" [[package]] name = "windows_x86_64_msvc" -version = "0.36.1" +version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c811ca4a8c853ef420abd8592ba53ddbbac90410fab6903b3e79972a631f7680" +checksum = "f40009d85759725a34da6d89a94e63d7bdc50a862acf0dbc7c8e488f1edcb6f5" diff --git a/src/tools/miri/test_dependencies/Cargo.toml b/src/tools/miri/test_dependencies/Cargo.toml index d8c70935fb144..3a80e8c964419 100644 --- a/src/tools/miri/test_dependencies/Cargo.toml +++ b/src/tools/miri/test_dependencies/Cargo.toml @@ -11,7 +11,7 @@ edition = "2021" # all dependencies (and their transitive ones) listed here can be used in `tests/`. tokio = { version = "1.0", features = ["full"] } libc = "0.2" -page_size = "0.4.1" +page_size = "0.5" num_cpus = "1.10.1" getrandom_1 = { package = "getrandom", version = "0.1" } diff --git a/src/tools/miri/tests/fail/crates/tokio_mvp.stderr b/src/tools/miri/tests/fail/crates/tokio_mvp.stderr index 016081d90adf7..5a80d1ac5a9b1 100644 --- a/src/tools/miri/tests/fail/crates/tokio_mvp.stderr +++ b/src/tools/miri/tests/fail/crates/tokio_mvp.stderr @@ -1,8 +1,8 @@ error: unsupported operation: can't call foreign function: epoll_create1 --> CARGO_REGISTRY/.../epoll.rs:LL:CC | -LL | syscall!(epoll_create1(flag)).map(|ep| Selector { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't call foreign function: epoll_create1 +LL | let res = syscall!(epoll_create1(libc::EPOLL_CLOEXEC)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't call foreign function: epoll_create1 | = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support = note: BACKTRACE: From 1ca3c293b2c196e02e188faeddf0a0d776d39fd6 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Tue, 22 Nov 2022 10:19:29 -0500 Subject: [PATCH 07/31] Document is_user_relevant Co-authored-by: Ralf Jung --- src/tools/miri/src/helpers.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 958ed50496b1f..70971cdc20ee2 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -968,6 +968,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { self.threads.active_thread_ref().top_user_relevant_frame() } + /// This is the source of truth for the `is_user_relevant` flag in our `FrameExtra`. pub fn is_user_relevant(&self, frame: &Frame<'mir, 'tcx, Provenance>) -> bool { let def_id = frame.instance.def_id(); (def_id.is_local() || self.local_crates.contains(&def_id.krate)) From a312329b0addbb2321ab4af5ac7132719f18e51c Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Tue, 22 Nov 2022 22:22:47 -0500 Subject: [PATCH 08/31] Update src/machine.rs Co-authored-by: Ralf Jung --- src/tools/miri/src/machine.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index f5fa8f5f09b46..b7f434b5557c6 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1192,6 +1192,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { if frame.extra.is_user_relevant { // All that we store is whether or not the frame we just removed is local, so now we // have no idea where the next topmost local frame is. So we recompute it. + // (If this ever becomes a bottleneck, we could have `push` store the previous + // user-relevant frame and restore that here.) ecx.active_thread_mut().recompute_top_user_relevant_frame(); } let timing = frame.extra.timing.take(); From 448f0444070b21abc2c054e778aabdf7b16e9d8f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 25 Nov 2022 08:57:16 +0100 Subject: [PATCH 09/31] replace a borrow_mut by get_mut --- src/tools/miri/src/stacked_borrows/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/stacked_borrows/mod.rs index bc52428910aa0..9fdd1562c1976 100644 --- a/src/tools/miri/src/stacked_borrows/mod.rs +++ b/src/tools/miri/src/stacked_borrows/mod.rs @@ -935,11 +935,11 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // Note that this asserts that the allocation is mutable -- but since we are creating a // mutable pointer, that seems reasonable. let (alloc_extra, machine) = this.get_alloc_extra_mut(alloc_id)?; - let mut stacked_borrows = alloc_extra + let stacked_borrows = alloc_extra .stacked_borrows .as_mut() .expect("we should have Stacked Borrows data") - .borrow_mut(); + .get_mut(); let item = Item::new(new_tag, perm, protect.is_some()); let range = alloc_range(base_offset, size); let global = machine.stacked_borrows.as_ref().unwrap().borrow(); From 2c456b5123d3546732683c4fe6dce5be822e15d1 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Thu, 24 Nov 2022 21:22:12 +0100 Subject: [PATCH 10/31] Test a small cargo-miri smoke test even in `run_tests_minimal` This makes sure that cargo-miri works on all targets. --- src/tools/miri/ci.sh | 4 +++ src/tools/miri/test-cargo-miri/Cargo.lock | 4 +++ src/tools/miri/test-cargo-miri/Cargo.toml | 1 + .../test-cargo-miri/no-std-smoke/Cargo.lock | 7 ++++ .../test-cargo-miri/no-std-smoke/Cargo.toml | 14 ++++++++ .../test-cargo-miri/no-std-smoke/src/main.rs | 34 +++++++++++++++++++ 6 files changed, 64 insertions(+) create mode 100644 src/tools/miri/test-cargo-miri/no-std-smoke/Cargo.lock create mode 100644 src/tools/miri/test-cargo-miri/no-std-smoke/Cargo.toml create mode 100644 src/tools/miri/test-cargo-miri/no-std-smoke/src/main.rs diff --git a/src/tools/miri/ci.sh b/src/tools/miri/ci.sh index 72b7b791a47e0..bf9e986bdc791 100755 --- a/src/tools/miri/ci.sh +++ b/src/tools/miri/ci.sh @@ -73,6 +73,10 @@ function run_tests_minimal { fi ./miri test -- "$@" + + # Ensure that a small smoke test of cargo-miri works. + # Note: This doesn't work on windows because of TLS. + cargo miri run --manifest-path test-cargo-miri/no-std-smoke/Cargo.toml } # host diff --git a/src/tools/miri/test-cargo-miri/Cargo.lock b/src/tools/miri/test-cargo-miri/Cargo.lock index 2c53c482bf9aa..d037a37e0f677 100644 --- a/src/tools/miri/test-cargo-miri/Cargo.lock +++ b/src/tools/miri/test-cargo-miri/Cargo.lock @@ -81,6 +81,10 @@ version = "0.1.0" name = "issue_rust_86261" version = "0.1.0" +[[package]] +name = "no-std-smoke" +version = "0.1.0" + [[package]] name = "proc-macro2" version = "1.0.44" diff --git a/src/tools/miri/test-cargo-miri/Cargo.toml b/src/tools/miri/test-cargo-miri/Cargo.toml index 5d9e5d143b3b3..37c996de6623c 100644 --- a/src/tools/miri/test-cargo-miri/Cargo.toml +++ b/src/tools/miri/test-cargo-miri/Cargo.toml @@ -1,5 +1,6 @@ [workspace] members = ["subcrate", "issue-1567", "exported-symbol-dep"] +exclude = ["no-std-smoke"] # it wants to be panic="abort" [package] name = "cargo-miri-test" diff --git a/src/tools/miri/test-cargo-miri/no-std-smoke/Cargo.lock b/src/tools/miri/test-cargo-miri/no-std-smoke/Cargo.lock new file mode 100644 index 0000000000000..b92a05fccf80f --- /dev/null +++ b/src/tools/miri/test-cargo-miri/no-std-smoke/Cargo.lock @@ -0,0 +1,7 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "no-std-smoke" +version = "0.1.0" diff --git a/src/tools/miri/test-cargo-miri/no-std-smoke/Cargo.toml b/src/tools/miri/test-cargo-miri/no-std-smoke/Cargo.toml new file mode 100644 index 0000000000000..3a056bedaa003 --- /dev/null +++ b/src/tools/miri/test-cargo-miri/no-std-smoke/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "no-std-smoke" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] + +[profile.dev] +panic = 'abort' + +[profile.release] +panic = 'abort' diff --git a/src/tools/miri/test-cargo-miri/no-std-smoke/src/main.rs b/src/tools/miri/test-cargo-miri/no-std-smoke/src/main.rs new file mode 100644 index 0000000000000..3a207b7d50aa2 --- /dev/null +++ b/src/tools/miri/test-cargo-miri/no-std-smoke/src/main.rs @@ -0,0 +1,34 @@ +// Copied from tests/pass/no-std.rs + +#![feature(start)] +#![no_std] + +// Plumbing to let us use `writeln!` to host stdout: + +extern "Rust" { + fn miri_write_to_stdout(bytes: &[u8]); +} + +struct Host; + +use core::fmt::Write; + +impl Write for Host { + fn write_str(&mut self, s: &str) -> core::fmt::Result { + unsafe { + miri_write_to_stdout(s.as_bytes()); + } + Ok(()) + } +} + +#[start] +fn start(_: isize, _: *const *const u8) -> isize { + writeln!(Host, "hello, world!").unwrap(); + 0 +} + +#[panic_handler] +fn panic_handler(_: &core::panic::PanicInfo) -> ! { + loop {} +} From 8961e13802946e3c956b8f9657aaabf2978027b6 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 25 Nov 2022 13:12:43 -0500 Subject: [PATCH 11/31] Use None when the stack is empty --- src/tools/miri/src/concurrency/thread.rs | 6 ++++-- src/tools/miri/src/helpers.rs | 15 +++++++-------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index 96d988eb9a3bb..dacb3a9b88f8f 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -179,12 +179,14 @@ impl<'mir, 'tcx> Thread<'mir, 'tcx> { self.top_user_relevant_frame = Some(frame_idx); } - pub fn top_user_relevant_frame(&self) -> usize { + /// Returns the topmost frame that is considered user-relevant, or the + /// top of the stack if there is no such frame, or `None` if the stack is empty. + pub fn top_user_relevant_frame(&self) -> Option { debug_assert_eq!(self.top_user_relevant_frame, self.compute_top_user_relevant_frame()); // This can be called upon creation of an allocation. We create allocations while setting up // parts of the Rust runtime when we do not have any stack frames yet, so we need to handle // empty stacks. - self.top_user_relevant_frame.unwrap_or_else(|| self.stack.len().saturating_sub(1)) + self.top_user_relevant_frame.or_else(|| self.stack.len().checked_sub(1)) } } diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 70971cdc20ee2..cfa47c5c004d5 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -940,9 +940,8 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { /// `#[track_caller]`. /// This function is backed by a cache, and can be assumed to be very fast. pub fn current_span(&self) -> Span { - self.stack() - .get(self.top_user_relevant_frame()) - .map(Frame::current_span) + self.top_user_relevant_frame() + .map(|frame_idx| self.stack()[frame_idx].current_span()) .unwrap_or(rustc_span::DUMMY_SP) } @@ -954,17 +953,17 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { pub fn caller_span(&self) -> Span { // We need to go down at least to the caller (len - 2), or however // far we have to go to find a frame in a local crate which is also not #[track_caller]. - let frame_idx = self.top_user_relevant_frame(); - let stack = self.stack(); - let frame_idx = cmp::min(frame_idx, stack.len().saturating_sub(2)); - stack.get(frame_idx).map(Frame::current_span).unwrap_or(rustc_span::DUMMY_SP) + self.top_user_relevant_frame() + .map(|frame_idx| cmp::min(frame_idx, self.stack().len() - 2)) + .map(|frame_idx| self.stack()[frame_idx].current_span()) + .unwrap_or(rustc_span::DUMMY_SP) } fn stack(&self) -> &[Frame<'mir, 'tcx, Provenance, machine::FrameData<'tcx>>] { self.threads.active_thread_stack() } - fn top_user_relevant_frame(&self) -> usize { + fn top_user_relevant_frame(&self) -> Option { self.threads.active_thread_ref().top_user_relevant_frame() } From 726b9d09d420e8b94428170f3e8a637f54928f50 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 26 Nov 2022 14:01:05 +0100 Subject: [PATCH 12/31] caller_span only makes sense when there are 2 frames on the stack --- src/tools/miri/src/helpers.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index cfa47c5c004d5..8c7bc9eff0016 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -939,6 +939,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { /// Get the current span in the topmost function which is workspace-local and not /// `#[track_caller]`. /// This function is backed by a cache, and can be assumed to be very fast. + /// It will work even when the stack is empty. pub fn current_span(&self) -> Span { self.top_user_relevant_frame() .map(|frame_idx| self.stack()[frame_idx].current_span()) @@ -953,10 +954,9 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { pub fn caller_span(&self) -> Span { // We need to go down at least to the caller (len - 2), or however // far we have to go to find a frame in a local crate which is also not #[track_caller]. - self.top_user_relevant_frame() - .map(|frame_idx| cmp::min(frame_idx, self.stack().len() - 2)) - .map(|frame_idx| self.stack()[frame_idx].current_span()) - .unwrap_or(rustc_span::DUMMY_SP) + let frame_idx = self.top_user_relevant_frame().unwrap(); + let frame_idx = cmp::min(frame_idx, self.stack().len().checked_sub(2).unwrap()); + self.stack()[frame_idx].current_span() } fn stack(&self) -> &[Frame<'mir, 'tcx, Provenance, machine::FrameData<'tcx>>] { From 166e60e2bb0713dc9c76b13f08e580cb908b232e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 26 Nov 2022 14:16:45 +0100 Subject: [PATCH 13/31] update lockfile --- src/tools/miri/test-cargo-miri/Cargo.lock | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/tools/miri/test-cargo-miri/Cargo.lock b/src/tools/miri/test-cargo-miri/Cargo.lock index d037a37e0f677..2c53c482bf9aa 100644 --- a/src/tools/miri/test-cargo-miri/Cargo.lock +++ b/src/tools/miri/test-cargo-miri/Cargo.lock @@ -81,10 +81,6 @@ version = "0.1.0" name = "issue_rust_86261" version = "0.1.0" -[[package]] -name = "no-std-smoke" -version = "0.1.0" - [[package]] name = "proc-macro2" version = "1.0.44" From 56a1d07cb7b78cadf7f8b260c1035bc48e33a17d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 26 Nov 2022 14:03:41 +0100 Subject: [PATCH 14/31] prettify our CI logs --- src/tools/miri/ci.sh | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/tools/miri/ci.sh b/src/tools/miri/ci.sh index bf9e986bdc791..01925e39da9f7 100755 --- a/src/tools/miri/ci.sh +++ b/src/tools/miri/ci.sh @@ -1,6 +1,17 @@ #!/bin/bash set -euo pipefail -set -x + +function begingroup { + echo "::group::$1" + set -x +} + +function endgroup { + set +x + echo "::endgroup" +} + +begingroup "Building Miri" # Determine configuration for installed build echo "Installing release version of Miri" @@ -14,14 +25,15 @@ export CARGO_EXTRA_FLAGS="--locked" ./miri check --no-default-features # make sure this can be built ./miri check --all-features # and this, too ./miri build --all-targets # the build that all the `./miri test` below will use -echo + +endgroup # Test function run_tests { if [ -n "${MIRI_TEST_TARGET+exists}" ]; then - echo "Testing foreign architecture $MIRI_TEST_TARGET" + begingroup "Testing foreign architecture $MIRI_TEST_TARGET" else - echo "Testing host architecture" + begingroup "Testing host architecture" fi ## ui test suite @@ -52,7 +64,6 @@ function run_tests { echo 'build.rustc-wrapper = "thisdoesnotexist"' > .cargo/config.toml # Run the actual test ${PYTHON} test-cargo-miri/run-test.py - echo # Clean up unset RUSTC MIRI rm -rf .cargo @@ -63,13 +74,15 @@ function run_tests { cargo miri run --manifest-path bench-cargo-miri/$BENCH/Cargo.toml done fi + + endgroup } function run_tests_minimal { if [ -n "${MIRI_TEST_TARGET+exists}" ]; then - echo "Testing MINIMAL foreign architecture $MIRI_TEST_TARGET: only testing $@" + begingroup "Testing MINIMAL foreign architecture $MIRI_TEST_TARGET: only testing $@" else - echo "Testing MINIMAL host architecture: only testing $@" + begingroup "Testing MINIMAL host architecture: only testing $@" fi ./miri test -- "$@" @@ -77,6 +90,8 @@ function run_tests_minimal { # Ensure that a small smoke test of cargo-miri works. # Note: This doesn't work on windows because of TLS. cargo miri run --manifest-path test-cargo-miri/no-std-smoke/Cargo.toml + + endgroup } # host From 0822c311fb062d0e82094cb78a00b12786ed0fd4 Mon Sep 17 00:00:00 2001 From: DrMeepster <19316085+DrMeepster@users.noreply.github.com> Date: Wed, 9 Nov 2022 00:39:03 -0800 Subject: [PATCH 15/31] add namespace to resolve_path --- src/tools/miri/src/eval.rs | 3 +- src/tools/miri/src/helpers.rs | 47 ++++++++++++++++------------- src/tools/miri/src/shims/unix/fs.rs | 8 +---- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index 81132db94cf18..363b647d6c684 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -9,6 +9,7 @@ use std::thread; use log::info; use rustc_data_structures::fx::FxHashSet; +use rustc_hir::def::Namespace; use rustc_hir::def_id::DefId; use rustc_middle::ty::{ self, @@ -195,7 +196,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( MiriMachine::late_init(&mut ecx, config)?; // Make sure we have MIR. We check MIR for some stable monomorphic function in libcore. - let sentinel = ecx.try_resolve_path(&["core", "ascii", "escape_default"]); + let sentinel = ecx.try_resolve_path(&["core", "ascii", "escape_default"], Namespace::ValueNS); if !matches!(sentinel, Some(s) if tcx.is_mir_available(s.def.def_id())) { tcx.sess.fatal( "the current sysroot was built without `-Zalways-encode-mir`, or libcore seems missing. \ diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 8c7bc9eff0016..17c9ceafba48f 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -2,12 +2,12 @@ pub mod convert; use std::cmp; use std::iter; -use std::mem; use std::num::NonZeroUsize; use std::time::Duration; use log::trace; +use rustc_hir::def::{DefKind, Namespace}; use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX}; use rustc_middle::mir; use rustc_middle::ty::{ @@ -74,40 +74,43 @@ const UNIX_IO_ERROR_TABLE: &[(&str, std::io::ErrorKind)] = { }; /// Gets an instance for a path. -fn try_resolve_did<'tcx>(tcx: TyCtxt<'tcx>, path: &[&str]) -> Option { +fn try_resolve_did<'tcx>(tcx: TyCtxt<'tcx>, path: &[&str], namespace: Namespace) -> Option { tcx.crates(()).iter().find(|&&krate| tcx.crate_name(krate).as_str() == path[0]).and_then( |krate| { let krate = DefId { krate: *krate, index: CRATE_DEF_INDEX }; let mut items = tcx.module_children(krate); - let mut path_it = path.iter().skip(1).peekable(); - while let Some(segment) = path_it.next() { - for item in mem::take(&mut items).iter() { - if item.ident.name.as_str() == *segment { - if path_it.peek().is_none() { - return Some(item.res.def_id()); - } + for &segment in &path[1..path.len() - 1] { + let next_mod = items.iter().find(|item| { + item.ident.name.as_str() == segment + && tcx.def_kind(item.res.def_id()) == DefKind::Mod + })?; - items = tcx.module_children(item.res.def_id()); - break; - } - } + items = tcx.module_children(next_mod.res.def_id()); } - None + + let item_name = *path.last().unwrap(); + + let item = items.iter().find(|item| { + item.ident.name.as_str() == item_name + && tcx.def_kind(item.res.def_id()).ns() == Some(namespace) + })?; + + Some(item.res.def_id()) }, ) } pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// Gets an instance for a path; fails gracefully if the path does not exist. - fn try_resolve_path(&self, path: &[&str]) -> Option> { - let did = try_resolve_did(self.eval_context_ref().tcx.tcx, path)?; + fn try_resolve_path(&self, path: &[&str], namespace: Namespace) -> Option> { + let did = try_resolve_did(self.eval_context_ref().tcx.tcx, path, namespace)?; Some(ty::Instance::mono(self.eval_context_ref().tcx.tcx, did)) } /// Gets an instance for a path. - fn resolve_path(&self, path: &[&str]) -> ty::Instance<'tcx> { - self.try_resolve_path(path) + fn resolve_path(&self, path: &[&str], namespace: Namespace) -> ty::Instance<'tcx> { + self.try_resolve_path(path, namespace) .unwrap_or_else(|| panic!("failed to find required Rust item: {path:?}")) } @@ -115,7 +118,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// if the path could be resolved, and None otherwise fn eval_path_scalar(&self, path: &[&str]) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_ref(); - let instance = this.resolve_path(path); + let instance = this.resolve_path(path, Namespace::ValueNS); let cid = GlobalId { instance, promoted: None }; // We don't give a span -- this isn't actually used directly by the program anyway. let const_val = this.eval_global(cid, None)?; @@ -147,7 +150,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// Helper function to get the `TyAndLayout` of a `libc` type fn libc_ty_layout(&self, name: &str) -> InterpResult<'tcx, TyAndLayout<'tcx>> { let this = self.eval_context_ref(); - let ty = this.resolve_path(&["libc", name]).ty(*this.tcx, ty::ParamEnv::reveal_all()); + let ty = this + .resolve_path(&["libc", name], Namespace::TypeNS) + .ty(*this.tcx, ty::ParamEnv::reveal_all()); this.layout_of(ty) } @@ -155,7 +160,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn windows_ty_layout(&self, name: &str) -> InterpResult<'tcx, TyAndLayout<'tcx>> { let this = self.eval_context_ref(); let ty = this - .resolve_path(&["std", "sys", "windows", "c", name]) + .resolve_path(&["std", "sys", "windows", "c", name], Namespace::TypeNS) .ty(*this.tcx, ty::ParamEnv::reveal_all()); this.layout_of(ty) } diff --git a/src/tools/miri/src/shims/unix/fs.rs b/src/tools/miri/src/shims/unix/fs.rs index b152082b4deb8..816e7d87e8586 100644 --- a/src/tools/miri/src/shims/unix/fs.rs +++ b/src/tools/miri/src/shims/unix/fs.rs @@ -11,7 +11,6 @@ use std::time::SystemTime; use log::trace; use rustc_data_structures::fx::FxHashMap; -use rustc_middle::ty::{self, layout::LayoutOf}; use rustc_target::abi::{Align, Size}; use crate::shims::os_str::bytes_to_os_str; @@ -1006,12 +1005,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // as `isize`s instead of having the proper types. Thus, we have to recover the layout of // `statxbuf_op` by using the `libc::statx` struct type. let statxbuf = { - // FIXME: This long path is required because `libc::statx` is an struct and also a - // function and `resolve_path` is returning the latter. - let statx_ty = this - .resolve_path(&["libc", "unix", "linux_like", "linux", "gnu", "statx"]) - .ty(*this.tcx, ty::ParamEnv::reveal_all()); - let statx_layout = this.layout_of(statx_ty)?; + let statx_layout = this.libc_ty_layout("statx")?; MPlaceTy::from_aligned_ptr(statxbuf_ptr, statx_layout) }; From 245857beb76094d07f8447c5072d9da386f42b91 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 26 Nov 2022 15:58:28 +0100 Subject: [PATCH 16/31] refactor try_resolve_did and also support resolving crates/modules --- src/tools/miri/src/helpers.rs | 80 +++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 26 deletions(-) diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 17c9ceafba48f..bf086a7c62330 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -74,38 +74,66 @@ const UNIX_IO_ERROR_TABLE: &[(&str, std::io::ErrorKind)] = { }; /// Gets an instance for a path. -fn try_resolve_did<'tcx>(tcx: TyCtxt<'tcx>, path: &[&str], namespace: Namespace) -> Option { - tcx.crates(()).iter().find(|&&krate| tcx.crate_name(krate).as_str() == path[0]).and_then( - |krate| { - let krate = DefId { krate: *krate, index: CRATE_DEF_INDEX }; - let mut items = tcx.module_children(krate); - - for &segment in &path[1..path.len() - 1] { - let next_mod = items.iter().find(|item| { - item.ident.name.as_str() == segment - && tcx.def_kind(item.res.def_id()) == DefKind::Mod - })?; - - items = tcx.module_children(next_mod.res.def_id()); - } - - let item_name = *path.last().unwrap(); - - let item = items.iter().find(|item| { - item.ident.name.as_str() == item_name - && tcx.def_kind(item.res.def_id()).ns() == Some(namespace) - })?; +/// +/// A `None` namespace indicates we are looking for a module. +fn try_resolve_did<'tcx>( + tcx: TyCtxt<'tcx>, + path: &[&str], + namespace: Option, +) -> Option { + /// Yield all children of the given item, that have the given name. + fn find_children<'tcx: 'a, 'a>( + tcx: TyCtxt<'tcx>, + item: DefId, + name: &'a str, + ) -> impl Iterator + 'a { + tcx.module_children(item) + .iter() + .filter(move |item| item.ident.name.as_str() == name) + .map(move |item| item.res.def_id()) + } - Some(item.res.def_id()) - }, - ) + // Take apart the path: leading crate, a sequence of modules, and potentially a final item. + let (&crate_name, path) = path.split_first().expect("paths must have at least one segment"); + let (modules, item) = if let Some(namespace) = namespace { + let (&item_name, modules) = + path.split_last().expect("non-module paths must have at least 2 segments"); + (modules, Some((item_name, namespace))) + } else { + (path, None) + }; + + // First find the crate. + let krate = + tcx.crates(()).iter().find(|&&krate| tcx.crate_name(krate).as_str() == crate_name)?; + let mut cur_item = DefId { krate: *krate, index: CRATE_DEF_INDEX }; + // Then go over the modules. + for &segment in modules { + cur_item = find_children(tcx, cur_item, segment) + .find(|item| tcx.def_kind(item) == DefKind::Mod)?; + } + // Finally, look up the desired item in this module, if any. + match item { + Some((item_name, namespace)) => + Some( + find_children(tcx, cur_item, item_name) + .find(|item| tcx.def_kind(item).ns() == Some(namespace))?, + ), + None => Some(cur_item), + } } pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { + /// Checks if the given crate/module exists. + fn have_module(&self, path: &[&str]) -> bool { + try_resolve_did(*self.eval_context_ref().tcx, path, None).is_some() + } + /// Gets an instance for a path; fails gracefully if the path does not exist. fn try_resolve_path(&self, path: &[&str], namespace: Namespace) -> Option> { - let did = try_resolve_did(self.eval_context_ref().tcx.tcx, path, namespace)?; - Some(ty::Instance::mono(self.eval_context_ref().tcx.tcx, did)) + let tcx = self.eval_context_ref().tcx.tcx; + let did = try_resolve_did(tcx, path, Some(namespace))?; + Some(ty::Instance::mono(tcx, did)) } /// Gets an instance for a path. From 3158a8d476d6f35664502fa50aae9f3e873a263e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 26 Nov 2022 15:58:44 +0100 Subject: [PATCH 17/31] support no_std on Windows --- src/tools/miri/ci.sh | 1 - src/tools/miri/src/shims/tls.rs | 5 +++++ src/tools/miri/tests/pass/no_std.rs | 4 ---- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tools/miri/ci.sh b/src/tools/miri/ci.sh index bf9e986bdc791..4e976ef248ae7 100755 --- a/src/tools/miri/ci.sh +++ b/src/tools/miri/ci.sh @@ -75,7 +75,6 @@ function run_tests_minimal { ./miri test -- "$@" # Ensure that a small smoke test of cargo-miri works. - # Note: This doesn't work on windows because of TLS. cargo miri run --manifest-path test-cargo-miri/no-std-smoke/Cargo.toml } diff --git a/src/tools/miri/src/shims/tls.rs b/src/tools/miri/src/shims/tls.rs index 430dedbc1700c..5fda8bd7b7de9 100644 --- a/src/tools/miri/src/shims/tls.rs +++ b/src/tools/miri/src/shims/tls.rs @@ -261,6 +261,11 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // (that would be basically https://github.com/rust-lang/miri/issues/450), // we specifically look up the static in libstd that we know is placed // in that section. + if !this.have_module(&["std"]) { + // Looks like we are running in a `no_std` crate. + // That also means no TLS dtors callback to call. + return Ok(()); + } let thread_callback = this.eval_windows("thread_local_key", "p_thread_callback")?.to_pointer(this)?; let thread_callback = this.get_ptr_fn(thread_callback)?.as_instance()?; diff --git a/src/tools/miri/tests/pass/no_std.rs b/src/tools/miri/tests/pass/no_std.rs index eb0e860e68ebf..3bece7783f798 100644 --- a/src/tools/miri/tests/pass/no_std.rs +++ b/src/tools/miri/tests/pass/no_std.rs @@ -1,9 +1,5 @@ #![feature(lang_items, start)] #![no_std] -// windows tls dtors go through libstd right now, thus this test -// cannot pass. When windows tls dtors go through the special magic -// windows linker section, we can run this test on windows again. -//@ignore-target-windows: no-std not supported on Windows // Plumbing to let us use `writeln!` to host stdout: From b19e0347453b9388266ecbcf6ec85d2c66e2f9f4 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 26 Nov 2022 22:35:05 +0100 Subject: [PATCH 18/31] Switch rustdoc-gui test to function call --- src/test/rustdoc-gui/sidebar-mobile.goml | 46 +++++++++++++++--------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/src/test/rustdoc-gui/sidebar-mobile.goml b/src/test/rustdoc-gui/sidebar-mobile.goml index 453873f1b81b6..fc36401733633 100644 --- a/src/test/rustdoc-gui/sidebar-mobile.goml +++ b/src/test/rustdoc-gui/sidebar-mobile.goml @@ -48,23 +48,35 @@ compare-elements-position-near: (".block.keyword li:nth-child(1)", ".mobile-topb // Now checking the background color of the sidebar. show-text: true -local-storage: {"rustdoc-use-system-theme": "false", "rustdoc-theme": "dark"} -reload: -// Open the sidebar menu. -click: ".sidebar-menu-toggle" -assert-css: (".sidebar", {"background-color": "rgb(80, 80, 80)", "color": "rgb(221, 221, 221)"}) - -local-storage: {"rustdoc-use-system-theme": "false", "rustdoc-theme": "ayu"} -reload: - -// Open the sidebar menu. -click: ".sidebar-menu-toggle" -assert-css: (".sidebar", {"background-color": "rgb(20, 25, 31)", "color": "rgb(197, 197, 197)"}) +define-function: ( + "check-colors", + (theme, color, background), + [ + ("local-storage", {"rustdoc-use-system-theme": "false", "rustdoc-theme": |theme|}), + ("reload"), -local-storage: {"rustdoc-use-system-theme": "false", "rustdoc-theme": "light"} -reload: + // Open the sidebar menu. + ("click", ".sidebar-menu-toggle"), + ("assert-css", (".sidebar", { + "background-color": |background|, + "color": |color|, + })), + ], +) -// Open the sidebar menu. -click: ".sidebar-menu-toggle" -assert-css: (".sidebar", {"background-color": "rgb(245, 245, 245)", "color": "rgb(0, 0, 0)"}) +call-function: ("check-colors", { + "theme": "ayu", + "color": "rgb(197, 197, 197)", + "background": "rgb(20, 25, 31)", +}) +call-function: ("check-colors", { + "theme": "dark", + "color": "rgb(221, 221, 221)", + "background": "rgb(80, 80, 80)", +}) +call-function: ("check-colors", { + "theme": "light", + "color": "rgb(0, 0, 0)", + "background": "rgb(245, 245, 245)", +}) From f479404b12ad5ca9b163a591a708ae7be63f402d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 26 Nov 2022 13:22:19 +0100 Subject: [PATCH 19/31] !Unpin retags must still be reads, to check dereferenceable also fix ICE on deallocation error and avoid redundant find_granting on retag --- .../miri/src/stacked_borrows/diagnostics.rs | 19 ++-- src/tools/miri/src/stacked_borrows/mod.rs | 96 +++++++++---------- src/tools/miri/src/stacked_borrows/stack.rs | 4 +- .../fail/stacked_borrows/illegal_dealloc1.rs | 14 +++ .../stacked_borrows/illegal_dealloc1.stderr | 30 ++++++ .../notunpin_dereferenceable_fakeread.rs | 17 ++++ .../notunpin_dereferenceable_fakeread.stderr | 28 ++++++ 7 files changed, 151 insertions(+), 57 deletions(-) create mode 100644 src/tools/miri/tests/fail/stacked_borrows/illegal_dealloc1.rs create mode 100644 src/tools/miri/tests/fail/stacked_borrows/illegal_dealloc1.stderr create mode 100644 src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.rs create mode 100644 src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.stderr diff --git a/src/tools/miri/src/stacked_borrows/diagnostics.rs b/src/tools/miri/src/stacked_borrows/diagnostics.rs index 662d8ada735eb..9970b79f8c7f1 100644 --- a/src/tools/miri/src/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/stacked_borrows/diagnostics.rs @@ -353,10 +353,12 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { /// Report a descriptive error when `new` could not be granted from `derived_from`. #[inline(never)] // This is only called on fatal code paths - pub(super) fn grant_error(&self, perm: Permission, stack: &Stack) -> InterpError<'tcx> { + pub(super) fn grant_error(&self, stack: &Stack) -> InterpError<'tcx> { let Operation::Retag(op) = &self.operation else { unreachable!("grant_error should only be called during a retag") }; + let perm = + op.permission.expect("`start_grant` must be called before calling `grant_error`"); let action = format!( "trying to retag from {:?} for {:?} permission at {:?}[{:#x}]", op.orig_tag, @@ -374,8 +376,11 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { /// Report a descriptive error when `access` is not permitted based on `tag`. #[inline(never)] // This is only called on fatal code paths pub(super) fn access_error(&self, stack: &Stack) -> InterpError<'tcx> { - let Operation::Access(op) = &self.operation else { - unreachable!("access_error should only be called during an access") + // Deallocation and retagging also do an access as part of their thing, so handle that here, too. + let op = match &self.operation { + Operation::Access(op) => op, + Operation::Retag(_) => return self.grant_error(stack), + Operation::Dealloc(_) => return self.dealloc_error(stack), }; let action = format!( "attempting a {access} using {tag:?} at {alloc_id:?}[{offset:#x}]", @@ -428,14 +433,16 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { } #[inline(never)] // This is only called on fatal code paths - pub fn dealloc_error(&self) -> InterpError<'tcx> { + pub fn dealloc_error(&self, stack: &Stack) -> InterpError<'tcx> { let Operation::Dealloc(op) = &self.operation else { unreachable!("dealloc_error should only be called during a deallocation") }; err_sb_ub( format!( - "no item granting write access for deallocation to tag {:?} at {:?} found in borrow stack", - op.tag, self.history.id, + "attempting deallocation using {tag:?} at {alloc_id:?}{cause}", + tag = op.tag, + alloc_id = self.history.id, + cause = error_cause(stack, op.tag), ), None, op.tag.and_then(|tag| self.get_logs_relevant_to(tag, None)), diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/stacked_borrows/mod.rs index db4e19f91184b..1759eee35855e 100644 --- a/src/tools/miri/src/stacked_borrows/mod.rs +++ b/src/tools/miri/src/stacked_borrows/mod.rs @@ -392,7 +392,7 @@ impl<'tcx> Stack { // Step 1: Find granting item. let granting_idx = - self.find_granting(access, tag, exposed_tags).map_err(|_| dcx.access_error(self))?; + self.find_granting(access, tag, exposed_tags).map_err(|()| dcx.access_error(self))?; // Step 2: Remove incompatible items above them. Make sure we do not remove protected // items. Behavior differs for reads and writes. @@ -476,8 +476,7 @@ impl<'tcx> Stack { ) -> InterpResult<'tcx> { // Step 1: Make a write access. // As part of this we do regular protector checking, i.e. even weakly protected items cause UB when popped. - self.access(AccessKind::Write, tag, global, dcx, exposed_tags) - .map_err(|_| dcx.dealloc_error())?; + self.access(AccessKind::Write, tag, global, dcx, exposed_tags)?; // Step 2: Pretend we remove the remaining items, checking if any are strongly protected. for idx in (0..self.len()).rev() { @@ -489,39 +488,42 @@ impl<'tcx> Stack { } /// Derive a new pointer from one with the given tag. - /// `weak` controls whether this operation is weak or strong: weak granting does not act as - /// an access, and they add the new item directly on top of the one it is derived - /// from instead of all the way at the top of the stack. - /// `range` refers the entire operation, and `offset` refers to the specific location in - /// `range` that we are currently checking. + /// + /// `access` indicates which kind of memory access this retag itself should correspond to. fn grant( &mut self, derived_from: ProvenanceExtra, new: Item, + access: Option, global: &GlobalStateInner, dcx: &mut DiagnosticCx<'_, '_, '_, 'tcx>, exposed_tags: &FxHashSet, ) -> InterpResult<'tcx> { dcx.start_grant(new.perm()); - // Figure out which access `perm` corresponds to. - let access = - if new.perm().grants(AccessKind::Write) { AccessKind::Write } else { AccessKind::Read }; - - // Now we figure out which item grants our parent (`derived_from`) this kind of access. - // We use that to determine where to put the new item. - let granting_idx = self - .find_granting(access, derived_from, exposed_tags) - .map_err(|_| dcx.grant_error(new.perm(), self))?; - // Compute where to put the new item. // Either way, we ensure that we insert the new item in a way such that between // `derived_from` and the new one, there are only items *compatible with* `derived_from`. - let new_idx = if new.perm() == Permission::SharedReadWrite { - assert!( - access == AccessKind::Write, - "this case only makes sense for stack-like accesses" - ); + let new_idx = if let Some(access) = access { + // Simple case: We are just a regular memory access, and then push our thing on top, + // like a regular stack. + // This ensures F2b for `Unique`, by removing offending `SharedReadOnly`. + self.access(access, derived_from, global, dcx, exposed_tags)?; + + // We insert "as far up as possible": We know only compatible items are remaining + // on top of `derived_from`, and we want the new item at the top so that we + // get the strongest possible guarantees. + // This ensures U1 and F1. + self.len() + } else { + // The tricky case: creating a new SRW permission without actually being an access. + assert!(new.perm() == Permission::SharedReadWrite); + + // First we figure out which item grants our parent (`derived_from`) this kind of access. + // We use that to determine where to put the new item. + let granting_idx = self + .find_granting(AccessKind::Write, derived_from, exposed_tags) + .map_err(|()| dcx.grant_error(self))?; let (Some(granting_idx), ProvenanceExtra::Concrete(_)) = (granting_idx, derived_from) else { // The parent is a wildcard pointer or matched the unknown bottom. @@ -538,17 +540,6 @@ impl<'tcx> Stack { // be popped to (i.e., we insert it above all the write-compatible items). // This ensures F2b by adding the new item below any potentially existing `SharedReadOnly`. self.find_first_write_incompatible(granting_idx) - } else { - // A "safe" reborrow for a pointer that actually expects some aliasing guarantees. - // Here, creating a reference actually counts as an access. - // This ensures F2b for `Unique`, by removing offending `SharedReadOnly`. - self.access(access, derived_from, global, dcx, exposed_tags)?; - - // We insert "as far up as possible": We know only compatible items are remaining - // on top of `derived_from`, and we want the new item at the top so that we - // get the strongest possible guarantees. - // This ensures U1 and F1. - self.len() }; // Put the new item there. @@ -864,18 +855,22 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // Update the stacks. // Make sure that raw pointers and mutable shared references are reborrowed "weak": // There could be existing unique pointers reborrowed from them that should remain valid! - let perm = match kind { - RefKind::Unique { two_phase: false } - if place.layout.ty.is_unpin(*this.tcx, this.param_env()) => - { - // Only if the type is unpin do we actually enforce uniqueness - Permission::Unique + let (perm, access) = match kind { + RefKind::Unique { two_phase } => { + // Permission is Unique only if the type is `Unpin` and this is not twophase + let perm = if !two_phase && place.layout.ty.is_unpin(*this.tcx, this.param_env()) { + Permission::Unique + } else { + Permission::SharedReadWrite + }; + // We do an access for all full borrows, even if `!Unpin`. + let access = if !two_phase { Some(AccessKind::Write) } else { None }; + (perm, access) } - RefKind::Unique { .. } => { - // Two-phase references and !Unpin references are treated as SharedReadWrite - Permission::SharedReadWrite + RefKind::Raw { mutable: true } => { + // Creating a raw ptr does not count as an access + (Permission::SharedReadWrite, None) } - RefKind::Raw { mutable: true } => Permission::SharedReadWrite, RefKind::Shared | RefKind::Raw { mutable: false } => { // Shared references and *const are a whole different kind of game, the // permission is not uniform across the entire range! @@ -892,10 +887,13 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // Adjust range. range.start += base_offset; // We are only ever `SharedReadOnly` inside the frozen bits. - let perm = if frozen { - Permission::SharedReadOnly + let (perm, access) = if frozen { + (Permission::SharedReadOnly, Some(AccessKind::Read)) } else { - Permission::SharedReadWrite + // Inside UnsafeCell, this does *not* count as an access, as there + // might actually be mutable references further up the stack that + // we have to keep alive. + (Permission::SharedReadWrite, None) }; let protected = if frozen { protect.is_some() @@ -914,7 +912,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' alloc_range(base_offset, size), ); stacked_borrows.for_each(range, dcx, |stack, dcx, exposed_tags| { - stack.grant(orig_tag, item, &global, dcx, exposed_tags) + stack.grant(orig_tag, item, access, &global, dcx, exposed_tags) }) })?; return Ok(Some(alloc_id)); @@ -941,7 +939,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' alloc_range(base_offset, size), ); stacked_borrows.for_each(range, dcx, |stack, dcx, exposed_tags| { - stack.grant(orig_tag, item, &global, dcx, exposed_tags) + stack.grant(orig_tag, item, access, &global, dcx, exposed_tags) })?; Ok(Some(alloc_id)) diff --git a/src/tools/miri/src/stacked_borrows/stack.rs b/src/tools/miri/src/stacked_borrows/stack.rs index aa549e34c5f65..51a6fba6df011 100644 --- a/src/tools/miri/src/stacked_borrows/stack.rs +++ b/src/tools/miri/src/stacked_borrows/stack.rs @@ -367,10 +367,10 @@ impl<'tcx> Stack { /// Find all `Unique` elements in this borrow stack above `granting_idx`, pass a copy of them /// to the `visitor`, then set their `Permission` to `Disabled`. - pub fn disable_uniques_starting_at crate::InterpResult<'tcx>>( + pub fn disable_uniques_starting_at( &mut self, disable_start: usize, - mut visitor: V, + mut visitor: impl FnMut(Item) -> crate::InterpResult<'tcx>, ) -> crate::InterpResult<'tcx> { #[cfg(feature = "stack-cache")] let unique_range = self.unique_range.clone(); diff --git a/src/tools/miri/tests/fail/stacked_borrows/illegal_dealloc1.rs b/src/tools/miri/tests/fail/stacked_borrows/illegal_dealloc1.rs new file mode 100644 index 0000000000000..670dd4baad8bc --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/illegal_dealloc1.rs @@ -0,0 +1,14 @@ +//@error-pattern: /deallocation .* tag does not exist in the borrow stack/ +use std::alloc::{alloc, dealloc, Layout}; + +fn main() { + unsafe { + let x = alloc(Layout::from_size_align_unchecked(1, 1)); + let ptr1 = (&mut *x) as *mut u8; + let ptr2 = (&mut *ptr1) as *mut u8; + // Invalidate ptr2 by writing to ptr1. + ptr1.write(0); + // Deallocate through ptr2. + dealloc(ptr2, Layout::from_size_align_unchecked(1, 1)); + } +} diff --git a/src/tools/miri/tests/fail/stacked_borrows/illegal_dealloc1.stderr b/src/tools/miri/tests/fail/stacked_borrows/illegal_dealloc1.stderr new file mode 100644 index 0000000000000..3b7802901a54e --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/illegal_dealloc1.stderr @@ -0,0 +1,30 @@ +error: Undefined Behavior: attempting deallocation using at ALLOC, but that tag does not exist in the borrow stack for this location + --> RUSTLIB/alloc/src/alloc.rs:LL:CC + | +LL | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempting deallocation using at ALLOC, but that tag does not exist in the borrow stack for this location + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental + = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information +help: was created by a SharedReadWrite retag at offsets [0x0..0x1] + --> $DIR/illegal_deALLOC.rs:LL:CC + | +LL | let ptr2 = (&mut *ptr1) as *mut u8; + | ^^^^^^^^^^^^ +help: was later invalidated at offsets [0x0..0x1] by a write access + --> $DIR/illegal_deALLOC.rs:LL:CC + | +LL | ptr1.write(0); + | ^^^^^^^^^^^^^ + = note: BACKTRACE: + = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC +note: inside `main` at $DIR/illegal_deALLOC.rs:LL:CC + --> $DIR/illegal_deALLOC.rs:LL:CC + | +LL | dealloc(ptr2, Layout::from_size_align_unchecked(1, 1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +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/stacked_borrows/notunpin_dereferenceable_fakeread.rs b/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.rs new file mode 100644 index 0000000000000..d660921bfe6e2 --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.rs @@ -0,0 +1,17 @@ +//! Reborrowing a `&mut !Unpin` must still act like a (fake) read. +use std::marker::PhantomPinned; + +struct NotUnpin(i32, PhantomPinned); + +fn main() { + unsafe { + let mut x = NotUnpin(0, PhantomPinned); + // Mutable borrow of `Unpin` field (with lifetime laundering) + let fieldref = &mut *(&mut x.0 as *mut i32); + // Mutable reborrow of the entire `x`, which is `!Unpin` but should + // still count as a read since we would add `dereferenceable`. + let _xref = &mut x; + // That read should have invalidated `fieldref`. + *fieldref = 0; //~ ERROR: /write access .* tag does not exist in the borrow stack/ + } +} diff --git a/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.stderr b/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.stderr new file mode 100644 index 0000000000000..3ef8a8e0e9c6a --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/notunpin_dereferenceable_fakeread.stderr @@ -0,0 +1,28 @@ +error: Undefined Behavior: attempting a write access using at ALLOC[0x0], but that tag does not exist in the borrow stack for this location + --> $DIR/notunpin_dereferenceable_fakeread.rs:LL:CC + | +LL | *fieldref = 0; + | ^^^^^^^^^^^^^ + | | + | attempting a write access using at ALLOC[0x0], but that tag does not exist in the borrow stack for this location + | this error occurs as part of an access at ALLOC[0x0..0x4] + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental + = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information +help: was created by a Unique retag at offsets [0x0..0x4] + --> $DIR/notunpin_dereferenceable_fakeread.rs:LL:CC + | +LL | let fieldref = &mut *(&mut x.0 as *mut i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: was later invalidated at offsets [0x0..0x4] by a SharedReadWrite retag + --> $DIR/notunpin_dereferenceable_fakeread.rs:LL:CC + | +LL | let _xref = &mut x; + | ^^^^^^ + = note: BACKTRACE: + = note: inside `main` at $DIR/notunpin_dereferenceable_fakeread.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + From a7f72218c89c2af7d6303f3b8cb6bd868c0af334 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 26 Nov 2022 14:42:27 +0100 Subject: [PATCH 20/31] slightly adjust and synchronize Machine passing for SB and DataRace --- src/tools/miri/src/concurrency/data_race.rs | 26 +++++++------- src/tools/miri/src/machine.rs | 40 ++++----------------- src/tools/miri/src/stacked_borrows/mod.rs | 13 +++---- 3 files changed, 24 insertions(+), 55 deletions(-) diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index b0f766462127f..d669cc1362a9a 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -838,18 +838,18 @@ impl VClockAlloc { &self, alloc_id: AllocId, range: AllocRange, - global: &GlobalState, - thread_mgr: &ThreadManager<'_, '_>, + machine: &MiriMachine<'_, '_>, ) -> InterpResult<'tcx> { + let global = machine.data_race.as_ref().unwrap(); if global.race_detecting() { - let (index, clocks) = global.current_thread_state(thread_mgr); + let (index, clocks) = global.current_thread_state(&machine.threads); let mut alloc_ranges = self.alloc_ranges.borrow_mut(); for (offset, range) in alloc_ranges.iter_mut(range.start, range.size) { if let Err(DataRace) = range.read_race_detect(&clocks, index) { // Report data-race. return Self::report_data_race( global, - thread_mgr, + &machine.threads, range, "Read", false, @@ -869,17 +869,17 @@ impl VClockAlloc { alloc_id: AllocId, range: AllocRange, write_type: WriteType, - global: &mut GlobalState, - thread_mgr: &ThreadManager<'_, '_>, + machine: &mut MiriMachine<'_, '_>, ) -> InterpResult<'tcx> { + let global = machine.data_race.as_mut().unwrap(); if global.race_detecting() { - let (index, clocks) = global.current_thread_state(thread_mgr); + let (index, clocks) = global.current_thread_state(&machine.threads); for (offset, range) in self.alloc_ranges.get_mut().iter_mut(range.start, range.size) { if let Err(DataRace) = range.write_race_detect(&clocks, index, write_type) { // Report data-race return Self::report_data_race( global, - thread_mgr, + &machine.threads, range, write_type.get_descriptor(), false, @@ -901,10 +901,9 @@ impl VClockAlloc { &mut self, alloc_id: AllocId, range: AllocRange, - global: &mut GlobalState, - thread_mgr: &ThreadManager<'_, '_>, + machine: &mut MiriMachine<'_, '_>, ) -> InterpResult<'tcx> { - self.unique_access(alloc_id, range, WriteType::Write, global, thread_mgr) + self.unique_access(alloc_id, range, WriteType::Write, machine) } /// Detect data-races for an unsynchronized deallocate operation, will not perform @@ -915,10 +914,9 @@ impl VClockAlloc { &mut self, alloc_id: AllocId, range: AllocRange, - global: &mut GlobalState, - thread_mgr: &ThreadManager<'_, '_>, + machine: &mut MiriMachine<'_, '_>, ) -> InterpResult<'tcx> { - self.unique_access(alloc_id, range, WriteType::Deallocate, global, thread_mgr) + self.unique_access(alloc_id, range, WriteType::Deallocate, machine) } } diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index b7f434b5557c6..edfef211dc675 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1003,21 +1003,12 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { range: AllocRange, ) -> InterpResult<'tcx> { if let Some(data_race) = &alloc_extra.data_race { - data_race.read( - alloc_id, - range, - machine.data_race.as_ref().unwrap(), - &machine.threads, - )?; + data_race.read(alloc_id, range, machine)?; } if let Some(stacked_borrows) = &alloc_extra.stacked_borrows { - stacked_borrows.borrow_mut().before_memory_read( - alloc_id, - prov_extra, - range, - machine.stacked_borrows.as_ref().unwrap(), - machine, - )?; + stacked_borrows + .borrow_mut() + .before_memory_read(alloc_id, prov_extra, range, machine)?; } if let Some(weak_memory) = &alloc_extra.weak_memory { weak_memory.memory_accessed(range, machine.data_race.as_ref().unwrap()); @@ -1034,21 +1025,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { range: AllocRange, ) -> InterpResult<'tcx> { if let Some(data_race) = &mut alloc_extra.data_race { - data_race.write( - alloc_id, - range, - machine.data_race.as_mut().unwrap(), - &machine.threads, - )?; + data_race.write(alloc_id, range, machine)?; } if let Some(stacked_borrows) = &mut alloc_extra.stacked_borrows { - stacked_borrows.get_mut().before_memory_write( - alloc_id, - prov_extra, - range, - machine.stacked_borrows.as_ref().unwrap(), - machine, - )?; + stacked_borrows.get_mut().before_memory_write(alloc_id, prov_extra, range, machine)?; } if let Some(weak_memory) = &alloc_extra.weak_memory { weak_memory.memory_accessed(range, machine.data_race.as_ref().unwrap()); @@ -1068,19 +1048,13 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { machine.emit_diagnostic(NonHaltingDiagnostic::FreedAlloc(alloc_id)); } if let Some(data_race) = &mut alloc_extra.data_race { - data_race.deallocate( - alloc_id, - range, - machine.data_race.as_mut().unwrap(), - &machine.threads, - )?; + data_race.deallocate(alloc_id, range, machine)?; } if let Some(stacked_borrows) = &mut alloc_extra.stacked_borrows { stacked_borrows.get_mut().before_memory_deallocation( alloc_id, prove_extra, range, - machine.stacked_borrows.as_ref().unwrap(), machine, ) } else { diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/stacked_borrows/mod.rs index 1759eee35855e..e412f64506638 100644 --- a/src/tools/miri/src/stacked_borrows/mod.rs +++ b/src/tools/miri/src/stacked_borrows/mod.rs @@ -644,7 +644,6 @@ impl Stacks { alloc_id: AllocId, tag: ProvenanceExtra, range: AllocRange, - state: &GlobalState, machine: &'ecx MiriMachine<'mir, 'tcx>, ) -> InterpResult<'tcx> where @@ -657,7 +656,7 @@ impl Stacks { range.size.bytes() ); let dcx = DiagnosticCxBuilder::read(machine, tag, range); - let state = state.borrow(); + let state = machine.stacked_borrows.as_ref().unwrap().borrow(); self.for_each(range, dcx, |stack, dcx, exposed_tags| { stack.access(AccessKind::Read, tag, &state, dcx, exposed_tags) }) @@ -669,8 +668,7 @@ impl Stacks { alloc_id: AllocId, tag: ProvenanceExtra, range: AllocRange, - state: &GlobalState, - machine: &'ecx MiriMachine<'mir, 'tcx>, + machine: &'ecx mut MiriMachine<'mir, 'tcx>, ) -> InterpResult<'tcx> { trace!( "write access with tag {:?}: {:?}, size {}", @@ -679,7 +677,7 @@ impl Stacks { range.size.bytes() ); let dcx = DiagnosticCxBuilder::write(machine, tag, range); - let state = state.borrow(); + let state = machine.stacked_borrows.as_ref().unwrap().borrow(); self.for_each(range, dcx, |stack, dcx, exposed_tags| { stack.access(AccessKind::Write, tag, &state, dcx, exposed_tags) }) @@ -691,12 +689,11 @@ impl Stacks { alloc_id: AllocId, tag: ProvenanceExtra, range: AllocRange, - state: &GlobalState, - machine: &'ecx MiriMachine<'mir, 'tcx>, + machine: &'ecx mut MiriMachine<'mir, 'tcx>, ) -> InterpResult<'tcx> { trace!("deallocation with tag {:?}: {:?}, size {}", tag, alloc_id, range.size.bytes()); let dcx = DiagnosticCxBuilder::dealloc(machine, tag); - let state = state.borrow(); + let state = machine.stacked_borrows.as_ref().unwrap().borrow(); self.for_each(range, dcx, |stack, dcx, exposed_tags| { stack.dealloc(tag, &state, dcx, exposed_tags) })?; From a3bd57823f602164fa94fde3e7997770bd97cea8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 26 Nov 2022 14:46:06 +0100 Subject: [PATCH 21/31] make Stacked Borrows retags act like data races --- src/tools/miri/src/stacked_borrows/mod.rs | 23 ++++++++++++-- .../tests/fail/data_race/alloc_read_race.rs | 2 +- .../tests/fail/data_race/alloc_write_race.rs | 2 +- .../data_race/atomic_read_na_write_race1.rs | 4 +-- .../data_race/atomic_read_na_write_race2.rs | 4 +-- .../data_race/atomic_write_na_read_race1.rs | 4 +-- .../data_race/atomic_write_na_read_race2.rs | 4 +-- .../data_race/atomic_write_na_write_race1.rs | 4 +-- .../data_race/atomic_write_na_write_race2.rs | 4 +-- .../data_race/dangling_thread_async_race.rs | 4 +-- .../fail/data_race/dangling_thread_race.rs | 4 +-- .../fail/data_race/dealloc_read_race1.rs | 4 +-- .../fail/data_race/dealloc_read_race2.rs | 4 +-- .../fail/data_race/dealloc_read_race_stack.rs | 2 +- .../fail/data_race/dealloc_write_race1.rs | 4 +-- .../fail/data_race/dealloc_write_race2.rs | 4 +-- .../data_race/dealloc_write_race_stack.rs | 2 +- .../data_race/enable_after_join_to_main.rs | 4 +-- .../tests/fail/data_race/fence_after_load.rs | 4 +-- .../tests/fail/data_race/read_write_race.rs | 4 +-- .../fail/data_race/read_write_race_stack.rs | 2 +- .../fail/data_race/relax_acquire_race.rs | 2 +- .../tests/fail/data_race/release_seq_race.rs | 2 +- .../data_race/release_seq_race_same_thread.rs | 2 +- .../miri/tests/fail/data_race/rmw_race.rs | 2 +- .../tests/fail/data_race/stack_pop_race.rs | 2 +- .../tests/fail/data_race/write_write_race.rs | 2 +- .../fail/data_race/write_write_race_stack.rs | 2 +- .../stacked_borrows/retag_data_race_read.rs | 31 +++++++++++++++++++ .../retag_data_race_read.stderr | 20 ++++++++++++ .../stacked_borrows/retag_data_race_write.rs | 31 +++++++++++++++++++ .../retag_data_race_write.stderr | 20 ++++++++++++ 32 files changed, 164 insertions(+), 45 deletions(-) create mode 100644 src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.rs create mode 100644 src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stderr create mode 100644 src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.rs create mode 100644 src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.stderr diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/stacked_borrows/mod.rs index e412f64506638..5093cddfd3b50 100644 --- a/src/tools/miri/src/stacked_borrows/mod.rs +++ b/src/tools/miri/src/stacked_borrows/mod.rs @@ -874,8 +874,8 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // We need a frozen-sensitive reborrow. // We have to use shared references to alloc/memory_extra here since // `visit_freeze_sensitive` needs to access the global state. - let extra = this.get_alloc_extra(alloc_id)?; - let mut stacked_borrows = extra + let alloc_extra = this.get_alloc_extra(alloc_id)?; + let mut stacked_borrows = alloc_extra .stacked_borrows .as_ref() .expect("we should have Stacked Borrows data") @@ -910,7 +910,16 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' ); stacked_borrows.for_each(range, dcx, |stack, dcx, exposed_tags| { stack.grant(orig_tag, item, access, &global, dcx, exposed_tags) - }) + })?; + drop(global); + if let Some(access) = access { + assert!(access == AccessKind::Read); + // Make sure the data race model also knows about this. + if let Some(data_race) = alloc_extra.data_race.as_ref() { + data_race.read(alloc_id, range, &this.machine)?; + } + } + Ok(()) })?; return Ok(Some(alloc_id)); } @@ -938,6 +947,14 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' stacked_borrows.for_each(range, dcx, |stack, dcx, exposed_tags| { stack.grant(orig_tag, item, access, &global, dcx, exposed_tags) })?; + drop(global); + if let Some(access) = access { + assert!(access == AccessKind::Write); + // Make sure the data race model also knows about this. + if let Some(data_race) = alloc_extra.data_race.as_mut() { + data_race.write(alloc_id, range, machine)?; + } + } Ok(Some(alloc_id)) } diff --git a/src/tools/miri/tests/fail/data_race/alloc_read_race.rs b/src/tools/miri/tests/fail/data_race/alloc_read_race.rs index 0bd3068af1ffe..6040452a166cb 100644 --- a/src/tools/miri/tests/fail/data_race/alloc_read_race.rs +++ b/src/tools/miri/tests/fail/data_race/alloc_read_race.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 +//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows #![feature(new_uninit)] use std::mem::MaybeUninit; diff --git a/src/tools/miri/tests/fail/data_race/alloc_write_race.rs b/src/tools/miri/tests/fail/data_race/alloc_write_race.rs index 7991280721e29..51d431b36f3a3 100644 --- a/src/tools/miri/tests/fail/data_race/alloc_write_race.rs +++ b/src/tools/miri/tests/fail/data_race/alloc_write_race.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 +//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows #![feature(new_uninit)] use std::ptr::null_mut; diff --git a/src/tools/miri/tests/fail/data_race/atomic_read_na_write_race1.rs b/src/tools/miri/tests/fail/data_race/atomic_read_na_write_race1.rs index 2b0446d724a02..79c6760b7c42a 100644 --- a/src/tools/miri/tests/fail/data_race/atomic_read_na_write_race1.rs +++ b/src/tools/miri/tests/fail/data_race/atomic_read_na_write_race1.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::sync::atomic::{AtomicUsize, Ordering}; use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/atomic_read_na_write_race2.rs b/src/tools/miri/tests/fail/data_race/atomic_read_na_write_race2.rs index ef5157515c64a..e069ac4ad6a83 100644 --- a/src/tools/miri/tests/fail/data_race/atomic_read_na_write_race2.rs +++ b/src/tools/miri/tests/fail/data_race/atomic_read_na_write_race2.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; diff --git a/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race1.rs b/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race1.rs index 8c17e76748438..9c025a0153d58 100644 --- a/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race1.rs +++ b/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race1.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; diff --git a/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race2.rs b/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race2.rs index f14d7c704dbb1..30b3c4863740c 100644 --- a/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race2.rs +++ b/src/tools/miri/tests/fail/data_race/atomic_write_na_read_race2.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::sync::atomic::{AtomicUsize, Ordering}; use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/atomic_write_na_write_race1.rs b/src/tools/miri/tests/fail/data_race/atomic_write_na_write_race1.rs index 0804b33407580..02b17cc57b61a 100644 --- a/src/tools/miri/tests/fail/data_race/atomic_write_na_write_race1.rs +++ b/src/tools/miri/tests/fail/data_race/atomic_write_na_write_race1.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::sync::atomic::{AtomicUsize, Ordering}; use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/atomic_write_na_write_race2.rs b/src/tools/miri/tests/fail/data_race/atomic_write_na_write_race2.rs index 658cddcc9c5b6..b5f4966d8842a 100644 --- a/src/tools/miri/tests/fail/data_race/atomic_write_na_write_race2.rs +++ b/src/tools/miri/tests/fail/data_race/atomic_write_na_write_race2.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; diff --git a/src/tools/miri/tests/fail/data_race/dangling_thread_async_race.rs b/src/tools/miri/tests/fail/data_race/dangling_thread_async_race.rs index af2588e923240..9922468e5f842 100644 --- a/src/tools/miri/tests/fail/data_race/dangling_thread_async_race.rs +++ b/src/tools/miri/tests/fail/data_race/dangling_thread_async_race.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-disable-isolation -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::mem; use std::thread::{sleep, spawn}; diff --git a/src/tools/miri/tests/fail/data_race/dangling_thread_race.rs b/src/tools/miri/tests/fail/data_race/dangling_thread_race.rs index 1ee619c3f99d5..8c8a6ac87f3ab 100644 --- a/src/tools/miri/tests/fail/data_race/dangling_thread_race.rs +++ b/src/tools/miri/tests/fail/data_race/dangling_thread_race.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-disable-isolation -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::mem; use std::thread::{sleep, spawn}; diff --git a/src/tools/miri/tests/fail/data_race/dealloc_read_race1.rs b/src/tools/miri/tests/fail/data_race/dealloc_read_race1.rs index cbc02549a2541..8e1216f5bf0a0 100644 --- a/src/tools/miri/tests/fail/data_race/dealloc_read_race1.rs +++ b/src/tools/miri/tests/fail/data_race/dealloc_read_race1.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/dealloc_read_race2.rs b/src/tools/miri/tests/fail/data_race/dealloc_read_race2.rs index 24cce5d6fac1c..38f76af9de137 100644 --- a/src/tools/miri/tests/fail/data_race/dealloc_read_race2.rs +++ b/src/tools/miri/tests/fail/data_race/dealloc_read_race2.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/dealloc_read_race_stack.rs b/src/tools/miri/tests/fail/data_race/dealloc_read_race_stack.rs index 5484370f35c17..665e5ce4a1706 100644 --- a/src/tools/miri/tests/fail/data_race/dealloc_read_race_stack.rs +++ b/src/tools/miri/tests/fail/data_race/dealloc_read_race_stack.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-disable-isolation -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 +//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::ptr::null_mut; use std::sync::atomic::{AtomicPtr, Ordering}; diff --git a/src/tools/miri/tests/fail/data_race/dealloc_write_race1.rs b/src/tools/miri/tests/fail/data_race/dealloc_write_race1.rs index 23bf73fe8c5ad..b36c6b5ac0e45 100644 --- a/src/tools/miri/tests/fail/data_race/dealloc_write_race1.rs +++ b/src/tools/miri/tests/fail/data_race/dealloc_write_race1.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/dealloc_write_race2.rs b/src/tools/miri/tests/fail/data_race/dealloc_write_race2.rs index 7c8033e2335e9..4af8b904626d2 100644 --- a/src/tools/miri/tests/fail/data_race/dealloc_write_race2.rs +++ b/src/tools/miri/tests/fail/data_race/dealloc_write_race2.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/dealloc_write_race_stack.rs b/src/tools/miri/tests/fail/data_race/dealloc_write_race_stack.rs index 1872abfe021b3..f851ce95785f7 100644 --- a/src/tools/miri/tests/fail/data_race/dealloc_write_race_stack.rs +++ b/src/tools/miri/tests/fail/data_race/dealloc_write_race_stack.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-disable-isolation -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 +//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::ptr::null_mut; use std::sync::atomic::{AtomicPtr, Ordering}; diff --git a/src/tools/miri/tests/fail/data_race/enable_after_join_to_main.rs b/src/tools/miri/tests/fail/data_race/enable_after_join_to_main.rs index c11239da7febb..27aa16a122f36 100644 --- a/src/tools/miri/tests/fail/data_race/enable_after_join_to_main.rs +++ b/src/tools/miri/tests/fail/data_race/enable_after_join_to_main.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/fence_after_load.rs b/src/tools/miri/tests/fail/data_race/fence_after_load.rs index ae443908598fe..4d436d51f9895 100644 --- a/src/tools/miri/tests/fail/data_race/fence_after_load.rs +++ b/src/tools/miri/tests/fail/data_race/fence_after_load.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-disable-isolation -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::sync::atomic::{fence, AtomicUsize, Ordering}; use std::sync::Arc; use std::thread; diff --git a/src/tools/miri/tests/fail/data_race/read_write_race.rs b/src/tools/miri/tests/fail/data_race/read_write_race.rs index 482dd2df7df91..b26ec6c41427a 100644 --- a/src/tools/miri/tests/fail/data_race/read_write_race.rs +++ b/src/tools/miri/tests/fail/data_race/read_write_race.rs @@ -1,5 +1,5 @@ -// We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +// We want to control preemption here. Stacked borrows interferes by having its own accesses. +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/read_write_race_stack.rs b/src/tools/miri/tests/fail/data_race/read_write_race_stack.rs index 1b4932439b010..2fbac173993e4 100644 --- a/src/tools/miri/tests/fail/data_race/read_write_race_stack.rs +++ b/src/tools/miri/tests/fail/data_race/read_write_race_stack.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-disable-isolation -Zmir-opt-level=0 -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 +//@compile-flags: -Zmir-opt-level=0 -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows // Note: mir-opt-level set to 0 to prevent the read of stack_var in thread 1 // from being optimized away and preventing the detection of the data-race. diff --git a/src/tools/miri/tests/fail/data_race/relax_acquire_race.rs b/src/tools/miri/tests/fail/data_race/relax_acquire_race.rs index 240b4c90eb225..24040a9496114 100644 --- a/src/tools/miri/tests/fail/data_race/relax_acquire_race.rs +++ b/src/tools/miri/tests/fail/data_race/relax_acquire_race.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 +//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::sync::atomic::{AtomicUsize, Ordering}; use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/release_seq_race.rs b/src/tools/miri/tests/fail/data_race/release_seq_race.rs index 5ae801278357b..2d7246858e108 100644 --- a/src/tools/miri/tests/fail/data_race/release_seq_race.rs +++ b/src/tools/miri/tests/fail/data_race/release_seq_race.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-disable-isolation -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 +//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::sync::atomic::{AtomicUsize, Ordering}; use std::thread::{sleep, spawn}; diff --git a/src/tools/miri/tests/fail/data_race/release_seq_race_same_thread.rs b/src/tools/miri/tests/fail/data_race/release_seq_race_same_thread.rs index 63e6dc2dd71b9..0f974e1c56d3f 100644 --- a/src/tools/miri/tests/fail/data_race/release_seq_race_same_thread.rs +++ b/src/tools/miri/tests/fail/data_race/release_seq_race_same_thread.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-disable-isolation -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 +//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::sync::atomic::{AtomicUsize, Ordering}; use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/rmw_race.rs b/src/tools/miri/tests/fail/data_race/rmw_race.rs index 122780d11aa1f..2d13da30b4639 100644 --- a/src/tools/miri/tests/fail/data_race/rmw_race.rs +++ b/src/tools/miri/tests/fail/data_race/rmw_race.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 +//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::sync::atomic::{AtomicUsize, Ordering}; use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/stack_pop_race.rs b/src/tools/miri/tests/fail/data_race/stack_pop_race.rs index 163f46eacc19f..cf5c2ed81cb1d 100644 --- a/src/tools/miri/tests/fail/data_race/stack_pop_race.rs +++ b/src/tools/miri/tests/fail/data_race/stack_pop_race.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-preemption-rate=0 +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::thread; #[derive(Copy, Clone)] diff --git a/src/tools/miri/tests/fail/data_race/write_write_race.rs b/src/tools/miri/tests/fail/data_race/write_write_race.rs index 13c31c87cbbae..60e9ac2ac6c38 100644 --- a/src/tools/miri/tests/fail/data_race/write_write_race.rs +++ b/src/tools/miri/tests/fail/data_race/write_write_race.rs @@ -1,5 +1,5 @@ // We want to control preemption here. -//@compile-flags: -Zmiri-preemption-rate=0 +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::thread::spawn; diff --git a/src/tools/miri/tests/fail/data_race/write_write_race_stack.rs b/src/tools/miri/tests/fail/data_race/write_write_race_stack.rs index 731ac8b26aa74..0a29dc13cba17 100644 --- a/src/tools/miri/tests/fail/data_race/write_write_race_stack.rs +++ b/src/tools/miri/tests/fail/data_race/write_write_race_stack.rs @@ -1,4 +1,4 @@ -//@compile-flags: -Zmiri-disable-isolation -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 +//@compile-flags: -Zmiri-disable-weak-memory-emulation -Zmiri-preemption-rate=0 -Zmiri-disable-stacked-borrows use std::ptr::null_mut; use std::sync::atomic::{AtomicPtr, Ordering}; diff --git a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.rs b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.rs new file mode 100644 index 0000000000000..309d7a22be64f --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.rs @@ -0,0 +1,31 @@ +//! Make sure that a retag acts like a write for the data race model. +//@compile-flags: -Zmiri-preemption-rate=0 +#[derive(Copy, Clone)] +struct SendPtr(*mut u8); + +unsafe impl Send for SendPtr {} + +fn thread_1(p: SendPtr) { + let p = p.0; + unsafe { + let _r = &*p; + } +} + +fn thread_2(p: SendPtr) { + let p = p.0; + unsafe { + *p = 5; //~ ERROR: Data race detected between Write on thread `` and Read on thread `` + } +} + +fn main() { + let mut x = 0; + let p = std::ptr::addr_of_mut!(x); + let p = SendPtr(p); + + let t1 = std::thread::spawn(move || thread_1(p)); + let t2 = std::thread::spawn(move || thread_2(p)); + let _ = t1.join(); + let _ = t2.join(); +} diff --git a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stderr b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stderr new file mode 100644 index 0000000000000..f25d689524d1b --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: Data race detected between Write on thread `` and Read on thread `` at ALLOC + --> $DIR/retag_data_race_read.rs:LL:CC + | +LL | *p = 5; + | ^^^^^^ Data race detected between Write on thread `` and Read on thread `` at ALLOC + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `thread_2` at $DIR/retag_data_race_read.rs:LL:CC +note: inside closure at $DIR/retag_data_race_read.rs:LL:CC + --> $DIR/retag_data_race_read.rs:LL:CC + | +LL | let t2 = std::thread::spawn(move || thread_2(p)); + | ^^^^^^^^^^^ + +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/stacked_borrows/retag_data_race_write.rs b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.rs new file mode 100644 index 0000000000000..9368a0a919eb3 --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.rs @@ -0,0 +1,31 @@ +//! Make sure that a retag acts like a write for the data race model. +//@compile-flags: -Zmiri-preemption-rate=0 +#[derive(Copy, Clone)] +struct SendPtr(*mut u8); + +unsafe impl Send for SendPtr {} + +fn thread_1(p: SendPtr) { + let p = p.0; + unsafe { + let _r = &mut *p; + } +} + +fn thread_2(p: SendPtr) { + let p = p.0; + unsafe { + *p = 5; //~ ERROR: Data race detected between Write on thread `` and Write on thread `` + } +} + +fn main() { + let mut x = 0; + let p = std::ptr::addr_of_mut!(x); + let p = SendPtr(p); + + let t1 = std::thread::spawn(move || thread_1(p)); + let t2 = std::thread::spawn(move || thread_2(p)); + let _ = t1.join(); + let _ = t2.join(); +} diff --git a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.stderr b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.stderr new file mode 100644 index 0000000000000..f97e6bb11e9d6 --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: Data race detected between Write on thread `` and Write on thread `` at ALLOC + --> $DIR/retag_data_race_write.rs:LL:CC + | +LL | *p = 5; + | ^^^^^^ Data race detected between Write on thread `` and Write on thread `` at ALLOC + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `thread_2` at $DIR/retag_data_race_write.rs:LL:CC +note: inside closure at $DIR/retag_data_race_write.rs:LL:CC + --> $DIR/retag_data_race_write.rs:LL:CC + | +LL | let t2 = std::thread::spawn(move || thread_2(p)); + | ^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + From edf8154695e697d5bef8fdd3341379a3f789fd62 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 27 Nov 2022 00:06:00 +0100 Subject: [PATCH 22/31] nits --- src/tools/miri/src/stacked_borrows/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/src/stacked_borrows/mod.rs b/src/tools/miri/src/stacked_borrows/mod.rs index 5093cddfd3b50..f6f5c063b39e8 100644 --- a/src/tools/miri/src/stacked_borrows/mod.rs +++ b/src/tools/miri/src/stacked_borrows/mod.rs @@ -913,7 +913,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' })?; drop(global); if let Some(access) = access { - assert!(access == AccessKind::Read); + assert_eq!(access, AccessKind::Read); // Make sure the data race model also knows about this. if let Some(data_race) = alloc_extra.data_race.as_ref() { data_race.read(alloc_id, range, &this.machine)?; @@ -949,7 +949,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' })?; drop(global); if let Some(access) = access { - assert!(access == AccessKind::Write); + assert_eq!(access, AccessKind::Write); // Make sure the data race model also knows about this. if let Some(data_race) = alloc_extra.data_race.as_mut() { data_race.write(alloc_id, range, machine)?; From 958d5918b0a3cf750c4fcf5bf4e584487eeb4552 Mon Sep 17 00:00:00 2001 From: nils <48135649+Nilstrieb@users.noreply.github.com> Date: Mon, 21 Nov 2022 09:01:26 +0100 Subject: [PATCH 23/31] Use `.wasm` extension when building for wasm in cargo-miri WASM uses the `.wasm` file extension for its binaries (just like how windows uses `.exe`), so we need to set that as well. --- src/tools/miri/cargo-miri/src/phases.rs | 3 ++- src/tools/miri/ci.sh | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index df36041c75ed3..64b3187305e1a 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/cargo-miri/src/phases.rs @@ -281,9 +281,10 @@ pub fn phase_rustc(mut args: impl Iterator, phase: RustcPhase) { eprintln!("[cargo-miri rustc] writing run info to `{}`", filename.display()); } info.store(&filename); - // For Windows, do the same thing again with `.exe` appended to the filename. + // For Windows and WASM, do the same thing again with `.exe`/`.wasm` appended to the filename. // (Need to do this here as cargo moves that "binary" to a different place before running it.) info.store(&out_filename("", ".exe")); + info.store(&out_filename("", ".wasm")); }; let runnable_crate = !info_query && is_runnable_crate(); diff --git a/src/tools/miri/ci.sh b/src/tools/miri/ci.sh index e528be8b037af..991463ec8662b 100755 --- a/src/tools/miri/ci.sh +++ b/src/tools/miri/ci.sh @@ -103,6 +103,7 @@ case $HOST_TARGET in MIRI_TEST_TARGET=i686-pc-windows-msvc run_tests MIRI_TEST_TARGET=x86_64-unknown-freebsd run_tests_minimal hello integer vec panic/panic concurrency/simple atomic data_race env/var MIRI_TEST_TARGET=aarch64-linux-android run_tests_minimal hello integer vec panic/panic + MIRI_TEST_TARGET=wasm32-wasi MIRI_NO_STD=1 run_tests_minimal no_std # supports std but miri doesn't support it MIRI_TEST_TARGET=thumbv7em-none-eabihf MIRI_NO_STD=1 run_tests_minimal no_std # no_std embedded architecture ;; x86_64-apple-darwin) From 1d42936b18d08ba414d9def35508d3baf2175c72 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Sun, 27 Nov 2022 11:15:06 +0000 Subject: [PATCH 24/31] Prefer doc comments over `//`-comments in compiler --- compiler/rustc_ast/src/ast.rs | 6 +- compiler/rustc_ast/src/attr/mod.rs | 12 ++-- compiler/rustc_ast/src/mut_visit.rs | 6 +- compiler/rustc_ast/src/token.rs | 20 +++--- compiler/rustc_ast/src/tokenstream.rs | 20 +++--- compiler/rustc_ast_pretty/src/helpers.rs | 4 +- .../rustc_ast_pretty/src/pprust/state/expr.rs | 8 +-- .../rustc_attr/src/session_diagnostics.rs | 4 +- .../rustc_borrowck/src/diagnostics/mod.rs | 4 +- compiler/rustc_borrowck/src/lib.rs | 2 +- .../src/type_check/liveness/polonius.rs | 4 +- .../src/deriving/generic/mod.rs | 12 ++-- .../rustc_builtin_macros/src/edition_panic.rs | 26 ++++---- .../rustc_builtin_macros/src/source_util.rs | 2 +- compiler/rustc_builtin_macros/src/test.rs | 14 ++-- .../rustc_builtin_macros/src/test_harness.rs | 4 +- .../src/value_and_place.rs | 4 +- compiler/rustc_codegen_gcc/src/context.rs | 4 +- .../src/coverageinfo/mod.rs | 2 +- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 2 +- compiler/rustc_codegen_llvm/src/llvm_util.rs | 8 +-- compiler/rustc_codegen_llvm/src/type_.rs | 2 +- compiler/rustc_codegen_ssa/src/back/link.rs | 2 +- compiler/rustc_codegen_ssa/src/back/linker.rs | 6 +- .../rustc_codegen_ssa/src/back/metadata.rs | 64 +++++++++---------- compiler/rustc_codegen_ssa/src/back/write.rs | 18 +++--- .../src/debuginfo/type_names.rs | 10 +-- compiler/rustc_codegen_ssa/src/mir/operand.rs | 4 +- .../rustc_const_eval/src/interpret/machine.rs | 4 +- .../src/interpret/projection.rs | 4 +- .../rustc_const_eval/src/interpret/visitor.rs | 2 +- .../src/transform/check_consts/mod.rs | 16 ++--- .../src/transform/check_consts/ops.rs | 2 +- .../rustc_const_eval/src/util/aggregate.rs | 3 +- compiler/rustc_driver/src/lib.rs | 4 +- compiler/rustc_expand/src/base.rs | 2 +- compiler/rustc_expand/src/build.rs | 2 +- compiler/rustc_expand/src/config.rs | 2 +- compiler/rustc_expand/src/expand.rs | 11 ++-- compiler/rustc_hir/src/hir.rs | 26 ++++---- .../rustc_hir_analysis/src/astconv/mod.rs | 4 +- .../rustc_hir_analysis/src/check/dropck.rs | 7 +- .../rustc_hir_analysis/src/variance/terms.rs | 14 ++-- compiler/rustc_hir_typeck/src/demand.rs | 4 +- compiler/rustc_hir_typeck/src/expectation.rs | 6 +- compiler/rustc_hir_typeck/src/expr.rs | 4 +- compiler/rustc_hir_typeck/src/inherited.rs | 20 +++--- .../src/persist/dirty_clean.rs | 6 +- .../rustc_infer/src/infer/free_regions.rs | 8 +-- .../rustc_infer/src/infer/opaque_types.rs | 26 ++++---- .../src/infer/opaque_types/table.rs | 8 +-- compiler/rustc_lint/src/context.rs | 2 +- compiler/rustc_metadata/src/rmeta/decoder.rs | 8 +-- .../rustc_middle/src/mir/interpret/value.rs | 4 +- compiler/rustc_middle/src/traits/mod.rs | 2 +- compiler/rustc_middle/src/ty/closure.rs | 6 +- compiler/rustc_middle/src/ty/context.rs | 38 +++++------ compiler/rustc_middle/src/ty/flags.rs | 2 +- compiler/rustc_middle/src/ty/mod.rs | 5 +- compiler/rustc_middle/src/ty/sty.rs | 2 +- compiler/rustc_middle/src/ty/trait_def.rs | 4 +- compiler/rustc_middle/src/ty/util.rs | 10 +-- .../src/build/expr/category.rs | 28 ++++---- compiler/rustc_mir_build/src/build/misc.rs | 4 +- compiler/rustc_mir_build/src/build/scope.rs | 8 ++- .../src/add_moves_for_packed_drops.rs | 61 +++++++++--------- .../src/elaborate_box_derefs.rs | 2 +- compiler/rustc_monomorphize/src/collector.rs | 6 +- .../rustc_parse/src/lexer/unicode_chars.rs | 4 +- .../rustc_parse/src/parser/attr_wrapper.rs | 2 +- .../rustc_parse/src/parser/diagnostics.rs | 6 +- .../rustc_query_system/src/dep_graph/graph.rs | 24 +++---- compiler/rustc_resolve/src/imports.rs | 8 +-- compiler/rustc_save_analysis/src/lib.rs | 4 +- compiler/rustc_serialize/src/leb128.rs | 2 +- compiler/rustc_serialize/src/opaque.rs | 26 ++++---- compiler/rustc_span/src/lib.rs | 14 ++-- compiler/rustc_span/src/source_map.rs | 22 +++---- compiler/rustc_span/src/symbol.rs | 4 +- compiler/rustc_target/src/spec/mod.rs | 14 ++-- .../src/traits/auto_trait.rs | 2 +- .../src/traits/project.rs | 2 +- .../src/traits/specialize/mod.rs | 2 +- 83 files changed, 400 insertions(+), 387 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index b48a7d29f5097..8bb4442d1bb27 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -111,8 +111,8 @@ impl HashStable for Path { } impl Path { - // Convert a span and an identifier to the corresponding - // one-segment path. + /// Convert a span and an identifier to the corresponding + /// one-segment path. pub fn from_ident(ident: Ident) -> Path { Path { segments: thin_vec![PathSegment::from_ident(ident)], span: ident.span, tokens: None } } @@ -1283,7 +1283,7 @@ impl Expr { ) } - // To a first-order approximation, is this a pattern + /// To a first-order approximation, is this a pattern? pub fn is_approximately_pattern(&self) -> bool { match &self.peel_parens().kind { ExprKind::Box(_) diff --git a/compiler/rustc_ast/src/attr/mod.rs b/compiler/rustc_ast/src/attr/mod.rs index 3e0129531150c..c948faeb35835 100644 --- a/compiler/rustc_ast/src/attr/mod.rs +++ b/compiler/rustc_ast/src/attr/mod.rs @@ -26,9 +26,9 @@ use thin_vec::thin_vec; pub struct MarkedAttrs(GrowableBitSet); impl MarkedAttrs { - // We have no idea how many attributes there will be, so just - // initiate the vectors with 0 bits. We'll grow them as necessary. pub fn new() -> Self { + // We have no idea how many attributes there will be, so just + // initiate the vectors with 0 bits. We'll grow them as necessary. MarkedAttrs(GrowableBitSet::new_empty()) } @@ -174,9 +174,11 @@ impl MetaItem { self.ident().unwrap_or_else(Ident::empty).name } - // Example: - // #[attribute(name = "value")] - // ^^^^^^^^^^^^^^ + /// ```text + /// Example: + /// #[attribute(name = "value")] + /// ^^^^^^^^^^^^^^ + /// ``` pub fn name_value_literal(&self) -> Option<&Lit> { match &self.kind { MetaItemKind::NameValue(v) => Some(v), diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index a5b24c403dd37..11def67c46365 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -725,10 +725,10 @@ pub fn visit_lazy_tts(lazy_tts: &mut Option, visit_lazy_tts_opt_mut(lazy_tts.as_mut(), vis); } +/// Applies ident visitor if it's an ident; applies other visits to interpolated nodes. +/// In practice the ident part is not actually used by specific visitors right now, +/// but there's a test below checking that it works. // No `noop_` prefix because there isn't a corresponding method in `MutVisitor`. -// Applies ident visitor if it's an ident; applies other visits to interpolated nodes. -// In practice the ident part is not actually used by specific visitors right now, -// but there's a test below checking that it works. pub fn visit_token(t: &mut Token, vis: &mut T) { let Token { kind, span } = t; match kind { diff --git a/compiler/rustc_ast/src/token.rs b/compiler/rustc_ast/src/token.rs index cb32925584c58..c0cc4e79a3d53 100644 --- a/compiler/rustc_ast/src/token.rs +++ b/compiler/rustc_ast/src/token.rs @@ -302,9 +302,9 @@ impl TokenKind { Literal(Lit::new(kind, symbol, suffix)) } - // An approximation to proc-macro-style single-character operators used by rustc parser. - // If the operator token can be broken into two tokens, the first of which is single-character, - // then this function performs that operation, otherwise it returns `None`. + /// An approximation to proc-macro-style single-character operators used by rustc parser. + /// If the operator token can be broken into two tokens, the first of which is single-character, + /// then this function performs that operation, otherwise it returns `None`. pub fn break_two_token_op(&self) -> Option<(TokenKind, TokenKind)> { Some(match *self { Le => (Lt, Eq), @@ -538,10 +538,10 @@ impl Token { } } - // A convenience function for matching on identifiers during parsing. - // Turns interpolated identifier (`$i: ident`) or lifetime (`$l: lifetime`) token - // into the regular identifier or lifetime token it refers to, - // otherwise returns the original token. + /// A convenience function for matching on identifiers during parsing. + /// Turns interpolated identifier (`$i: ident`) or lifetime (`$l: lifetime`) token + /// into the regular identifier or lifetime token it refers to, + /// otherwise returns the original token. pub fn uninterpolate(&self) -> Cow<'_, Token> { match &self.kind { Interpolated(nt) => match **nt { @@ -621,7 +621,7 @@ impl Token { false } - // Is the token an interpolated block (`$b:block`)? + /// Is the token an interpolated block (`$b:block`)? pub fn is_whole_block(&self) -> bool { if let Interpolated(nt) = &self.kind && let NtBlock(..) = **nt { return true; @@ -665,8 +665,8 @@ impl Token { self.is_non_raw_ident_where(Ident::is_path_segment_keyword) } - // Returns true for reserved identifiers used internally for elided lifetimes, - // unnamed method parameters, crate root module, error recovery etc. + /// Returns true for reserved identifiers used internally for elided lifetimes, + /// unnamed method parameters, crate root module, error recovery etc. pub fn is_special_ident(&self) -> bool { self.is_non_raw_ident_where(Ident::is_special) } diff --git a/compiler/rustc_ast/src/tokenstream.rs b/compiler/rustc_ast/src/tokenstream.rs index 015f5c1ee8ae5..58c6d397ea270 100644 --- a/compiler/rustc_ast/src/tokenstream.rs +++ b/compiler/rustc_ast/src/tokenstream.rs @@ -86,12 +86,12 @@ impl TokenTree { } } - // Create a `TokenTree::Token` with alone spacing. + /// Create a `TokenTree::Token` with alone spacing. pub fn token_alone(kind: TokenKind, span: Span) -> TokenTree { TokenTree::Token(Token::new(kind, span), Spacing::Alone) } - // Create a `TokenTree::Token` with joint spacing. + /// Create a `TokenTree::Token` with joint spacing. pub fn token_joint(kind: TokenKind, span: Span) -> TokenTree { TokenTree::Token(Token::new(kind, span), Spacing::Joint) } @@ -413,17 +413,17 @@ impl TokenStream { TokenStream(Lrc::new(self.0.iter().enumerate().map(|(i, tree)| f(i, tree)).collect())) } - // Create a token stream containing a single token with alone spacing. + /// Create a token stream containing a single token with alone spacing. pub fn token_alone(kind: TokenKind, span: Span) -> TokenStream { TokenStream::new(vec![TokenTree::token_alone(kind, span)]) } - // Create a token stream containing a single token with joint spacing. + /// Create a token stream containing a single token with joint spacing. pub fn token_joint(kind: TokenKind, span: Span) -> TokenStream { TokenStream::new(vec![TokenTree::token_joint(kind, span)]) } - // Create a token stream containing a single `Delimited`. + /// Create a token stream containing a single `Delimited`. pub fn delimited(span: DelimSpan, delim: Delimiter, tts: TokenStream) -> TokenStream { TokenStream::new(vec![TokenTree::Delimited(span, delim, tts)]) } @@ -522,8 +522,8 @@ impl TokenStream { } } - // Push `tt` onto the end of the stream, possibly gluing it to the last - // token. Uses `make_mut` to maximize efficiency. + /// Push `tt` onto the end of the stream, possibly gluing it to the last + /// token. Uses `make_mut` to maximize efficiency. pub fn push_tree(&mut self, tt: TokenTree) { let vec_mut = Lrc::make_mut(&mut self.0); @@ -534,9 +534,9 @@ impl TokenStream { } } - // Push `stream` onto the end of the stream, possibly gluing the first - // token tree to the last token. (No other token trees will be glued.) - // Uses `make_mut` to maximize efficiency. + /// Push `stream` onto the end of the stream, possibly gluing the first + /// token tree to the last token. (No other token trees will be glued.) + /// Uses `make_mut` to maximize efficiency. pub fn push_stream(&mut self, stream: TokenStream) { let vec_mut = Lrc::make_mut(&mut self.0); diff --git a/compiler/rustc_ast_pretty/src/helpers.rs b/compiler/rustc_ast_pretty/src/helpers.rs index 5ec71cddf7de6..c3e0eccd3d404 100644 --- a/compiler/rustc_ast_pretty/src/helpers.rs +++ b/compiler/rustc_ast_pretty/src/helpers.rs @@ -36,8 +36,8 @@ impl Printer { self.nbsp() } - // Synthesizes a comment that was not textually present in the original - // source file. + /// Synthesizes a comment that was not textually present in the original + /// source file. pub fn synth_comment(&mut self, text: impl Into>) { self.word("/*"); self.space(); diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs index 4b37fa027f53b..949d98f96ab6a 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs @@ -58,10 +58,10 @@ impl<'a> State<'a> { self.print_expr_cond_paren(expr, Self::cond_needs_par(expr)) } - // Does `expr` need parentheses when printed in a condition position? - // - // These cases need parens due to the parse error observed in #26461: `if return {}` - // parses as the erroneous construct `if (return {})`, not `if (return) {}`. + /// Does `expr` need parentheses when printed in a condition position? + /// + /// These cases need parens due to the parse error observed in #26461: `if return {}` + /// parses as the erroneous construct `if (return {})`, not `if (return) {}`. pub(super) fn cond_needs_par(expr: &ast::Expr) -> bool { match expr.kind { ast::ExprKind::Break(..) diff --git a/compiler/rustc_attr/src/session_diagnostics.rs b/compiler/rustc_attr/src/session_diagnostics.rs index edccfa1c8ffa2..91c6bcb08a079 100644 --- a/compiler/rustc_attr/src/session_diagnostics.rs +++ b/compiler/rustc_attr/src/session_diagnostics.rs @@ -41,7 +41,7 @@ pub(crate) struct IncorrectMetaItem { pub span: Span, } -// Error code: E0541 +/// Error code: E0541 pub(crate) struct UnknownMetaItem<'a> { pub span: Span, pub item: String, @@ -200,7 +200,7 @@ pub(crate) struct InvalidReprHintNoValue { pub name: String, } -// Error code: E0565 +/// Error code: E0565 pub(crate) struct UnsupportedLiteral { pub span: Span, pub reason: UnsupportedLiteralReason, diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index c500cbc49e4a3..86c5d9cfa8121 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -590,7 +590,7 @@ impl UseSpans<'_> { } } - // Add a span label to the arguments of the closure, if it exists. + /// Add a span label to the arguments of the closure, if it exists. pub(super) fn args_span_label(self, err: &mut Diagnostic, message: impl Into) { if let UseSpans::ClosureUse { args_span, .. } = self { err.span_label(args_span, message); @@ -628,7 +628,7 @@ impl UseSpans<'_> { } } - // Add a span label to the use of the captured variable, if it exists. + /// Add a span label to the use of the captured variable, if it exists. pub(super) fn var_span_label( self, err: &mut Diagnostic, diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 163170a1d1aa0..4d87ecf5e44be 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -83,7 +83,7 @@ mod type_check; mod universal_regions; mod used_muts; -// A public API provided for the Rust compiler consumers. +/// A public API provided for the Rust compiler consumers. pub mod consumers; use borrow_set::{BorrowData, BorrowSet}; diff --git a/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs b/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs index bc76a465e3c3a..b344ab46adbde 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/polonius.rs @@ -121,8 +121,8 @@ pub(super) fn populate_access_facts<'a, 'tcx>( } } -// For every potentially drop()-touched region `region` in `local`'s type -// (`kind`), emit a Polonius `use_of_var_derefs_origin(local, origin)` fact. +/// For every potentially drop()-touched region `region` in `local`'s type +/// (`kind`), emit a Polonius `use_of_var_derefs_origin(local, origin)` fact. pub(super) fn add_drop_of_var_derefs_origin<'tcx>( typeck: &mut TypeChecker<'_, 'tcx>, local: Local, diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index 3309fab224fb7..1467d4eaec068 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -300,12 +300,12 @@ struct TypeParameter { ty: P, } -// The code snippets built up for derived code are sometimes used as blocks -// (e.g. in a function body) and sometimes used as expressions (e.g. in a match -// arm). This structure avoids committing to either form until necessary, -// avoiding the insertion of any unnecessary blocks. -// -// The statements come before the expression. +/// The code snippets built up for derived code are sometimes used as blocks +/// (e.g. in a function body) and sometimes used as expressions (e.g. in a match +/// arm). This structure avoids committing to either form until necessary, +/// avoiding the insertion of any unnecessary blocks. +/// +/// The statements come before the expression. pub struct BlockOrExpr(Vec, Option>); impl BlockOrExpr { diff --git a/compiler/rustc_builtin_macros/src/edition_panic.rs b/compiler/rustc_builtin_macros/src/edition_panic.rs index cae648cd11aff..b2a21611db7f9 100644 --- a/compiler/rustc_builtin_macros/src/edition_panic.rs +++ b/compiler/rustc_builtin_macros/src/edition_panic.rs @@ -6,15 +6,15 @@ use rustc_span::edition::Edition; use rustc_span::symbol::sym; use rustc_span::Span; -// This expands to either -// - `$crate::panic::panic_2015!(...)` or -// - `$crate::panic::panic_2021!(...)` -// depending on the edition. -// -// This is used for both std::panic!() and core::panic!(). -// -// `$crate` will refer to either the `std` or `core` crate depending on which -// one we're expanding from. +/// This expands to either +/// - `$crate::panic::panic_2015!(...)` or +/// - `$crate::panic::panic_2021!(...)` +/// depending on the edition. +/// +/// This is used for both std::panic!() and core::panic!(). +/// +/// `$crate` will refer to either the `std` or `core` crate depending on which +/// one we're expanding from. pub fn expand_panic<'cx>( cx: &'cx mut ExtCtxt<'_>, sp: Span, @@ -24,10 +24,10 @@ pub fn expand_panic<'cx>( expand(mac, cx, sp, tts) } -// This expands to either -// - `$crate::panic::unreachable_2015!(...)` or -// - `$crate::panic::unreachable_2021!(...)` -// depending on the edition. +/// This expands to either +/// - `$crate::panic::unreachable_2015!(...)` or +/// - `$crate::panic::unreachable_2021!(...)` +/// depending on the edition. pub fn expand_unreachable<'cx>( cx: &'cx mut ExtCtxt<'_>, sp: Span, diff --git a/compiler/rustc_builtin_macros/src/source_util.rs b/compiler/rustc_builtin_macros/src/source_util.rs index 3411bd40c9de5..0b17e92efe936 100644 --- a/compiler/rustc_builtin_macros/src/source_util.rs +++ b/compiler/rustc_builtin_macros/src/source_util.rs @@ -164,7 +164,7 @@ pub fn expand_include<'cx>( Box::new(ExpandResult { p, node_id: cx.current_expansion.lint_node_id }) } -// include_str! : read the given file, insert it as a literal string expr +/// `include_str!`: read the given file, insert it as a literal string expr pub fn expand_include_str( cx: &mut ExtCtxt<'_>, sp: Span, diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs index b62840d4bc822..82baf1da28f2f 100644 --- a/compiler/rustc_builtin_macros/src/test.rs +++ b/compiler/rustc_builtin_macros/src/test.rs @@ -13,13 +13,13 @@ use rustc_span::Span; use std::iter; use thin_vec::thin_vec; -// #[test_case] is used by custom test authors to mark tests -// When building for test, it needs to make the item public and gensym the name -// Otherwise, we'll omit the item. This behavior means that any item annotated -// with #[test_case] is never addressable. -// -// We mark item with an inert attribute "rustc_test_marker" which the test generation -// logic will pick up on. +/// #[test_case] is used by custom test authors to mark tests +/// When building for test, it needs to make the item public and gensym the name +/// Otherwise, we'll omit the item. This behavior means that any item annotated +/// with #[test_case] is never addressable. +/// +/// We mark item with an inert attribute "rustc_test_marker" which the test generation +/// logic will pick up on. pub fn expand_test_case( ecx: &mut ExtCtxt<'_>, attr_sp: Span, diff --git a/compiler/rustc_builtin_macros/src/test_harness.rs b/compiler/rustc_builtin_macros/src/test_harness.rs index b8b8351a36f41..3269f62b105b9 100644 --- a/compiler/rustc_builtin_macros/src/test_harness.rs +++ b/compiler/rustc_builtin_macros/src/test_harness.rs @@ -34,8 +34,8 @@ struct TestCtxt<'a> { test_runner: Option, } -// Traverse the crate, collecting all the test functions, eliding any -// existing main functions, and synthesizing a main test harness +/// Traverse the crate, collecting all the test functions, eliding any +/// existing main functions, and synthesizing a main test harness pub fn inject(sess: &Session, resolver: &mut dyn ResolverExpand, krate: &mut ast::Crate) { let span_diagnostic = sess.diagnostic(); let panic_strategy = sess.panic_strategy(); diff --git a/compiler/rustc_codegen_cranelift/src/value_and_place.rs b/compiler/rustc_codegen_cranelift/src/value_and_place.rs index c5bd574623df6..34746ff6b6645 100644 --- a/compiler/rustc_codegen_cranelift/src/value_and_place.rs +++ b/compiler/rustc_codegen_cranelift/src/value_and_place.rs @@ -108,8 +108,8 @@ impl<'tcx> CValue<'tcx> { } // FIXME remove - // Forces the data value of a dyn* value to the stack and returns a pointer to it as well as the - // vtable pointer. + /// Forces the data value of a dyn* value to the stack and returns a pointer to it as well as the + /// vtable pointer. pub(crate) fn dyn_star_force_data_on_stack( self, fx: &mut FunctionCx<'_, '_, 'tcx>, diff --git a/compiler/rustc_codegen_gcc/src/context.rs b/compiler/rustc_codegen_gcc/src/context.rs index 2e71c3665daa7..837708aeb0ea9 100644 --- a/compiler/rustc_codegen_gcc/src/context.rs +++ b/compiler/rustc_codegen_gcc/src/context.rs @@ -88,9 +88,9 @@ pub struct CodegenCx<'gcc, 'tcx> { pub vtables: RefCell, Option>), RValue<'gcc>>>, // TODO(antoyo): improve the SSA API to not require those. - // Mapping from function pointer type to indexes of on stack parameters. + /// Mapping from function pointer type to indexes of on stack parameters. pub on_stack_params: RefCell, FxHashSet>>, - // Mapping from function to indexes of on stack parameters. + /// Mapping from function to indexes of on stack parameters. pub on_stack_function_params: RefCell, FxHashSet>>, /// Cache of emitted const globals (value -> global) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 964a632b6eedd..ace15cfb02477 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -37,7 +37,7 @@ const VAR_ALIGN_BYTES: usize = 8; /// A context object for maintaining all state needed by the coverageinfo module. pub struct CrateCoverageContext<'ll, 'tcx> { - // Coverage data for each instrumented function identified by DefId. + /// Coverage data for each instrumented function identified by DefId. pub(crate) function_coverage_map: RefCell, FunctionCoverage<'tcx>>>, pub(crate) pgo_func_name_var_map: RefCell, &'ll llvm::Value>>, } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index f451984973048..c14e1656291e8 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -35,7 +35,7 @@ pub enum LLVMRustResult { pub struct LLVMRustCOFFShortExport { pub name: *const c_char, pub ordinal_present: bool, - // value of `ordinal` only important when `ordinal_present` is true + /// value of `ordinal` only important when `ordinal_present` is true pub ordinal: u16, } diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index 4af1aaec0a112..bc3a94a402706 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -194,8 +194,8 @@ pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> SmallVec<[&'a str; 2] } } -// Given a map from target_features to whether they are enabled or disabled, -// ensure only valid combinations are allowed. +/// Given a map from target_features to whether they are enabled or disabled, +/// ensure only valid combinations are allowed. pub fn check_tied_features( sess: &Session, features: &FxHashMap<&str, bool>, @@ -213,8 +213,8 @@ pub fn check_tied_features( return None; } -// Used to generate cfg variables and apply features -// Must express features in the way Rust understands them +/// Used to generate cfg variables and apply features +/// Must express features in the way Rust understands them pub fn target_features(sess: &Session, allow_unstable: bool) -> Vec { let target_machine = create_informational_target_machine(sess); let mut features: Vec = supported_target_features(sess) diff --git a/compiler/rustc_codegen_llvm/src/type_.rs b/compiler/rustc_codegen_llvm/src/type_.rs index 5eec7dc613028..5772b7e1d812a 100644 --- a/compiler/rustc_codegen_llvm/src/type_.rs +++ b/compiler/rustc_codegen_llvm/src/type_.rs @@ -238,7 +238,7 @@ impl Type { unsafe { llvm::LLVMInt8TypeInContext(llcx) } } - // Creates an integer type with the given number of bits, e.g., i24 + /// Creates an integer type with the given number of bits, e.g., i24 pub fn ix_llcx(llcx: &llvm::Context, num_bits: u64) -> &Type { unsafe { llvm::LLVMIntTypeInContext(llcx, num_bits as c_uint) } } diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 762430c618721..39cd4a35f1778 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1179,7 +1179,7 @@ pub fn ignored_for_lto(sess: &Session, info: &CrateInfo, cnum: CrateNum) -> bool && (info.compiler_builtins == Some(cnum) || info.is_no_builtins.contains(&cnum)) } -// This functions tries to determine the appropriate linker (and corresponding LinkerFlavor) to use +/// This functions tries to determine the appropriate linker (and corresponding LinkerFlavor) to use pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) { fn infer_from( sess: &Session, diff --git a/compiler/rustc_codegen_ssa/src/back/linker.rs b/compiler/rustc_codegen_ssa/src/back/linker.rs index 7f0c2861f7e29..ff0c1ac4916f2 100644 --- a/compiler/rustc_codegen_ssa/src/back/linker.rs +++ b/compiler/rustc_codegen_ssa/src/back/linker.rs @@ -34,9 +34,9 @@ pub fn disable_localization(linker: &mut Command) { linker.env("VSLANG", "1033"); } -// The third parameter is for env vars, used on windows to set up the -// path for MSVC to find its DLLs, and gcc to find its bundled -// toolchain +/// The third parameter is for env vars, used on windows to set up the +/// path for MSVC to find its DLLs, and gcc to find its bundled +/// toolchain pub fn get_linker<'a>( sess: &'a Session, linker: &Path, diff --git a/compiler/rustc_codegen_ssa/src/back/metadata.rs b/compiler/rustc_codegen_ssa/src/back/metadata.rs index 780a385003681..51c5c375d5191 100644 --- a/compiler/rustc_codegen_ssa/src/back/metadata.rs +++ b/compiler/rustc_codegen_ssa/src/back/metadata.rs @@ -191,38 +191,38 @@ pub enum MetadataPosition { Last, } -// For rlibs we "pack" rustc metadata into a dummy object file. -// -// Historically it was needed because rustc linked rlibs as whole-archive in some cases. -// In that case linkers try to include all files located in an archive, so if metadata is stored -// in an archive then it needs to be of a form that the linker is able to process. -// Now it's not clear whether metadata still needs to be wrapped into an object file or not. -// -// Note, though, that we don't actually want this metadata to show up in any -// final output of the compiler. Instead this is purely for rustc's own -// metadata tracking purposes. -// -// With the above in mind, each "flavor" of object format gets special -// handling here depending on the target: -// -// * MachO - macos-like targets will insert the metadata into a section that -// is sort of fake dwarf debug info. Inspecting the source of the macos -// linker this causes these sections to be skipped automatically because -// it's not in an allowlist of otherwise well known dwarf section names to -// go into the final artifact. -// -// * WebAssembly - we actually don't have any container format for this -// target. WebAssembly doesn't support the `dylib` crate type anyway so -// there's no need for us to support this at this time. Consequently the -// metadata bytes are simply stored as-is into an rlib. -// -// * COFF - Windows-like targets create an object with a section that has -// the `IMAGE_SCN_LNK_REMOVE` flag set which ensures that if the linker -// ever sees the section it doesn't process it and it's removed. -// -// * ELF - All other targets are similar to Windows in that there's a -// `SHF_EXCLUDE` flag we can set on sections in an object file to get -// automatically removed from the final output. +/// For rlibs we "pack" rustc metadata into a dummy object file. +/// +/// Historically it was needed because rustc linked rlibs as whole-archive in some cases. +/// In that case linkers try to include all files located in an archive, so if metadata is stored +/// in an archive then it needs to be of a form that the linker is able to process. +/// Now it's not clear whether metadata still needs to be wrapped into an object file or not. +/// +/// Note, though, that we don't actually want this metadata to show up in any +/// final output of the compiler. Instead this is purely for rustc's own +/// metadata tracking purposes. +/// +/// With the above in mind, each "flavor" of object format gets special +/// handling here depending on the target: +/// +/// * MachO - macos-like targets will insert the metadata into a section that +/// is sort of fake dwarf debug info. Inspecting the source of the macos +/// linker this causes these sections to be skipped automatically because +/// it's not in an allowlist of otherwise well known dwarf section names to +/// go into the final artifact. +/// +/// * WebAssembly - we actually don't have any container format for this +/// target. WebAssembly doesn't support the `dylib` crate type anyway so +/// there's no need for us to support this at this time. Consequently the +/// metadata bytes are simply stored as-is into an rlib. +/// +/// * COFF - Windows-like targets create an object with a section that has +/// the `IMAGE_SCN_LNK_REMOVE` flag set which ensures that if the linker +/// ever sees the section it doesn't process it and it's removed. +/// +/// * ELF - All other targets are similar to Windows in that there's a +/// `SHF_EXCLUDE` flag we can set on sections in an object file to get +/// automatically removed from the final output. pub fn create_wrapper_file( sess: &Session, section_name: Vec, diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index e3d28a1aca20e..12fca64968aac 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -340,20 +340,20 @@ pub struct CodegenContext { pub split_debuginfo: rustc_target::spec::SplitDebuginfo, pub split_dwarf_kind: rustc_session::config::SplitDwarfKind, - // Number of cgus excluding the allocator/metadata modules + /// Number of cgus excluding the allocator/metadata modules pub total_cgus: usize, - // Handler to use for diagnostics produced during codegen. + /// Handler to use for diagnostics produced during codegen. pub diag_emitter: SharedEmitter, - // LLVM optimizations for which we want to print remarks. + /// LLVM optimizations for which we want to print remarks. pub remark: Passes, - // Worker thread number + /// Worker thread number pub worker: usize, - // The incremental compilation session directory, or None if we are not - // compiling incrementally + /// The incremental compilation session directory, or None if we are not + /// compiling incrementally pub incr_comp_session_dir: Option, - // Used to update CGU re-use information during the thinlto phase. + /// Used to update CGU re-use information during the thinlto phase. pub cgu_reuse_tracker: CguReuseTracker, - // Channel back to the main control thread to send messages to + /// Channel back to the main control thread to send messages to pub coordinator_send: Sender>, } @@ -756,7 +756,7 @@ fn execute_work_item( } } -// Actual LTO type we end up choosing based on multiple factors. +/// Actual LTO type we end up choosing based on multiple factors. pub enum ComputedLtoType { No, Thin, diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index 8647fbace2a75..b004fbf85a97f 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -1,4 +1,4 @@ -// Type Names for Debug Info. +//! Type Names for Debug Info. // Notes on targeting MSVC: // In general, MSVC's debugger attempts to parse all arguments as C++ expressions, @@ -26,10 +26,10 @@ use std::fmt::Write; use crate::debuginfo::wants_c_like_enum_debuginfo; -// Compute the name of the type as it should be stored in debuginfo. Does not do -// any caching, i.e., calling the function twice with the same type will also do -// the work twice. The `qualified` parameter only affects the first level of the -// type name, further levels (i.e., type parameters) are always fully qualified. +/// Compute the name of the type as it should be stored in debuginfo. Does not do +/// any caching, i.e., calling the function twice with the same type will also do +/// the work twice. The `qualified` parameter only affects the first level of the +/// type name, further levels (i.e., type parameters) are always fully qualified. pub fn compute_debuginfo_type_name<'tcx>( tcx: TyCtxt<'tcx>, t: Ty<'tcx>, diff --git a/compiler/rustc_codegen_ssa/src/mir/operand.rs b/compiler/rustc_codegen_ssa/src/mir/operand.rs index e6ba642a7ed0e..34a5b638d7eba 100644 --- a/compiler/rustc_codegen_ssa/src/mir/operand.rs +++ b/compiler/rustc_codegen_ssa/src/mir/operand.rs @@ -40,10 +40,10 @@ pub enum OperandValue { /// instead. #[derive(Copy, Clone)] pub struct OperandRef<'tcx, V> { - // The value. + /// The value. pub val: OperandValue, - // The layout of value, based on its Rust type. + /// The layout of value, based on its Rust type. pub layout: TyAndLayout<'tcx>, } diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index 351152eba01f6..88d25be6bd861 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -417,8 +417,8 @@ pub trait Machine<'mir, 'tcx>: Sized { } } -// A lot of the flexibility above is just needed for `Miri`, but all "compile-time" machines -// (CTFE and ConstProp) use the same instance. Here, we share that code. +/// A lot of the flexibility above is just needed for `Miri`, but all "compile-time" machines +/// (CTFE and ConstProp) use the same instance. Here, we share that code. pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) { type Provenance = AllocId; type ProvenanceExtra = (); diff --git a/compiler/rustc_const_eval/src/interpret/projection.rs b/compiler/rustc_const_eval/src/interpret/projection.rs index 4966fd6ea80c1..2ffd73eef3ef8 100644 --- a/compiler/rustc_const_eval/src/interpret/projection.rs +++ b/compiler/rustc_const_eval/src/interpret/projection.rs @@ -206,8 +206,8 @@ where } } - // Iterates over all fields of an array. Much more efficient than doing the - // same by repeatedly calling `operand_index`. + /// Iterates over all fields of an array. Much more efficient than doing the + /// same by repeatedly calling `operand_index`. pub fn operand_array_fields<'a>( &self, base: &'a OpTy<'tcx, Prov>, diff --git a/compiler/rustc_const_eval/src/interpret/visitor.rs b/compiler/rustc_const_eval/src/interpret/visitor.rs index aee1f93b1a39c..1a10851a9f901 100644 --- a/compiler/rustc_const_eval/src/interpret/visitor.rs +++ b/compiler/rustc_const_eval/src/interpret/visitor.rs @@ -324,7 +324,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueMut<'mir, 'tcx, M> macro_rules! make_value_visitor { ($visitor_trait:ident, $value_trait:ident, $($mutability:ident)?) => { - // How to traverse a value and what to do when we are at the leaves. + /// How to traverse a value and what to do when we are at the leaves. pub trait $visitor_trait<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>: Sized { type V: $value_trait<'mir, 'tcx, M>; diff --git a/compiler/rustc_const_eval/src/transform/check_consts/mod.rs b/compiler/rustc_const_eval/src/transform/check_consts/mod.rs index b90e0962ce6ad..655ec345ed377 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/mod.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/mod.rs @@ -75,14 +75,14 @@ pub fn rustc_allow_const_fn_unstable( attr::rustc_allow_const_fn_unstable(&tcx.sess, attrs).any(|name| name == feature_gate) } -// Returns `true` if the given `const fn` is "const-stable". -// -// Panics if the given `DefId` does not refer to a `const fn`. -// -// Const-stability is only relevant for `const fn` within a `staged_api` crate. Only "const-stable" -// functions can be called in a const-context by users of the stable compiler. "const-stable" -// functions are subject to more stringent restrictions than "const-unstable" functions: They -// cannot use unstable features and can only call other "const-stable" functions. +/// Returns `true` if the given `const fn` is "const-stable". +/// +/// Panics if the given `DefId` does not refer to a `const fn`. +/// +/// Const-stability is only relevant for `const fn` within a `staged_api` crate. Only "const-stable" +/// functions can be called in a const-context by users of the stable compiler. "const-stable" +/// functions are subject to more stringent restrictions than "const-unstable" functions: They +/// cannot use unstable features and can only call other "const-stable" functions. pub fn is_const_stable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> bool { // A default body in a `#[const_trait]` is not const-stable because const // trait fns currently cannot be const-stable. We shouldn't diff --git a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs b/compiler/rustc_const_eval/src/transform/check_consts/ops.rs index bfc950eff5c06..27fb95dcd4031 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/ops.rs @@ -686,7 +686,7 @@ impl<'tcx> NonConstOp<'tcx> for ThreadLocalAccess { } } -// Types that cannot appear in the signature or locals of a `const fn`. +/// Types that cannot appear in the signature or locals of a `const fn`. pub mod ty { use super::*; diff --git a/compiler/rustc_const_eval/src/util/aggregate.rs b/compiler/rustc_const_eval/src/util/aggregate.rs index 180a40043db07..c43de3368c62f 100644 --- a/compiler/rustc_const_eval/src/util/aggregate.rs +++ b/compiler/rustc_const_eval/src/util/aggregate.rs @@ -9,10 +9,11 @@ use std::iter::TrustedLen; /// Expand `lhs = Rvalue::Aggregate(kind, operands)` into assignments to the fields. /// /// Produces something like -/// +/// ```ignore (ilustrative) /// (lhs as Variant).field0 = arg0; // We only have a downcast if this is an enum /// (lhs as Variant).field1 = arg1; /// discriminant(lhs) = variant_index; // If lhs is an enum or generator. +/// ``` pub fn expand_aggregate<'tcx>( orig_lhs: Place<'tcx>, operands: impl Iterator, Ty<'tcx>)> + TrustedLen, diff --git a/compiler/rustc_driver/src/lib.rs b/compiler/rustc_driver/src/lib.rs index 8e176efb2a9ed..380fbd732d505 100644 --- a/compiler/rustc_driver/src/lib.rs +++ b/compiler/rustc_driver/src/lib.rs @@ -1336,8 +1336,8 @@ mod signal_handler { } } - // When an error signal (such as SIGABRT or SIGSEGV) is delivered to the - // process, print a stack trace and then exit. + /// When an error signal (such as SIGABRT or SIGSEGV) is delivered to the + /// process, print a stack trace and then exit. pub(super) fn install() { unsafe { const ALT_STACK_SIZE: usize = libc::MINSIGSTKSZ + 64 * 1024; diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 8955abebf1e0f..9fe5d588b1f5d 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -242,8 +242,8 @@ pub enum ExpandResult { Retry(U), } -// `meta_item` is the attribute, and `item` is the item being modified. pub trait MultiItemModifier { + /// `meta_item` is the attribute, and `item` is the item being modified. fn expand( &self, ecx: &mut ExtCtxt<'_>, diff --git a/compiler/rustc_expand/src/build.rs b/compiler/rustc_expand/src/build.rs index e17cba1478ab6..234cf1b315a23 100644 --- a/compiler/rustc_expand/src/build.rs +++ b/compiler/rustc_expand/src/build.rs @@ -193,7 +193,7 @@ impl<'a> ExtCtxt<'a> { self.stmt_local(local, sp) } - // Generates `let _: Type;`, which is usually used for type assertions. + /// Generates `let _: Type;`, which is usually used for type assertions. pub fn stmt_let_type_only(&self, span: Span, ty: P) -> ast::Stmt { let local = P(ast::Local { pat: self.pat_wild(span), diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index 1d2b1298a68f6..2510795c2e3ed 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -200,7 +200,7 @@ fn get_features( features } -// `cfg_attr`-process the crate's attributes and compute the crate's features. +/// `cfg_attr`-process the crate's attributes and compute the crate's features. pub fn features( sess: &Session, mut krate: ast::Crate, diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index c2b1b96cd6465..3e98b024c73b9 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -401,7 +401,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { krate } - // Recursively expand all macro invocations in this AST fragment. + /// Recursively expand all macro invocations in this AST fragment. pub fn fully_expand_fragment(&mut self, input_fragment: AstFragment) -> AstFragment { let orig_expansion_data = self.cx.current_expansion.clone(); let orig_force_mode = self.cx.force_mode; @@ -1931,9 +1931,12 @@ pub struct ExpansionConfig<'feat> { pub features: Option<&'feat Features>, pub recursion_limit: Limit, pub trace_mac: bool, - pub should_test: bool, // If false, strip `#[test]` nodes - pub span_debug: bool, // If true, use verbose debugging for `proc_macro::Span` - pub proc_macro_backtrace: bool, // If true, show backtraces for proc-macro panics + /// If false, strip `#[test]` nodes + pub should_test: bool, + /// If true, use verbose debugging for `proc_macro::Span` + pub span_debug: bool, + /// If true, show backtraces for proc-macro panics + pub proc_macro_backtrace: bool, } impl<'feat> ExpansionConfig<'feat> { diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 473a04f33a9ad..042e65fc4dff4 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -969,8 +969,8 @@ pub struct Pat<'hir> { pub hir_id: HirId, pub kind: PatKind<'hir>, pub span: Span, - // Whether to use default binding modes. - // At present, this is false only for destructuring assignment. + /// Whether to use default binding modes. + /// At present, this is false only for destructuring assignment. pub default_binding_modes: bool, } @@ -1078,7 +1078,7 @@ impl fmt::Display for RangeEnd { pub struct DotDotPos(u32); impl DotDotPos { - // Panics if n >= u32::MAX. + /// Panics if n >= u32::MAX. pub fn new(n: Option) -> Self { match n { Some(n) => { @@ -1682,10 +1682,10 @@ impl Expr<'_> { } } - // Whether this looks like a place expr, without checking for deref - // adjustments. - // This will return `true` in some potentially surprising cases such as - // `CONSTANT.field`. + /// Whether this looks like a place expr, without checking for deref + /// adjustments. + /// This will return `true` in some potentially surprising cases such as + /// `CONSTANT.field`. pub fn is_syntactic_place_expr(&self) -> bool { self.is_place_expr(|_| true) } @@ -1826,7 +1826,7 @@ impl Expr<'_> { } } - // To a first-order approximation, is this a pattern + /// To a first-order approximation, is this a pattern? pub fn is_approximately_pattern(&self) -> bool { match &self.kind { ExprKind::Box(_) @@ -2148,11 +2148,11 @@ impl fmt::Display for LoopIdError { #[derive(Copy, Clone, Encodable, Debug, HashStable_Generic)] pub struct Destination { - // This is `Some(_)` iff there is an explicit user-specified `label + /// This is `Some(_)` iff there is an explicit user-specified 'label pub label: Option