Skip to content

Commit

Permalink
Auto merge of rust-lang#2196 - carbotaniuman:permissive-stacked-borro…
Browse files Browse the repository at this point in the history 
…ws, r=RalfJung

Handle wildcard pointers in SB

This uses an permissive `Unknown` implementation, where a wildcard pointer (and any SRW derived from a wildcard pointer) can access any previously-exposed SB tag. This is missing any meaningful test-cases, and all of the edge-cases have not yet been worked through.

I think there's also some bugs here with differing Unknowns in different ranges and having things behave really weirdly too, alongside some issues with retagging to `SRO` or `Unique`.
  • Loading branch information
@bors
bors committed Jun 25, 2022
2 parents a1226c4 + 58c79c5 commit 3b4402c
Show file tree
Hide file tree
Showing 19 changed files with 555 additions and 163 deletions.
4 changes: 1 addition & 3 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,9 +358,7 @@ to Miri failing to detect cases of undefined behavior in a program.
for pointer-to-int and int-to-pointer casts, respectively. This will
necessarily miss some bugs as those semantics are not efficiently
implementable in a sanitizer, but it will only miss bugs that concerns
memory/pointers which is subject to these operations. Also note that this flag
is currently incompatible with Stacked Borrows, so you will have to also pass
`-Zmiri-disable-stacked-borrows` to use this.
memory/pointers which is subject to these operations.
* `-Zmiri-symbolic-alignment-check` makes the alignment check more strict. By
default, alignment is checked by casting the pointer to an integer, and making
sure that is a multiple of the alignment. This can lead to cases where a
Expand Down
8 changes: 4 additions & 4 deletions src/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_middle::ty;
use rustc_span::{source_map::DUMMY_SP, Span, SpanData, Symbol};

use crate::helpers::HexRange;
use crate::stacked_borrows::{diagnostics::TagHistory, AccessKind, SbTag};
use crate::stacked_borrows::{diagnostics::TagHistory, AccessKind};
use crate::*;

/// Details of premature program termination.
Expand Down Expand Up @@ -61,9 +61,9 @@ impl MachineStopType for TerminationInfo {}
/// Miri specific diagnostics
pub enum NonHaltingDiagnostic {
CreatedPointerTag(NonZeroU64),
/// This `Item` was popped from the borrow stack, either due to a grant of
/// `AccessKind` to `SbTag` or a deallocation when the second argument is `None`.
PoppedPointerTag(Item, Option<(SbTag, AccessKind)>),
/// This `Item` was popped from the borrow stack, either due to an access with the given tag or
/// a deallocation when the second argument is `None`.
PoppedPointerTag(Item, Option<(SbTagExtra, AccessKind)>),
CreatedCallId(CallId),
CreatedAlloc(AllocId),
FreedAlloc(AllocId),
Expand Down
18 changes: 9 additions & 9 deletions src/intptrcast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,16 @@ impl<'mir, 'tcx> GlobalStateInner {
}
}

pub fn expose_addr(ecx: &MiriEvalContext<'mir, 'tcx>, alloc_id: AllocId) {
trace!("Exposing allocation id {:?}", alloc_id);

let mut global_state = ecx.machine.intptrcast.borrow_mut();
pub fn expose_ptr(ecx: &mut MiriEvalContext<'mir, 'tcx>, alloc_id: AllocId, sb: SbTag) {
let global_state = ecx.machine.intptrcast.get_mut();
// In legacy and strict mode, we don't need this, so we can save some cycles
// by not tracking it.
if global_state.provenance_mode == ProvenanceMode::Permissive {
trace!("Exposing allocation id {alloc_id:?}");
global_state.exposed.insert(alloc_id);
if ecx.machine.stacked_borrows.is_some() {
ecx.expose_tag(alloc_id, sb);
}
}
}

Expand Down Expand Up @@ -140,9 +142,7 @@ impl<'mir, 'tcx> GlobalStateInner {
// Determine the allocation this points to at cast time.
let alloc_id = Self::alloc_id_from_addr(ecx, addr);
Pointer::new(
alloc_id.map(|alloc_id| {
Tag::Concrete(ConcreteTag { alloc_id, sb: SbTag::Untagged })
}),
alloc_id.map(|alloc_id| Tag::Concrete { alloc_id, sb: SbTag::Untagged }),
Size::from_bytes(addr),
)
}
Expand Down Expand Up @@ -220,8 +220,8 @@ impl<'mir, 'tcx> GlobalStateInner {
) -> Option<(AllocId, Size)> {
let (tag, addr) = ptr.into_parts(); // addr is absolute (Tag provenance)

let alloc_id = if let Tag::Concrete(concrete) = tag {
concrete.alloc_id
let alloc_id = if let Tag::Concrete { alloc_id, .. } = tag {
alloc_id
} else {
// A wildcard pointer.
assert_eq!(ecx.machine.intptrcast.borrow().provenance_mode, ProvenanceMode::Permissive);
Expand Down
10 changes: 6 additions & 4 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#![feature(let_else)]
#![feature(io_error_more)]
#![feature(yeet_expr)]
#![feature(is_some_with)]
#![feature(nonzero_ops)]
#![warn(rust_2018_idioms)]
#![allow(
clippy::collapsible_else_if,
Expand Down Expand Up @@ -80,15 +82,15 @@ pub use crate::eval::{
pub use crate::helpers::{CurrentSpan, EvalContextExt as HelpersEvalContextExt};
pub use crate::intptrcast::ProvenanceMode;
pub use crate::machine::{
AllocExtra, ConcreteTag, Evaluator, FrameData, MiriEvalContext, MiriEvalContextExt,
MiriMemoryKind, Tag, NUM_CPUS, PAGE_SIZE, STACK_ADDR, STACK_SIZE,
AllocExtra, Evaluator, FrameData, MiriEvalContext, MiriEvalContextExt, MiriMemoryKind, Tag,
NUM_CPUS, PAGE_SIZE, STACK_ADDR, STACK_SIZE,
};
pub use crate::mono_hash_map::MonoHashMap;
pub use crate::operator::EvalContextExt as OperatorEvalContextExt;
pub use crate::range_map::RangeMap;
pub use crate::stacked_borrows::{
CallId, EvalContextExt as StackedBorEvalContextExt, Item, Permission, PtrId, SbTag, Stack,
Stacks,
CallId, EvalContextExt as StackedBorEvalContextExt, Item, Permission, PtrId, SbTag, SbTagExtra,
Stack, Stacks,
};
pub use crate::sync::{CondvarId, EvalContextExt as SyncEvalContextExt, MutexId, RwLockId};
pub use crate::thread::{
Expand Down
44 changes: 21 additions & 23 deletions src/machine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,14 @@ impl fmt::Display for MiriMemoryKind {
/// Pointer provenance (tag).
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum Tag {
Concrete(ConcreteTag),
Concrete {
alloc_id: AllocId,
/// Stacked Borrows tag.
sb: SbTag,
},
Wildcard,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct ConcreteTag {
pub alloc_id: AllocId,
/// Stacked Borrows tag.
pub sb: SbTag,
}

#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
static_assert_size!(Pointer<Tag>, 24);
// #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
Expand All @@ -160,15 +157,15 @@ impl Provenance for Tag {
write!(f, "0x{:x}", addr.bytes())?;

match tag {
Tag::Concrete(tag) => {
Tag::Concrete { alloc_id, sb } => {
// Forward `alternate` flag to `alloc_id` printing.
if f.alternate() {
write!(f, "[{:#?}]", tag.alloc_id)?;
write!(f, "[{:#?}]", alloc_id)?;
} else {
write!(f, "[{:?}]", tag.alloc_id)?;
write!(f, "[{:?}]", alloc_id)?;
}
// Print Stacked Borrows tag.
write!(f, "{:?}", tag.sb)?;
write!(f, "{:?}", sb)?;
}
Tag::Wildcard => {
write!(f, "[Wildcard]")?;
Expand All @@ -180,7 +177,7 @@ impl Provenance for Tag {

fn get_alloc_id(self) -> Option<AllocId> {
match self {
Tag::Concrete(concrete) => Some(concrete.alloc_id),
Tag::Concrete { alloc_id, .. } => Some(alloc_id),
Tag::Wildcard => None,
}
}
Expand Down Expand Up @@ -489,7 +486,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
type AllocExtra = AllocExtra;

type PointerTag = Tag;
type TagExtra = SbTag;
type TagExtra = SbTagExtra;

type MemoryMap =
MonoHashMap<AllocId, (MemoryKind<MiriMemoryKind>, Allocation<Tag, Self::AllocExtra>)>;
Expand Down Expand Up @@ -649,7 +646,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
let alloc: Allocation<Tag, Self::AllocExtra> = alloc.convert_tag_add_extra(
&ecx.tcx,
AllocExtra {
stacked_borrows: stacks,
stacked_borrows: stacks.map(RefCell::new),
data_race: race_alloc,
weak_memory: buffer_alloc,
},
Expand Down Expand Up @@ -682,7 +679,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
SbTag::Untagged
};
Pointer::new(
Tag::Concrete(ConcreteTag { alloc_id: ptr.provenance, sb: sb_tag }),
Tag::Concrete { alloc_id: ptr.provenance, sb: sb_tag },
Size::from_bytes(absolute_addr),
)
}
Expand All @@ -708,8 +705,9 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
ptr: Pointer<Self::PointerTag>,
) -> InterpResult<'tcx> {
match ptr.provenance {
Tag::Concrete(concrete) =>
intptrcast::GlobalStateInner::expose_addr(ecx, concrete.alloc_id),
Tag::Concrete { alloc_id, sb } => {
intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, sb);
}
Tag::Wildcard => {
// No need to do anything for wildcard pointers as
// their provenances have already been previously exposed.
Expand All @@ -728,8 +726,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {

rel.map(|(alloc_id, size)| {
let sb = match ptr.provenance {
Tag::Concrete(ConcreteTag { sb, .. }) => sb,
Tag::Wildcard => SbTag::Untagged,
Tag::Concrete { sb, .. } => SbTagExtra::Concrete(sb),
Tag::Wildcard => SbTagExtra::Wildcard,
};
(alloc_id, size, sb)
})
Expand All @@ -747,7 +745,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
data_race.read(alloc_id, range, machine.data_race.as_ref().unwrap())?;
}
if let Some(stacked_borrows) = &alloc_extra.stacked_borrows {
stacked_borrows.memory_read(
stacked_borrows.borrow_mut().memory_read(
alloc_id,
tag,
range,
Expand All @@ -773,7 +771,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
data_race.write(alloc_id, range, machine.data_race.as_mut().unwrap())?;
}
if let Some(stacked_borrows) = &mut alloc_extra.stacked_borrows {
stacked_borrows.memory_written(
stacked_borrows.get_mut().memory_written(
alloc_id,
tag,
range,
Expand Down Expand Up @@ -802,7 +800,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
data_race.deallocate(alloc_id, range, machine.data_race.as_mut().unwrap())?;
}
if let Some(stacked_borrows) = &mut alloc_extra.stacked_borrows {
stacked_borrows.memory_deallocated(
stacked_borrows.get_mut().memory_deallocated(
alloc_id,
tag,
range,
Expand Down
Loading

0 comments on commit 3b4402c

Please sign in to comment.