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

Remove duplicate remove_outer_derefs and refactor mod deref #573

Open
wants to merge 5 commits into
base: tests.refs.fields
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
7 changes: 7 additions & 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 dynamic_instrumentation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ edition = "2021"

[dependencies]
anyhow = "1.0"
apply = "0.3"
bincode = "1.0.1"
c2rust-analysis-rt = { path = "../analysis/runtime"}
cargo = "0.62"
Expand Down
86 changes: 32 additions & 54 deletions dynamic_instrumentation/src/instrument.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use anyhow::Context;
use apply::Apply;
use c2rust_analysis_rt::metadata::Metadata;
use c2rust_analysis_rt::mir_loc::{self, EventMetadata, Func, MirLoc, MirLocId, TransferKind};
use c2rust_analysis_rt::HOOK_FUNCTIONS;
Expand All @@ -20,7 +21,7 @@ use std::sync::Mutex;

use crate::arg::{ArgKind, InstrumentationArg};
use crate::hooks::Hooks;
use crate::mir_utils::{has_outer_deref, remove_outer_deref, strip_all_deref};
use crate::mir_utils::{remove_outer_deref, strip_all_deref, try_remove_outer_deref};
use crate::point::cast_ptr_to_usize;
use crate::point::InstrumentationAdder;
use crate::point::InstrumentationApplier;
Expand Down Expand Up @@ -229,25 +230,21 @@ impl<'tcx> Visitor<'tcx> for InstrumentationAdder<'_, 'tcx> {
}
}
_ if !is_region_or_unsafe_ptr(value_ty) => {}
Rvalue::AddressOf(_, p)
if has_outer_deref(p)
&& place_ty(&remove_outer_deref(*p, self.tcx())).is_region_ptr() =>
{
let source = remove_outer_deref(*p, self.tcx());
// Instrument which local's address is taken
self.loc(location.successor_within_block(), copy_fn)
.arg_var(dest)
.source(&source)
.dest(&dest)
.debug_mir(location)
.add_to(self);
}
Rvalue::AddressOf(_, p) => {
// Instrument which local's address is taken
self.loc(location.successor_within_block(), addr_local_fn)
let source = try_remove_outer_deref(*p, self.tcx())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't want to combine these, there are more complicated cases incoming and it'll be much clearer if they're separate

.filter(|source| place_ty(source).is_region_ptr());
let func = match source {
Some(_) => copy_fn,
None => addr_local_fn,
};
self.loc(location.successor_within_block(), func)
.arg_var(dest)
.arg_index_of(p.local)
.source(p)
.apply(|b| match source {
Some(_) => b,
None => b.arg_index_of(p.local),
})
.source(source.as_ref().unwrap_or(p))
.dest(&dest)
.debug_mir(location)
.add_to(self);
Expand Down Expand Up @@ -281,49 +278,30 @@ impl<'tcx> Visitor<'tcx> for InstrumentationAdder<'_, 'tcx> {
.debug_mir(location)
.add_to(self);
}
Rvalue::Ref(_, bkind, p) if has_outer_deref(p) => {
Rvalue::Ref(_, bkind, p) => {
// this is a reborrow or field reference, i.e. _2 = &(*_1)
let source = remove_outer_deref(*p, self.tcx());
if let BorrowKind::Mut { .. } = bkind {
let source = try_remove_outer_deref(*p, self.tcx());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't want to combine these, there are more complicated cases incoming and it'll be much clearer if they're separate

let loc = if let BorrowKind::Mut { .. } = bkind {
// Instrument which local's address is taken
self.loc(location, copy_fn)
.arg_addr_of(*p)
.source(&source)
.dest(&dest)
.debug_mir(location)
.add_to(self);
location
} else {
// Instrument immutable borrows by tracing the reference itself
self.loc(location.successor_within_block(), copy_fn)
.arg_var(dest)
.source(&source)
.dest(&dest)
.debug_mir(location)
.add_to(self);
location.successor_within_block()
};
}
Rvalue::Ref(_, bkind, p) if !p.is_indirect() => {
// this is a reborrow or field reference, i.e. _2 = &(*_1)
let source = remove_outer_deref(*p, self.tcx());
if let BorrowKind::Mut { .. } = bkind {
// Instrument which local's address is taken
self.loc(location, addr_local_fn)
.arg_addr_of(*p)
.arg_index_of(p.local)
.source(&source)
.dest(&dest)
.debug_mir(location)
.add_to(self);
} else {
// Instrument immutable borrows by tracing the reference itself
self.loc(location.successor_within_block(), addr_local_fn)
.arg_var(dest)
.arg_index_of(p.local)
.source(&source)
.dest(&dest)
.debug_mir(location)
.add_to(self);
let func = match source {
Some(_) => copy_fn,
None => addr_local_fn,
};
self.loc(loc, func)
.arg_addr_of(*p)
.apply(|b| match source {
Some(_) => b,
None => b.arg_index_of(p.local),
})
.source(source.as_ref().unwrap_or(p))
.dest(&dest)
.debug_mir(location)
.add_to(self);
}
_ => (),
}
Expand Down
36 changes: 20 additions & 16 deletions dynamic_instrumentation/src/mir_utils/deref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,6 @@ use rustc_middle::{
ty::TyCtxt,
};

pub fn has_outer_deref(p: &Place) -> bool {
matches!(
p.iter_projections().last(),
Some((_, ProjectionElem::Deref))
)
}

/// Get the inner-most dereferenced [`Place`].
pub fn strip_all_deref<'tcx>(p: &Place<'tcx>, tcx: TyCtxt<'tcx>) -> Place<'tcx> {
let mut base_dest = p.as_ref();
Expand All @@ -27,18 +20,29 @@ pub fn strip_all_deref<'tcx>(p: &Place<'tcx>, tcx: TyCtxt<'tcx>) -> Place<'tcx>
}
}

/// Strip the initital [`Deref`](ProjectionElem::Deref)
/// from a [`projection`](PlaceRef::projection) sequence.
pub fn remove_outer_deref<'tcx>(p: Place<'tcx>, tcx: TyCtxt<'tcx>) -> Place<'tcx> {
fn try_remove_outer_deref_as_ref<'tcx>(p: &Place<'tcx>) -> Option<PlaceRef<'tcx>> {
// Remove outer deref if present
match p.as_ref() {
PlaceRef {
local,
projection: &[ref base @ .., ProjectionElem::Deref],
} => Place {
local,
projection: tcx.intern_place_elems(base),
},
_ => p,
projection: &[ref projection @ .., ProjectionElem::Deref],
} => Some(PlaceRef { local, projection }),
_ => None,
}
}

/// Try to strip the initital [`Deref`](ProjectionElem::Deref)
/// from a [`projection`](PlaceRef::projection) sequence.
pub fn try_remove_outer_deref<'tcx>(p: Place<'tcx>, tcx: TyCtxt<'tcx>) -> Option<Place<'tcx>> {
try_remove_outer_deref_as_ref(&p).map(|PlaceRef { local, projection }| Place {
local,
projection: tcx.intern_place_elems(projection),
})
}

/// Strip the initital [`Deref`](ProjectionElem::Deref)
/// from a [`projection`](PlaceRef::projection) sequence
/// if there is one.
pub fn remove_outer_deref<'tcx>(p: Place<'tcx>, tcx: TyCtxt<'tcx>) -> Place<'tcx> {
try_remove_outer_deref(p, tcx).unwrap_or(p)
}
2 changes: 1 addition & 1 deletion dynamic_instrumentation/src/point/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_span::def_id::DefId;

use crate::{arg::InstrumentationArg, hooks::Hooks, util::Convert};

pub use apply::InstrumentationApplier;
pub use self::apply::InstrumentationApplier;
pub use cast::cast_ptr_to_usize;

pub struct InstrumentationPoint<'tcx> {
Expand Down