Auto merge of rust-lang#105605 - inquisitivecrystal:attr-validation, …

…r=cjgillot

Don't perform invalid checks in `codegen_attrs`

The attributes `#[track_caller]` and `#[cmse_nonsecure_entry]` are only valid on functions. When validating one of these attributes, codegen_attrs previously called `fn_sig`, [which can only be used on functions](rust-lang#105201), on the item the attribute was attached to, assuming that the item was a function without checking. This led to [ICEs in situations where the attribute was incorrectly used on non-functions](rust-lang#105594).

With this change, we skip calling `fn_sig` if the item the attribute is attached to must be a function but isn't, because `check_attr` will reject such cases without codegen_attrs's intervention.

As a side note, some of the attributes in codegen_attrs are only valid on functions, but that property isn't actually checked. I'm planning to fix that in a follow up PR since it's a behavior change that will need to be validated rather than an obvious bugfix. Thankfully, all the attributes like that I've found so far are unstable.

Fixes rust-lang#105594.

r? `@cjgillot`

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

@@ -2,6 +2,7 @@ use rustc_ast::{ast, MetaItemKind, NestedMetaItem};
 use rustc_attr::{list_contains_name, InlineAttr, InstructionSetAttr, OptimizeAttr};
 use rustc_errors::struct_span_err;
 use rustc_hir as hir;
+use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE};
 use rustc_hir::{lang_items, weak_lang_items::WEAK_LANG_ITEMS, LangItem};
 use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
@@ -60,6 +61,21 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs {
 
     let supported_target_features = tcx.supported_target_features(LOCAL_CRATE);
 
+    // In some cases, attribute are only valid on functions, but it's the `check_attr`
+    // pass that check that they aren't used anywhere else, rather this module.
+    // In these cases, we bail from performing further checks that are only meaningful for
+    // functions (such as calling `fn_sig`, which ICEs if given a non-function). We also
+    // report a delayed bug, just in case `check_attr` isn't doing its job.
+    let validate_fn_only_attr = |attr_sp| -> bool {
+        let def_kind = tcx.def_kind(did);
+        if let DefKind::Fn | DefKind::AssocFn | DefKind::Variant | DefKind::Ctor(..) = def_kind {
+            true
+        } else {
+            tcx.sess.delay_span_bug(attr_sp, "this attribute can only be applied to functions");
+            false
+        }
+    };
+
     let mut inline_span = None;
     let mut link_ordinal_span = None;
     let mut no_sanitize_span = None;
@@ -197,7 +213,9 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs {
                 }
             }
         } else if attr.has_name(sym::cmse_nonsecure_entry) {
-            if !matches!(tcx.fn_sig(did).abi(), abi::Abi::C { .. }) {
+            if validate_fn_only_attr(attr.span)
+                && !matches!(tcx.fn_sig(did).abi(), abi::Abi::C { .. })
+            {
                 struct_span_err!(
                     tcx.sess,
                     attr.span,
@@ -214,7 +232,10 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs {
         } else if attr.has_name(sym::thread_local) {
             codegen_fn_attrs.flags |= CodegenFnAttrFlags::THREAD_LOCAL;
         } else if attr.has_name(sym::track_caller) {
-            if !tcx.is_closure(did.to_def_id()) && tcx.fn_sig(did).abi() != abi::Abi::Rust {
+            if !tcx.is_closure(did.to_def_id())
+                && validate_fn_only_attr(attr.span)
+                && tcx.fn_sig(did).abi() != abi::Abi::Rust
+            {
                 struct_span_err!(tcx.sess, attr.span, E0737, "`#[track_caller]` requires Rust ABI")
                     .emit();
             }

src/test/ui/attributes/issue-105594-invalid-attr-validation.rs

@@ -0,0 +1,13 @@
+// This checks that the attribute validation ICE in issue #105594 doesn't
+// recur.
+//
+// ignore-thumbv8m.base
+#![feature(cmse_nonsecure_entry)]
+
+fn main() {}
+
+#[track_caller] //~ ERROR attribute should be applied to a function
+static _A: () = ();
+
+#[cmse_nonsecure_entry] //~ ERROR attribute should be applied to a function
+static _B: () = (); //~| ERROR #[cmse_nonsecure_entry]` is only valid for targets

src/test/ui/attributes/issue-105594-invalid-attr-validation.stderr

@@ -0,0 +1,26 @@
+error[E0739]: attribute should be applied to a function definition
+  --> $DIR/issue-105594-invalid-attr-validation.rs:9:1
+   |
+LL | #[track_caller]
+   | ^^^^^^^^^^^^^^^
+LL | static _A: () = ();
+   | ------------------- not a function definition
+
+error: attribute should be applied to a function definition
+  --> $DIR/issue-105594-invalid-attr-validation.rs:12:1
+   |
+LL | #[cmse_nonsecure_entry]
+   | ^^^^^^^^^^^^^^^^^^^^^^^
+LL | static _B: () = ();
+   | ------------------- not a function definition
+
+error[E0775]: `#[cmse_nonsecure_entry]` is only valid for targets with the TrustZone-M extension
+  --> $DIR/issue-105594-invalid-attr-validation.rs:12:1
+   |
+LL | #[cmse_nonsecure_entry]
+   | ^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors
+
+Some errors have detailed explanations: E0739, E0775.
+For more information about an error, try `rustc --explain E0739`.