-
Notifications
You must be signed in to change notification settings - Fork 0
krabcake: Successfully send intrinsics::assume data to Valgrind #1
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
base: master-before-krabcakeeeee-squash
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
use crate::MirPass; | ||
|
||
use rustc_index::vec::Idx; | ||
use rustc_middle::mir::patch::MirPatch; | ||
use rustc_middle::mir::{ | ||
AggregateKind, BasicBlockData, Body, Constant, ConstantKind, Location, NonDivergingIntrinsic, | ||
Operand, Place, Rvalue, StatementKind, TerminatorKind, | ||
}; | ||
use rustc_middle::ty::InternalSubsts; | ||
use rustc_middle::ty::TyCtxt; | ||
use rustc_target::abi::VariantIdx; | ||
|
||
pub struct ValgrindClientRequest; | ||
|
||
impl<'tcx> MirPass<'tcx> for ValgrindClientRequest { | ||
#[instrument(skip(self, tcx, body))] | ||
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { | ||
if !tcx.sess.opts.unstable_opts.instrument_krabcake { | ||
info!("Not instrumenting for krabcake"); | ||
return; | ||
} | ||
info!("Instrumenting for krabcake now..."); | ||
let mut patch = MirPatch::new(body); | ||
|
||
for (block_index, block_data) in body.basic_blocks.iter_enumerated() { | ||
for (stmt_index, stmt) in block_data.statements.iter().enumerate() { | ||
match &stmt.kind { | ||
StatementKind::Intrinsic(box NonDivergingIntrinsic::Assume(operand)) => { | ||
let loc = Location { block: block_index, statement_index: stmt_index }; | ||
info!("Found assume intrinsic (operand={operand:?}. At {loc:?}"); | ||
patch = call_ikr(tcx, patch, body, loc, operand); | ||
} | ||
_ => (), | ||
} | ||
} | ||
} | ||
|
||
patch.apply(body); | ||
} | ||
} | ||
|
||
fn call_ikr<'tcx>( | ||
tcx: TyCtxt<'tcx>, | ||
mut patch: MirPatch<'tcx>, | ||
body: &Body<'tcx>, | ||
loc: Location, | ||
_operand: &Operand<'tcx>, | ||
) -> MirPatch<'tcx> { | ||
let span = patch.source_info_for_location(body, loc).span; | ||
|
||
let op = |flag: bool| { | ||
Operand::Constant(Box::new(Constant { | ||
span, | ||
user_ty: None, | ||
literal: ConstantKind::from_bool(tcx, flag), | ||
})) | ||
}; | ||
|
||
let (place, rvalue) = { | ||
let krabcake_req_did = tcx.lang_items().krabcake_request().unwrap(); | ||
let krabcake_req_substs = InternalSubsts::identity_for_item(tcx, krabcake_req_did); | ||
let krabcake_req_def = tcx.adt_def(krabcake_req_did); | ||
let krabcake_req_ty = tcx.mk_adt(krabcake_req_def, krabcake_req_substs); | ||
let rvalue = Rvalue::Aggregate( | ||
Box::new(AggregateKind::Adt( | ||
krabcake_req_did, | ||
VariantIdx::new(0), | ||
&krabcake_req_substs, | ||
None, | ||
None, | ||
)), | ||
vec![op(true)], | ||
); | ||
let temp = patch.new_temp(krabcake_req_ty, span); | ||
let place = Place::from(temp); | ||
(place, rvalue) | ||
}; | ||
|
||
patch.add_assign(loc, place, rvalue); | ||
|
||
let krabcake_req_operand = Operand::Copy(place); | ||
|
||
let orig_terminator = patch.terminator_for_location(body, loc); | ||
let ikr_did = tcx.lang_items().insert_krabcake_request_fn().unwrap(); | ||
let ikr_substs = InternalSubsts::identity_for_item(tcx, ikr_did); | ||
let ikr_ty = tcx.mk_fn_def(ikr_did, ikr_substs); | ||
|
||
let func = Operand::Constant(Box::new(Constant { | ||
span, | ||
user_ty: None, | ||
literal: ConstantKind::zero_sized(ikr_ty), | ||
})); | ||
let storage = patch.new_temp(tcx.mk_mut_ptr(tcx.types.unit), span); | ||
let storage = Place::from(storage); | ||
let fn_call_terminator_kind = TerminatorKind::Call { | ||
func, | ||
args: vec![krabcake_req_operand], | ||
destination: storage, | ||
target: Some(loc.block + 1), | ||
cleanup: None, | ||
from_hir_call: false, | ||
fn_span: span, | ||
}; | ||
|
||
patch.patch_terminator(loc.block, fn_call_terminator_kind); | ||
|
||
let new_bb = | ||
BasicBlockData { statements: vec![], terminator: orig_terminator, is_cleanup: false }; | ||
|
||
patch.new_block(new_bb); | ||
patch | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
use core::arch::asm; | ||
|
||
mod vg_c_compatible { | ||
const fn vg_userreq_tool_base(a: u64, b: u64) -> u64 { | ||
((a) & 0xff) << 24 | ((b) & 0xff) << 16 | ||
} | ||
|
||
/// blah | ||
#[allow(dead_code)] // For currently unused variants | ||
#[derive(Debug, Clone, Copy)] | ||
#[repr(u64)] | ||
pub(super) enum ValgrindClientRequestCode { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm. Question: Should we be extending Here are our options, as I see them:
I'm trying to envisage the cases where we would want to do option (3). The main benefit of option (3) AFAICT is that you get to write source code in your demos that reference the enum values, so you have more control over fine-tuning (if you wanted to e.g. experiment with alternative protocols for how you interact with the valgrind krabcake tool). But I think in all such cases, I would opt to actually include a copy of the relevant enum in the source code (i.e. option (1.) over option (3.) @bryangarza did you adopt this strategy because it was simpler to implement than option (2.)? Or am I missing some benefit of option (3.)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not sure I quite follow what you mean in option 2; do you mean basically being able to have the MirPass insert the krabcake request into the MIR but without needing a Lang Item? (example) Or do you mean some method that doesn't use a MirPass?
The reason I made it a lang item is so that it would be more maintainable to have most of the logic defined as Rust code instead of needing to handwrite a lot of MIR. Also, I created a separate data-type |
||
/// blah | ||
BorrowMut = vg_userreq_tool_base('K' as u64, 'C' as u64), | ||
/// blah | ||
BorrowShr, | ||
/// blah | ||
AsRaw, | ||
/// blah | ||
AsBorrowMut, | ||
/// blah | ||
AsBorrowShr, | ||
/// blah | ||
RetagFnPrologue, | ||
/// blah | ||
RetagAssign, | ||
/// blah | ||
RetagRaw, | ||
/// blah | ||
IntrinsicsAssume, | ||
/// blah | ||
KrabcakeRecordOverlapError = vg_userreq_tool_base('K' as u64, 'C' as u64) + 256, | ||
} | ||
|
||
/// Ultimately, this should map to a [u64; 6], which is what Valgrind expects | ||
#[derive(Debug, Clone, Copy)] | ||
#[repr(C)] | ||
struct ValgrindClientRequest<T> | ||
where | ||
T: Into<u64>, | ||
{ | ||
req_code: ValgrindClientRequestCode, | ||
args: [T; 5], | ||
} | ||
} | ||
|
||
/// blah | ||
/// #[doc(hidden)] | ||
// There is no issue created for krabcake yet so just using a placeholder | ||
#[unstable(feature = "instrument_krabcake", issue = "1")] | ||
#[cfg_attr(not(bootstrap), lang = "KrabcakeRequest")] | ||
#[derive(Debug, Clone, Copy)] | ||
#[repr(C)] | ||
pub enum KrabcakeRequest { | ||
/// blah | ||
IntrinsicsAssume { | ||
/// blah | ||
flag: bool, | ||
}, | ||
} | ||
|
||
#[doc(hidden)] | ||
// There is no issue created for krabcake yet so just using a placeholder | ||
#[unstable(feature = "instrument_krabcake", issue = "1")] | ||
#[inline(never)] | ||
#[cfg_attr(not(bootstrap), lang = "insert_krabcake_request")] | ||
pub fn insert_krabcake_request(req: KrabcakeRequest) { | ||
// `res` not used for IntrinsicsAssume request | ||
let mut res = false as u64; | ||
let args: [u64; 6] = match req { | ||
KrabcakeRequest::IntrinsicsAssume { flag } => [ | ||
vg_c_compatible::ValgrindClientRequestCode::IntrinsicsAssume as u64, | ||
flag.into(), | ||
0, | ||
0, | ||
0, | ||
0, | ||
], | ||
}; | ||
|
||
// SAFETY: | ||
// This is a valid safety comment | ||
unsafe { | ||
asm!( | ||
"rol rdi, 3", | ||
"rol rdi, 13", | ||
"rol rdi, 61", | ||
"rol rdi, 51", | ||
"xchg rbx, rbx", | ||
inout("di") res, | ||
in("ax") &args, | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could have a terminator or intrinsic for emitting valgrind client requests, right? That would probably be more efficient as you can avoid the overhead of a non-inlineable call. Valgrind is relatively slow, so every bit helps I guess. |
||
} | ||
let _ = res; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Get the bool from the existing MIR
intrinsics::assume
itselfThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
namely, extract it from the
_operand
parameter you are passing above, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct