Increase the precision of the exportable MIR check

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -1554,6 +1554,13 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             self.tables.unused_generic_params.set(def_id.local_def_index, unused);
         }
 
+        // Encode promoted_mir for all functions that we inlined
+        let prev_len = tcx.inlined_internal_defs.lock().len();
+        for def_id in tcx.inlined_internal_defs.lock().clone() {
+            record!(self.tables.promoted_mir[def_id.to_def_id()] <- tcx.promoted_mir(def_id));
+        }
+        assert_eq!(tcx.inlined_internal_defs.lock().len(), prev_len);
+
         // Encode all the deduced parameter attributes for everything that has MIR, even for items
         // that can't be inlined. But don't if we aren't optimizing in non-incremental mode, to
         // save the query traffic.

compiler/rustc_middle/src/ty/context.rs

@@ -558,6 +558,8 @@ pub struct GlobalCtxt<'tcx> {
 
     /// Stores memory for globals (statics/consts).
     pub(crate) alloc_map: Lock<interpret::AllocMap<'tcx>>,
+
+    pub inlined_internal_defs: Lock<FxHashSet<LocalDefId>>,
 }
 
 impl<'tcx> GlobalCtxt<'tcx> {
@@ -706,6 +708,7 @@ impl<'tcx> TyCtxt<'tcx> {
             new_solver_evaluation_cache: Default::default(),
             data_layout,
             alloc_map: Lock::new(interpret::AllocMap::new()),
+            inlined_internal_defs: Default::default(),
         }
     }
 

compiler/rustc_mir_transform/src/inline.rs

@@ -5,6 +5,7 @@ use rustc_hir::def_id::DefId;
 use rustc_index::bit_set::BitSet;
 use rustc_index::Idx;
 use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
+use rustc_middle::mir::interpret::{ConstValue, GlobalAlloc, Scalar};
 use rustc_middle::mir::visit::*;
 use rustc_middle::mir::*;
 use rustc_middle::ty::TypeVisitableExt;
@@ -87,12 +88,19 @@ fn inline<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
 
     let param_env = tcx.param_env_reveal_all_normalized(def_id);
 
+    let codegen_fn_attrs = tcx.codegen_fn_attrs(def_id);
+
+    let is_exported = tcx.is_constructor(def_id.into())
+        || tcx.generics_of(def_id).requires_monomorphization(tcx)
+        || codegen_fn_attrs.requests_inline();
+
     let mut this = Inliner {
         tcx,
         param_env,
-        codegen_fn_attrs: tcx.codegen_fn_attrs(def_id),
+        codegen_fn_attrs,
         history: Vec::new(),
         changed: false,
+        is_exported,
     };
     let blocks = START_BLOCK..body.basic_blocks.next_index();
     this.process_blocks(body, blocks);
@@ -112,6 +120,7 @@ struct Inliner<'tcx> {
     history: Vec<DefId>,
     /// Indicates that the caller body has been modified.
     changed: bool,
+    is_exported: bool,
 }
 
 impl<'tcx> Inliner<'tcx> {
@@ -146,6 +155,12 @@ impl<'tcx> Inliner<'tcx> {
                     debug!("inlined {}", callsite.callee);
                     self.changed = true;
 
+                    if !matches!(callsite.callee.def, InstanceDef::CloneShim(..)) {
+                        if let Some(def_id) = callsite.callee.def_id().as_local() {
+                            self.tcx.inlined_internal_defs.lock().insert(def_id);
+                        }
+                    }
+
                     self.history.push(callsite.callee.def_id());
                     self.process_blocks(caller_body, new_blocks);
                     self.history.pop();
@@ -184,6 +199,9 @@ impl<'tcx> Inliner<'tcx> {
 
         self.check_mir_is_available(caller_body, &callsite.callee)?;
         let callee_body = try_instance_mir(self.tcx, callsite.callee.def)?;
+        if self.is_exported {
+            self.check_mir_is_exportable(&callsite.callee, callee_attrs, callee_body)?;
+        }
         self.check_mir_body(callsite, callee_body, callee_attrs)?;
 
         if !self.tcx.consider_optimizing(|| {
@@ -353,6 +371,43 @@ impl<'tcx> Inliner<'tcx> {
         None
     }
 
+    fn check_mir_is_exportable(
+        &self,
+        callee: &Instance<'tcx>,
+        callee_attrs: &CodegenFnAttrs,
+        callee_body: &Body<'tcx>,
+    ) -> Result<(), &'static str> {
+        // If the callee is not local, then it must be exportable because we passed the check for
+        // if MIR is available.
+        if !callee.def_id().is_local() {
+            return Ok(());
+        }
+        if self.tcx.is_constructor(callee.def_id()) {
+            return Ok(());
+        }
+        if callee_attrs.requests_inline() {
+            return Ok(());
+        }
+        let is_generic = callee.substs.non_erasable_generics().next().is_some();
+        if is_generic {
+            return Ok(());
+        }
+
+        // The general situation we need to avoid is this:
+        // * We inline a non-exported function into an exported function
+        // * The non-exported function pulls a symbol previously determined to be internal into an
+        // exported function
+        // * Later, we inline the exported function (which is now not exportable) into another crate
+        // * Linker error (or ICE)
+        // So we search the callee body for anything that might cause problems if exported.
+        if !may_contain_unexportable_constant(self.tcx, callee_body) {
+            debug!("Has no pointer constants, must be exportable");
+            return Ok(());
+        }
+
+        Err("not exported")
+    }
+
     /// Returns an error if inlining is not possible based on codegen attributes alone. A success
     /// indicates that inlining decision should be based on other criteria.
     fn check_codegen_attributes(
@@ -364,16 +419,6 @@ impl<'tcx> Inliner<'tcx> {
             return Err("never inline hint");
         }
 
-        // Only inline local functions if they would be eligible for cross-crate
-        // inlining. This is to ensure that the final crate doesn't have MIR that
-        // reference unexported symbols
-        if callsite.callee.def_id().is_local() {
-            let is_generic = callsite.callee.substs.non_erasable_generics().next().is_some();
-            if !is_generic && !callee_attrs.requests_inline() {
-                return Err("not exported");
-            }
-        }
-
         if callsite.fn_sig.c_variadic() {
             return Err("C variadic");
         }
@@ -1148,3 +1193,45 @@ fn try_instance_mir<'tcx>(
         _ => Ok(tcx.instance_mir(instance)),
     }
 }
+
+fn may_contain_unexportable_constant<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> bool {
+    let mut finder = UnexportableConstFinder { tcx, found: false };
+    finder.visit_body(body);
+    finder.found
+}
+
+struct UnexportableConstFinder<'tcx> {
+    tcx: TyCtxt<'tcx>,
+    found: bool,
+}
+
+impl<'tcx> Visitor<'_> for UnexportableConstFinder<'tcx> {
+    fn visit_constant(&mut self, constant: &Constant<'_>, location: Location) {
+        debug!("Visiting constant: {:?}", constant.literal);
+        // Unevaluated constants could be anything. Ouch.
+        if let ConstantKind::Unevaluated(..) = constant.literal {
+            self.found = true;
+            return;
+        }
+
+        if let ConstantKind::Val(val, ty) = constant.literal {
+            // Constants which contain a pointer to a static are mentions of a static. Since
+            // statics can be private, exporting this MIR may cause a linker error.
+            if let ConstValue::Scalar(Scalar::Ptr(ptr, _size)) = val {
+                if let Some(GlobalAlloc::Static(_)) =
+                    self.tcx.try_get_global_alloc(ptr.into_parts().0)
+                {
+                    self.found = true;
+                }
+            }
+
+            // fn constants are mentions of other functions; if those functions are not exportable,
+            // exporting this function would break compilation.
+            if ty.is_fn() {
+                self.found = true;
+            }
+        }
+
+        self.super_constant(constant, location);
+    }
+}

tests/codegen-units/item-collection/items-within-generic-items.rs

@@ -3,6 +3,7 @@
 #![deny(dead_code)]
 #![feature(start)]
 
+#[inline(never)]
 fn generic_fn<T>(a: T) -> (T, i32) {
     //~ MONO_ITEM fn generic_fn::nested_fn
     fn nested_fn(a: i32) -> i32 {

tests/codegen/call-metadata.rs

@@ -12,6 +12,7 @@ pub fn test() {
 }
 
 #[no_mangle]
+#[inline(never)]
 fn some_true() -> Option<bool> {
     Some(true)
 }

tests/codegen/cold-call-declare-and-call.rs

@@ -9,6 +9,7 @@
 // CHECK: call coldcc void @this_should_never_happen(i16
 
 #[no_mangle]
+#[inline(never)]
 pub extern "rust-cold" fn this_should_never_happen(x: u16) {}
 
 pub fn do_things(x: u16) {

tests/codegen/drop.rs

@@ -11,6 +11,7 @@ impl Drop for SomeUniqueName {
     }
 }
 
+#[inline(never)]
 pub fn possibly_unwinding() {
 }
 

tests/codegen/personality_lifetimes.rs

@@ -13,6 +13,7 @@ impl Drop for S {
     }
 }
 
+#[inline(never)]
 fn might_unwind() {
 }
 

tests/codegen/target-cpu-on-functions.rs

@@ -15,7 +15,8 @@ pub extern "C" fn exported() {
 
 // CHECK-LABEL: ; target_cpu_on_functions::not_exported
 // CHECK-NEXT: ; Function Attrs:
-// CHECK-NEXT: define {{.*}}() {{.*}} #0
+// CHECK-NEXT: define {{.*}}() {{.*}} #1
+#[inline(never)]
 fn not_exported() {}
 
 // CHECK: attributes #0 = {{.*}} "target-cpu"="{{.*}}"

tests/codegen/tune-cpu-on-functions.rs

@@ -15,7 +15,8 @@ pub extern fn exported() {
 
 // CHECK-LABEL: ; tune_cpu_on_functions::not_exported
 // CHECK-NEXT: ; Function Attrs:
-// CHECK-NEXT: define {{.*}}() {{.*}} #0
+// CHECK-NEXT: define {{.*}}() {{.*}} #1
+#[inline(never)]
 fn not_exported() {}
 
 // CHECK: attributes #0 = {{.*}} "tune-cpu"="{{.*}}"

tests/incremental/hashes/let_expressions.rs

@@ -142,9 +142,9 @@ pub fn add_ref_in_pattern() {
 }
 
 #[cfg(not(any(cfail1,cfail4)))]
-#[rustc_clean(cfg="cfail2", except="hir_owner_nodes,typeck,optimized_mir")]
+#[rustc_clean(cfg="cfail2", except="hir_owner_nodes,typeck,optimized_mir,promoted_mir")]
 #[rustc_clean(cfg="cfail3")]
-#[rustc_clean(cfg="cfail5", except="hir_owner_nodes,typeck,optimized_mir")]
+#[rustc_clean(cfg="cfail5", except="hir_owner_nodes,typeck,optimized_mir,promoted_mir")]
 #[rustc_clean(cfg="cfail6")]
 pub fn add_ref_in_pattern() {
     let (ref _a, _b) = (1u8, 'y');

tests/mir-opt/const_prop/issue_66971.main.ConstProp.panic-abort.diff

@@ -3,17 +3,42 @@
 
   fn main() -> () {
       let mut _0: ();
-      let _1: ();
-      let mut _2: ((), u8, u8);
+      let mut _5: ();
+      let mut _6: u8;
+      let mut _7: u8;
+      scope 1 (inlined encode) {
+          debug this => ((), u8, u8){ .0 => _5, .1 => _6, .2 => _7, };
+          let mut _1: bool;
+          let mut _2: bool;
+          let mut _3: u8;
+          let mut _4: !;
+      }
 
       bb0: {
+          StorageLive(_5);
+          StorageLive(_6);
+          _5 = const ();
+          _6 = const 0_u8;
+          _7 = const 0_u8;
+          StorageLive(_1);
           StorageLive(_2);
-          _2 = (const (), const 0_u8, const 0_u8);
-          _1 = encode(move _2) -> [return: bb1, unwind unreachable];
+-         _2 = Eq(_7, const 0_u8);
+-         _1 = Not(move _2);
++         _2 = const true;
++         _1 = const false;
+          StorageDead(_2);
+-         switchInt(move _1) -> [0: bb2, otherwise: bb1];
++         switchInt(const false) -> [0: bb2, otherwise: bb1];
       }
 
       bb1: {
-          StorageDead(_2);
+          _4 = core::panicking::panic(const "assertion failed: this.2 == 0") -> unwind unreachable;
+      }
+
+      bb2: {
+          StorageDead(_1);
+          StorageDead(_5);
+          StorageDead(_6);
           return;
       }
   }

tests/mir-opt/const_prop/issue_66971.main.ConstProp.panic-unwind.diff

@@ -3,17 +3,42 @@
 
   fn main() -> () {
       let mut _0: ();
-      let _1: ();
-      let mut _2: ((), u8, u8);
+      let mut _5: ();
+      let mut _6: u8;
+      let mut _7: u8;
+      scope 1 (inlined encode) {
+          debug this => ((), u8, u8){ .0 => _5, .1 => _6, .2 => _7, };
+          let mut _1: bool;
+          let mut _2: bool;
+          let mut _3: u8;
+          let mut _4: !;
+      }
 
       bb0: {
+          StorageLive(_5);
+          StorageLive(_6);
+          _5 = const ();
+          _6 = const 0_u8;
+          _7 = const 0_u8;
+          StorageLive(_1);
           StorageLive(_2);
-          _2 = (const (), const 0_u8, const 0_u8);
-          _1 = encode(move _2) -> bb1;
+-         _2 = Eq(_7, const 0_u8);
+-         _1 = Not(move _2);
++         _2 = const true;
++         _1 = const false;
+          StorageDead(_2);
+-         switchInt(move _1) -> [0: bb2, otherwise: bb1];
++         switchInt(const false) -> [0: bb2, otherwise: bb1];
       }
 
       bb1: {
-          StorageDead(_2);
+          _4 = core::panicking::panic(const "assertion failed: this.2 == 0");
+      }
+
+      bb2: {
+          StorageDead(_1);
+          StorageDead(_5);
+          StorageDead(_6);
           return;
       }
   }