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

Do not rewrite ADTs mentioned in extern blocks #960

Merged
merged 25 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b90b646
Addresses #948
aneksteind Jun 14, 2023
aa23b35
use extern "C" in test case
aneksteind Jun 14, 2023
9f35bbf
support foreign statics
aneksteind Jun 14, 2023
e789039
update doc FIX -> FIXED
aneksteind Jun 14, 2023
795438a
walk reachable types and fields
aneksteind Jun 15, 2023
ddc074e
fix infinite recursion bug in lighttpd
aneksteind Jun 15, 2023
91777bf
comment out breaking portions of lighttpd-minimal
aneksteind Jun 15, 2023
e52aa3e
CHECK-DAG -> CHECK-LABEL
aneksteind Jun 15, 2023
1105f6b
CHECK-DAG -> CHECK-LABEL
aneksteind Jun 15, 2023
d4655d1
fix CHECK-LABEL failure
aneksteind Jun 15, 2023
320d752
minor cosmetic changes
aneksteind Jun 15, 2023
de55129
docs
aneksteind Jun 15, 2023
f15d804
use adt_def instead of type_of given that the type is guaranteed to b…
aneksteind Jun 15, 2023
9c856bc
Docstring
aneksteind Jun 16, 2023
a2e99a5
documentation of walk_args_and_fields
aneksteind Jun 16, 2023
130bda9
add support for walking function pointer types
aneksteind Jun 16, 2023
d0a6aac
fn foreign_mentioned_tys
aneksteind Jun 16, 2023
88472f7
walk_args_and_fields -> walk_ffi_safe_ty
aneksteind Jun 16, 2023
79502f7
docs
aneksteind Jun 16, 2023
db64462
Revert "add support for walking function pointer types"
aneksteind Jun 16, 2023
e98ff18
simplify field walk
aneksteind Jun 16, 2023
8e34144
simplify foreign_mentioned_tys(..)
aneksteind Jun 16, 2023
0a4557d
walk_args_and_fields -> walk_adts
aneksteind Jun 16, 2023
b854368
simplify ADT walk
aneksteind Jun 16, 2023
7893da8
update documentation
aneksteind Jun 16, 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
76 changes: 38 additions & 38 deletions analysis/tests/lighttpd-minimal/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1195,12 +1195,12 @@ unsafe extern "C" fn connection_init(
);
}
(*con).srv = srv;
(*con).plugin_slots = (*srv).plugin_slots;
(*con).config_data_base = (*srv).config_data_base;
// (*con).plugin_slots = (*srv).plugin_slots;
// (*con).config_data_base = (*srv).config_data_base;
let r: *mut request_st = &mut (*con).request;
// request_init_data(r, con, srv);
(*con).write_queue = &mut (*r).write_queue;
(*con).read_queue = &mut (*r).read_queue;
// (*con).write_queue = &mut (*r).write_queue;
// (*con).read_queue = &mut (*r).read_queue;
// (*con).plugin_ctx = calloc(
// 1 as libc::c_int as libc::c_ulong,
// (((*srv).plugins.used).wrapping_add(1 as libc::c_int as libc::c_uint) as libc::c_ulong)
Expand All @@ -1223,18 +1223,18 @@ unsafe extern "C" fn connections_get_new_connection(
) -> *mut connection {
// let mut con: *mut connection = 0 as *mut connection;
(*srv).lim_conns = ((*srv).lim_conns).wrapping_sub(1);
if !((*srv).conns_pool).is_null() {
con = (*srv).conns_pool;
(*srv).conns_pool = (*con).next;
} else {
// if !((*srv).conns_pool).is_null() {
// con = (*srv).conns_pool;
// (*srv).conns_pool = (*con).next;
// } else {
con = connection_init(srv, con);
connection_reset(con);
}
(*con).next = (*srv).conns;
if !((*con).next).is_null() {
(*(*con).next).prev = con;
}
(*srv).conns = con;
// }
// (*con).next = (*srv).conns;
// if !((*con).next).is_null() {
// (*(*con).next).prev = con;
// }
// (*srv).conns = con;
return (*srv).conns;
}

Expand All @@ -1246,15 +1246,15 @@ pub unsafe extern "C" fn fdevent_register(
// mut ctx: *mut libc::c_void,
fdn: *mut fdnode,
) -> *mut fdnode {
let ref mut fresh0 = *((*ev).fdarray).offset(fd as isize);
*fresh0 = fdnode_init(fdn);
let mut fdn: *mut fdnode = *fresh0;
// let ref mut fresh0 = *((*ev).fdarray).offset(fd as isize);
// *fresh0 = fdnode_init(fdn);
// let mut fdn: *mut fdnode = *fresh0;
// (*fdn).handler = handler;
(*fdn).fd = fd;
// (*fdn).fd = fd;
// (*fdn).ctx = ctx;
(*fdn).events = 0 as libc::c_int;
(*fdn).fde_ndx = -(1 as libc::c_int);
return fdn;
// (*fdn).events = 0 as libc::c_int;
// (*fdn).fde_ndx = -(1 as libc::c_int);
return 0 as *mut fdnode;
}

unsafe extern "C" fn fdnode_init(fdn: *mut fdnode) -> *mut fdnode {
Expand All @@ -1275,17 +1275,17 @@ unsafe extern "C" fn fdnode_init(fdn: *mut fdnode) -> *mut fdnode {
}

unsafe extern "C" fn connection_del(mut srv: *mut server, mut con: *mut connection) {
if !((*con).next).is_null() {
(*(*con).next).prev = (*con).prev;
}
if !((*con).prev).is_null() {
(*(*con).prev).next = (*con).next;
} else {
(*srv).conns = (*con).next;
}
(*con).prev = con; // 0 as *mut connection;
(*con).next = (*srv).conns_pool;
(*srv).conns_pool = con;
// if !((*con).next).is_null() {
// (*(*con).next).prev = (*con).prev;
// }
// if !((*con).prev).is_null() {
// (*(*con).prev).next = (*con).next;
// } else {
// (*srv).conns = (*con).next;
// }
// (*con).prev = con; // 0 as *mut connection;
// (*con).next = (*srv).conns_pool;
// (*srv).conns_pool = con;
(*srv).lim_conns = ((*srv).lim_conns).wrapping_add(1);
}

Expand All @@ -1302,7 +1302,7 @@ unsafe extern "C" fn connection_close(mut con: *mut connection) {
(*con).request_count = 0 as libc::c_int as uint32_t;
(*con).is_ssl_sock = 0 as libc::c_int as libc::c_char;
(*con).revents_err = 0 as libc::c_int as uint16_t;
fdevent_fdnode_event_del((*srv).ev, (*con).fdn);
// fdevent_fdnode_event_del((*srv).ev, (*con).fdn);
fdevent_unregister((*srv).ev, (*con).fd);
// (*con).fdn = 0 as *mut fdnode;
if 0 as libc::c_int == close((*con).fd) {
Expand Down Expand Up @@ -1348,11 +1348,11 @@ unsafe extern "C" fn fdevent_fdnode_event_unsetter(mut ev: *mut fdevents, mut fd

#[no_mangle]
pub unsafe extern "C" fn fdevent_unregister(mut ev: *mut fdevents, mut fd: libc::c_int) {
let mut fdn: *mut fdnode = *((*ev).fdarray).offset(fd as isize);
if fdn as uintptr_t & 0x3 as libc::c_int as libc::c_ulong != 0 {
return;
}
let ref mut fresh1 = *((*ev).fdarray).offset(fd as isize);
// let mut fdn: *mut fdnode = *((*ev).fdarray).offset(fd as isize);
// if fdn as uintptr_t & 0x3 as libc::c_int as libc::c_ulong != 0 {
// return;
// }
// let ref mut fresh1 = *((*ev).fdarray).offset(fd as isize);
// *fresh1 = 0 as *mut fdnode;
// fdnode_free(fdn);
}
Expand Down
6 changes: 5 additions & 1 deletion c2rust-analyze/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use rustc_middle::ty::{
tls, AdtDef, FieldDef, GenericArgKind, GenericParamDefKind, Ty, TyCtxt, TyKind,
};
use rustc_type_ir::RegionKind::{ReEarlyBound, ReStatic};
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::fmt::Debug;
use std::ops::Index;

Expand Down Expand Up @@ -287,6 +287,8 @@ pub struct GlobalAnalysisCtxt<'tcx> {
pub addr_of_static: HashMap<DefId, PointerId>,

pub adt_metadata: AdtMetadataTable<'tcx>,

pub foreign_mentioned_tys: HashSet<DefId>,
}

pub struct AnalysisCtxt<'a, 'tcx> {
Expand Down Expand Up @@ -536,6 +538,7 @@ impl<'tcx> GlobalAnalysisCtxt<'tcx> {
static_tys: HashMap::new(),
addr_of_static: HashMap::new(),
adt_metadata: construct_adt_metadata(tcx),
foreign_mentioned_tys: HashSet::new(),
}
}

Expand Down Expand Up @@ -582,6 +585,7 @@ impl<'tcx> GlobalAnalysisCtxt<'tcx> {
ref mut static_tys,
ref mut addr_of_static,
adt_metadata: _,
foreign_mentioned_tys: _,
} = *self;

*ptr_info = remap_global_ptr_info(ptr_info, map, counter.num_pointers());
Expand Down
69 changes: 68 additions & 1 deletion c2rust-analyze/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use rustc_middle::mir::{
AggregateKind, BindingForm, Body, Constant, LocalDecl, LocalInfo, LocalKind, Location, Operand,
Rvalue, StatementKind,
};
use rustc_middle::ty::{Ty, TyCtxt, TyKind, WithOptConstParam};
use rustc_middle::ty::{GenericArgKind, Ty, TyCtxt, TyKind, WithOptConstParam};
use rustc_span::Span;
use std::collections::{HashMap, HashSet};
use std::env;
Expand Down Expand Up @@ -278,6 +278,53 @@ fn update_pointer_info<'tcx>(acx: &mut AnalysisCtxt<'_, 'tcx>, mir: &Body<'tcx>)
}
}

fn foreign_mentioned_tys(tcx: TyCtxt) -> HashSet<DefId> {
let mut foreign_mentioned_tys = HashSet::new();
for ty in tcx
.hir_crate_items(())
.foreign_items()
.map(|item| item.def_id.to_def_id())
.filter_map(|did| match tcx.def_kind(did) {
DefKind::Fn | DefKind::AssocFn => Some(tcx.mk_fn_ptr(tcx.fn_sig(did))),
DefKind::Static(_) => Some(tcx.type_of(did)),
_ => None,
})
{
walk_adts(tcx, ty, &mut |did| foreign_mentioned_tys.insert(did));
}
foreign_mentioned_tys
}

/// Walks the type `ty` and applies a function `f` to it if it's an ADT
/// `f` gets applied recursively to `ty`'s generic types and fields (if applicable)
/// If `f` returns false, recursion terminates cases where the ADT is recursive,
/// otherwise the function will naturally terminate given that the generic types
/// of a type are finite in length.
/// We only look for ADTs rather than other FFI-crossing types because ADTs
/// are the only nominal ones, which are the ones that we may rewrite.
fn walk_adts<'tcx, F>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, f: &mut F)
where
F: FnMut(DefId) -> bool,
{
// the first type encountered in the walk is `ty`, and the
// relevant handling for that is below, so we can skip it here
for arg in ty.walk().skip(1) {
if let GenericArgKind::Type(ty) = arg.unpack() {
walk_adts(tcx, ty, f);
}
}
aneksteind marked this conversation as resolved.
Show resolved Hide resolved

if let TyKind::Adt(adt_def, _) = ty.kind() {
if !f(adt_def.did()) {
return;
}
for field in adt_def.all_fields() {
let field_ty = tcx.type_of(field.did);
walk_adts(tcx, field_ty, f);
}
}
}

fn run(tcx: TyCtxt) {
let mut gacx = GlobalAnalysisCtxt::new(tcx);
let mut func_info = HashMap::new();
Expand Down Expand Up @@ -474,6 +521,10 @@ fn run(tcx: TyCtxt) {
|| info.contains(PointerInfo::NOT_TEMPORARY_REF))
}

// track all types mentioned in extern blocks, we
// don't want to rewrite those
gacx.foreign_mentioned_tys = foreign_mentioned_tys(tcx);

let mut gasn =
GlobalAssignment::new(gacx.num_pointers(), PermissionSet::UNIQUE, FlagSet::empty());
for (ptr, &info) in gacx.ptr_info().iter() {
Expand All @@ -482,6 +533,22 @@ fn run(tcx: TyCtxt) {
}
}

// FIX the fields of structs mentioned in extern blocks
aneksteind marked this conversation as resolved.
Show resolved Hide resolved
for adt_did in &gacx.adt_metadata.struct_dids {
if gacx.foreign_mentioned_tys.contains(adt_did) {
let adt_def = tcx.adt_def(adt_did);
let fields = adt_def.all_fields();
for field in fields {
let field_lty = gacx.field_ltys[&field.did];
eprintln!(
"adding FIXED permission for {adt_did:?} field {:?}",
field.did
);
make_ty_fixed(&mut gasn, field_lty);
}
}
}

for info in func_info.values_mut() {
let num_pointers = info.acx_data.num_pointers();
let mut lasn = LocalAssignment::new(num_pointers, PermissionSet::UNIQUE, FlagSet::empty());
Expand Down
22 changes: 12 additions & 10 deletions c2rust-analyze/src/rewrite/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,10 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for HirTyVisitor<'a, 'tcx> {
#[allow(clippy::single_match)]
match &item.kind {
ItemKind::Struct(VariantData::Struct(field_defs, _), generics) => {
if self.acx.gacx.foreign_mentioned_tys.contains(&did) {
eprintln!("Avoiding rewrite for foreign-mentioned type: {did:?}");
return;
}
let adt_metadata = &self.acx.gacx.adt_metadata.table[&did];
let updated_lifetime_params = &adt_metadata.lifetime_params;

Expand Down Expand Up @@ -538,16 +542,14 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for HirTyVisitor<'a, 'tcx> {
f_lty,
field_metadata.origin_args,
&mut |pointer_lty, lifetime_lty, args| {
{
create_rewrite_label(
pointer_lty,
args,
&self.asn.perms(),
&self.asn.flags(),
lifetime_lty.label,
&self.acx.gacx.adt_metadata,
)
}
create_rewrite_label(
pointer_lty,
args,
&self.asn.perms(),
&self.asn.flags(),
lifetime_lty.label,
&self.acx.gacx.adt_metadata,
)
},
);

Expand Down
1 change: 1 addition & 0 deletions c2rust-analyze/tests/filecheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ define_tests! {
fields,
field_temp,
fixed,
foreign,
insertion_sort,
insertion_sort_driver,
insertion_sort_rewrites,
Expand Down
66 changes: 66 additions & 0 deletions c2rust-analyze/tests/filecheck/foreign.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
extern "C" {
fn foo(bar: Alias) -> Baz;
}

type Alias = Bar;

// CHECK-DAG: br: ({{.*}}) perms = UNIQUE, flags = FIXED
// CHECK-DAG: bz: ({{.*}}) perms = UNIQUE, flags = FIXED
// CHECK-DAG: x: ({{.*}}) perms = UNIQUE, flags = FIXED
// CHECK-DAG: y: ({{.*}}) perms = UNIQUE, flags = FIXED

// CHECK-LABEL: BEGIN{{.*}}foreign.rs

// CHECK-LABEL: struct Bar
#[repr(C)]
struct Bar {
// CHECK-DAG: br: *mut i32
br: *mut i32
}

// CHECK-LABEL: struct Baz
#[repr(C)]
struct Baz {
// CHECK-DAG: bz: *mut i32
bz: *mut i32
}

// we need something to get rewritten in order
// to check that `Bar` and `Baz` get output
// in the rewrite string
fn fizz(i: *const i32) {}

extern "C" {
static mut s: S;
}

#[derive(Copy, Clone)]
#[repr(C)]
// CHECK-LABEL: struct S
pub struct S {
// CHECK-DAG: pub x: *const i32
pub x: *const i32,
// CHECK-DAG: pub y: *const i32
pub y: *const i32,
}

// CHECK-LABEL: struct Nit
#[repr(C)]
struct Nit {
// CHECK-DAG: x: *mut i32
x: *mut i32,
// CHECK-DAG: y: *mut i32
y: *mut i32,
}

// CHECK-LABEL: struct Bin
#[repr(C)]
struct Bin {
// CHECK-DAG: nit: *mut Nit
nit: *mut Nit,
}

extern "C" {
// CHECK-DAG: fn f(bin: *mut Bin)
fn f(bin: *mut Bin);
}