Skip to content

Commit

Permalink
analyze: misc fixes for simple_buffer (#1148)
Browse files Browse the repository at this point in the history
This branch has fixes for various minor issues uncovered while testing
on `simple_buffer.c` (#1103).

* Fix borrowck handling of casts to `*mut my_struct` when `my_struct`
has lifetimes
* Fix various cases of compile errors introduced by `DynOwned` rewrites
* Fix precedence of method calls when generating rewritten source code
* Treat `realloc(NULL, n)` like `malloc(n)`
* Support nullable pointers in the `ZeroizeTy` initializers used for
`malloc`/`memset` rewrites
  • Loading branch information
spernsteiner authored Nov 26, 2024
2 parents 644d80b + 14cf393 commit 68de84c
Show file tree
Hide file tree
Showing 7 changed files with 260 additions and 43 deletions.
21 changes: 21 additions & 0 deletions c2rust-analyze/src/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,26 @@ fn run_polonius<'tcx>(
local_ltys.push(lty);
}

// Assign origins to `rvalue_tys`. We sort the keys first to ensure that we assign origins in
// a deterministic order.
let mut keys = acx.rvalue_tys.keys().collect::<Vec<_>>();
keys.sort();
let rvalue_ltys = keys
.into_iter()
.map(|&loc| {
let lty = acx.rvalue_tys[&loc];
let new_lty = assign_origins(
ltcx,
hypothesis,
&mut facts,
&mut maps,
&acx.gacx.adt_metadata,
lty,
);
(loc, new_lty)
})
.collect::<HashMap<_, _>>();

// Gather field permissions
let field_permissions = field_ltys
.iter()
Expand All @@ -348,6 +368,7 @@ fn run_polonius<'tcx>(
&mut maps,
&mut loans,
&local_ltys,
&rvalue_ltys,
&field_permissions,
hypothesis,
mir,
Expand Down
10 changes: 7 additions & 3 deletions c2rust-analyze/src/borrowck/type_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ struct TypeChecker<'tcx, 'a> {
maps: &'a mut AtomMaps<'tcx>,
loans: &'a mut HashMap<Local, Vec<(Path, Loan, BorrowKind)>>,
local_ltys: &'a [LTy<'tcx>],
rvalue_ltys: &'a HashMap<Location, LTy<'tcx>>,
field_permissions: &'a HashMap<DefId, PermissionSet>,
hypothesis: &'a PointerTableMut<'a, PermissionSet>,
local_decls: &'a IndexVec<Local, LocalDecl<'tcx>>,
Expand Down Expand Up @@ -389,16 +390,17 @@ impl<'tcx> TypeChecker<'tcx, '_> {
self.do_assign(result_lty, op_lty);
result_lty
}
Rvalue::Cast(_, _, ty) => self.ltcx.label(ty, &mut |_ty| {
Rvalue::Cast(_, _, _ty) => {
// TODO: handle Unsize casts at minimum
/*
assert!(
!matches!(ty.kind(), TyKind::RawPtr(..) | TyKind::Ref(..)),
"pointer Cast NYI"
);
*/
Label::default()
}),
// TODO: connect this type to the type of the operand
self.rvalue_ltys[&self.current_location]
}
Rvalue::Aggregate(ref kind, ref ops) => match **kind {
AggregateKind::Array(..) => {
let ty = rv.ty(self.local_decls, *self.ltcx);
Expand Down Expand Up @@ -623,6 +625,7 @@ pub fn visit_body<'tcx>(
maps: &mut AtomMaps<'tcx>,
loans: &mut HashMap<Local, Vec<(Path, Loan, BorrowKind)>>,
local_ltys: &[LTy<'tcx>],
rvalue_ltys: &HashMap<Location, LTy<'tcx>>,
field_permissions: &HashMap<DefId, PermissionSet>,
hypothesis: &PointerTableMut<PermissionSet>,
mir: &Body<'tcx>,
Expand All @@ -635,6 +638,7 @@ pub fn visit_body<'tcx>(
maps,
loans,
local_ltys,
rvalue_ltys,
field_permissions,
hypothesis,
local_decls: &mir.local_decls,
Expand Down
2 changes: 1 addition & 1 deletion c2rust-analyze/src/rewrite/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ impl<S: Sink> Emitter<'_, S> {
})
}
Rewrite::MethodCall(ref method, ref receiver_rw, ref arg_rws) => {
self.emit(receiver_rw, 0)?;
self.emit(receiver_rw, 3)?;
self.emit_str(".")?;
self.emit_str(method)?;
self.emit_parenthesized(true, |slf| {
Expand Down
38 changes: 32 additions & 6 deletions c2rust-analyze/src/rewrite/expr/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ impl<'tcx> ConvertVisitor<'tcx> {
elem_size,
src_single,
dest_single,
option,
} => {
// `realloc(p, n)` -> `Box::new(...)`
assert!(matches!(hir_rw, Rewrite::Identity));
Expand All @@ -364,9 +365,14 @@ impl<'tcx> ConvertVisitor<'tcx> {
}
let expr = match (src_single, dest_single) {
(false, false) => {
let src = if option {
"src_ptr.unwrap_or(Box::new([]))"
} else {
"src_ptr"
};
stmts.push(Rewrite::Let1(
"mut dest_ptr".into(),
Box::new(Rewrite::Text("Vec::from(src_ptr)".into())),
Box::new(format_rewrite!("Vec::from({src})")),
));
stmts.push(format_rewrite!(
"dest_ptr.resize_with(dest_n, || {})",
Expand All @@ -375,8 +381,9 @@ impl<'tcx> ConvertVisitor<'tcx> {
Rewrite::Text("dest_ptr.into_boxed_slice()".into())
}
(false, true) => {
let opt_flatten = if option { ".flatten()" } else { "" };
format_rewrite!(
"src_ptr.into_iter().next().unwrap_or_else(|| {})",
"src_ptr.into_iter(){opt_flatten}.next().unwrap_or_else(|| {})",
zeroize_expr
)
}
Expand All @@ -385,16 +392,28 @@ impl<'tcx> ConvertVisitor<'tcx> {
"mut dest_ptr".into(),
Box::new(Rewrite::Text("Vec::with_capacity(dest_n)".into())),
));
stmts.push(Rewrite::Text(
"if dest_n >= 1 { dest_ptr.push(*src_ptr); }".into(),
));
if option {
stmts.push(Rewrite::Text(
"if dest_n >= 1 { if let Some(src) = src_ptr { dest_ptr.push(*src); } }".into(),
));
} else {
stmts.push(Rewrite::Text(
"if dest_n >= 1 { dest_ptr.push(*src_ptr); }".into(),
));
}
stmts.push(format_rewrite!(
"dest_ptr.resize_with(dest_n, || {})",
zeroize_expr,
));
Rewrite::Text("dest_ptr.into_boxed_slice()".into())
}
(true, true) => Rewrite::Text("src_ptr".into()),
(true, true) => {
if option {
format_rewrite!("src_ptr.unwrap_or_else(|| Box::new({}))", zeroize_expr)
} else {
Rewrite::Text("src_ptr".into())
}
}
};
Rewrite::Block(stmts, Some(Box::new(expr)))
}
Expand Down Expand Up @@ -679,6 +698,7 @@ fn generate_zeroize_code(zero_ty: &ZeroizeType, lv: &str) -> String {
match *zero_ty {
ZeroizeType::Int => format!("{lv} = 0"),
ZeroizeType::Bool => format!("{lv} = false"),
ZeroizeType::Option => format!("{lv} = None"),
ZeroizeType::Array(ref elem_zero_ty) => format!(
"
{{
Expand Down Expand Up @@ -712,6 +732,7 @@ fn generate_zeroize_expr(zero_ty: &ZeroizeType) -> String {
match *zero_ty {
ZeroizeType::Int => format!("0"),
ZeroizeType::Bool => format!("false"),
ZeroizeType::Option => format!("None"),
ZeroizeType::Array(ref elem_zero_ty) => format!(
"std::array::from_fn(|| {})",
generate_zeroize_expr(elem_zero_ty)
Expand Down Expand Up @@ -758,6 +779,11 @@ pub fn convert_cast_rewrite(kind: &mir_op::RewriteKind, hir_rw: Rewrite) -> Rewr
Rewrite::Ref(Box::new(place), mutbl_from_bool(mutbl))
}

mir_op::RewriteKind::Deref => {
// `p` -> `*p`
Rewrite::Deref(Box::new(hir_rw))
}

mir_op::RewriteKind::OptionUnwrap => {
// `p` -> `p.unwrap()`
Rewrite::MethodCall("unwrap".to_string(), Box::new(hir_rw), vec![])
Expand Down
Loading

0 comments on commit 68de84c

Please sign in to comment.