Rollup merge of #156242 - Jamesbarford:feat/remove-alwaysinline+target-feature, r=RalfJung,saethlin

Remove unsound `target_feature_inline_always` feature

## Summary
- Remove `target_feature_inline_always`
- Update stdarch generators to only use `#[inline]` & regenerate stdarch.

## Why?
Succinctly; the feature relies on LLVMs `AlwaysInlinerPass()` running before LLVMs heuristic based inliner pass. Which is not a basis for sound code.

This has been discussed in [the tracking issue](#145574).

If the ordering of the passes were to change, of which they have in the past, it is very possible we could inline functions across callsites with mismatching target features leading to unsound code. Checks proposed in; #155426 would only take into account caller -> callee which is not enough to guard against possibly of generating unsound code if the pass ordering were to change.

There doesn't seem to be a way, presently, this this mechanism to provide soundness guarantees nor does it seem like `AlwaysInlinerPass()` is a desired feature of LLVM, which this feature relies on.

r? @RalfJung

Author: JonathanBrouwer

compiler/rustc_codegen_llvm/src/attributes.rs

@@ -41,20 +41,13 @@ pub(crate) fn remove_string_attr_from_llfn(llfn: &Value, name: &str) {
 }
 
 /// Get LLVM attribute for the provided inline heuristic.
-pub(crate) fn inline_attr<'ll, 'tcx>(
+#[inline]
+fn inline_attr<'ll>(
     cx: &SimpleCx<'ll>,
-    tcx: TyCtxt<'tcx>,
-    instance: ty::Instance<'tcx>,
+    sess: &Session,
+    inline: InlineAttr,
 ) -> Option<&'ll Attribute> {
-    // `optnone` requires `noinline`
-    let codegen_fn_attrs = tcx.codegen_fn_attrs(instance.def_id());
-    let inline = match (codegen_fn_attrs.inline, &codegen_fn_attrs.optimize) {
-        (_, OptimizeAttr::DoNotOptimize) => InlineAttr::Never,
-        (InlineAttr::None, _) if instance.def.requires_inline(tcx) => InlineAttr::Hint,
-        (inline, _) => inline,
-    };
-
-    if !tcx.sess.opts.unstable_opts.inline_llvm {
+    if !sess.opts.unstable_opts.inline_llvm {
         // disable LLVM inlining
         return Some(AttributeKind::NoInline.create_attr(cx.llcx));
     }
@@ -64,7 +57,7 @@ pub(crate) fn inline_attr<'ll, 'tcx>(
             Some(AttributeKind::AlwaysInline.create_attr(cx.llcx))
         }
         InlineAttr::Never => {
-            if tcx.sess.target.arch != Arch::AmdGpu {
+            if sess.target.arch != Arch::AmdGpu {
                 Some(AttributeKind::NoInline.create_attr(cx.llcx))
             } else {
                 None
@@ -418,6 +411,17 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
         OptimizeAttr::Speed => {}
     }
 
+    if let Some(instance) = instance {
+        // `optnone` requires `noinline`
+        let inline = match (codegen_fn_attrs.inline, &codegen_fn_attrs.optimize) {
+            (_, OptimizeAttr::DoNotOptimize) => InlineAttr::Never,
+            (InlineAttr::None, _) if instance.def.requires_inline(tcx) => InlineAttr::Hint,
+            (inline, _) => inline,
+        };
+
+        to_add.extend(inline_attr(cx, sess, inline));
+    }
+
     if sess.must_emit_unwind_tables() {
         to_add.push(uwtable_attr(cx.llcx, sess.opts.unstable_opts.use_sync_unwind));
     }
@@ -568,16 +572,6 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
     let function_features =
         codegen_fn_attrs.target_features.iter().map(|f| f.name.as_str()).collect::<Vec<&str>>();
 
-    // Apply function attributes as per usual if there are no user defined
-    // target features otherwise this will get applied at the callsite.
-    if function_features.is_empty() {
-        if let Some(instance) = instance
-            && let Some(inline_attr) = inline_attr(cx, tcx, instance)
-        {
-            to_add.push(inline_attr);
-        }
-    }
-
     let function_features = function_features
         .iter()
         // Convert to LLVMFeatures and filter out unavailable ones

compiler/rustc_codegen_llvm/src/builder.rs

@@ -15,7 +15,7 @@ use rustc_codegen_ssa::mir::place::PlaceRef;
 use rustc_codegen_ssa::traits::*;
 use rustc_data_structures::small_c_str::SmallCStr;
 use rustc_hir::def_id::DefId;
-use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrs, TargetFeature, TargetFeatureKind};
+use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
 use rustc_middle::ty::layout::{
     FnAbiError, FnAbiOfHelpers, FnAbiRequest, HasTypingEnv, LayoutError, LayoutOfHelpers,
     TyAndLayout,
@@ -1419,32 +1419,6 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
             )
         };
 
-        if let Some(callee_instance) = callee_instance {
-            // Attributes on the function definition being called
-            let callee_attrs = self.cx.tcx.codegen_fn_attrs(callee_instance.def_id());
-            if let Some(caller_attrs) = caller_attrs
-                // If there is an inline attribute and a target feature that matches
-                // we will add the attribute to the callsite otherwise we'll omit
-                // this and not add the attribute to prevent soundness issues.
-                && let Some(inlining_rule) = attributes::inline_attr(&self.cx, self.cx.tcx, callee_instance)
-                && self.cx.tcx.is_target_feature_call_safe(
-                    &callee_attrs.target_features,
-                    &caller_attrs.target_features.iter().cloned().chain(
-                        self.cx.tcx.sess.target_features.iter().map(|feat| TargetFeature {
-                            name: *feat,
-                            kind: TargetFeatureKind::Implied,
-                        })
-                    ).collect::<Vec<_>>(),
-                )
-            {
-                attributes::apply_to_callsite(
-                    call,
-                    llvm::AttributePlace::Function,
-                    &[inlining_rule],
-                );
-            }
-        }
-
         if let Some(fn_abi) = fn_abi {
             fn_abi.apply_attrs_callsite(self, call);
         }

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

@@ -419,16 +419,16 @@ fn check_result(
     // llvm/llvm-project#70563).
     if !codegen_fn_attrs.target_features.is_empty()
         && matches!(codegen_fn_attrs.inline, InlineAttr::Always)
-        && !tcx.features().target_feature_inline_always()
         && let Some(span) = interesting_spans.inline
     {
-        feature_err(
-            tcx.sess,
-            sym::target_feature_inline_always,
-            span,
-            "cannot use `#[inline(always)]` with `#[target_feature]`",
-        )
-        .emit();
+        let mut diag = tcx
+            .dcx()
+            .struct_span_err(span, "cannot use `#[inline(always)]` with `#[target_feature]`");
+        diag.note(
+            "See this issue for full discussion: \
+            https://github.com/rust-lang/rust/issues/145574",
+        );
+        diag.emit();
     }
 
     // warn that inline has no effect when no_sanitize is present

compiler/rustc_feature/src/removed.rs

@@ -282,6 +282,9 @@ declare_features! (
     /// Allows string patterns to dereference values to match them.
     (removed, string_deref_patterns, "1.94.0", Some(87121), Some("superseded by `deref_patterns`"), 150530),
     (removed, struct_inherit, "1.0.0", None, None),
+    /// Allows the use of target_feature when a function is marked inline(always).
+    (removed, target_feature_inline_always, "CURRENT_RUSTC_VERSION", Some(145574),
+     Some("removed because of unfixable soundness issues")),
     (removed, test_removed_feature, "1.0.0", None, None),
     /// Allows using items which are missing stability attributes
     (removed, unmarked_api, "1.0.0", None, None),

compiler/rustc_feature/src/unstable.rs

@@ -727,8 +727,6 @@ declare_features! (
     (unstable, super_let, "1.88.0", Some(139076)),
     /// Allows subtrait items to shadow supertrait items.
     (unstable, supertrait_item_shadowing, "1.86.0", Some(89151)),
-    /// Allows the use of target_feature when a function is marked inline(always).
-    (unstable, target_feature_inline_always, "1.91.0", Some(145574)),
     /// Allows using `#[thread_local]` on `static` items.
     (unstable, thread_local, "1.0.0", Some(29594)),
     /// Allows defining `trait X = A + B;` alias items.

compiler/rustc_lint/src/lib.rs

@@ -644,6 +644,10 @@ fn register_builtins(store: &mut LintStore) {
     );
     store.register_removed("wasm_c_abi", "the wasm C ABI has been fixed");
     store.register_removed("soft_unstable", "the general soft-unstable mechanism has been removed");
+    store.register_removed(
+        "inline_always_mismatching_target_features",
+        "replaced by a hard error for `#[inline(always)]` with `#[target_feature]`",
+    );
 }
 
 fn register_internals(store: &mut LintStore) {

compiler/rustc_lint_defs/src/builtin.rs

@@ -56,7 +56,6 @@ declare_lint_pass! {
         ILL_FORMED_ATTRIBUTE_INPUT,
         INCOMPLETE_INCLUDE,
         INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
-        INLINE_ALWAYS_MISMATCHING_TARGET_FEATURES,
         INLINE_NO_SANITIZE,
         INVALID_DOC_ATTRIBUTES,
         INVALID_MACRO_EXPORT_ARGUMENTS,
@@ -5492,61 +5491,6 @@ declare_lint! {
     "detects tail calls of functions marked with `#[track_caller]`",
     @feature_gate = explicit_tail_calls;
 }
-declare_lint! {
-    /// The `inline_always_mismatching_target_features` lint will trigger when a
-    /// function with the `#[inline(always)]` and `#[target_feature(enable = "...")]`
-    /// attributes is called and cannot be inlined due to missing target features in the caller.
-    ///
-    /// ### Example
-    ///
-    /// ```rust,ignore (fails on x86_64)
-    /// #[inline(always)]
-    /// #[target_feature(enable = "fp16")]
-    /// unsafe fn callee() {
-    ///     // operations using fp16 types
-    /// }
-    ///
-    /// // Caller does not enable the required target feature
-    /// fn caller() {
-    ///     unsafe { callee(); }
-    /// }
-    ///
-    /// fn main() {
-    ///     caller();
-    /// }
-    /// ```
-    ///
-    /// This will produce:
-    ///
-    /// ```text
-    /// warning: call to `#[inline(always)]`-annotated `callee` requires the same target features. Function will not have `alwaysinline` attribute applied
-    ///   --> $DIR/builtin.rs:5192:14
-    ///    |
-    /// 10 |     unsafe { callee(); }
-    ///    |              ^^^^^^^^
-    ///    |
-    /// note: `fp16` target feature enabled in `callee` here but missing from `caller`
-    ///   --> $DIR/builtin.rs:5185:1
-    ///    |
-    /// 3  | #[target_feature(enable = "fp16")]
-    ///    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-    /// 4  | unsafe fn callee() {
-    ///    | ------------------
-    ///    = note: `#[warn(inline_always_mismatching_target_features)]` on by default
-    /// warning: 1 warning emitted
-    /// ```
-    ///
-    /// ### Explanation
-    ///
-    /// Inlining a function with a target feature attribute into a caller that
-    /// lacks the corresponding target feature can lead to unsound behavior.
-    /// LLVM may select the wrong instructions or registers, or reorder
-    /// operations, potentially resulting in runtime errors.
-    pub INLINE_ALWAYS_MISMATCHING_TARGET_FEATURES,
-    Warn,
-    r#"detects when a function annotated with `#[inline(always)]` and `#[target_feature(enable = "..")]` is inlined into a caller without the required target feature"#,
-}
-
 declare_lint! {
     /// The `repr_c_enums_larger_than_int` lint detects `repr(C)` enums with discriminant
     /// values that do not fit into a C `int` or `unsigned int`.

compiler/rustc_mir_transform/src/check_inline_always_target_features.rs

@@ -5,52 +5,11 @@ use rustc_errors::{
 };
 use rustc_macros::{Diagnostic, Subdiagnostic};
 use rustc_middle::mir::AssertKind;
-use rustc_middle::query::QueryKey;
 use rustc_middle::ty::TyCtxt;
 use rustc_session::lint::{self, Lint};
 use rustc_span::def_id::DefId;
 use rustc_span::{Ident, Span, Symbol};
 
-/// Emit diagnostic for calls to `#[inline(always)]`-annotated functions with a
-/// `#[target_feature]` attribute where the caller enables a different set of target features.
-pub(crate) fn emit_inline_always_target_feature_diagnostic<'a, 'tcx>(
-    tcx: TyCtxt<'tcx>,
-    call_span: Span,
-    callee_def_id: DefId,
-    caller_def_id: DefId,
-    callee_only: &[&'a str],
-) {
-    tcx.emit_node_span_lint(
-        lint::builtin::INLINE_ALWAYS_MISMATCHING_TARGET_FEATURES,
-        tcx.local_def_id_to_hir_id(caller_def_id.as_local().unwrap()),
-        call_span,
-        rustc_errors::DiagDecorator(|lint| {
-            let callee = tcx.def_path_str(callee_def_id);
-            let caller = tcx.def_path_str(caller_def_id);
-
-            lint.primary_message(format!(
-                "call to `#[inline(always)]`-annotated `{callee}` \
-                requires the same target features to be inlined"
-            ));
-            lint.note("function will not be inlined");
-
-            lint.note(format!(
-                "the following target features are on `{callee}` but missing from `{caller}`: {}",
-                callee_only.join(", ")
-            ));
-            lint.span_note(callee_def_id.default_span(tcx), format!("`{callee}` is defined here"));
-
-            let feats = callee_only.join(",");
-            lint.span_suggestion(
-                tcx.def_span(caller_def_id).shrink_to_lo(),
-                format!("add `#[target_feature]` attribute to `{caller}`"),
-                format!("#[target_feature(enable = \"{feats}\")]\n"),
-                lint::Applicability::MaybeIncorrect,
-            );
-        }),
-    );
-}
-
 #[derive(Diagnostic)]
 #[diag("function cannot return without recursing")]
 #[help("a `loop` may express intention better if this is on purpose")]

compiler/rustc_mir_transform/src/errors.rs

@@ -122,7 +122,6 @@ declare_passes! {
     mod add_subtyping_projections : Subtyper;
     mod check_inline : CheckForceInline;
     mod check_call_recursion : CheckCallRecursion, CheckDropRecursion;
-    mod check_inline_always_target_features: CheckInlineAlwaysTargetFeature;
     mod check_alignment : CheckAlignment;
     mod check_enums : CheckEnums;
     mod check_const_item_mutation : CheckConstItemMutation;
@@ -403,9 +402,6 @@ fn mir_built(tcx: TyCtxt<'_>, def: LocalDefId) -> &Steal<Body<'_>> {
             // MIR-level lints.
             &Lint(check_inline::CheckForceInline),
             &Lint(check_call_recursion::CheckCallRecursion),
-            // Check callee's target features match callers target features when
-            // using `#[inline(always)]`
-            &Lint(check_inline_always_target_features::CheckInlineAlwaysTargetFeature),
             &Lint(check_packed_ref::CheckPackedRef),
             &Lint(check_const_item_mutation::CheckConstItemMutation),
             &Lint(function_item_references::FunctionItemReferences),