analyze: omit unused hypothetical lifetimes during rewriting

c2rust-analyze/src/context.rs

@@ -217,6 +217,7 @@ bitflags! {
     }
 }
 
+#[derive(Default)]
 pub struct AdtMetadataTable<'tcx> {
     pub table: HashMap<DefId, AdtMetadata<'tcx>>,
     pub struct_dids: Vec<DefId>,
@@ -392,6 +393,7 @@ pub struct FnSigOrigins<'tcx> {
     pub output: LabeledTy<'tcx, &'tcx [OriginArg<'tcx>]>,
 }
 
+#[derive(Default)]
 pub struct FnOriginMap<'tcx> {
     pub fn_info: HashMap<DefId, FnSigOrigins<'tcx>>,
 }
@@ -489,7 +491,13 @@ fn fn_origin_args_params<'tcx>(
     FnOriginMap { fn_info }
 }
 
-fn construct_adt_metadata<'tcx>(tcx: TyCtxt<'tcx>) -> AdtMetadataTable {
+/// Build the `AdtMetadataTable`, which records the region parameters to be added to each ADT.  A
+/// region parameter will be created for each raw pointer where `needs_region(lty)` returns `true`.
+fn construct_adt_metadata<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    field_ltys: &HashMap<DefId, LTy<'tcx>>,
+    mut needs_region: impl FnMut(LTy<'tcx>) -> bool,
+) -> AdtMetadataTable<'tcx> {
     let struct_dids: Vec<_> = tcx
         .hir_crate_items(())
         .definitions()
@@ -509,24 +517,20 @@ fn construct_adt_metadata<'tcx>(tcx: TyCtxt<'tcx>) -> AdtMetadataTable {
         struct_dids,
     };
 
-    // Gather known lifetime parameters for each struct
+    // Gather existing lifetime parameters for each struct
     for struct_did in &adt_metadata_table.struct_dids {
         let struct_ty = tcx.type_of(struct_did);
         if let TyKind::Adt(adt_def, substs) = struct_ty.kind() {
             adt_metadata_table
                 .table
                 .insert(adt_def.did(), AdtMetadata::default());
+            let metadata = adt_metadata_table.table.get_mut(&adt_def.did()).unwrap();
             eprintln!("gathering known lifetimes for {adt_def:?}");
             for sub in substs.iter() {
                 if let GenericArgKind::Lifetime(r) = sub.unpack() {
                     eprintln!("\tfound lifetime {r:?} in {adt_def:?}");
                     assert_matches!(r.kind(), ReEarlyBound(eb) => {
-                        let _ = adt_metadata_table
-                        .table
-                        .entry(adt_def.did())
-                        .and_modify(|metadata| {
-                            metadata.lifetime_params.insert(OriginParam::Actual(eb));
-                        });
+                        metadata.lifetime_params.insert(OriginParam::Actual(eb));
                     });
                 }
             }
@@ -535,6 +539,14 @@ fn construct_adt_metadata<'tcx>(tcx: TyCtxt<'tcx>) -> AdtMetadataTable {
         }
     }
 
+    // TODO: Build dependency graph of ADTs and work by depth-first traversal of strongly-connected
+    // components.  Currently, if `Foo` contains a pointer and `Bar` contains two `Foo`s, `Bar`
+    // gets one region param that is used for both pointers.  Since this case is acyclic, we could
+    // instead give `Bar` two region params and pass a different one to each `Foo`.  Within each
+    // SCC, we need to use the first approach, otherwise every ADT might require an infinite number
+    // of regions.  But for mentions of ADTs in other SCCs, we can use the second approach, which
+    // is more flexible.
+
     let ltcx = LabeledTyCtxt::<'tcx, &[OriginArg<'tcx>]>::new(tcx);
     let mut loop_count = 0;
     loop {
@@ -557,35 +569,39 @@ fn construct_adt_metadata<'tcx>(tcx: TyCtxt<'tcx>) -> AdtMetadataTable {
             let adt_def = tcx.adt_def(struct_did);
             eprintln!("gathering lifetimes and lifetime parameters for {adt_def:?}");
             for field in adt_def.all_fields() {
-                let field_ty: Ty = tcx.type_of(field.did);
+                let field_lty = field_ltys
+                    .get(&field.did)
+                    .unwrap_or_else(|| panic!("missing field_ltys entry for {:?}", field.did));
                 eprintln!("\t{adt_def:?}.{:}", field.name);
-                let field_origin_args = ltcx.label(field_ty, &mut |ty| {
+                let field_origin_args = ltcx.relabel(field_lty, &mut |lty| {
                     let mut field_origin_args = IndexSet::new();
-                    match ty.kind() {
+                    match lty.kind() {
                         TyKind::RawPtr(ty) => {
-                            eprintln!(
-                                "\t\tfound pointer that requires hypothetical lifetime: *{:}",
-                                if let Mutability::Mut = ty.mutbl {
-                                    "mut"
-                                } else {
-                                    "const"
-                                }
-                            );
-                            adt_metadata_table
-                                .table
-                                .entry(*struct_did)
-                                .and_modify(|adt| {
-                                    let origin_arg = OriginArg::Hypothetical(next_hypo_origin_id);
-                                    let origin_param =
-                                        OriginParam::Hypothetical(next_hypo_origin_id);
-                                    eprintln!(
-                                        "\t\t\tinserting origin {origin_param:?} into {adt_def:?}"
-                                    );
-
-                                    adt.lifetime_params.insert(origin_param);
-                                    next_hypo_origin_id += 1;
-                                    field_origin_args.insert(origin_arg);
-                                });
+                            if needs_region(lty) {
+                                eprintln!(
+                                    "\t\tfound pointer that requires hypothetical lifetime: *{:}",
+                                    if let Mutability::Mut = ty.mutbl {
+                                        "mut"
+                                    } else {
+                                        "const"
+                                    }
+                                );
+                                adt_metadata_table
+                                    .table
+                                    .entry(*struct_did)
+                                    .and_modify(|adt| {
+                                        let origin_arg = OriginArg::Hypothetical(next_hypo_origin_id);
+                                        let origin_param =
+                                            OriginParam::Hypothetical(next_hypo_origin_id);
+                                        eprintln!(
+                                            "\t\t\tinserting origin {origin_param:?} into {adt_def:?}"
+                                        );
+
+                                        adt.lifetime_params.insert(origin_param);
+                                        next_hypo_origin_id += 1;
+                                        field_origin_args.insert(origin_arg);
+                                    });
+                            }
                         }
                         TyKind::Ref(reg, _ty, _mutability) => {
                             eprintln!("\t\tfound reference field lifetime: {reg:}");
@@ -664,8 +680,6 @@ fn construct_adt_metadata<'tcx>(tcx: TyCtxt<'tcx>) -> AdtMetadataTable {
 
 impl<'tcx> GlobalAnalysisCtxt<'tcx> {
     pub fn new(tcx: TyCtxt<'tcx>) -> GlobalAnalysisCtxt<'tcx> {
-        let adt_metadata = construct_adt_metadata(tcx);
-        let fn_origins = fn_origin_args_params(tcx, &adt_metadata);
         GlobalAnalysisCtxt {
             tcx,
             lcx: LabeledTyCtxt::new(tcx),
@@ -679,12 +693,25 @@ impl<'tcx> GlobalAnalysisCtxt<'tcx> {
             field_ltys: HashMap::new(),
             static_tys: HashMap::new(),
             addr_of_static: HashMap::new(),
-            adt_metadata,
-            fn_origins,
+            adt_metadata: AdtMetadataTable::default(),
+            fn_origins: FnOriginMap::default(),
             foreign_mentioned_tys: HashSet::new(),
         }
     }
 
+    /// Initialize `self.adt_metadata` and `self.fn_origins`.  This requires that all field types
+    /// in the crate have already been labeled in `field_ltys`.
+    pub fn construct_region_metadata(&mut self) {
+        debug_assert!(self.adt_metadata.table.is_empty());
+        debug_assert!(self.fn_origins.fn_info.is_empty());
+        self.construct_region_metadata_filtered(|_| true);
+    }
+
+    pub fn construct_region_metadata_filtered(&mut self, filter: impl FnMut(LTy<'tcx>) -> bool) {
+        self.adt_metadata = construct_adt_metadata(self.tcx, &self.field_ltys, filter);
+        self.fn_origins = fn_origin_args_params(self.tcx, &self.adt_metadata);
+    }
+
     pub fn function_context<'a>(&'a mut self, mir: &'a Body<'tcx>) -> AnalysisCtxt<'a, 'tcx> {
         AnalysisCtxt::new(self, mir)
     }

c2rust-analyze/src/main.rs

@@ -25,7 +25,9 @@ use crate::equiv::{GlobalEquivSet, LocalEquivSet};
 use crate::labeled_ty::LabeledTyCtxt;
 use crate::log::init_logger;
 use crate::panic_detail::PanicDetail;
+use crate::type_desc::Ownership;
 use crate::util::{Callee, TestAttr};
+use ::log::warn;
 use context::AdtMetadataTable;
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{CrateNum, DefId, DefIndex, LocalDefId};
@@ -538,6 +540,10 @@ fn run(tcx: TyCtxt) {
         gacx.assign_pointer_to_fields(did);
     }
 
+    // Compute hypothetical region data for all ADTs and functions.  This can only be done after
+    // all field types are labeled.
+    gacx.construct_region_metadata();
+
     // ----------------------------------
     // Compute dataflow constraints
     // ----------------------------------
@@ -943,15 +949,86 @@ fn run(tcx: TyCtxt) {
     }
     eprintln!("reached fixpoint in {} iterations", loop_count);
 
+    // Do final processing on each function.
+    for &ldid in &all_fn_ldids {
+        if gacx.fn_failed(ldid.to_def_id()) {
+            continue;
+        }
+
+        let info = func_info.get_mut(&ldid).unwrap();
+        let ldid_const = WithOptConstParam::unknown(ldid);
+        let mir = tcx.mir_built(ldid_const);
+        let mir = mir.borrow();
+        let acx = gacx.function_context_with_data(&mir, info.acx_data.take());
+        let mut asn = gasn.and(&mut info.lasn);
+
+        let r = panic_detail::catch_unwind(AssertUnwindSafe(|| {
+            // Add the CELL permission to pointers that need it.
+            info.dataflow.propagate_cell(&mut asn);
+
+            acx.check_string_literal_perms(&asn);
+        }));
+        match r {
+            Ok(()) => {}
+            Err(pd) => {
+                gacx.mark_fn_failed(ldid.to_def_id(), pd);
+                continue;
+            }
+        }
+
+        info.acx_data.set(acx.into_data());
+    }
+
     // Check that these perms haven't changed.
+    let mut known_perm_error_ptrs = HashSet::new();
     for (ptr, perms) in gacx.known_fn_ptr_perms() {
-        assert_eq!(perms, gasn.perms[ptr]);
+        if gasn.perms[ptr] != perms {
+            known_perm_error_ptrs.insert(ptr);
+            warn!(
+                "known permissions changed for PointerId {ptr:?}: {perms:?} -> {:?}",
+                gasn.perms[ptr]
+            );
+        }
+    }
+
+    let mut known_perm_error_fns = HashSet::new();
+    for (&def_id, lsig) in &gacx.fn_sigs {
+        if !tcx.is_foreign_item(def_id) {
+            continue;
+        }
+        for lty in lsig.inputs_and_output().flat_map(|lty| lty.iter()) {
+            let ptr = lty.label;
+            if !ptr.is_none() && known_perm_error_ptrs.contains(&ptr) {
+                known_perm_error_fns.insert(def_id);
+                warn!("known permissions changed for {def_id:?}: {lsig:?}");
+                break;
+            }
+        }
     }
 
     // ----------------------------------
     // Generate rewrites
     // ----------------------------------
 
+    // Regenerate region metadata, with hypothetical regions only in places where we intend to
+    // introduce refs.
+    gacx.construct_region_metadata_filtered(|lty| {
+        let ptr = lty.label;
+        if ptr.is_none() {
+            return false;
+        }
+        let flags = gasn.flags[ptr];
+        if flags.contains(FlagSet::FIXED) {
+            return false;
+        }
+        let perms = gasn.perms[ptr];
+        let desc = type_desc::perms_to_desc(lty.ty, perms, flags);
+        match desc.own {
+            Ownership::Imm | Ownership::Cell | Ownership::Mut => true,
+            Ownership::Raw | Ownership::RawMut | Ownership::Rc | Ownership::Box => false,
+        }
+    });
+
     // For testing, putting #[c2rust_analyze_test::fail_before_rewriting] on a function marks it as
     // failed at this point.
     for &ldid in &all_fn_ldids {
@@ -1005,14 +1082,9 @@ fn run(tcx: TyCtxt) {
             let mir = tcx.mir_built(ldid_const);
             let mir = mir.borrow();
             let acx = gacx.function_context_with_data(&mir, info.acx_data.take());
-            let mut asn = gasn.and(&mut info.lasn);
+            let asn = gasn.and(&mut info.lasn);
 
             let r = panic_detail::catch_unwind(AssertUnwindSafe(|| {
-                // Add the CELL permission to pointers that need it.
-                info.dataflow.propagate_cell(&mut asn);
-
-                acx.check_string_literal_perms(&asn);
-
                 if util::has_test_attr(tcx, ldid, TestAttr::SkipRewrite) {
                     return;
                 }
@@ -1263,6 +1335,13 @@ fn run(tcx: TyCtxt) {
         gacx.fns_failed.len(),
         all_fn_ldids.len()
     );
+
+    if known_perm_error_fns.len() > 0 {
+        eprintln!(
+            "saw permission errors in {} known fns",
+            known_perm_error_fns.len()
+        );
+    }
 }
 
 trait AssignPointerIds<'tcx> {

c2rust-analyze/tests/filecheck.rs

@@ -56,6 +56,7 @@ define_tests! {
     offset1,
     offset2,
     ptrptr1,
+    regions_fixed,
     statics,
     test_attrs,
     trivial,

c2rust-analyze/tests/filecheck/aggregate1.rs

@@ -71,7 +71,7 @@ pub union Onion {
     y: *mut u8,
 }
 
-// CHECK-DAG: struct UseOnion<'h2,'h1> {
+// CHECK-DAG: struct UseOnion<'h1,'h2> {
 struct UseOnion {
     // CHECK-DAG: foo: &'h2 (Onion<'h1>)
     foo: *mut Onion,

c2rust-analyze/tests/filecheck/cast.rs

@@ -27,7 +27,7 @@ extern "C" {
     fn bar(f: Foo);
 }
 
-// CHECK-LABEL: pub unsafe fn cell_as_mut_as_cell<'h0,'h1>(mut x: &'h0 core::cell::Cell<(i32)>, mut f: Foo<'h1>) {
+// CHECK-LABEL: pub unsafe fn cell_as_mut_as_cell<'h0>(mut x: &'h0 core::cell::Cell<(i32)>, mut f: Foo) {
 pub unsafe fn cell_as_mut_as_cell(mut x: *mut i32, mut f: Foo) {
     let z = x;
     let r = x;

c2rust-analyze/tests/filecheck/fields.rs

@@ -29,9 +29,9 @@ pub struct A<'a> {
     pub pra: *mut &'a mut A<'a>,
 }
 
-// CHECK-DAG: struct VecTup<'a,'h3,'h4,'h0,'h1,'h2> {
+// CHECK-DAG: struct VecTup<'a,'h0,'h1,'h2,'h3,'h4> {
 struct VecTup<'a> {
-    // CHECK-DAG: bar: &'h3 std::vec::Vec<(VecTup<'a,'h3,'h4,'h0,'h1,'h2>,&'h4 A<'a,'h0,'h1,'h2>),std::alloc::Global>
+    // CHECK-DAG: bar: &'h4 std::vec::Vec<(VecTup<'a,'h0,'h1,'h2,'h3,'h4>,&'h3 A<'a,'h0,'h1,'h2>),std::alloc::Global>
     bar: *mut Vec<(VecTup<'a>, *mut A<'a>)>,
 }
 
@@ -86,9 +86,9 @@ unsafe fn _field_access<'d, 'a: 'd, T: Clone + Copy>(ra: &'d mut A<'d>, ppd: *mu
 // CHECK-DAG: pub struct A<'a,'h0,'h1,'h2> {
 // CHECK-DAG: pub rd: &'a Data<'a,'h0,'h1,'h2>,
 // CHECK-DAG: pub pra: &'h2 core::cell::Cell<(&'a mut A<'a,'h0,'h1,'h2>)>,
-// CHECK-DAG: bar: &'h3 (Vec<(VecTup<'a,'h3,'h4,'h0,'h1,'h2>, &'h4 (A<'a,'h0,'h1,'h2>))>),
+// CHECK-DAG: bar: &'h4 (Vec<(VecTup<'a,'h0,'h1,'h2,'h3,'h4>, &'h3 (A<'a,'h0,'h1,'h2>))>),
 
-// CHECK-DAG: struct HypoWrapper<'h6,'h5>
+// CHECK-DAG: struct HypoWrapper<'h5,'h6>
 // CHECK-DAG: hw: &'h6 (Hypo<'h5>)
 
 // CHECK-DAG: unsafe fn _field_access<'d, 'a: 'd,'h0,'h1,'h2,'h3,'h4,'h5,'h6,'h7, T: Clone + Copy>(ra: &'d mut A<'d,'h0,'h1,'h2>, ppd: &'h3 mut (&'h4 mut (Data<'d,'h5,'h6,'h7>))) {

c2rust-analyze/tests/filecheck/foreign.rs

@@ -67,3 +67,23 @@ extern "C" {
     // CHECK-DAG: fn f(bin: *mut Bin)
     fn f(bin: *mut Bin);
 }
+
+extern "C" {
+    // CHECK-DAG: fn epoll_wait(events: *mut epoll_event);
+    fn epoll_wait(events: *mut epoll_event);
+}
+
+// CHECK-DAG: pub struct fdevents<'h0> {
+pub struct fdevents {
+    // CHECK-DAG: pub epoll_events: &'h0 (epoll_event),
+    pub epoll_events: *mut epoll_event,
+}
+
+// CHECK-DAG: pub struct epoll_event {
+pub struct epoll_event {
+    // CHECK-DAG: pub ptr: *mut u8,
+    pub ptr: *mut u8,
+}
+
+// CHECK-DAG: fn events<'h0>(f: fdevents<'h0>) {}
+fn events(f: fdevents) {}