Merge #30

30: Prevent early finalization r=ltratt a=jacob-hughes

This is a draft PR because I'm still debugging the test suite.

Currently `Gc` is defined in the libgc crate. The compiler isn't aware
of this so can't perform any specific operations on the `Gc` type. To
fix this, we introduce a `GcSmartPointer` trait which `libgc::Gc` will
implement.

This should be temporary. Eventually, the `Gc` smart pointer will be
part of the standard library. Right now though, it's convenient for
development that they are separated.

Co-authored-by: Jake Hughes <jh@jakehughes.uk>

compiler/rustc_hir/src/lang_items.rs

@@ -208,6 +208,7 @@ language_item_table! {
 
     NoTrace,                 sym::notrace,             no_trace_trait,             Target::Trait;
     Conservative,            sym::conservative,        conservative_trait,         Target::Trait;
+    GcSmartPointer,          sym::gcsp,                gc_smart_pointer_trait,     Target::Trait;
 
     CoerceUnsized,           sym::coerce_unsized,      coerce_unsized_trait,       Target::Trait;
     DispatchFromDyn,         sym::dispatch_from_dyn,   dispatch_from_dyn_trait,    Target::Trait;

compiler/rustc_middle/src/query/mod.rs

@@ -1060,6 +1060,10 @@ rustc_queries! {
         query is_conservative_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
             desc { "computing whether `{}` is `Conservative`", env.value }
         }
+        /// Query backing `TyS::is_gc_smart_pointer`.
+        query is_gc_smart_pointer_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
+            desc { "computing whether `{}` is `GcSmartPointer`", env.value }
+        }
         /// Query backing `TyS::needs_drop`.
         query needs_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
             desc { "computing whether `{}` needs drop", env.value }

compiler/rustc_middle/src/ty/util.rs

@@ -755,6 +755,14 @@ impl<'tcx> ty::TyS<'tcx> {
         !self.is_conservative(tcx_at, param_env)
     }
 
+    pub fn is_gc_smart_pointer(
+        &'tcx self,
+        tcx_at: TyCtxtAt<'tcx>,
+        param_env: ty::ParamEnv<'tcx>,
+    ) -> bool {
+        tcx_at.is_gc_smart_pointer_raw(param_env.and(self))
+    }
+
     /// Fast path helper for testing if a type is `Freeze`.
     ///
     /// Returning true means the type is known to be `Freeze`. Returning

compiler/rustc_mir/src/transform/mod.rs

@@ -38,6 +38,7 @@ pub mod match_branches;
 pub mod multiple_return_terminators;
 pub mod no_landing_pads;
 pub mod nrvo;
+pub mod prevent_early_finalization;
 pub mod promote_consts;
 pub mod remove_noop_landing_pads;
 pub mod remove_unneeded_drops;
@@ -274,6 +275,7 @@ fn mir_const<'tcx>(
             &function_item_references::FunctionItemReferences,
             // What we need to do constant evaluation.
             &simplify::SimplifyCfg::new("initial"),
+            &prevent_early_finalization::PreventEarlyFinalization,
             &rustc_peek::SanityCheck,
         ]],
     );

compiler/rustc_mir/src/transform/prevent_early_finalization.rs

@@ -0,0 +1,86 @@
+use crate::{transform::MirPass, util::patch::MirPatch};
+use rustc_ast::{LlvmAsmDialect, StrStyle};
+use rustc_hir as hir;
+use rustc_middle::mir::*;
+use rustc_middle::ty::{ParamEnv, TyCtxt};
+use rustc_span::symbol::{kw, sym, Symbol};
+use rustc_span::DUMMY_SP;
+
+#[derive(PartialEq)]
+pub struct PreventEarlyFinalization;
+
+impl<'tcx> MirPass<'tcx> for PreventEarlyFinalization {
+    fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
+        if tcx.lang_items().gc_smart_pointer_trait().is_none() {
+            return;
+        }
+        let barrier = build_asm_barrier();
+        let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id());
+        let gc_locals = body
+            .local_decls()
+            .iter_enumerated()
+            .filter(|(_, d)| needs_barrier(d, tcx, param_env))
+            .map(|(l, _)| l)
+            .collect::<Vec<_>>();
+
+        if gc_locals.is_empty() {
+            return;
+        }
+        // First, remove the StorageDead for each local, so that a new one can
+        // be inserted after the barrier. Usually, this is at the end of the
+        // body anyway, so it's quicker to iterate backwards over the MIR.
+        for data in body.basic_blocks_mut() {
+            for stmt in data.statements.iter_mut().rev() {
+                if let StatementKind::StorageDead(ref local) = stmt.kind {
+                    if gc_locals.contains(local) {
+                        stmt.make_nop();
+                    }
+                }
+            }
+        }
+
+        let mut patch = MirPatch::new(body);
+        // There can be many BBs which terminate with a return, so to be safe,
+        // we add an asm barrier to each one.
+        let return_blocks =
+            body.basic_blocks().iter_enumerated().filter(|(_, b)| is_return(b.terminator()));
+        for (block, data) in return_blocks {
+            for local in gc_locals.iter() {
+                let loc = Location { block, statement_index: data.statements.len() };
+                let mut asm = barrier.clone();
+                asm.inputs = Box::new([(DUMMY_SP, Operand::Copy(Place::from(*local)))]);
+                patch.add_statement(loc, StatementKind::LlvmInlineAsm(asm));
+                patch.add_statement(loc, StatementKind::StorageDead(*local));
+            }
+        }
+        patch.apply(body);
+    }
+}
+
+fn is_return<'tcx>(terminator: &Terminator<'tcx>) -> bool {
+    match terminator.kind {
+        TerminatorKind::Return => true,
+        _ => false,
+    }
+}
+
+fn needs_barrier(local: &LocalDecl<'tcx>, tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>) -> bool {
+    local.is_user_variable()
+        && !local.ty.is_no_finalize_modulo_regions(tcx.at(DUMMY_SP), param_env)
+        && local.ty.is_gc_smart_pointer(tcx.at(DUMMY_SP), param_env)
+}
+
+fn build_asm_barrier() -> Box<LlvmInlineAsm<'tcx>> {
+    let asm_inner = hir::LlvmInlineAsmInner {
+        asm: kw::Empty,
+        asm_str_style: StrStyle::Cooked,
+        outputs: Vec::new(),
+        inputs: vec![Symbol::intern("r")],
+        clobbers: vec![sym::memory],
+        volatile: true,
+        alignstack: false,
+        dialect: LlvmAsmDialect::Att,
+    };
+
+    Box::new(LlvmInlineAsm { asm: asm_inner, inputs: Box::new([]), outputs: Box::new([]) })
+}

compiler/rustc_span/src/symbol.rs

@@ -576,6 +576,7 @@ symbols! {
         future,
         future_trait,
         gc_layout,
+        gcsp,
         ge,
         gen_future,
         gen_kill,

compiler/rustc_ty_utils/src/common_traits.rs

@@ -30,6 +30,13 @@ fn is_no_finalize_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'
     is_item_raw(tcx, query, LangItem::NoFinalize)
 }
 
+fn is_gc_smart_pointer_raw<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
+) -> bool {
+    is_item_raw(tcx, query, LangItem::GcSmartPointer)
+}
+
 fn is_item_raw<'tcx>(
     tcx: TyCtxt<'tcx>,
     query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
@@ -55,6 +62,7 @@ pub(crate) fn provide(providers: &mut ty::query::Providers) {
         is_freeze_raw,
         is_conservative_raw,
         is_no_trace_raw,
+        is_gc_smart_pointer_raw,
         is_no_finalize_raw,
         ..*providers
     };

library/core/src/gc.rs

@@ -25,6 +25,14 @@ pub struct Trace {
     pub size: u64,
 }
 
+#[unstable(feature = "gc", issue = "none")]
+#[cfg_attr(not(bootstrap), lang = "gcsp")]
+/// An internal trait which is only implemented by the `Gc` type. This exists so
+/// that the `Gc` type -- which is defined externally -- can be used as part of
+/// static analysis in the compiler. Evenually, this won't be necessary because
+/// `Gc` will be defined in the standard library.
+pub unsafe trait GcSmartPointer {}
+
 #[unstable(feature = "gc", issue = "none")]
 impl Trace {
     #[inline]

src/test/mir-opt/prevent_early_finalization.preserve_args.PreventEarlyFinalization.diff

@@ -0,0 +1,41 @@
+- // MIR for `preserve_args` before PreventEarlyFinalization
++ // MIR for `preserve_args` after PreventEarlyFinalization
+
+  fn preserve_args() -> () {
+      let mut _0: ();                      // return place in scope 0 at $DIR/prevent_early_finalization.rs:28:20: 28:20
+      let _1: Finalizable;                 // in scope 0 at $DIR/prevent_early_finalization.rs:29:9: 29:12
+      let mut _2: Finalizable;             // in scope 0 at $DIR/prevent_early_finalization.rs:29:35: 29:51
+      let mut _3: Finalizable;             // in scope 0 at $DIR/prevent_early_finalization.rs:29:53: 29:69
+      scope 1 {
+          debug ret => _1;                 // in scope 1 at $DIR/prevent_early_finalization.rs:29:9: 29:12
+      }
+
+      bb0: {
+          StorageLive(_1);                 // scope 0 at $DIR/prevent_early_finalization.rs:29:9: 29:12
+          StorageLive(_2);                 // scope 0 at $DIR/prevent_early_finalization.rs:29:35: 29:51
+          _2 = Finalizable(const 123_usize); // scope 0 at $DIR/prevent_early_finalization.rs:29:35: 29:51
+          StorageLive(_3);                 // scope 0 at $DIR/prevent_early_finalization.rs:29:53: 29:69
+          _3 = Finalizable(const 456_usize); // scope 0 at $DIR/prevent_early_finalization.rs:29:53: 29:69
+          _1 = preserve_args_inner(move _2, move _3) -> [return: bb1, unwind: bb2]; // scope 0 at $DIR/prevent_early_finalization.rs:29:15: 29:70
+                                           // mir::Constant
+                                           // + span: $DIR/prevent_early_finalization.rs:29:15: 29:34
+                                           // + literal: Const { ty: fn(Finalizable, Finalizable) -> Finalizable {preserve_args_inner}, val: Value(Scalar(<ZST>)) }
+      }
+
+      bb1: {
+          StorageDead(_3);                 // scope 0 at $DIR/prevent_early_finalization.rs:29:69: 29:70
+          StorageDead(_2);                 // scope 0 at $DIR/prevent_early_finalization.rs:29:69: 29:70
+          FakeRead(ForLet, _1);            // scope 0 at $DIR/prevent_early_finalization.rs:29:9: 29:12
+          _0 = const ();                   // scope 0 at $DIR/prevent_early_finalization.rs:28:20: 30:2
+-         StorageDead(_1);                 // scope 0 at $DIR/prevent_early_finalization.rs:30:1: 30:2
++         nop;                             // scope 0 at $DIR/prevent_early_finalization.rs:30:1: 30:2
++         llvm_asm!(LlvmInlineAsmInner { asm: "", asm_str_style: Cooked, outputs: [], inputs: ["r"], clobbers: ["memory"], volatile: true, alignstack: false, dialect: Att } : [] : [($DIR/prevent_early_finalization.rs:1:1: 1:1 (#0), _1)]); // scope 0 at $DIR/prevent_early_finalization.rs:30:2: 30:2
++         StorageDead(_1);                 // scope 0 at $DIR/prevent_early_finalization.rs:30:2: 30:2
+          return;                          // scope 0 at $DIR/prevent_early_finalization.rs:30:2: 30:2
+      }
+
+      bb2 (cleanup): {
+          resume;                          // scope 0 at $DIR/prevent_early_finalization.rs:28:1: 30:2
+      }
+  }
+