Skip to content

Commit

Permalink
Keep track of unique projection combinations in Project
Browse files Browse the repository at this point in the history
Add an index value to Project events to differentiate between
different projections with the same pointer offset, e.g.,
(*p).x and (*p).x.a if a is the first field of the structure.

This is implemented as an IndexSet<Vec<usize>> where each element
is a unique combination of field projections. The Project index
points to an element in this set.
  • Loading branch information
ahomescu committed Jul 24, 2024
1 parent 6ff83d4 commit a7aac0d
Show file tree
Hide file tree
Showing 11 changed files with 100 additions and 50 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions analysis/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ enum_dispatch = "0.3"
fs-err = "2"
crossbeam-queue = "0.3"
crossbeam-utils = "0.8"
indexmap = { version = "1.9", features = ["serde"] }
8 changes: 6 additions & 2 deletions analysis/runtime/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ pub enum EventKind {
CopyRef,

/// Projection. Used for operations like `_2 = &(*_1).0`.
Project(Pointer, Pointer),
/// The third value is a "projection index" that points to an element
/// of the projections data structure in the metadata. It is used to
/// disambiguate between different projections with the same pointer,
/// e.g., `(*p).x` and `(*p).x.a` where `a` is at offset 0.
Project(Pointer, Pointer, usize),

Alloc {
size: usize,
Expand Down Expand Up @@ -88,7 +92,7 @@ impl Debug for EventKind {
use EventKind::*;
match *self {
CopyPtr(ptr) => write!(f, "copy(0x{:x})", ptr),
Project(ptr, new_ptr) => write!(f, "project(0x{:x}, 0x{:x})", ptr, new_ptr),
Project(ptr, new_ptr, idx) => write!(f, "project(0x{:x}, 0x{:x}, [{}])", ptr, new_ptr, idx),
Alloc { size, ptr } => {
write!(f, "malloc({}) -> 0x{:x}", size, ptr)
}
Expand Down
4 changes: 2 additions & 2 deletions analysis/runtime/src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ pub const HOOK_FUNCTIONS: &[&str] = &[
hook_fn!(offset),
];

pub fn ptr_project(mir_loc: MirLocId, ptr: usize, new_ptr: usize) {
pub fn ptr_project(mir_loc: MirLocId, ptr: usize, new_ptr: usize, proj_idx: usize) {
RUNTIME.send_event(Event {
mir_loc,
kind: EventKind::Project(ptr, new_ptr),
kind: EventKind::Project(ptr, new_ptr, proj_idx),
});
}

Expand Down
6 changes: 5 additions & 1 deletion analysis/runtime/src/metadata.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use indexmap::IndexSet;
use std::{
collections::HashMap,
fmt::{self, Debug, Formatter},
Expand All @@ -13,6 +14,7 @@ use crate::mir_loc::{Func, FuncId, MirLoc, MirLocId};
pub struct Metadata {
pub locs: Vec<MirLoc>,
pub functions: HashMap<FuncId, String>,
pub projections: IndexSet<Vec<usize>>,
}

impl Metadata {
Expand Down Expand Up @@ -46,11 +48,13 @@ impl FromIterator<Metadata> for Metadata {
fn from_iter<I: IntoIterator<Item = Metadata>>(iter: I) -> Self {
let mut locs = Vec::new();
let mut functions = HashMap::new();
let mut projections = IndexSet::new();
for metadata in iter {
locs.extend(metadata.locs);
functions.extend(metadata.functions);
projections.extend(metadata.projections);
}
Self { locs, functions }
Self { locs, functions, projections }
}
}

Expand Down
23 changes: 21 additions & 2 deletions dynamic_instrumentation/src/instrument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@ use crate::point::{cast_ptr_to_usize, InstrumentationPriority};
use crate::point::{
CollectAddressTakenLocals, CollectInstrumentationPoints, RewriteAddressTakenLocals,
};
use crate::point::ProjectionSet;
use crate::util::Convert;

#[derive(Default)]
pub struct Instrumenter {
mir_locs: Mutex<IndexSet<MirLoc>>,
functions: Mutex<HashMap<FuncId, String>>,
projections: Mutex<IndexSet<Vec<usize>>>,
}

impl Instrumenter {
Expand Down Expand Up @@ -72,9 +74,10 @@ impl Instrumenter {
pub fn finalize(&self, metadata_path: &Path) -> anyhow::Result<()> {
let mut locs = self.mir_locs.lock().unwrap();
let mut functions = self.functions.lock().unwrap();
let projections = std::mem::take(&mut *self.projections.lock().unwrap());
let locs = locs.drain(..).collect::<Vec<_>>();
let functions = functions.drain().collect::<HashMap<_, _>>();
let metadata = Metadata { locs, functions };
let metadata = Metadata { locs, functions, projections };
let bytes = bincode::serialize(&metadata).context("Location serialization failed")?;
let mut file = OpenOptions::new()
.append(true)
Expand Down Expand Up @@ -115,6 +118,12 @@ impl Instrumenter {
}
}

impl ProjectionSet for Instrumenter {
fn add_proj(&self, proj: Vec<usize>) -> usize {
self.projections.lock().unwrap().insert_full(proj).0
}
}

fn is_shared_or_unsafe_ptr(ty: Ty) -> bool {
ty.is_unsafe_ptr() || (ty.is_region_ptr() && !ty.is_mutable_ptr())
}
Expand Down Expand Up @@ -374,7 +383,14 @@ impl<'tcx> Visitor<'tcx> for CollectInstrumentationPoints<'_, 'tcx> {

while let Some((inner_deref_base, PlaceElem::Deref)) = proj_iter.next() {
// Find the next Deref or end of projections
// Meanwhile, collect all `Field`s into a vector that we
// can add to the `projections` IndexSet
let mut fields = vec![];
let (outer_deref_base, have_outer_deref) = loop {
if let Some((_, PlaceElem::Field(idx, _))) = proj_iter.peek() {
fields.push(idx.index());
}

match proj_iter.peek() {
Some((base, PlaceElem::Deref)) => break (base.clone(), true),
// Reached the end, we can use the full place
Expand All @@ -387,6 +403,8 @@ impl<'tcx> Visitor<'tcx> for CollectInstrumentationPoints<'_, 'tcx> {

// We have some other elements between the two projections
if outer_deref_base.projection.len() - inner_deref_base.projection.len() > 1 {
let proj_idx = self.projections.add_proj(fields);

// There are non-deref projection elements between the two derefs.
// Add a Project event between the start pointer of those field/index
// projections and their final address.
Expand All @@ -400,6 +418,7 @@ impl<'tcx> Visitor<'tcx> for CollectInstrumentationPoints<'_, 'tcx> {
self.loc(location, location, project_fn)
.arg_var(inner_deref_base)
.arg_addr_of(outer_deref_base.clone())
.arg_var(proj_idx)
.add_to(self);
}

Expand Down Expand Up @@ -749,7 +768,7 @@ fn instrument_body<'a, 'tcx>(

// collect instrumentation points
let points = {
let mut collector = CollectInstrumentationPoints::new(tcx, hooks, body, local_to_address);
let mut collector = CollectInstrumentationPoints::new(tcx, hooks, body, local_to_address, state);
collector.visit_body(body);
collector.into_instrumentation_points()
};
Expand Down
14 changes: 14 additions & 0 deletions dynamic_instrumentation/src/into_operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,20 @@ impl<'tcx> IntoOperand<'tcx> for u32 {
}
}

impl<'tcx> IntoOperand<'tcx> for usize {
fn op(self, tcx: TyCtxt<'tcx>) -> Operand<'tcx> {
Operand::Constant(Box::new(Constant {
span: DUMMY_SP,
user_ty: None,
literal: ConstantKind::Ty(ty::Const::from_bits(
tcx,
self.try_into().unwrap(),
ParamEnv::empty().and(tcx.types.usize),
)),
}))
}
}

impl<'tcx> IntoOperand<'tcx> for Local {
fn op(self, tcx: TyCtxt<'tcx>) -> Operand<'tcx> {
Place::from(self).op(tcx)
Expand Down
7 changes: 7 additions & 0 deletions dynamic_instrumentation/src/point/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,18 @@ impl<'tcx> RewriteAddressTakenLocals<'tcx> {
}
}

pub trait ProjectionSet {
fn add_proj(&self, proj: Vec<usize>) -> usize;
}

pub struct CollectInstrumentationPoints<'a, 'tcx: 'a> {
tcx: TyCtxt<'tcx>,
hooks: Hooks<'tcx>,
pub body: &'a Body<'tcx>,
pub instrumentation_points: Vec<InstrumentationPoint<'tcx>>,
assignment: Option<(Place<'tcx>, Rvalue<'tcx>)>,
pub addr_taken_local_addresses: IndexMap<Local, Local>,
pub projections: &'a dyn ProjectionSet,
}

impl<'a, 'tcx: 'a> CollectInstrumentationPoints<'a, 'tcx> {
Expand All @@ -127,6 +132,7 @@ impl<'a, 'tcx: 'a> CollectInstrumentationPoints<'a, 'tcx> {
hooks: Hooks<'tcx>,
body: &'a Body<'tcx>,
addr_taken_local_addresses: IndexMap<Local, Local>,
projections: &'a dyn ProjectionSet,
) -> Self {
Self {
tcx,
Expand All @@ -135,6 +141,7 @@ impl<'a, 'tcx: 'a> CollectInstrumentationPoints<'a, 'tcx> {
instrumentation_points: Default::default(),
assignment: Default::default(),
addr_taken_local_addresses,
projections,
}
}

Expand Down
2 changes: 1 addition & 1 deletion pdg/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl EventKindExt for EventKind {
Realloc { .. } => NodeKind::Alloc(1),
Free { .. } => NodeKind::Free,
CopyPtr(..) | CopyRef => NodeKind::Copy,
Project(base_ptr, new_ptr) => NodeKind::Project(new_ptr - base_ptr),
Project(base_ptr, new_ptr, idx) => NodeKind::Project(new_ptr - base_ptr, idx),
LoadAddr(..) => NodeKind::LoadAddr,
StoreAddr(..) => NodeKind::StoreAddr,
StoreAddrTaken(..) => NodeKind::StoreAddr,
Expand Down
4 changes: 2 additions & 2 deletions pdg/src/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub enum NodeKind {
/// [`Field`] projection.
///
/// Used for operations like `_2 = &(*_1).0`.
Project(usize),
Project(usize, usize),

/// Pointer arithmetic.
///
Expand Down Expand Up @@ -119,7 +119,7 @@ impl Display for NodeKind {
use NodeKind::*;
match self {
Copy => write!(f, "copy"),
Project(offset) => write!(f, "project.{offset}"),
Project(offset, idx) => write!(f, "project.{offset}[{idx}]"),
Offset(offset) => write!(f, "offset[{offset}]"),
AddrOfLocal(local) => write!(f, "&{local:?}"),
_AddrOfStatic(static_) => write!(f, "&'static {static_:?}"),
Expand Down
Loading

0 comments on commit a7aac0d

Please sign in to comment.