Use FSA to check for thread-locals

Finalizers in Alloy are run on a separate thread, so if a user tries to
access a thread-local from within a drop method used as a finalizer it
is almost certainly incorrect. This commit disallows this and emit an
error when it happens.

compiler/rustc_middle/src/ty/util.rs

@@ -1224,6 +1224,13 @@ impl<'tcx> Ty<'tcx> {
         return false;
     }
 
+    pub fn is_thread_local(self, tcx: TyCtxt<'tcx>) -> bool {
+        if let ty::Adt(adt_def, ..) = self.kind() {
+            return tcx.get_diagnostic_item(sym::LocalKey).map_or(false, |t| adt_def.did() == t);
+        }
+        return false;
+    }
+
     pub fn is_finalize_unchecked(self, tcx: TyCtxt<'tcx>) -> bool {
         if let ty::Adt(adt_def, ..) = self.kind() {
             return tcx

compiler/rustc_mir_transform/src/check_finalizers.rs

@@ -4,6 +4,7 @@ use rustc_data_structures::fx::FxHashSet;
 use rustc_hir::def_id::DefId;
 use rustc_hir::lang_items::LangItem;
 use rustc_middle::mir::visit::PlaceContext;
+use rustc_middle::mir::visit::TyContext;
 use rustc_middle::mir::visit::Visitor;
 use rustc_middle::mir::*;
 use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt};
@@ -35,6 +36,7 @@ enum FinalizerErrorKind<'tcx> {
     InlineAsm(FnInfo<'tcx>),
     /// Contains a field projection where one of the projection elements is a raw pointer.
     RawPtr(FnInfo<'tcx>, ProjInfo<'tcx>),
+    ThreadLocal(FnInfo<'tcx>),
 }
 
 /// Information about the projection which caused the FSA error.
@@ -423,6 +425,17 @@ impl<'tcx> FSAEntryPointCtxt<'tcx> {
                 err.span_label(pi.span, "or be safe to use across threads.");
                 err.help("`Gc` runs finalizers on a separate thread, so drop methods\ncannot safely dereference raw pointers. If you are sure that this is safe,\nconsider wrapping it in a type which implements `Send + Sync`.");
             }
+            FinalizerErrorKind::ThreadLocal(fi) => {
+                err = self.tcx.sess.psess.dcx.struct_span_err(
+                    self.arg_span,
+                    format!("The drop method for `{0}` cannot be safely finalized.", fi.drop_ty),
+                );
+                err.span_label(
+                    fi.span,
+                    format!("this thread-local is not safe to run in a finalizer"),
+                );
+                err.help("`Gc` runs finalizers on a separate thread, so thread-locals cannot be accessed.");
+            }
         }
         err.span_label(
             self.fn_span,
@@ -454,6 +467,7 @@ struct DropCtxt<'ecx, 'tcx> {
     /// us to deal with recursive function calls. Without this, recursive calls in `drop` would
     /// cause FSA to loop forever.
     visited_fns: FxHashSet<ty::Instance<'tcx>>,
+    return_early: bool,
 }
 
 impl<'ecx, 'tcx> DropCtxt<'ecx, 'tcx> {
@@ -464,7 +478,7 @@ impl<'ecx, 'tcx> DropCtxt<'ecx, 'tcx> {
     ) -> Self {
         let mut callsites = VecDeque::default();
         callsites.push_back(drop_instance);
-        Self { callsites, ecx, drop_ty, visited_fns: FxHashSet::default() }
+        Self { callsites, ecx, drop_ty, visited_fns: FxHashSet::default(), return_early: false }
     }
 
     fn check(mut self) -> Result<(), Vec<FinalizerErrorKind<'tcx>>> {
@@ -528,6 +542,9 @@ impl<'dcx, 'ecx, 'tcx> FuncCtxt<'dcx, 'ecx, 'tcx> {
 
 impl<'dcx, 'ecx, 'tcx> Visitor<'tcx> for FuncCtxt<'dcx, 'ecx, 'tcx> {
     fn visit_projection(&mut self, place_ref: PlaceRef<'tcx>, _: PlaceContext, location: Location) {
+        if self.dcx.return_early {
+            return;
+        }
         // A single projection can be comprised of other 'inner' projections (e.g. self.a.b.c), so
         // this loop ensures that the types of each intermediate projection is extracted and then
         // checked.
@@ -562,6 +579,9 @@ impl<'dcx, 'ecx, 'tcx> Visitor<'tcx> for FuncCtxt<'dcx, 'ecx, 'tcx> {
     }
 
     fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
+        if self.dcx.return_early {
+            return;
+        }
         let (instance, info) = match &terminator.kind {
             TerminatorKind::Call { func, fn_span, .. } => {
                 match func.ty(self.body, self.tcx()).kind() {
@@ -645,6 +665,25 @@ impl<'dcx, 'ecx, 'tcx> Visitor<'tcx> for FuncCtxt<'dcx, 'ecx, 'tcx> {
         };
         self.super_terminator(terminator, location);
     }
+
+    fn visit_ty(&mut self, ty: Ty<'tcx>, cx: TyContext) {
+        if self.dcx.return_early {
+            return;
+        }
+        if !ty.is_thread_local(self.tcx()) {
+            self.super_ty(ty);
+            return;
+        }
+
+        let TyContext::Location(loc) = cx else {
+            bug!();
+        };
+
+        // We've found a thread-local inside a finalizer, which is almost certainly incorrect.
+        let info = FnInfo::new(self.body.source_info(loc).span, self.dcx.drop_ty);
+        self.push_error(loc, FinalizerErrorKind::ThreadLocal(info));
+        self.dcx.return_early = true;
+    }
 }
 
 fn in_std_lib<'tcx>(tcx: TyCtxt<'tcx>, did: DefId) -> bool {

tests/ui/static/gc/fsa/thread_locals.rs

@@ -0,0 +1,27 @@
+#![feature(gc)]
+#![feature(negative_impls)]
+#![allow(dead_code)]
+#![allow(unused_variables)]
+include!{"./auxiliary/types.rs"}
+
+use std::cell::Cell;
+
+thread_local! {
+    static COUNTER: Cell<u32> = Cell::new(0);
+}
+
+
+#[derive(Debug)]
+struct S;
+
+impl Drop for S {
+    fn drop(&mut self) {
+        // Access the thread-local variable
+        let x = COUNTER.get();
+    }
+}
+
+fn main() {
+    Gc::new(FinalizerUnsafeWrapper(S));
+    //~^ ERROR: The drop method for `S` cannot be safely finalized.
+}

tests/ui/static/gc/fsa/thread_locals.stderr

@@ -0,0 +1,13 @@
+error: The drop method for `S` cannot be safely finalized.
+  --> $DIR/thread_locals.rs:25:13
+   |
+LL |         let x = COUNTER.get();
+   |                 ------- this thread-local is not safe to run in a finalizer
+...
+LL |     Gc::new(FinalizerUnsafeWrapper(S));
+   |     --------^^^^^^^^^^^^^^^^^^^^^^^^^- caused by trying to construct a `Gc<FinalizerUnsafeWrapper<S>>` here.
+   |
+   = help: `Gc` runs finalizers on a separate thread, so thread-locals cannot be accessed.
+
+error: aborting due to 1 previous error
+