diff --git a/c2rust-analyze/src/borrowck/def_use.rs b/c2rust-analyze/src/borrowck/def_use.rs index ca0fd4bfd2..e92ecceeff 100644 --- a/c2rust-analyze/src/borrowck/def_use.rs +++ b/c2rust-analyze/src/borrowck/def_use.rs @@ -81,6 +81,10 @@ pub fn categorize(context: PlaceContext) -> Option { // Debug info is neither def nor use. PlaceContext::NonUse(NonUseContext::VarDebugInfo) => None, + + PlaceContext::MutatingUse(MutatingUseContext::Deinit | MutatingUseContext::SetDiscriminant) => { + panic!("These statements are not allowed in this MIR phase") + } } } @@ -127,7 +131,7 @@ impl<'tcx> Visitor<'tcx> for DefUseVisitor<'tcx, '_> { } } - fn visit_local(&mut self, local: &Local, context: PlaceContext, location: Location) { + fn visit_local(&mut self, local: Local, context: PlaceContext, location: Location) { eprintln!( "visit local {:?} with context {:?} = {:?} at {:?}", local, @@ -135,7 +139,7 @@ impl<'tcx> Visitor<'tcx> for DefUseVisitor<'tcx, '_> { categorize(context), location ); - let var = self.maps.variable(*local); + let var = self.maps.variable(local); let point = self.maps.point_mid_location(location); match categorize(context) { Some(DefUse::Def) => { @@ -255,7 +259,7 @@ impl<'tcx> Visitor<'tcx> for LoanInvalidatedAtVisitor<'tcx, '_> { } } - fn visit_local(&mut self, local: &Local, context: PlaceContext, location: Location) { + fn visit_local(&mut self, local: Local, context: PlaceContext, location: Location) { eprintln!( "loan_invalidated_at: visit local {:?} with context {:?} = {:?} at {:?}", local, @@ -264,7 +268,7 @@ impl<'tcx> Visitor<'tcx> for LoanInvalidatedAtVisitor<'tcx, '_> { location ); - let local_loans = self.loans.get(local).map_or(&[] as &[_], |x| x); + let local_loans = self.loans.get(&local).map_or(&[] as &[_], |x| x); for &(_path, loan, borrow_kind) in local_loans { // All paths rooted in this local overlap the local. self.access_loan_at_location(loan, borrow_kind, context, location); diff --git a/c2rust-analyze/src/borrowck/mod.rs b/c2rust-analyze/src/borrowck/mod.rs index 9bc50feaf3..50b6d367fc 100644 --- a/c2rust-analyze/src/borrowck/mod.rs +++ b/c2rust-analyze/src/borrowck/mod.rs @@ -138,7 +138,7 @@ fn run_polonius<'tcx>( let term_start = maps.point(bb, term_idx, SubPoint::Start); let term_mid = maps.point(bb, term_idx, SubPoint::Mid); facts.cfg_edge.push((term_start, term_mid)); - for &succ in bb_data.terminator().successors() { + for succ in bb_data.terminator().successors() { let succ_start = maps.point(succ, 0, SubPoint::Start); facts.cfg_edge.push((term_mid, succ_start)); } diff --git a/c2rust-analyze/src/borrowck/type_check.rs b/c2rust-analyze/src/borrowck/type_check.rs index 4f20b94c62..224132bbc2 100644 --- a/c2rust-analyze/src/borrowck/type_check.rs +++ b/c2rust-analyze/src/borrowck/type_check.rs @@ -210,6 +210,7 @@ impl<'tcx> TypeChecker<'tcx, '_> { ref func, ref args, destination, + target, .. } => { let func_ty = func.ty(self.local_decls, *self.ltcx); @@ -218,9 +219,10 @@ impl<'tcx> TypeChecker<'tcx, '_> { Some(Callee::PtrOffset { .. }) => { // We handle this like a pointer assignment. - // `destination` must be `Some` because the function doesn't diverge. - let destination = destination.unwrap(); - let pl_lty = self.visit_place(destination.0); + // `target` must be `Some` because the function doesn't diverge. + // TODO(kkysen) I kept the `.unwrap()` so that the behavior is identical. Do we need this? + target.unwrap(); + let pl_lty = self.visit_place(destination); assert!(args.len() == 2); let rv_lty = self.visit_operand(&args[0]); self.do_assign(pl_lty, rv_lty); diff --git a/c2rust-analyze/src/context.rs b/c2rust-analyze/src/context.rs index 3a15f5e36e..b78a24fd69 100644 --- a/c2rust-analyze/src/context.rs +++ b/c2rust-analyze/src/context.rs @@ -342,6 +342,7 @@ impl<'tcx> TypeOf<'tcx> for Rvalue<'tcx> { match *self { Rvalue::Use(ref op) => acx.type_of(op), + Rvalue::CopyForDeref(pl) => acx.type_of(pl), Rvalue::Repeat(ref op, _) => { let op_lty = acx.type_of(op); let ty = self.ty(acx, acx.tcx()); diff --git a/c2rust-analyze/src/dataflow/type_check.rs b/c2rust-analyze/src/dataflow/type_check.rs index ae8d833d4e..13a42ed18b 100644 --- a/c2rust-analyze/src/dataflow/type_check.rs +++ b/c2rust-analyze/src/dataflow/type_check.rs @@ -196,6 +196,7 @@ impl<'tcx> TypeChecker<'tcx, '_> { ref func, ref args, destination, + target, .. } => { let func_ty = func.ty(self.mir, tcx); @@ -204,10 +205,11 @@ impl<'tcx> TypeChecker<'tcx, '_> { Some(Callee::PtrOffset { .. }) => { // We handle this like a pointer assignment. - // `destination` must be `Some` because the function doesn't diverge. - let destination = destination.unwrap(); + // `target` must be `Some` because the function doesn't diverge. + // TODO(kkysen) I kept the `.unwrap()` so that the behavior is identical. Do we need this? + target.unwrap(); let ctx = PlaceContext::MutatingUse(MutatingUseContext::Store); - let pl_lty = self.visit_place(destination.0, ctx); + let pl_lty = self.visit_place(destination, ctx); assert!(args.len() == 2); let rv_lty = self.visit_operand(&args[0]); self.do_assign(pl_lty.label, rv_lty.label); diff --git a/c2rust-analyze/src/expr_rewrite.rs b/c2rust-analyze/src/expr_rewrite.rs index e28193e623..d6f065e05a 100644 --- a/c2rust-analyze/src/expr_rewrite.rs +++ b/c2rust-analyze/src/expr_rewrite.rs @@ -3,7 +3,7 @@ use crate::pointer_id::PointerTable; use crate::type_desc::{self, Ownership, Quantity}; use crate::util::{self, Callee}; use rustc_middle::mir::{ - BasicBlock, Body, Location, Operand, Rvalue, Statement, StatementKind, Terminator, + BasicBlock, Body, Location, Operand, Place, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, }; use rustc_span::{Span, DUMMY_SP}; @@ -132,6 +132,7 @@ impl<'a, 'tcx> ExprRewriteVisitor<'a, 'tcx> { } StatementKind::FakeRead(..) => {} StatementKind::SetDiscriminant { .. } => todo!("statement {:?}", stmt), + StatementKind::Deinit(..) => {} StatementKind::StorageLive(..) => {} StatementKind::StorageDead(..) => {} StatementKind::Retag(..) => {} @@ -163,14 +164,16 @@ impl<'a, 'tcx> ExprRewriteVisitor<'a, 'tcx> { ref func, ref args, destination, + target, .. } => { let func_ty = func.ty(self.mir, tcx); - let pl_ty = destination.map(|(pl, _)| self.acx.type_of(pl)); + let pl_ty = self.acx.type_of(destination); if let Some(callee) = util::ty_callee(tcx, func_ty) { // Special cases for particular functions. - let pl_ty = pl_ty.unwrap(); + // TODO(kkysen) I kept the `.unwrap()` so that the behavior is identical. Do we need this? + target.unwrap(); match callee { Callee::PtrOffset { .. } => { self.visit_ptr_offset(&args[0], pl_ty); @@ -249,23 +252,29 @@ impl<'a, 'tcx> ExprRewriteVisitor<'a, 'tcx> { Rvalue::ShallowInitBox(ref _op, _ty) => { // TODO } + Rvalue::CopyForDeref(pl) => { + self.enter_rvalue_operand(0, |v| v.visit_place(pl, expect_ty)); + } } } fn visit_operand(&mut self, op: &Operand<'tcx>, expect_ty: LTy<'tcx>) { match *op { Operand::Copy(pl) | Operand::Move(pl) => { - if let Some(ptr) = self.acx.ptr_of(pl) { - let expect_ptr = expect_ty.label; - self.emit_ptr_cast(ptr, expect_ptr); - } - - // TODO: walk over `pl` to handle all derefs (casts, `*x` -> `(*x).get()`) + self.visit_place(pl, expect_ty); } Operand::Constant(..) => {} } } + fn visit_place(&mut self, pl: Place<'tcx>, expect_ty: LTy<'tcx>) { + if let Some(ptr) = self.acx.ptr_of(pl) { + let expect_ptr = expect_ty.label; + self.emit_ptr_cast(ptr, expect_ptr); + } + // TODO: walk over `pl` to handle all derefs (casts, `*x` -> `(*x).get()`) + } + fn visit_operand_desc( &mut self, op: &Operand<'tcx>, diff --git a/c2rust-analyze/src/labeled_ty.rs b/c2rust-analyze/src/labeled_ty.rs index 028f33ab38..73aa60d46f 100644 --- a/c2rust-analyze/src/labeled_ty.rs +++ b/c2rust-analyze/src/labeled_ty.rs @@ -153,7 +153,7 @@ impl<'tcx, L: Copy> LabeledTyCtxt<'tcx, L> { /// Label a `Ty` using a callback. The callback runs at every type constructor to produce a /// label for that node in the tree. pub fn label) -> L>(&self, ty: Ty<'tcx>, f: &mut F) -> LabeledTy<'tcx, L> { - use rustc_middle::ty::TyKind::*; + use rustc_type_ir::TyKind::*; let label = f(ty); match ty.kind() { // Types with no arguments @@ -166,11 +166,11 @@ impl<'tcx, L: Copy> LabeledTyCtxt<'tcx, L> { let args = substs.types().map(|t| self.label(t, f)).collect::>(); self.mk(ty, self.mk_slice(&args), label) } - Array(elem, _) => { + &Array(elem, _) => { let args = [self.label(elem, f)]; self.mk(ty, self.mk_slice(&args), label) } - Slice(elem) => { + &Slice(elem) => { let args = [self.label(elem, f)]; self.mk(ty, self.mk_slice(&args), label) } @@ -178,7 +178,7 @@ impl<'tcx, L: Copy> LabeledTyCtxt<'tcx, L> { let args = [self.label(mty.ty, f)]; self.mk(ty, self.mk_slice(&args), label) } - Ref(_, mty, _) => { + &Ref(_, mty, _) => { let args = [self.label(mty, f)]; self.mk(ty, self.mk_slice(&args), label) } @@ -199,10 +199,7 @@ impl<'tcx, L: Copy> LabeledTyCtxt<'tcx, L> { self.mk(ty, self.mk_slice(&args), label) } Tuple(elems) => { - let args = elems - .types() - .map(|ty| self.label(ty, f)) - .collect::>(); + let args = elems.iter().map(|ty| self.label(ty, f)).collect::>(); self.mk(ty, self.mk_slice(&args), label) } @@ -220,7 +217,7 @@ impl<'tcx, L: Copy> LabeledTyCtxt<'tcx, L> { where F: FnMut(Ty<'tcx>) -> L, { - self.mk_slice(&tys.iter().map(|ty| self.label(ty, f)).collect::>()) + self.mk_slice(&tys.iter().map(|&ty| self.label(ty, f)).collect::>()) } /// Substitute in arguments for any type parameter references (`Param`) in a labeled type. @@ -291,7 +288,7 @@ impl<'tcx, L: Copy> LabeledTyCtxt<'tcx, L> { where F: FnMut(Ty<'tcx>, &[Ty<'tcx>], L) -> Ty<'tcx>, { - use rustc_middle::ty::TyKind::*; + use rustc_type_ir::TyKind::*; let args = lty .args .iter() diff --git a/c2rust-analyze/src/main.rs b/c2rust-analyze/src/main.rs index 122b03cdd4..1fcb3957e3 100644 --- a/c2rust-analyze/src/main.rs +++ b/c2rust-analyze/src/main.rs @@ -12,6 +12,7 @@ extern crate rustc_mir_build; extern crate rustc_session; extern crate rustc_span; extern crate rustc_target; +extern crate rustc_type_ir; use crate::context::{ AnalysisCtxt, FlagSet, GlobalAnalysisCtxt, GlobalAssignment, LTy, LocalAssignment, diff --git a/c2rust-analyze/src/util.rs b/c2rust-analyze/src/util.rs index 50acf7cc74..61bfeaa15d 100644 --- a/c2rust-analyze/src/util.rs +++ b/c2rust-analyze/src/util.rs @@ -82,7 +82,7 @@ pub fn ty_callee<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Option> match name.as_str() { "offset" => { // The `offset` inherent method of `*const T` and `*mut T`. - let parent_did = tcx.parent(did)?; + let parent_did = tcx.parent(did); if tcx.def_kind(parent_did) != DefKind::Impl { return None; } diff --git a/c2rust-transpile/src/translator/atomics.rs b/c2rust-transpile/src/translator/atomics.rs index 2327aab0e9..7e46cf695d 100644 --- a/c2rust-transpile/src/translator/atomics.rs +++ b/c2rust-transpile/src/translator/atomics.rs @@ -75,28 +75,28 @@ impl<'c> Translation<'c> { }) } + fn order_name(order: Ordering) -> &'static str { + use Ordering::*; + match order { + SeqCst => "seqcst", + AcqRel => "acqrel", + Acquire => "acquire", + Release => "release", + Relaxed => "relaxed", + _ => unreachable!( + "new variants added to `{}`", + std::any::type_name::() + ), + } + } + match name { "__atomic_load" | "__atomic_load_n" => ptr.and_then(|ptr| { - use Ordering::*; - let intrinsic_name = match static_order(order) { - SeqCst => Some("atomic_load"), - AcqRel => None, - Acquire => Some("atomic_load_acq"), - Release => None, - Relaxed => Some("atomic_load_relaxed"), - _ => unreachable!("Did we not handle a case above??"), - } - .ok_or_else(|| { - format_translation_err!( - self.ast_context - .display_loc(&self.ast_context[order_id].loc), - "Invalid memory ordering for __atomic_load", - ) - })?; + let intrinsic_name = format!("atomic_load_{}", order_name(static_order(order))); self.use_feature("core_intrinsics"); - let atomic_load = mk().abs_path_expr(vec!["core", "intrinsics", intrinsic_name]); + let atomic_load = mk().abs_path_expr(vec!["core", "intrinsics", &intrinsic_name]); let call = mk().call_expr(atomic_load, vec![ptr]); if name == "__atomic_load" { let ret = val1.expect("__atomic_load should have a ret argument"); @@ -124,27 +124,13 @@ impl<'c> Translation<'c> { let val = val1.expect("__atomic_store must have a val argument"); ptr.and_then(|ptr| { val.and_then(|val| { - use Ordering::*; - let intrinsic_name = match static_order(order) { - SeqCst => Some("atomic_store"), - AcqRel => None, - Acquire => None, - Release => Some("atomic_store_rel"), - Relaxed => Some("atomic_store_relaxed"), - _ => unreachable!("Did we not handle a case above??"), - } - .ok_or_else(|| { - format_translation_err!( - self.ast_context - .display_loc(&self.ast_context[order_id].loc), - "Invalid memory ordering for __atomic_store", - ) - })?; + let intrinsic_name = + format!("atomic_store_{}", order_name(static_order(order))); self.use_feature("core_intrinsics"); let atomic_store = - mk().abs_path_expr(vec!["core", "intrinsics", intrinsic_name]); + mk().abs_path_expr(vec!["core", "intrinsics", &intrinsic_name]); let val = if name == "__atomic_store" { mk().unary_expr(UnOp::Deref(Default::default()), val) } else { @@ -164,27 +150,13 @@ impl<'c> Translation<'c> { let val = val1.expect("__atomic_store must have a val argument"); ptr.and_then(|ptr| { val.and_then(|val| { - use Ordering::*; - let intrinsic_name = match static_order(order) { - SeqCst => Some("atomic_xchg"), - AcqRel => Some("atomic_xchg_acqrel"), - Acquire => Some("atomic_xchg_acq"), - Release => Some("atomic_xchg_rel"), - Relaxed => Some("atomic_xchg_relaxed"), - _ => unreachable!("Did we not handle a case above??"), - } - .ok_or_else(|| { - format_translation_err!( - self.ast_context - .display_loc(&self.ast_context[order_id].loc), - "Invalid memory ordering for __atomic_exchange", - ) - })?; + let intrinsic_name = + format!("atomic_xchg_{}", order_name(static_order(order))); self.use_feature("core_intrinsics"); let fn_path = - mk().abs_path_expr(vec!["core", "intrinsics", intrinsic_name]); + mk().abs_path_expr(vec!["core", "intrinsics", &intrinsic_name]); let val = if name == "__atomic_exchange" { mk().unary_expr(UnOp::Deref(Default::default()), val) } else { @@ -230,27 +202,23 @@ impl<'c> Translation<'c> { let order = static_order(order); let order_fail = static_order(order_fail); use Ordering::*; - let intrinsic_name = (|| { - Some(match (order, order_fail) { - (_, Release | AcqRel) => return None, - (SeqCst, SeqCst) => "", - (SeqCst, Acquire) => "_failacq", - (SeqCst, Relaxed) => "_failrelaxed", - (AcqRel, Acquire) => "_acqrel", - (AcqRel, Relaxed) => "_acqrel_failrelaxed", - (Release, Relaxed) => "_rel", - (Acquire, Acquire) => "_acq", - (Acquire, Relaxed) => "_acq_failrelaxed", - (Relaxed, Relaxed) => "_relaxed", - (SeqCst | AcqRel | Release | Acquire | Relaxed, _) => { - return None - } - - (_, _) => unreachable!("Did we not handle a case above??"), - }) - })() - .map(|suffix| { - format!("atomic_cxchg{}{}", if weak { "weak" } else { "" }, suffix) + let intrinsic_name = match (order, order_fail) { + (_, Release | AcqRel) => None, + (SeqCst, SeqCst | Acquire | Relaxed) + | (AcqRel, Acquire | Relaxed) + | (Release, Relaxed) + | (Acquire | Relaxed, Acquire | Relaxed) => { + Some((order, order_fail)) + } + (SeqCst | AcqRel | Release | Acquire | Relaxed, _) => None, + + (_, _) => unreachable!("Did we not handle a case above??"), + } + .map(|(order, order_fail)| { + let weak = if weak { "weak" } else { "" }; + let order = order_name(order); + let order_fail = order_name(order_fail); + format!("atomic_cxchg{weak}_{order}_{order_fail}") }) .ok_or_else(|| { format_translation_err!( @@ -320,16 +288,8 @@ impl<'c> Translation<'c> { "atomic_and" }; - use Ordering::*; - let intrinsic_suffix = match static_order(order) { - SeqCst => "", - AcqRel => "_acqrel", - Acquire => "_acq", - Release => "_rel", - Relaxed => "_relaxed", - _ => unreachable!("Unknown memory ordering"), - }; - let intrinsic_name = format!("{intrinsic_name}{intrinsic_suffix}"); + let intrinsic_suffix = order_name(static_order(order)); + let intrinsic_name = format!("{intrinsic_name}_{intrinsic_suffix}"); let fetch_first = name.starts_with("__atomic_fetch"); let val = val1.expect("__atomic arithmetic operations must have a val argument"); diff --git a/c2rust-transpile/src/translator/builtins.rs b/c2rust-transpile/src/translator/builtins.rs index 5262c5830f..244ccfc7dd 100644 --- a/c2rust-transpile/src/translator/builtins.rs +++ b/c2rust-transpile/src/translator/builtins.rs @@ -422,7 +422,7 @@ impl<'c> Translation<'c> { let returns_val = builtin_name.starts_with("__sync_val"); self.convert_atomic_cxchg( ctx, - "atomic_cxchg", + "atomic_cxchg_seqcst_seqcst", arg0, arg1, arg2, @@ -493,18 +493,18 @@ impl<'c> Translation<'c> { | "__sync_nand_and_fetch_8" | "__sync_nand_and_fetch_16" => { let func_name = if builtin_name.contains("_add_") { - "atomic_xadd" + "atomic_xadd_seqcst" } else if builtin_name.contains("_sub_") { - "atomic_xsub" + "atomic_xsub_seqcst" } else if builtin_name.contains("_or_") { - "atomic_or" + "atomic_or_seqcst" } else if builtin_name.contains("_xor_") { - "atomic_xor" + "atomic_xor_seqcst" } else if builtin_name.contains("_nand_") { - "atomic_nand" + "atomic_nand_seqcst" } else { // We can't explicitly check for "_and_" since they all contain it - "atomic_and" + "atomic_and_seqcst" }; let arg0 = self.convert_expr(ctx.used(), args[0])?; @@ -520,7 +520,8 @@ impl<'c> Translation<'c> { "__sync_synchronize" => { self.use_feature("core_intrinsics"); - let atomic_func = mk().abs_path_expr(vec!["core", "intrinsics", "atomic_fence"]); + let atomic_func = + mk().abs_path_expr(vec!["core", "intrinsics", "atomic_fence_seqcst"]); let call_expr = mk().call_expr(atomic_func, vec![]); self.convert_side_effects_expr( ctx, @@ -536,8 +537,9 @@ impl<'c> Translation<'c> { | "__sync_lock_test_and_set_16" => { self.use_feature("core_intrinsics"); - // Emit `atomic_xchg_acq(arg0, arg1)` - let atomic_func = mk().abs_path_expr(vec!["core", "intrinsics", "atomic_xchg_acq"]); + // Emit `atomic_xchg_acquire(arg0, arg1)` + let atomic_func = + mk().abs_path_expr(vec!["core", "intrinsics", "atomic_xchg_acquire"]); let arg0 = self.convert_expr(ctx.used(), args[0])?; let arg1 = self.convert_expr(ctx.used(), args[1])?; arg0.and_then(|arg0| { @@ -559,9 +561,9 @@ impl<'c> Translation<'c> { | "__sync_lock_release_16" => { self.use_feature("core_intrinsics"); - // Emit `atomic_store_rel(arg0, 0)` + // Emit `atomic_store_release(arg0, 0)` let atomic_func = - mk().abs_path_expr(vec!["core", "intrinsics", "atomic_store_rel"]); + mk().abs_path_expr(vec!["core", "intrinsics", "atomic_store_release"]); let arg0 = self.convert_expr(ctx.used(), args[0])?; arg0.and_then(|arg0| { let zero = mk().lit_expr(mk().int_lit(0, "")); diff --git a/dynamic_instrumentation/src/instrument.rs b/dynamic_instrumentation/src/instrument.rs index 1258b0eb23..586e43e608 100644 --- a/dynamic_instrumentation/src/instrument.rs +++ b/dynamic_instrumentation/src/instrument.rs @@ -12,7 +12,7 @@ use rustc_middle::mir::{ BasicBlock, BasicBlockData, Body, BorrowKind, HasLocalDecls, Local, LocalDecl, Location, Operand, Place, PlaceElem, Rvalue, SourceInfo, Terminator, TerminatorKind, START_BLOCK, }; -use rustc_middle::ty::{self, TyCtxt, TyS}; +use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_span::def_id::{DefId, DefPathHash}; use rustc_span::DUMMY_SP; use std::collections::HashMap; @@ -117,11 +117,11 @@ impl Instrumenter { } } -fn is_shared_or_unsafe_ptr(ty: &TyS) -> bool { +fn is_shared_or_unsafe_ptr(ty: Ty) -> bool { ty.is_unsafe_ptr() || (ty.is_region_ptr() && !ty.is_mutable_ptr()) } -fn is_region_or_unsafe_ptr(ty: &TyS) -> bool { +fn is_region_or_unsafe_ptr(ty: Ty) -> bool { ty.is_unsafe_ptr() || ty.is_region_ptr() } @@ -343,6 +343,7 @@ impl<'tcx> Visitor<'tcx> for CollectInstrumentationPoints<'_, 'tcx> { func, args, destination, + target, .. } => { let mut callee_arg: Place = Local::new(1).into(); @@ -376,8 +377,7 @@ impl<'tcx> Visitor<'tcx> for CollectInstrumentationPoints<'_, 'tcx> { callee_arg.local.increment_by(1); } } - if let (&ty::FnDef(def_id, _), &Some(destination)) = (func_kind, destination) { - let (dest_place, dest_block) = destination; + if let (&ty::FnDef(def_id, _), &Some(target)) = (func_kind, target) { println!("term: {:?}", terminator.kind); let fn_name = self.tcx().item_name(def_id); if HOOK_FUNCTIONS.contains(&fn_name.as_str()) { @@ -386,24 +386,24 @@ impl<'tcx> Visitor<'tcx> for CollectInstrumentationPoints<'_, 'tcx> { // Hooked function called; trace args self.loc(location, location, func_def_id) .source(args) - .dest(&dest_place) + .dest(destination) .after_call() .transfer(TransferKind::Ret(self.func_hash())) .arg_vars(args.iter().cloned()) .add_to(self); - } else if is_region_or_unsafe_ptr(dest_place.ty(self, self.tcx()).ty) { + } else if is_region_or_unsafe_ptr(destination.ty(self, self.tcx()).ty) { let instrumentation_location = Location { statement_index: 0, - block: dest_block, + block: target, }; self.loc(location, instrumentation_location, arg_fn) .source(&0) - .dest(&dest_place) + .dest(destination) .transfer(TransferKind::Ret( self.tcx().def_path_hash(def_id).convert(), )) - .arg_var(dest_place) + .arg_var(*destination) .add_to(self); } } @@ -487,7 +487,9 @@ pub fn insert_call<'tcx>( mut args: Vec>, ) -> (BasicBlock, Local) { println!("ST: {:?}", statement_index); - let (blocks, locals) = body.basic_blocks_and_local_decls_mut(); + + let blocks = body.basic_blocks.as_mut(); + let locals = &mut body.local_decls; let successor_stmts = blocks[block].statements.split_off(statement_index); let successor_terminator = blocks[block].terminator.take(); @@ -516,7 +518,8 @@ pub fn insert_call<'tcx>( kind: TerminatorKind::Call { func, args: args.iter().map(|arg| arg.inner().clone()).collect(), - destination: Some((ret_local.into(), successor_block)), + destination: ret_local.into(), + target: Some(successor_block), cleanup: None, from_hir_call: true, fn_span: DUMMY_SP, diff --git a/dynamic_instrumentation/src/into_operand.rs b/dynamic_instrumentation/src/into_operand.rs index f59ec3ef0b..fcf0c4ed58 100644 --- a/dynamic_instrumentation/src/into_operand.rs +++ b/dynamic_instrumentation/src/into_operand.rs @@ -1,5 +1,5 @@ use rustc_middle::{ - mir::{Constant, Local, Operand, Place}, + mir::{Constant, ConstantKind, Local, Operand, Place}, ty::{self, ParamEnv, TyCtxt}, }; use rustc_span::DUMMY_SP; @@ -37,6 +37,10 @@ fn make_const(tcx: TyCtxt, idx: u32) -> Operand { Operand::Constant(Box::new(Constant { span: DUMMY_SP, user_ty: None, - literal: ty::Const::from_bits(tcx, idx.into(), ParamEnv::empty().and(tcx.types.u32)).into(), + literal: ConstantKind::Ty(ty::Const::from_bits( + tcx, + idx.into(), + ParamEnv::empty().and(tcx.types.u32), + )), })) } diff --git a/dynamic_instrumentation/src/mir_utils/mod.rs b/dynamic_instrumentation/src/mir_utils/mod.rs index bf501bbf46..e16176ce7d 100644 --- a/dynamic_instrumentation/src/mir_utils/mod.rs +++ b/dynamic_instrumentation/src/mir_utils/mod.rs @@ -5,7 +5,7 @@ mod deref; pub use deref::*; /// Get the one and only input [`Place`], if applicable. -pub fn rv_place<'tcx>(rv: &'tcx Rvalue) -> Option> { +pub fn rv_place<'tcx>(rv: &Rvalue<'tcx>) -> Option> { use Rvalue::*; match rv { Use(op) => op.place(), diff --git a/dynamic_instrumentation/src/point/apply.rs b/dynamic_instrumentation/src/point/apply.rs index 2a7c9804d0..f434ea3385 100644 --- a/dynamic_instrumentation/src/point/apply.rs +++ b/dynamic_instrumentation/src/point/apply.rs @@ -69,7 +69,8 @@ impl<'tcx, 'a> InstrumentationApplier<'tcx, 'a> { state.add_fn(callee_id, tcx); } - let (blocks, locals) = body.basic_blocks_and_local_decls_mut(); + let blocks = body.basic_blocks.as_mut(); + let locals = &mut body.local_decls; // Add the MIR location as the first argument to the instrumentation function let loc_idx = state.get_mir_loc_idx(body_def, original_location, metadata.clone()); @@ -82,7 +83,9 @@ impl<'tcx, 'a> InstrumentationApplier<'tcx, 'a> { if after_call { let call = blocks[instrumentation_location.block].terminator_mut(); let ret_value = if let TerminatorKind::Call { - destination: Some((place, _next_block)), + destination: place, + // TODO(kkysen) I kept the `Some` pattern so that the `match` is identical. Do we need this? + target: Some(_next_block), args, .. } = &mut call.kind @@ -118,7 +121,7 @@ impl<'tcx, 'a> InstrumentationApplier<'tcx, 'a> { let (successor_block, _) = insert_call( tcx, - *body, + body, instrumentation_location.block, instrumentation_location.statement_index, func, @@ -136,11 +139,11 @@ impl<'tcx, 'a> InstrumentationApplier<'tcx, 'a> { let orig_call = blocks[successor_block].terminator_mut(); if let ( TerminatorKind::Call { - destination: Some((_, instrument_dest)), + target: Some(instrument_dest), .. }, TerminatorKind::Call { - destination: Some((_, orig_dest)), + target: Some(orig_dest), .. }, ) = (&mut instrument_call.kind, &mut orig_call.kind) diff --git a/dynamic_instrumentation/src/point/build.rs b/dynamic_instrumentation/src/point/build.rs index 360a203d51..826262b243 100644 --- a/dynamic_instrumentation/src/point/build.rs +++ b/dynamic_instrumentation/src/point/build.rs @@ -163,7 +163,9 @@ impl<'tcx> InstrumentationBuilder<'_, 'tcx> { match &block.terminator().kind { TerminatorKind::Call { args, - destination: Some((destination, _)), + destination, + // TODO(kkysen) I kept the `Some` pattern so that the `match` is identical. Do we need this? + target: Some(_), func, .. } => { diff --git a/dynamic_instrumentation/src/point/cast.rs b/dynamic_instrumentation/src/point/cast.rs index 4b5db5fa4c..e15f12336d 100644 --- a/dynamic_instrumentation/src/point/cast.rs +++ b/dynamic_instrumentation/src/point/cast.rs @@ -4,7 +4,7 @@ use rustc_middle::{ CastKind, Local, LocalDecl, Mutability, Operand, ProjectionElem, Rvalue, SourceInfo, Statement, StatementKind, }, - ty::{self, TyCtxt}, + ty::{self, TyCtxt, TypeAndMut}, }; use rustc_span::DUMMY_SP; @@ -30,7 +30,7 @@ pub fn cast_ptr_to_usize<'tcx>( let arg_ty = arg.inner().ty(locals, tcx); let ptr = match arg { - // If we were given an address as a usize, no conversion is necessary + // If we were given an address as a `usize`, no conversion is necessary. InstrumentationArg::Op(ArgKind::AddressUsize(_arg)) => { assert!( arg_ty.is_integral() || arg_ty.is_unit(), @@ -40,11 +40,10 @@ pub fn cast_ptr_to_usize<'tcx>( ); return None; } - // From a reference `r`, cast through raw ptr to usize: `r as *mut _ as usize` + // From a reference `r`, cast through a raw ptr to a `usize`: `r as *mut _ as usize`. InstrumentationArg::Op(ArgKind::Reference(arg)) => { assert!(arg_ty.is_region_ptr()); let inner_ty = arg_ty.builtin_deref(false).unwrap(); - let raw_ptr_ty = tcx.mk_ptr(inner_ty); let raw_ptr_local = locals.push(LocalDecl::new(raw_ptr_ty, DUMMY_SP)); @@ -63,7 +62,7 @@ pub fn cast_ptr_to_usize<'tcx>( new_stmts.push(cast_stmt); Operand::Move(raw_ptr_local.into()) } - // From a raw pointer `r`, cast: `r as usize` + // From a raw pointer `r`, cast: `r as usize`. InstrumentationArg::Op(ArgKind::RawPtr(arg)) => { assert!( arg_ty.is_unsafe_ptr(), @@ -73,8 +72,8 @@ pub fn cast_ptr_to_usize<'tcx>( ); arg.to_copy() } - // From a place to which a reference is also constructed, create a raw - // ptr with `addr_of!` + // From a place to which a reference is also constructed, + // create a raw ptr with `addr_of!`. InstrumentationArg::AddrOf(arg) => { let arg_place = arg.place().expect("Can't get the address of a constant"); let arg_place = remove_outer_deref(arg_place, tcx); @@ -100,8 +99,26 @@ pub fn cast_ptr_to_usize<'tcx>( } }; - // Cast the raw ptr to a usize before passing to the - // instrumentation function + let ptr = { + // Use `*const [(); 0]` as the opaque pointer type. + let thin_raw_ptr_ty = tcx.mk_ptr(TypeAndMut { + ty: tcx.mk_array(tcx.mk_unit(), 0), + mutbl: Mutability::Not, + }); + let casted_local = locals.push(LocalDecl::new(thin_raw_ptr_ty, DUMMY_SP)); + let casted_arg = Operand::Move(casted_local.into()); + let cast_stmt = Statement { + source_info: SourceInfo::outermost(DUMMY_SP), + kind: StatementKind::Assign(Box::new(( + casted_local.into(), + Rvalue::Cast(CastKind::Misc, ptr, thin_raw_ptr_ty), + ))), + }; + new_stmts.push(cast_stmt); + casted_arg + }; + + // Cast the raw ptr to a `usize` before passing to the instrumentation function. let usize_ty = tcx.mk_mach_uint(ty::UintTy::Usize); let casted_local = locals.push(LocalDecl::new(usize_ty, DUMMY_SP)); let casted_arg = Operand::Move(casted_local.into()); @@ -109,7 +126,7 @@ pub fn cast_ptr_to_usize<'tcx>( source_info: SourceInfo::outermost(DUMMY_SP), kind: StatementKind::Assign(Box::new(( casted_local.into(), - Rvalue::Cast(CastKind::Misc, ptr, usize_ty), + Rvalue::Cast(CastKind::PointerExposeAddress, ptr, usize_ty), ))), }; new_stmts.push(cast_stmt); diff --git a/pdg/src/builder.rs b/pdg/src/builder.rs index 7ed9194084..6d7e0b3d64 100644 --- a/pdg/src/builder.rs +++ b/pdg/src/builder.rs @@ -62,7 +62,7 @@ impl EventKindExt for EventKind { } fn parent(&self, obj: (GraphId, NodeId)) -> Option<(GraphId, NodeId)> { - self.has_parent().then(|| obj) + self.has_parent().then_some(obj) } fn to_node_kind(&self) -> Option { diff --git a/pdg/src/snapshots/c2rust_pdg__tests__analysis_test_pdg_snapshot.snap b/pdg/src/snapshots/c2rust_pdg__tests__analysis_test_pdg_snapshot.snap index 7765e0ab03..08b5091d63 100644 --- a/pdg/src/snapshots/c2rust_pdg__tests__analysis_test_pdg_snapshot.snap +++ b/pdg/src/snapshots/c2rust_pdg__tests__analysis_test_pdg_snapshot.snap @@ -63,10 +63,10 @@ nodes_that_need_write = [] g { n[0]: copy _ => _38 @ bb27[6]: fn main; _38 = null_mut(); n[1]: copy n[0] => _2 @ bb0[0]: fn push; _36 = push(move _37, move _38); - n[2]: value.store _ => _20.* @ bb4[7]: fn invalid; (*_20) = const 0_usize as *mut pointers::S (Misc); - n[3]: value.store _ => _17.* @ bb8[4]: fn fdevent_unregister; (*_17) = const 0_usize as *mut pointers::fdnode_st (Misc); - n[4]: int_to_ptr _ => _2 @ bb0[2]: fn test_ref_field; _2 = const 0_usize as *const pointers::S (Misc); - n[5]: int_to_ptr _ => _5 @ bb0[8]: fn test_ref_field; _5 = const 0_usize as *const pointers::S (Misc); + n[2]: value.store _ => _20.* @ bb4[7]: fn invalid; (*_20) = const 0_usize as *mut pointers::S (PointerFromExposedAddress); + n[3]: value.store _ => _17.* @ bb8[4]: fn fdevent_unregister; (*_17) = const 0_usize as *mut pointers::fdnode_st (PointerFromExposedAddress); + n[4]: int_to_ptr _ => _2 @ bb0[2]: fn test_ref_field; _2 = const 0_usize as *const pointers::S (PointerFromExposedAddress); + n[5]: int_to_ptr _ => _5 @ bb0[8]: fn test_ref_field; _5 = const 0_usize as *const pointers::S (PointerFromExposedAddress); } nodes_that_need_write = [] @@ -385,19 +385,19 @@ g { nodes_that_need_write = [] g { - n[0]: copy _ => _5 @ bb2[2]: fn no_owner; _5 = const {alloc2: *mut *mut pointers::S}; + n[0]: copy _ => _5 @ bb2[2]: fn no_owner; _5 = const {alloc8: *mut *mut pointers::S}; n[1]: addr.store n[0] => _ @ bb2[3]: fn no_owner; (*_5) = move _2 as *mut pointers::S (Misc); - n[2]: copy _ => _5 @ bb2[2]: fn no_owner; _5 = const {alloc2: *mut *mut pointers::S}; + n[2]: copy _ => _5 @ bb2[2]: fn no_owner; _5 = const {alloc8: *mut *mut pointers::S}; n[3]: addr.store n[2] => _ @ bb2[3]: fn no_owner; (*_5) = move _2 as *mut pointers::S (Misc); - n[4]: copy _ => _12 @ bb3[4]: fn no_owner; _12 = const {alloc2: *mut *mut pointers::S}; + n[4]: copy _ => _12 @ bb3[4]: fn no_owner; _12 = const {alloc8: *mut *mut pointers::S}; n[5]: addr.load n[4] => _ @ bb3[5]: fn no_owner; _11 = (*_12); - n[6]: copy _ => _6 @ bb2[9]: fn invalid; _6 = const {alloc2: *mut *mut pointers::S}; + n[6]: copy _ => _6 @ bb2[9]: fn invalid; _6 = const {alloc8: *mut *mut pointers::S}; n[7]: addr.store n[6] => _ @ bb2[10]: fn invalid; (*_6) = move _5; - n[8]: copy _ => _19 @ bb3[17]: fn invalid; _19 = const {alloc2: *mut *mut pointers::S}; + n[8]: copy _ => _19 @ bb3[17]: fn invalid; _19 = const {alloc8: *mut *mut pointers::S}; n[9]: field.0 n[8] => _18 @ bb3[18]: fn invalid; _18 = ((*(*_19)).0: i32); n[10]: addr.load n[9] => _ @ bb3[18]: fn invalid; _18 = ((*(*_19)).0: i32); - n[11]: copy _ => _20 @ bb4[6]: fn invalid; _20 = const {alloc2: *mut *mut pointers::S}; - n[12]: addr.store n[11] => _ @ bb4[7]: fn invalid; (*_20) = const 0_usize as *mut pointers::S (Misc); + n[11]: copy _ => _20 @ bb4[6]: fn invalid; _20 = const {alloc8: *mut *mut pointers::S}; + n[12]: addr.store n[11] => _ @ bb4[7]: fn invalid; (*_20) = const 0_usize as *mut pointers::S (PointerFromExposedAddress); } nodes_that_need_write = [12, 11, 7, 6, 3, 2, 1, 0] @@ -451,7 +451,7 @@ g { n[3]: copy n[2] => _7 @ bb2[9]: fn simple1; _7 = move _8 as *mut libc::c_void (Misc); n[4]: free n[3] => _6 @ bb3[2]: fn simple1; _6 = realloc(move _7, move _9); n[5]: copy n[1] => _16 @ bb4[20]: fn simple1; _16 = _1; - n[6]: ptr_to_int n[5] => _ @ bb4[21]: fn simple1; _15 = move _16 as usize (Misc); + n[6]: ptr_to_int n[5] => _ @ bb4[21]: fn simple1; _15 = move _16 as usize (PointerExposeAddress); n[7]: copy n[1] => _21 @ bb4[33]: fn simple1; _21 = _1; n[8]: copy n[7] => _20 @ bb4[34]: fn simple1; _20 = move _21 as *mut libc::c_void (Misc); n[9]: free n[8] => _19 @ bb4[36]: fn simple1; _19 = free(move _20); @@ -466,7 +466,7 @@ g { n[4]: addr.store n[3] => _ @ bb4[8]: fn simple1; ((*_11).0: i32) = const 10_i32; n[5]: copy n[1] => _12 @ bb4[10]: fn simple1; _12 = _5; n[6]: copy n[2] => _13 @ bb4[13]: fn simple1; _13 = _11; - n[7]: int_to_ptr _ => _17 @ bb4[27]: fn simple1; _17 = move _18 as *const libc::c_void (Misc); + n[7]: int_to_ptr _ => _17 @ bb4[27]: fn simple1; _17 = move _18 as *const libc::c_void (PointerFromExposedAddress); } nodes_that_need_write = [4, 3, 2, 1, 0] @@ -491,7 +491,7 @@ g { n[12]: value.load _ => _19 @ bb7[4]: fn fdevent_unregister; _19 = ((*_1).0: *mut *mut pointers::fdnode_st); n[13]: offset[0] n[12] => _18 @ bb7[10]: fn fdevent_unregister; _18 = offset(move _19, move _20); n[14]: copy n[13] => _17 @ bb8[3]: fn fdevent_unregister; _17 = &mut (*_18); - n[15]: addr.store n[14] => _ @ bb8[4]: fn fdevent_unregister; (*_17) = const 0_usize as *mut pointers::fdnode_st (Misc); + n[15]: addr.store n[14] => _ @ bb8[4]: fn fdevent_unregister; (*_17) = const 0_usize as *mut pointers::fdnode_st (PointerFromExposedAddress); n[16]: copy n[1] => _20 @ bb6[6]: fn lighttpd_test; _20 = _1; n[17]: copy n[16] => _19 @ bb6[7]: fn lighttpd_test; _19 = move _20 as *mut libc::c_void (Misc); n[18]: free n[17] => _18 @ bb6[9]: fn lighttpd_test; _18 = free(move _19); @@ -596,7 +596,7 @@ g { n[28]: addr.load n[27] => _ @ bb1[3]: fn fdevent_fdnode_event_unsetter; _8 = ((*_2).4: i32); n[29]: value.load _ => _3 @ bb1[2]: fn fdevent_unregister; _3 = (*_4); n[30]: copy n[29] => _12 @ bb1[11]: fn fdevent_unregister; _12 = _3; - n[31]: ptr_to_int n[30] => _ @ bb1[12]: fn fdevent_unregister; _11 = move _12 as usize (Misc); + n[31]: ptr_to_int n[30] => _ @ bb1[12]: fn fdevent_unregister; _11 = move _12 as usize (PointerExposeAddress); n[32]: copy n[29] => _23 @ bb8[7]: fn fdevent_unregister; _23 = _3; n[33]: copy n[32] => _1 @ bb0[0]: fn fdnode_free; _22 = fdnode_free(move _23); n[34]: copy n[33] => _4 @ bb0[3]: fn fdnode_free; _4 = _1; @@ -803,8 +803,8 @@ nodes_that_need_write = [5, 4, 1, 0] g { n[0]: alloc _ => _1 @ bb1[2]: fn test_ptr_int_ptr; _1 = malloc(move _2); n[1]: copy n[0] => _5 @ bb2[4]: fn test_ptr_int_ptr; _5 = _1; - n[2]: ptr_to_int n[1] => _ @ bb2[5]: fn test_ptr_int_ptr; _4 = move _5 as usize (Misc); - n[3]: int_to_ptr _ => _1 @ bb2[10]: fn test_ptr_int_ptr; _1 = move _6 as *mut libc::c_void (Misc); + n[2]: ptr_to_int n[1] => _ @ bb2[5]: fn test_ptr_int_ptr; _4 = move _5 as usize (PointerExposeAddress); + n[3]: int_to_ptr _ => _1 @ bb2[10]: fn test_ptr_int_ptr; _1 = move _6 as *mut libc::c_void (PointerFromExposedAddress); n[4]: copy n[3] => _8 @ bb2[14]: fn test_ptr_int_ptr; _8 = _1; n[5]: free n[4] => _7 @ bb2[15]: fn test_ptr_int_ptr; _7 = free(move _8); } diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 72a896fdd2..7b98babb0e 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2022-02-14" +channel = "nightly-2022-08-08" components = ["rustfmt-preview", "rustc-dev", "rust-src"]