Do not rewrite ADTs mentioned in extern blocks (#960)

Fixes #948. Fixes an issue where we would rewrite structs and their
fields even though they appear in extern blocks.

Author: aneksteind

analysis/tests/lighttpd-minimal/src/main.rs

@@ -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)
@@ -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;
 }
 
@@ -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 {
@@ -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);
 }
 
@@ -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) {
@@ -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);
 }

c2rust-analyze/src/context.rs

@@ -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;
 
@@ -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> {
@@ -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(),
         }
     }
 
@@ -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());

c2rust-analyze/src/main.rs

@@ -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;
@@ -278,6 +278,49 @@ 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, the fields of the ADT are not recursed into.
+/// 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,
+{
+    for arg in ty.walk() {
+        if let GenericArgKind::Type(ty) = arg.unpack() {
+            if let TyKind::Adt(adt_def, _) = ty.kind() {
+                if !f(adt_def.did()) {
+                    continue;
+                }
+                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();
@@ -474,6 +517,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() {
@@ -482,6 +529,22 @@ fn run(tcx: TyCtxt) {
         }
     }
 
+    // FIX the fields of structs mentioned in extern blocks
+    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());

c2rust-analyze/src/rewrite/ty.rs

@@ -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;
 
@@ -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,
+                            )
                         },
                     );
 

c2rust-analyze/tests/filecheck.rs

@@ -48,6 +48,7 @@ define_tests! {
     fields,
     field_temp,
     fixed,
+    foreign,
     insertion_sort,
     insertion_sort_driver,
     insertion_sort_rewrites,

c2rust-analyze/tests/filecheck/foreign.rs

@@ -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);
+}