Rollup merge of #148261 - aDotInTheVoid:no-sync-for-smir, r=celinval

rustc_public: Make Id types !Send / !Sync

These types are Id's to a table stored in TLS, so using them from another tread will either panic, or give wrong results.

Therefor, I've added a ~~`ReferencesTls`~~`ThreadLocalIndex`  marker type, which ensures types arn't `Send`/`Sync`.

This is a breaking change for users of the `rustc_public` crate.

~~It also changes how `DefId` and similar are `Serialize`d. It would be possible to preserve the old behavior if that's needed, but I couldn't see any tests for these.~~ EDIT: Not anymore: #148261 (comment)

Zulip Discussion: https://rust-lang.zulipchat.com/#narrow/channel/320896-project-stable-mir/topic/WDYM.20.22should.20not.20.20be.20shared.20across.20threads.22/with/547374171

Author: matthiaskrgr

Cargo.lock

@@ -4472,6 +4472,7 @@ dependencies = [
  "rustc_target",
  "scoped-tls",
  "serde",
+ "serde_json",
  "tracing",
 ]
 

compiler/rustc_public/Cargo.toml

@@ -13,10 +13,15 @@ rustc_session = { path = "../rustc_session" }
 rustc_span = { path = "../rustc_span" }
 rustc_target = { path = "../rustc_target" }
 scoped-tls = "1.0"
-serde = { version = "1.0.125", features = [ "derive" ] }
+serde = { version = "1.0.125", features = ["derive"] }
 tracing = "0.1"
 # tidy-alphabetical-end
 
+[dev-dependencies]
+# tidy-alphabetical-start
+serde_json = "1.0.142"
+# tidy-alphabetical-end
+
 [features]
 # tidy-alphabetical-start
 # Provides access to APIs that expose internals of the rust compiler.

compiler/rustc_public/src/abi.rs

@@ -7,8 +7,8 @@ use serde::Serialize;
 use crate::compiler_interface::with;
 use crate::mir::FieldIdx;
 use crate::target::{MachineInfo, MachineSize as Size};
-use crate::ty::{Align, Ty, VariantIdx};
-use crate::{Error, Opaque, error};
+use crate::ty::{Align, Ty, VariantIdx, index_impl};
+use crate::{Error, Opaque, ThreadLocalIndex, error};
 
 /// A function ABI definition.
 #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize)]
@@ -109,24 +109,16 @@ impl LayoutShape {
     }
 }
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Serialize)]
-pub struct Layout(usize);
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+pub struct Layout(usize, ThreadLocalIndex);
+index_impl!(Layout);
 
 impl Layout {
     pub fn shape(self) -> LayoutShape {
         with(|cx| cx.layout_shape(self))
     }
 }
 
-impl crate::IndexedVal for Layout {
-    fn to_val(index: usize) -> Self {
-        Layout(index)
-    }
-    fn to_index(&self) -> usize {
-        self.0
-    }
-}
-
 /// Describes how the fields of a type are shaped in memory.
 #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize)]
 pub enum FieldsShape {

compiler/rustc_public/src/compiler_interface.rs

@@ -25,7 +25,7 @@ use crate::ty::{
 use crate::unstable::{RustcInternal, Stable, new_item_kind};
 use crate::{
     AssocItems, Crate, CrateDef, CrateItem, CrateItems, CrateNum, DefId, Error, Filename,
-    ImplTraitDecls, ItemKind, Symbol, TraitDecls, alloc, mir,
+    ImplTraitDecls, ItemKind, Symbol, ThreadLocalIndex, TraitDecls, alloc, mir,
 };
 
 pub struct BridgeTys;
@@ -866,7 +866,7 @@ fn smir_crate<'tcx>(
 ) -> Crate {
     let name = cx.crate_name(crate_num);
     let is_local = cx.crate_is_local(crate_num);
-    let id = cx.crate_num_id(crate_num);
+    let id = CrateNum(cx.crate_num_id(crate_num), ThreadLocalIndex);
     debug!(?name, ?crate_num, "smir_crate");
     Crate { id, name, is_local }
 }

compiler/rustc_public/src/crate_def.rs

@@ -1,14 +1,13 @@
 //! Module that define a common trait for things that represent a crate definition,
 //! such as, a function, a trait, an enum, and any other definitions.
 
-use serde::Serialize;
-
-use crate::ty::{GenericArgs, Span, Ty};
-use crate::{AssocItems, Crate, Symbol, with};
+use crate::ty::{GenericArgs, Span, Ty, index_impl};
+use crate::{AssocItems, Crate, Symbol, ThreadLocalIndex, with};
 
 /// A unique identification number for each item accessible for the current compilation unit.
-#[derive(Clone, Copy, PartialEq, Eq, Hash, Serialize)]
-pub struct DefId(pub(crate) usize);
+#[derive(Clone, Copy, PartialEq, Eq, Hash)]
+pub struct DefId(pub(crate) usize, ThreadLocalIndex);
+index_impl!(DefId);
 
 impl DefId {
     /// Return fully qualified name of this definition

compiler/rustc_public/src/lib.rs

@@ -17,6 +17,7 @@
 //! [crates.io](https://crates.io).
 
 use std::fmt::Debug;
+use std::marker::PhantomData;
 use std::{fmt, io};
 
 pub(crate) use rustc_public_bridge::IndexedVal;
@@ -33,7 +34,10 @@ pub use crate::crate_def::{CrateDef, CrateDefItems, CrateDefType, DefId};
 pub use crate::error::*;
 use crate::mir::mono::StaticDef;
 use crate::mir::{Body, Mutability};
-use crate::ty::{AssocItem, FnDef, ForeignModuleDef, ImplDef, ProvenanceMap, Span, TraitDef, Ty};
+use crate::ty::{
+    AssocItem, FnDef, ForeignModuleDef, ImplDef, ProvenanceMap, Span, TraitDef, Ty,
+    serialize_index_impl,
+};
 use crate::unstable::Stable;
 
 pub mod abi;
@@ -46,31 +50,25 @@ pub mod compiler_interface;
 pub mod error;
 pub mod mir;
 pub mod target;
+#[cfg(test)]
+mod tests;
 pub mod ty;
 pub mod visitor;
 
 /// Use String for now but we should replace it.
 pub type Symbol = String;
 
 /// The number that identifies a crate.
-pub type CrateNum = usize;
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
+pub struct CrateNum(pub(crate) usize, ThreadLocalIndex);
+serialize_index_impl!(CrateNum);
 
 impl Debug for DefId {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("DefId").field("id", &self.0).field("name", &self.name()).finish()
     }
 }
 
-impl IndexedVal for DefId {
-    fn to_val(index: usize) -> Self {
-        DefId(index)
-    }
-
-    fn to_index(&self) -> usize {
-        self.0
-    }
-}
-
 /// A list of crate items.
 pub type CrateItems = Vec<CrateItem>;
 
@@ -297,3 +295,25 @@ impl rustc_public_bridge::bridge::Allocation<compiler_interface::BridgeTys>
         }
     }
 }
+
+#[derive(Clone, Copy, Hash, PartialEq, Eq, Default)]
+/// Marker type for indexes into thread local structures.
+///
+/// Makes things `!Send`/`!Sync`, so users don't move `rustc_public` types to
+/// thread with no (or worse, different) `rustc_public` pointer.
+///
+/// Note. This doesn't make it impossible to confuse TLS. You could return a
+/// `DefId` from one `run!` invocation, and then use it inside a different
+/// `run!` invocation with different tables.
+pub(crate) struct ThreadLocalIndex {
+    _phantom: PhantomData<*const ()>,
+}
+#[expect(non_upper_case_globals)]
+/// Emulating unit struct `struct ThreadLocalIndex`;
+pub(crate) const ThreadLocalIndex: ThreadLocalIndex = ThreadLocalIndex { _phantom: PhantomData };
+
+impl fmt::Debug for ThreadLocalIndex {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_tuple("ThreadLocalIndex").finish()
+    }
+}

compiler/rustc_public/src/mir/alloc.rs

@@ -6,8 +6,8 @@ use serde::Serialize;
 
 use crate::mir::mono::{Instance, StaticDef};
 use crate::target::{Endian, MachineInfo};
-use crate::ty::{Allocation, Binder, ExistentialTraitRef, Ty};
-use crate::{Error, IndexedVal, with};
+use crate::ty::{Allocation, Binder, ExistentialTraitRef, Ty, index_impl};
+use crate::{Error, ThreadLocalIndex, with};
 
 /// An allocation in the rustc_public's IR global memory can be either a function pointer,
 /// a static, or a "real" allocation with some data in it.
@@ -47,17 +47,9 @@ impl GlobalAlloc {
 }
 
 /// A unique identification number for each provenance
-#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, Serialize)]
-pub struct AllocId(usize);
-
-impl IndexedVal for AllocId {
-    fn to_val(index: usize) -> Self {
-        AllocId(index)
-    }
-    fn to_index(&self) -> usize {
-        self.0
-    }
-}
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)]
+pub struct AllocId(usize, ThreadLocalIndex);
+index_impl!(AllocId);
 
 /// Utility function used to read an allocation data into a unassigned integer.
 pub(crate) fn read_target_uint(mut bytes: &[u8]) -> Result<u128, Error> {

compiler/rustc_public/src/mir/mono.rs

@@ -7,8 +7,8 @@ use serde::Serialize;
 use crate::abi::FnAbi;
 use crate::crate_def::CrateDef;
 use crate::mir::Body;
-use crate::ty::{Allocation, ClosureDef, ClosureKind, FnDef, GenericArgs, Ty};
-use crate::{CrateItem, DefId, Error, IndexedVal, ItemKind, Opaque, Symbol, with};
+use crate::ty::{Allocation, ClosureDef, ClosureKind, FnDef, GenericArgs, Ty, index_impl};
+use crate::{CrateItem, DefId, Error, ItemKind, Opaque, Symbol, ThreadLocalIndex, with};
 
 #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize)]
 pub enum MonoItem {
@@ -241,8 +241,9 @@ impl From<StaticDef> for CrateItem {
     }
 }
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize)]
-pub struct InstanceDef(usize);
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
+pub struct InstanceDef(usize, ThreadLocalIndex);
+index_impl!(InstanceDef);
 
 impl CrateDef for InstanceDef {
     fn def_id(&self) -> DefId {
@@ -294,12 +295,3 @@ impl StaticDef {
         with(|cx| cx.eval_static_initializer(*self))
     }
 }
-
-impl IndexedVal for InstanceDef {
-    fn to_val(index: usize) -> Self {
-        InstanceDef(index)
-    }
-    fn to_index(&self) -> usize {
-        self.0
-    }
-}

compiler/rustc_public/src/rustc_internal/mod.rs

@@ -53,7 +53,7 @@ where
 }
 
 pub fn crate_num(item: &crate::Crate) -> CrateNum {
-    item.id.into()
+    item.id.0.into()
 }
 
 /// Loads the current context and calls a function with it.

compiler/rustc_public/src/tests.rs

@@ -0,0 +1,63 @@
+use rustc_public_bridge::IndexedVal;
+
+use crate::abi::Layout;
+use crate::mir::alloc::AllocId;
+use crate::mir::mono::InstanceDef;
+use crate::ty::{MirConstId, TyConstId, VariantIdx};
+use crate::{CrateNum, DefId, Span, ThreadLocalIndex, Ty};
+
+#[track_caller]
+fn check_serialize<T: serde::Serialize>(value: T, expected_json: &str) {
+    let got_json = serde_json::to_string(&value).unwrap();
+    assert_eq!(got_json, expected_json, "didn't get expected json for serializing");
+}
+
+#[test]
+fn serialize_cratenum() {
+    check_serialize(CrateNum(1, ThreadLocalIndex), "1");
+}
+
+#[test]
+fn serialize_defid() {
+    check_serialize(DefId::to_val(2), "2");
+}
+
+#[test]
+fn serialize_layout() {
+    check_serialize(Layout::to_val(3), "3");
+}
+
+#[test]
+fn serialize_allocid() {
+    check_serialize(AllocId::to_val(4), "4");
+}
+
+#[test]
+fn serialize_ty() {
+    check_serialize(Ty::to_val(5), "5");
+}
+
+#[test]
+fn serialize_tyconstid() {
+    check_serialize(TyConstId::to_val(6), "6");
+}
+
+#[test]
+fn serialize_mirconstid() {
+    check_serialize(MirConstId::to_val(7), "7");
+}
+
+#[test]
+fn serialize_span() {
+    check_serialize(Span::to_val(8), "8");
+}
+
+#[test]
+fn serialize_variantidx() {
+    check_serialize(VariantIdx::to_val(9), "9");
+}
+
+#[test]
+fn serialize_instancedef() {
+    check_serialize(InstanceDef::to_val(10), "10");
+}