Add a scheme for moving away from extern "rust-intrinsic" entirely

oli-obk · oli-obk · commit b364460d4f6e · 2024-02-21T15:57:42.000Z
diff --git a/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs b/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
@@ -1255,7 +1255,17 @@ fn codegen_regular_intrinsic_call<'tcx>(
 
         // Unimplemented intrinsics must have a fallback body. The fallback body is obtained
         // by converting the `InstanceDef::Intrinsic` to an `InstanceDef::Item`.
-        _ => return Err(Instance::new(instance.def_id(), instance.args)),
+        _ => {
+            let intrinsic = fx.tcx.intrinsic(instance.def_id()).unwrap();
+            if intrinsic.must_be_overridden {
+                span_bug!(
+                    source_info.span,
+                    "intrinsic {} must be overridden by codegen_cranelift, but isn't",
+                    intrinsic.name,
+                );
+            }
+            return Err(Instance::new(instance.def_id(), instance.args));
+        }
     }
 
     let ret_block = fx.get_block(destination.unwrap());
diff --git a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs
@@ -16,6 +16,7 @@ use rustc_middle::ty::{self, SymbolName, TyCtxt};
 use rustc_middle::ty::{GenericArgKind, GenericArgsRef};
 use rustc_middle::util::Providers;
 use rustc_session::config::{CrateType, OomStrategy};
+use rustc_span::sym;
 use rustc_target::spec::{SanitizerSet, TlsModel};
 
 pub fn threshold(tcx: TyCtxt<'_>) -> SymbolExportLevel {
@@ -81,6 +82,10 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap<S
                 return library.kind.is_statically_included().then_some(def_id);
             }
 
+            if tcx.has_attr(def_id, sym::rustc_intrinsic_must_be_overridden) {
+                return None;
+            }
+
             // Only consider nodes that actually have exported symbols.
             match tcx.def_kind(def_id) {
                 DefKind::Fn | DefKind::Static(_) => {}
diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs
@@ -896,7 +896,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                             MergingSucc::False
                         };
                     }
-                    Err(instance) => Some(instance),
+                    Err(instance) => {
+                        if intrinsic.must_be_overridden {
+                            span_bug!(
+                                span,
+                                "intrinsic {} must be overridden by codegen backend, but isn't",
+                                intrinsic.name,
+                            );
+                        }
+                        Some(instance)
+                    }
                 }
             }
         };
diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs
@@ -792,6 +792,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         rustc_intrinsic, Normal, template!(Word), ErrorFollowing,
         "the `#[rustc_intrinsic]` attribute is used to declare intrinsics with function bodies",
     ),
+    rustc_attr!(
+        rustc_intrinsic_must_be_overridden, Normal, template!(Word), ErrorFollowing,
+        "the `#[rustc_intrinsic_must_be_overridden]` attribute is used to declare intrinsics without real bodies",
+    ),
 
     // ==========================================================================
     // Internal attributes, Testing:
diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs
@@ -1051,13 +1051,18 @@ fn should_encode_mir(
         // Coroutines require optimized MIR to compute layout.
         DefKind::Closure if tcx.is_coroutine(def_id.to_def_id()) => (false, true),
         // Full-fledged functions + closures
-        DefKind::AssocFn | DefKind::Fn | DefKind::Closure => {
+        def_kind @ (DefKind::AssocFn | DefKind::Fn | DefKind::Closure) => {
             let generics = tcx.generics_of(def_id);
-            let opt = tcx.sess.opts.unstable_opts.always_encode_mir
+            let mut opt = tcx.sess.opts.unstable_opts.always_encode_mir
                 || (tcx.sess.opts.output_types.should_codegen()
                     && reachable_set.contains(&def_id)
                     && (generics.requires_monomorphization(tcx)
                         || tcx.cross_crate_inlinable(def_id)));
+            if matches!(def_kind, DefKind::AssocFn | DefKind::Fn) {
+                if let Some(intrinsic) = tcx.intrinsic(def_id) {
+                    opt &= !intrinsic.must_be_overridden;
+                }
+            }
             // The function has a `const` modifier or is in a `#[const_trait]`.
             let is_const_fn = tcx.is_const_fn_raw(def_id.to_def_id())
                 || tcx.is_const_default_method(def_id.to_def_id());
diff --git a/compiler/rustc_middle/src/ty/intrinsic.rs b/compiler/rustc_middle/src/ty/intrinsic.rs
@@ -5,6 +5,8 @@ use super::TyCtxt;
 #[derive(Copy, Clone, Debug, Decodable, Encodable, HashStable)]
 pub struct IntrinsicDef {
     pub name: Symbol,
+    /// Whether the intrinsic has no meaningful body and all backends need to shim all calls to it.
+    pub must_be_overridden: bool,
 }
 
 impl TyCtxt<'_> {
diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs
@@ -1646,7 +1646,10 @@ pub fn intrinsic(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<ty::IntrinsicDef
     if matches!(tcx.fn_sig(def_id).skip_binder().abi(), Abi::RustIntrinsic | Abi::PlatformIntrinsic)
         || tcx.has_attr(def_id, sym::rustc_intrinsic)
     {
-        Some(ty::IntrinsicDef { name: tcx.item_name(def_id.into()) })
+        Some(ty::IntrinsicDef {
+            name: tcx.item_name(def_id.into()),
+            must_be_overridden: tcx.has_attr(def_id, sym::rustc_intrinsic_must_be_overridden),
+        })
     } else {
         None
     }
diff --git a/compiler/rustc_mir_transform/src/cross_crate_inline.rs b/compiler/rustc_mir_transform/src/cross_crate_inline.rs
@@ -9,6 +9,7 @@ use rustc_middle::query::Providers;
 use rustc_middle::ty::TyCtxt;
 use rustc_session::config::InliningThreshold;
 use rustc_session::config::OptLevel;
+use rustc_span::sym;
 
 pub fn provide(providers: &mut Providers) {
     providers.cross_crate_inlinable = cross_crate_inlinable;
@@ -22,6 +23,10 @@ fn cross_crate_inlinable(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
         return false;
     }
 
+    if tcx.has_attr(def_id, sym::rustc_intrinsic_must_be_overridden) {
+        return false;
+    }
+
     // This just reproduces the logic from Instance::requires_inline.
     match tcx.def_kind(def_id) {
         DefKind::Ctor(..) | DefKind::Closure => return true,
diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs
@@ -632,6 +632,12 @@ fn optimized_mir(tcx: TyCtxt<'_>, did: LocalDefId) -> &Body<'_> {
 }
 
 fn inner_optimized_mir(tcx: TyCtxt<'_>, did: LocalDefId) -> Body<'_> {
+    if let Some(attr) = tcx.get_attr(did, sym::rustc_intrinsic_must_be_overridden) {
+        span_bug!(
+            attr.span,
+            "this intrinsic must be overridden by the codegen backend, it has no meaningful body",
+        )
+    }
     if tcx.is_constructor(did.to_def_id()) {
         // There's no reason to run all of the MIR passes on constructors when
         // we can just output the MIR we want directly. This also saves const
diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs
@@ -1019,6 +1019,11 @@ fn should_codegen_locally<'tcx>(tcx: TyCtxt<'tcx>, instance: &Instance<'tcx>) ->
         return false;
     }
 
+    if tcx.has_attr(def_id, sym::rustc_intrinsic_must_be_overridden) {
+        // These are implemented by backends directly and have no meaningful body.
+        return false;
+    }
+
     if def_id.is_local() {
         // Local items cannot be referred to locally without monomorphizing them locally.
         return true;
diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
@@ -1428,6 +1428,7 @@ symbols! {
         rustc_inherit_overflow_checks,
         rustc_insignificant_dtor,
         rustc_intrinsic,
+        rustc_intrinsic_must_be_overridden,
         rustc_layout,
         rustc_layout_scalar_valid_range_end,
         rustc_layout_scalar_valid_range_start,
diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs
@@ -2475,9 +2475,8 @@ extern "rust-intrinsic" {
     #[rustc_nounwind]
     pub fn black_box<T>(dummy: T) -> T;
 
-    /// `ptr` must point to a vtable.
-    /// The intrinsic will return the size stored in that vtable.
     #[rustc_nounwind]
+    #[cfg(bootstrap)]
     pub fn vtable_size(ptr: *const ()) -> usize;
 
     /// `ptr` must point to a vtable.
@@ -2654,6 +2653,17 @@ pub const unsafe fn const_allocate(_size: usize, _align: usize) -> *mut u8 {
 #[cfg_attr(not(bootstrap), rustc_intrinsic)]
 pub const unsafe fn const_deallocate(_ptr: *mut u8, _size: usize, _align: usize) {}
 
+/// `ptr` must point to a vtable.
+/// The intrinsic will return the size stored in that vtable.
+#[rustc_nounwind]
+#[unstable(feature = "core_intrinsics", issue = "none")]
+#[cfg_attr(not(bootstrap), rustc_intrinsic)]
+#[cfg_attr(not(bootstrap), rustc_intrinsic_must_be_overridden)]
+#[cfg(not(bootstrap))]
+pub unsafe fn vtable_size(_ptr: *const ()) -> usize {
+    unreachable!()
+}
+
 // Some functions are defined here because they accidentally got made
 // available in this module on stable. See <https://github.com/rust-lang/rust/issues/15702>.
 // (`transmute` also falls into this category, but it cannot be wrapped due to the
diff --git a/src/doc/unstable-book/src/language-features/intrinsics.md b/src/doc/unstable-book/src/language-features/intrinsics.md
@@ -50,12 +50,23 @@ with any regular function.
 Various intrinsics have native MIR operations that they correspond to. Instead of requiring
 backends to implement both the intrinsic and the MIR operation, the `lower_intrinsics` pass
 will convert the calls to the MIR operation. Backends do not need to know about these intrinsics
-at all.
+at all. These intrinsics only make sense without a body, and can either be declared as a "rust-intrinsic"
+or as a `#[rustc_intrinsic]`. The body is never used, as calls to the intrinsic do not exist
+anymore after MIR analyses.
 
 ## Intrinsics without fallback logic
 
 These must be implemented by all backends.
 
+### `#[rustc_intrinsic]` declarations
+
+These are written like intrinsics with fallback bodies, but the body is irrelevant.
+Use `loop {}` for the body or call the intrinsic recursively and add
+`#[rustc_intrinsic_must_be_overridden]` to the function to ensure that backends don't
+invoke the body.
+
+### Legacy extern ABI based intrinsics
+
 These are imported as if they were FFI functions, with the special
 `rust-intrinsic` ABI. For example, if one was in a freestanding
 context, but wished to be able to `transmute` between types, and
diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs
@@ -335,7 +335,7 @@ fn check_terminator<'tcx>(
                 // within const fns. `transmute` is allowed in all other const contexts.
                 // This won't really scale to more intrinsics or functions. Let's allow const
                 // transmutes in const fn before we add more hacks to this.
-                if matches!(tcx.intrinsic(fn_def_id), Some(sym::transmute)) {
+                if tcx.is_intrinsic(fn_def_id, sym::transmute) {
                     return Err((
                         span,
                         "can only call `transmute` from const items, not `const fn`".into(),
diff --git a/tests/ui/intrinsics/always-gets-overridden.rs b/tests/ui/intrinsics/always-gets-overridden.rs
@@ -0,0 +1,20 @@
+//! Check that `vtable_size` gets overridden by llvm backend even if there is no
+//! `rustc_intrinsic_must_be_overridden` attribute on this usage.
+#![feature(rustc_attrs)]
+//@run-pass
+
+#[rustc_intrinsic]
+pub unsafe fn vtable_size(_ptr: *const ()) -> usize {
+    panic!();
+}
+
+trait Trait {}
+impl Trait for () {}
+
+fn main() {
+    let x: &dyn Trait = &();
+    unsafe {
+        let (_data, vtable): (*const (), *const ()) = core::mem::transmute(x);
+        assert_eq!(vtable_size(vtable), 0);
+    }
+}
diff --git a/tests/ui/intrinsics/not-overridden.rs b/tests/ui/intrinsics/not-overridden.rs
@@ -0,0 +1,18 @@
+//! Check that intrinsics that do not get overridden, but are marked as such,
+//! cause an error instead of silently invoking the body.
+#![feature(rustc_attrs, effects)]
+//@ build-fail
+//@ failure-status:101
+//@ normalize-stderr-test ".*note: .*\n\n" -> ""
+//@ normalize-stderr-test "thread 'rustc' panicked.*:\n.*\n" -> ""
+//@ normalize-stderr-test "internal compiler error:.*: intrinsic const_deallocate " -> ""
+//@ rustc-env:RUST_BACKTRACE=0
+
+#[rustc_intrinsic]
+#[rustc_intrinsic_must_be_overridden]
+pub const unsafe fn const_deallocate(_ptr: *mut u8, _size: usize, _align: usize) {}
+
+fn main() {
+    unsafe { const_deallocate(std::ptr::null_mut(), 0, 0) }
+    //~^ ERROR: must be overridden
+}
diff --git a/tests/ui/intrinsics/not-overridden.stderr b/tests/ui/intrinsics/not-overridden.stderr
@@ -0,0 +1,10 @@
+error: must be overridden by codegen backend, but isn't
+  --> $DIR/not-overridden.rs:16:14
+   |
+LL |     unsafe { const_deallocate(std::ptr::null_mut(), 0, 0) }
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+query stack during panic:
+end of query stack
+error: aborting due to 1 previous error
+

Original file line number	Diff line number	Diff line change
`@@ -896,7 +896,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {`
`896`	`896`	`MergingSucc::False`
`897`	`897`	`};`
`898`	`898`	`}`
`899`		`- Err(instance) => Some(instance),`
	`899`	`+ Err(instance) => {`
	`900`	`+ if intrinsic.must_be_overridden {`
	`901`	`+ span_bug!(`
	`902`	`+ span,`
	`903`	`+ "intrinsic {} must be overridden by codegen backend, but isn't",`
	`904`	`+ intrinsic.name,`
	`905`	`+ );`
	`906`	`+ }`
	`907`	`+ Some(instance)`
	`908`	`+ }`
`900`	`909`	`}`
`901`	`910`	`}`
`902`	`911`	`};`
Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,8 @@ use super::TyCtxt;`
`5`	`5`	`#[derive(Copy, Clone, Debug, Decodable, Encodable, HashStable)]`
`6`	`6`	`pub struct IntrinsicDef {`
`7`	`7`	`pub name: Symbol,`
	`8`	`+ /// Whether the intrinsic has no meaningful body and all backends need to shim all calls to it.`
	`9`	`+ pub must_be_overridden: bool,`
`8`	`10`	`}`
`9`	`11`
`10`	`12`	`impl TyCtxt<'_> {`
Original file line number	Diff line number	Diff line change
`@@ -1646,7 +1646,10 @@ pub fn intrinsic(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<ty::IntrinsicDef`
`1646`	`1646`	`if matches!(tcx.fn_sig(def_id).skip_binder().abi(), Abi::RustIntrinsic \| Abi::PlatformIntrinsic)`
`1647`	`1647`	`\|\| tcx.has_attr(def_id, sym::rustc_intrinsic)`
`1648`	`1648`	`{`
`1649`		`- Some(ty::IntrinsicDef { name: tcx.item_name(def_id.into()) })`
	`1649`	`+ Some(ty::IntrinsicDef {`
	`1650`	`+ name: tcx.item_name(def_id.into()),`
	`1651`	`+ must_be_overridden: tcx.has_attr(def_id, sym::rustc_intrinsic_must_be_overridden),`
	`1652`	`+ })`
`1650`	`1653`	`} else {`
`1651`	`1654`	`None`
`1652`	`1655`	`}`