Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle wildcard pointers in SB #2196

Merged
merged 8 commits into from
Jun 25, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions README.md
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
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
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
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
36 changes: 17 additions & 19 deletions src/machine.rs
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 @@ -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 Down
Loading