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.

Sign up for GitHub

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

Support intermediate casts in calls. #1022

Merged
merged 3 commits into from
Sep 18, 2023
Merged

Support intermediate casts in calls. #1022

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d57764
Support intermediate casts in calls.
aneksteind Aug 29, 2023
a1fbda0
get source place of Use or Copy instead of the Place of any arbitrary…
aneksteind Sep 18, 2023
8eddd72
docs
aneksteind Sep 18, 2023
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
75 changes: 60 additions & 15 deletions c2rust-analyze/src/c_void_casts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::borrow::Borrow;
use std::collections::{HashMap, HashSet};

use rustc_middle::mir::{BasicBlock, LocalKind};
use rustc_middle::ty::Ty;
use rustc_middle::{
mir::{
BasicBlockData, Body, LocalDecls, Location, Place, Rvalue, Statement, StatementKind,
Expand Down Expand Up @@ -53,6 +54,16 @@ impl CVoidCastDirection {
}
}

pub fn is_c_void_ptr(tcx: TyCtxt, ty: Ty) -> bool {
if let Some(TyKind::Adt(adt, _)) = ty.builtin_deref(true).map(|ty_mut| ty_mut.ty.kind()) {
let def_path = tcx.def_path(adt.did()).data[0].to_string();
let item_name = tcx.item_name(adt.did());
def_path == "ffi" && item_name.as_str() == "c_void"
} else {
false
}
}

/// The [`Place`] of a [`*c_void`].
///
/// It is checked to be a [`*c_void`] upon construction.
Expand All @@ -77,14 +88,8 @@ impl<'tcx> CVoidPtr<'tcx> {
local_decls: &LocalDecls<'tcx>,
tcx: TyCtxt<'tcx>,
) -> Option<Self> {
let deref_ty = place.ty(local_decls, tcx).ty.builtin_deref(true)?;

if let TyKind::Adt(adt, _) = deref_ty.ty.kind() {
if tcx.def_path(adt.did()).data[0].to_string() == "ffi"
&& tcx.item_name(adt.did()).as_str() == "c_void"
{
return Some(Self { place });
}
if is_c_void_ptr(tcx, place.ty(local_decls, tcx).ty) {
return Some(Self { place });
}

None
Expand Down Expand Up @@ -251,6 +256,27 @@ pub struct CVoidCasts<'tcx> {
to: CVoidCastsUniDirectional<'tcx>,
}

pub fn rv_place<'tcx>(rv: &Rvalue<'tcx>) -> Option<Place<'tcx>> {
use Rvalue::*;
match rv {
Use(op) => op.place(),
Repeat(op, _) => op.place(),
Ref(_, _, p) => Some(*p),
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the purpose of this function? It doesn't seem like a very meaningful operation, since the Places it returns are used in all sorts of different ways depending on what kind of Rvalue was passed. For example, some Rvalues read from rv_place(rv), while others don't.

Looking at the use site below, it seems like this is part of the logic for propagating through multiple levels of casts, as in p as *const c_char as *const c_void. If that's the case, can this be reduced to the Cast case only, where the meaning is clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed, now only addressing Use and Cast in a1fbda0

// ThreadLocalRef
AddressOf(_, p) => Some(*p),
Len(p) => Some(*p),
Cast(_, op, _) => op.place(),
// BinaryOp
// CheckedBinaryOp
// NullaryOp
UnaryOp(_, op) => op.place(),
Discriminant(p) => Some(*p),
// Aggregate
ShallowInitBox(op, _) => op.place(),
_ => None,
}
}

impl<'tcx> CVoidCasts<'tcx> {
pub fn direction(&self, direction: CVoidCastDirection) -> &CVoidCastsUniDirectional<'tcx> {
use CVoidCastDirection::*;
Expand Down Expand Up @@ -407,6 +433,8 @@ impl<'tcx> CVoidCasts<'tcx> {
/// * `calloc`
/// * `realloc`
/// * `free`
/// * `memcpy`
/// * `memset`
///
/// and insert their casts to and from [`*c_void`].
///
Expand Down Expand Up @@ -511,14 +539,36 @@ impl<'tcx> CVoidCasts<'tcx> {
let mut inserted_places = HashSet::new();
let mut modifying_statements = Vec::new();

for (current_block, current_block_data) in body.basic_blocks().iter_enumerated() {
let mut current_place = c_void_ptr.place;

for (current_block, current_block_data) in body.basic_blocks().iter_enumerated().rev() {
for (index, stmt) in current_block_data.statements.iter().enumerate().rev() {
if let Some((_, rhs)) =
get_assign_sides(stmt).filter(|(lhs, _)| *lhs == current_place)
{
// ancestors of a *libc::c_void are given special treatment;
// casts between these ancestors are exempt from scrutiny
if let Some(place) = rv_place(rhs) {
if get_cast_place(rhs).is_some() {
let location = Location {
statement_index: index,
block: current_block,
};
self.insert_cast(direction, location);
}

current_place = place;
}
}
}

modifying_statements.extend(Self::find_modifying_assignments(
current_block,
current_block_data,
&c_void_ptr,
));

if let Some((statement_index, cast)) =
if let Some((_, cast)) =
Self::find_last_cast(&current_block_data.statements, c_void_ptr)
{
assert!(
Expand All @@ -530,11 +580,6 @@ impl<'tcx> CVoidCasts<'tcx> {
body.local_kind(c_void_ptr.place.local) == LocalKind::Temp,
"Unsupported cast into non-temporary local"
);
let location = Location {
statement_index,
block: current_block,
};
self.insert_cast(direction, location);
self.insert_call(direction, terminator_location(current_block, bb_data), cast);
}
}
Expand Down
32 changes: 23 additions & 9 deletions c2rust-analyze/src/dataflow/type_check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::DataflowConstraints;
use crate::c_void_casts::CVoidCastDirection;
use crate::c_void_casts::{is_c_void_ptr, CVoidCastDirection};
use crate::context::{AnalysisCtxt, LTy, PermissionSet, PointerId};
use crate::panic_detail;
use crate::util::{
Expand Down Expand Up @@ -143,9 +143,17 @@ impl<'tcx> TypeChecker<'tcx, '_> {
match is_transmutable_ptr_cast(from_ty, to_ty) {
Some(true) => {
// TODO add other dataflow constraints
},
Some(false) => ::log::error!("TODO: unsupported ptr-to-ptr cast between pointee types not yet supported as safely transmutable: `{from_ty:?} as {to_ty:?}`"),
None => {}, // not a ptr cast (no dataflow constraints needed); let rustc typeck this
}
Some(false) => {
if is_c_void_ptr(self.acx.tcx(), to_lty.ty) {
// allow casts to c_void
Copy link
Contributor Author

@aneksteind aneksteind Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@spernsteiner this feels off to me, but the logic to exempt ancestor casts to *libc::c_void got complicated when considering function call boundaries. Is there anything wrong with making exceptions for casts to *libc::c_void?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems okay for now. I'm hoping most of the void* cast logic will be superseded by the new pointee type analysis I'm working on.

self.do_assign_pointer_ids(to_lty.label, from_lty.label);
} else {
::log::error!("TODO: unsupported ptr-to-ptr cast between pointee types not yet supported as safely transmutable: `{from_ty:?} as {to_ty:?}`");
}
}

None => {} // not a ptr cast (no dataflow constraints needed); let rustc typeck this
};
}
}
Expand Down Expand Up @@ -521,22 +529,28 @@ impl<'tcx> TypeChecker<'tcx, '_> {
let src_ptr = args[1]
.place()
.expect("Casts to/from null pointer are not yet supported");
let src_ptr = self.acx.c_void_casts.get_adjusted_place_or_default_to(
let src_ptr_casted_from = self.acx.c_void_casts.get_adjusted_place_or_default_to(
loc,
CVoidCastDirection::To,
src_ptr,
);

self.visit_place(out_ptr, Mutability::Mut);
let dest_ptr_lty = self.acx.type_of(out_ptr);
assert!(args.len() == 3);
self.visit_place(src_ptr, Mutability::Not);
let src_ptr_lty = self.acx.type_of(src_ptr);
self.visit_place(src_ptr_casted_from, Mutability::Not);
let src_ptr_casted_lty = self.acx.type_of(src_ptr_casted_from);

// input needs READ permission
let perms = PermissionSet::READ;
self.constraints.add_all_perms(src_ptr_lty.label, perms);
self.constraints
.add_all_perms(src_ptr_casted_lty.label, perms);

// Perform a pseudo-assignment for *dest = *src
// Perform a pseudo-assignment for *dest = *src.
// We use `src_ptr` instead of `src_ptr_casted_from` because the type that was
// casted to the libc::c_void_ptr that `memcpy` takes likely differs from the
// type that's pointed to by `dest_ptr`
let src_ptr_lty = self.acx.type_of(src_ptr);
self.do_equivalence_nested(dest_ptr_lty.args[0], src_ptr_lty.args[0]);
}
Callee::Memset => {
Expand Down
66 changes: 32 additions & 34 deletions c2rust-analyze/tests/filecheck/algo_md5.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#![feature(rustc_private)]

extern crate libc;

extern "C" {
fn memcpy(_: *mut libc::c_void, _: *const libc::c_void, _: libc::c_ulong) -> *mut libc::c_void;
fn memset(_: *mut libc::c_void, _: libc::c_int, _: libc::c_ulong) -> *mut libc::c_void;
Expand Down Expand Up @@ -81,8 +80,6 @@ static mut PADDING: [libc::c_uchar; 64] = [
0 as libc::c_int as libc::c_uchar,
0 as libc::c_int as libc::c_uchar,
];

// CHECK-LABEL: MD5_Init
#[no_mangle]
pub unsafe extern "C" fn MD5_Init(mut context: *mut MD5_CTX) {
(*context).count[1 as libc::c_int as usize] = 0 as libc::c_int as uint32_t;
Expand All @@ -95,13 +92,13 @@ pub unsafe extern "C" fn MD5_Init(mut context: *mut MD5_CTX) {
#[no_mangle]
pub unsafe extern "C" fn MD5_Update(
mut context: *mut MD5_CTX,
mut input: *const libc::c_void,
mut _input: *const libc::c_void,
mut inputLen: libc::c_uint,
) {
let mut i: libc::c_uint = 0;
let mut ndx: libc::c_uint = 0;
let mut partLen: libc::c_uint = 0;
// let mut input: *const libc::c_uchar = _input as *const libc::c_uchar;
let mut input: *const libc::c_uchar = _input as *const libc::c_uchar;
ndx = (*context).count[0 as libc::c_int as usize] >> 3 as libc::c_int
& 0x3f as libc::c_int as libc::c_uint;
(*context).count[0 as libc::c_int as usize] =
Expand All @@ -119,7 +116,7 @@ pub unsafe extern "C" fn MD5_Update(
memcpy(
&mut *((*context).buffer).as_mut_ptr().offset(ndx as isize) as *mut libc::c_uchar
as *mut libc::c_void,
input /* as *mut libc::c_uchar */ as *const libc::c_void,
input as *mut libc::c_uchar as *const libc::c_void,
partLen as libc::c_ulong,
);
li_MD5Transform(
Expand All @@ -128,52 +125,51 @@ pub unsafe extern "C" fn MD5_Update(
);
i = partLen;
while i.wrapping_add(63 as libc::c_int as libc::c_uint) < inputLen {
// li_MD5Transform(((*context).state).as_mut_ptr(), &*input.offset(i as isize));
li_MD5Transform(((*context).state).as_mut_ptr(), &*input.offset(i as isize));
i = i.wrapping_add(64 as libc::c_int as libc::c_uint);
}
ndx = 0 as libc::c_int as libc::c_uint;
} else {
i = 0 as libc::c_int as libc::c_uint;
}
// memcpy(
// &mut *((*context).buffer).as_mut_ptr().offset(ndx as isize) as *mut libc::c_uchar
// as *mut libc::c_void,
// &*input.offset(i as isize) as *const libc::c_uchar as *mut libc::c_uchar
// as *const libc::c_void,
// inputLen.wrapping_sub(i) as libc::c_ulong,
// );
memcpy(
&mut *((*context).buffer).as_mut_ptr().offset(ndx as isize) as *mut libc::c_uchar
as *mut libc::c_void,
&*input.offset(i as isize) as *const libc::c_uchar as *mut libc::c_uchar
as *const libc::c_void,
inputLen.wrapping_sub(i) as libc::c_ulong,
);
}
#[no_mangle]
pub unsafe extern "C" fn MD5_Final(mut digest: *mut libc::c_uchar, mut context: *mut MD5_CTX) {
let mut bits: [libc::c_uchar; 8] = [0; 8];
let mut ndx: libc::c_uint = 0;
let mut padLen: libc::c_uint = 0;
// Encode(
// bits.as_mut_ptr(),
// ((*context).count).as_mut_ptr(),
// 8 as libc::c_int as libc::c_uint,
// );
Encode(
bits.as_mut_ptr(),
((*context).count).as_mut_ptr(),
8 as libc::c_int as libc::c_uint,
);
ndx = (*context).count[0 as libc::c_int as usize] >> 3 as libc::c_int
& 0x3f as libc::c_int as libc::c_uint;
padLen = if ndx < 56 as libc::c_int as libc::c_uint {
(56 as libc::c_int as libc::c_uint).wrapping_sub(ndx)
} else {
(120 as libc::c_int as libc::c_uint).wrapping_sub(ndx)
};
// MD5_Update(context, PADDING.as_mut_ptr(),// as *const libc::c_void,
// padLen);
// MD5_Update(
// context,
// bits.as_mut_ptr(), // as *const libc::c_void,
// 8 as libc::c_int as libc::c_uint,
// );
// Encode(
// digest,
// ((*context).state).as_mut_ptr(),
// 16 as libc::c_int as libc::c_uint,
// );
MD5_Update(context, PADDING.as_mut_ptr() as *const libc::c_void, padLen);
MD5_Update(
context,
bits.as_mut_ptr() as *const libc::c_void,
8 as libc::c_int as libc::c_uint,
);
Encode(
digest,
((*context).state).as_mut_ptr(),
16 as libc::c_int as libc::c_uint,
);
memset(
context /* as *mut libc::c_uchar */ as *mut libc::c_void,
context as *mut libc::c_uchar as *mut libc::c_void,
0 as libc::c_int,
::std::mem::size_of::<MD5_CTX>() as libc::c_ulong,
);
Expand All @@ -184,7 +180,7 @@ unsafe extern "C" fn li_MD5Transform(mut state: *mut uint32_t, mut block: *const
let mut c: uint32_t = *state.offset(2 as libc::c_int as isize);
let mut d: uint32_t = *state.offset(3 as libc::c_int as isize);
let mut x: [uint32_t; 16] = [0; 16];
// Decode(x.as_mut_ptr(), block, 64 as libc::c_int as libc::c_uint);
Decode(x.as_mut_ptr(), block, 64 as libc::c_int as libc::c_uint);
a = (a as libc::c_uint).wrapping_add(
(b & c | !b & d)
.wrapping_add(x[0 as libc::c_int as usize])
Expand Down Expand Up @@ -642,7 +638,7 @@ unsafe extern "C" fn li_MD5Transform(mut state: *mut uint32_t, mut block: *const
let ref mut fresh3 = *state.offset(3 as libc::c_int as isize);
*fresh3 = (*fresh3 as libc::c_uint).wrapping_add(d) as uint32_t as uint32_t;
memset(
x.as_mut_ptr() /* as *mut libc::c_uchar */ as *mut libc::c_void,
x.as_mut_ptr() as *mut libc::c_uchar as *mut libc::c_void,
0 as libc::c_int,
::std::mem::size_of::<[uint32_t; 16]>() as libc::c_ulong,
);
Expand Down Expand Up @@ -672,6 +668,8 @@ unsafe extern "C" fn Encode(
j = j.wrapping_add(4 as libc::c_int as libc::c_uint);
}
}

// CHECK-LABEL: Decode
unsafe extern "C" fn Decode(
mut output: *mut uint32_t,
mut input: *const libc::c_uchar,
Expand Down