Auto merge of #88680 - ehuss:more-attr-validation, r=petrochenkov

Validate builtin attributes for macro args.

This adds some validation for `path`, `crate_type`, and `recursion_limit` attributes so that they will now return an error if you attempt to pass a macro into them (such as `#[path = foo!()]`). Previously, the attribute would be completely ignored. These attributes are special because their values need to be known before/during expansion.

cc #87681

compiler/rustc_expand/src/module.rs

@@ -3,6 +3,7 @@ use rustc_ast::ptr::P;
 use rustc_ast::{token, Attribute, Inline, Item};
 use rustc_errors::{struct_span_err, DiagnosticBuilder};
 use rustc_parse::new_parser_from_file;
+use rustc_parse::validate_attr;
 use rustc_session::parse::ParseSess;
 use rustc_session::Session;
 use rustc_span::symbol::{sym, Ident};
@@ -168,7 +169,25 @@ fn mod_file_path_from_attr(
     dir_path: &Path,
 ) -> Option<PathBuf> {
     // Extract path string from first `#[path = "path_string"]` attribute.
-    let path_string = sess.first_attr_value_str_by_name(attrs, sym::path)?.as_str();
+    let first_path = attrs.iter().find(|at| at.has_name(sym::path))?;
+    let path_string = match first_path.value_str() {
+        Some(s) => s.as_str(),
+        None => {
+            // This check is here mainly to catch attempting to use a macro,
+            // such as #[path = concat!(...)]. This isn't currently supported
+            // because otherwise the InvocationCollector would need to defer
+            // loading a module until the #[path] attribute was expanded, and
+            // it doesn't support that (and would likely add a bit of
+            // complexity). Usually bad forms are checked in AstValidator (via
+            // `check_builtin_attribute`), but by the time that runs the macro
+            // is expanded, and it doesn't give an error.
+            validate_attr::emit_fatal_malformed_builtin_attribute(
+                &sess.parse_sess,
+                first_path,
+                sym::path,
+            );
+        }
+    };
 
     // On windows, the base path might have the form
     // `\\?\foo\bar` in which case it does not tolerate

compiler/rustc_interface/src/passes.rs

@@ -23,7 +23,7 @@ use rustc_middle::middle::cstore::{MetadataLoader, MetadataLoaderDyn};
 use rustc_middle::ty::query::Providers;
 use rustc_middle::ty::{self, GlobalCtxt, ResolverOutputs, TyCtxt};
 use rustc_mir_build as mir_build;
-use rustc_parse::{parse_crate_from_file, parse_crate_from_source_str};
+use rustc_parse::{parse_crate_from_file, parse_crate_from_source_str, validate_attr};
 use rustc_passes::{self, hir_stats, layout_test};
 use rustc_plugin_impl as plugin;
 use rustc_query_impl::{OnDiskCache, Queries as TcxQueries};
@@ -33,8 +33,8 @@ use rustc_session::config::{CrateType, Input, OutputFilenames, OutputType, PpMod
 use rustc_session::lint;
 use rustc_session::output::{filename_for_input, filename_for_metadata};
 use rustc_session::search_paths::PathKind;
-use rustc_session::Session;
-use rustc_span::symbol::{Ident, Symbol};
+use rustc_session::{Limit, Session};
+use rustc_span::symbol::{sym, Ident, Symbol};
 use rustc_span::FileName;
 use rustc_trait_selection::traits;
 use rustc_typeck as typeck;
@@ -311,8 +311,7 @@ pub fn configure_and_expand(
 
         // Create the config for macro expansion
         let features = sess.features_untracked();
-        let recursion_limit =
-            rustc_middle::middle::limits::get_recursion_limit(&krate.attrs, &sess);
+        let recursion_limit = get_recursion_limit(&krate.attrs, &sess);
         let cfg = rustc_expand::expand::ExpansionConfig {
             features: Some(&features),
             recursion_limit,
@@ -1070,3 +1069,24 @@ pub fn start_codegen<'tcx>(
 
     codegen
 }
+
+fn get_recursion_limit(krate_attrs: &[ast::Attribute], sess: &Session) -> Limit {
+    if let Some(attr) = krate_attrs
+        .iter()
+        .find(|attr| attr.has_name(sym::recursion_limit) && attr.value_str().is_none())
+    {
+        // This is here mainly to check for using a macro, such as
+        // #![recursion_limit = foo!()]. That is not supported since that
+        // would require expanding this while in the middle of expansion,
+        // which needs to know the limit before expanding. Otherwise,
+        // validation would normally be caught in AstValidator (via
+        // `check_builtin_attribute`), but by the time that runs the macro
+        // is expanded, and it doesn't give an error.
+        validate_attr::emit_fatal_malformed_builtin_attribute(
+            &sess.parse_sess,
+            attr,
+            sym::recursion_limit,
+        );
+    }
+    rustc_middle::middle::limits::get_recursion_limit(krate_attrs, sess)
+}

compiler/rustc_interface/src/util.rs

@@ -10,6 +10,7 @@ use rustc_errors::registry::Registry;
 use rustc_metadata::dynamic_lib::DynamicLibrary;
 #[cfg(parallel_compiler)]
 use rustc_middle::ty::tls;
+use rustc_parse::validate_attr;
 #[cfg(parallel_compiler)]
 use rustc_query_impl::QueryCtxt;
 use rustc_resolve::{self, Resolver};
@@ -475,7 +476,7 @@ pub fn get_codegen_sysroot(
 }
 
 pub(crate) fn check_attr_crate_type(
-    _sess: &Session,
+    sess: &Session,
     attrs: &[ast::Attribute],
     lint_buffer: &mut LintBuffer,
 ) {
@@ -515,6 +516,19 @@ pub(crate) fn check_attr_crate_type(
                         );
                     }
                 }
+            } else {
+                // This is here mainly to check for using a macro, such as
+                // #![crate_type = foo!()]. That is not supported since the
+                // crate type needs to be known very early in compilation long
+                // before expansion. Otherwise, validation would normally be
+                // caught in AstValidator (via `check_builtin_attribute`), but
+                // by the time that runs the macro is expanded, and it doesn't
+                // give an error.
+                validate_attr::emit_fatal_malformed_builtin_attribute(
+                    &sess.parse_sess,
+                    a,
+                    sym::crate_type,
+                );
             }
         }
     }

compiler/rustc_parse/src/validate_attr.rs

@@ -4,7 +4,7 @@ use crate::parse_in;
 
 use rustc_ast::tokenstream::{DelimSpan, TokenTree};
 use rustc_ast::{self as ast, Attribute, MacArgs, MacDelimiter, MetaItem, MetaItemKind};
-use rustc_errors::{Applicability, PResult};
+use rustc_errors::{Applicability, FatalError, PResult};
 use rustc_feature::{AttributeTemplate, BUILTIN_ATTRIBUTE_MAP};
 use rustc_session::lint::builtin::ILL_FORMED_ATTRIBUTE_INPUT;
 use rustc_session::parse::ParseSess;
@@ -91,27 +91,39 @@ pub fn check_builtin_attribute(
     // Some special attributes like `cfg` must be checked
     // before the generic check, so we skip them here.
     let should_skip = |name| name == sym::cfg;
-    // Some of previously accepted forms were used in practice,
-    // report them as warnings for now.
-    let should_warn = |name| {
-        name == sym::doc
-            || name == sym::ignore
-            || name == sym::inline
-            || name == sym::link
-            || name == sym::test
-            || name == sym::bench
-    };
 
     match parse_meta(sess, attr) {
         Ok(meta) => {
             if !should_skip(name) && !is_attr_template_compatible(&template, &meta.kind) {
+                emit_malformed_attribute(sess, attr, name, template);
+            }
+        }
+        Err(mut err) => {
+            err.emit();
+        }
+    }
+}
+
+fn emit_malformed_attribute(
+    sess: &ParseSess,
+    attr: &Attribute,
+    name: Symbol,
+    template: AttributeTemplate,
+) {
+    // Some of previously accepted forms were used in practice,
+    // report them as warnings for now.
+    let should_warn = |name| {
+        matches!(name, sym::doc | sym::ignore | sym::inline | sym::link | sym::test | sym::bench)
+    };
+
     let error_msg = format!("malformed `{}` attribute input", name);
     let mut msg = "attribute must be of the form ".to_owned();
     let mut suggestions = vec![];
     let mut first = true;
+    let inner = if attr.style == ast::AttrStyle::Inner { "!" } else { "" };
     if template.word {
         first = false;
-                    let code = format!("#[{}]", name);
+        let code = format!("#{}[{}]", inner, name);
         msg.push_str(&format!("`{}`", &code));
         suggestions.push(code);
     }
@@ -120,7 +132,7 @@ pub fn check_builtin_attribute(
             msg.push_str(" or ");
         }
         first = false;
-                    let code = format!("#[{}({})]", name, descr);
+        let code = format!("#{}[{}({})]", inner, name, descr);
         msg.push_str(&format!("`{}`", &code));
         suggestions.push(code);
     }
@@ -128,22 +140,17 @@ pub fn check_builtin_attribute(
         if !first {
             msg.push_str(" or ");
         }
-                    let code = format!("#[{} = \"{}\"]", name, descr);
+        let code = format!("#{}[{} = \"{}\"]", inner, name, descr);
         msg.push_str(&format!("`{}`", &code));
         suggestions.push(code);
     }
     if should_warn(name) {
-                    sess.buffer_lint(
-                        &ILL_FORMED_ATTRIBUTE_INPUT,
-                        meta.span,
-                        ast::CRATE_NODE_ID,
-                        &msg,
-                    );
+        sess.buffer_lint(&ILL_FORMED_ATTRIBUTE_INPUT, attr.span, ast::CRATE_NODE_ID, &msg);
     } else {
         sess.span_diagnostic
-                        .struct_span_err(meta.span, &error_msg)
+            .struct_span_err(attr.span, &error_msg)
             .span_suggestions(
-                            meta.span,
+                attr.span,
                 if suggestions.len() == 1 {
                     "must be of the form"
                 } else {
@@ -154,10 +161,16 @@ pub fn check_builtin_attribute(
             )
             .emit();
     }
-            }
-        }
-        Err(mut err) => {
-            err.emit();
-        }
-    }
+}
+
+pub fn emit_fatal_malformed_builtin_attribute(
+    sess: &ParseSess,
+    attr: &Attribute,
+    name: Symbol,
+) -> ! {
+    let template = BUILTIN_ATTRIBUTE_MAP.get(&name).expect("builtin attr defined").2;
+    emit_malformed_attribute(sess, attr, name, template);
+    // This is fatal, otherwise it will likely cause a cascade of other errors
+    // (and an error here is expected to be very rare).
+    FatalError.raise()
 }

src/test/ui/attributes/register-attr-tool-fail.stderr

@@ -30,13 +30,13 @@ error: malformed `register_attr` attribute input
   --> $DIR/register-attr-tool-fail.rs:4:1
    |
 LL | #![register_attr]
-   | ^^^^^^^^^^^^^^^^^ help: must be of the form: `#[register_attr(attr1, attr2, ...)]`
+   | ^^^^^^^^^^^^^^^^^ help: must be of the form: `#![register_attr(attr1, attr2, ...)]`
 
 error: malformed `register_tool` attribute input
   --> $DIR/register-attr-tool-fail.rs:5:1
    |
 LL | #![register_tool]
-   | ^^^^^^^^^^^^^^^^^ help: must be of the form: `#[register_tool(tool1, tool2, ...)]`
+   | ^^^^^^^^^^^^^^^^^ help: must be of the form: `#![register_tool(tool1, tool2, ...)]`
 
 error: aborting due to 6 previous errors
 

src/test/ui/gated-bad-feature.stderr

@@ -20,13 +20,13 @@ error: malformed `feature` attribute input
   --> $DIR/gated-bad-feature.rs:5:1
    |
 LL | #![feature]
-   | ^^^^^^^^^^^ help: must be of the form: `#[feature(name1, name1, ...)]`
+   | ^^^^^^^^^^^ help: must be of the form: `#![feature(name1, name1, ...)]`
 
 error: malformed `feature` attribute input
   --> $DIR/gated-bad-feature.rs:6:1
    |
 LL | #![feature = "foo"]
-   | ^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#[feature(name1, name1, ...)]`
+   | ^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#![feature(name1, name1, ...)]`
 
 error: aborting due to 5 previous errors
 

src/test/ui/invalid/invalid-crate-type-macro.rs

@@ -0,0 +1,7 @@
+#![crate_type = foo!()] //~ ERROR malformed `crate_type` attribute
+
+macro_rules! foo {
+    () => {"rlib"};
+}
+
+fn main() {}

src/test/ui/invalid/invalid-crate-type-macro.stderr

@@ -0,0 +1,8 @@
+error: malformed `crate_type` attribute input
+  --> $DIR/invalid-crate-type-macro.rs:1:1
+   |
+LL | #![crate_type = foo!()]
+   | ^^^^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#![crate_type = "bin|lib|..."]`
+
+error: aborting due to previous error
+

src/test/ui/invalid_crate_type_syntax.stderr

@@ -2,7 +2,7 @@ error: malformed `crate_type` attribute input
   --> $DIR/invalid_crate_type_syntax.rs:2:1
    |
 LL | #![crate_type(lib)]
-   | ^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#[crate_type = "bin|lib|..."]`
+   | ^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#![crate_type = "bin|lib|..."]`
 
 error: aborting due to previous error
 

src/test/ui/lint/lint-malformed.stderr

@@ -14,7 +14,7 @@ error: malformed `deny` attribute input
   --> $DIR/lint-malformed.rs:1:1
    |
 LL | #![deny = "foo"]
-   | ^^^^^^^^^^^^^^^^ help: must be of the form: `#[deny(lint1, lint2, ..., /*opt*/ reason = "...")]`
+   | ^^^^^^^^^^^^^^^^ help: must be of the form: `#![deny(lint1, lint2, ..., /*opt*/ reason = "...")]`
 
 error[E0452]: malformed lint attribute input
   --> $DIR/lint-malformed.rs:2:10

src/test/ui/malformed/malformed-plugin-1.stderr

@@ -2,7 +2,7 @@ error: malformed `plugin` attribute input
   --> $DIR/malformed-plugin-1.rs:2:1
    |
 LL | #![plugin]
-   | ^^^^^^^^^^ help: must be of the form: `#[plugin(name)]`
+   | ^^^^^^^^^^ help: must be of the form: `#![plugin(name)]`
 
 warning: use of deprecated attribute `plugin`: compiler plugins are deprecated. See https://github.com/rust-lang/rust/pull/64675
   --> $DIR/malformed-plugin-1.rs:2:1

src/test/ui/malformed/malformed-plugin-2.stderr

@@ -2,7 +2,7 @@ error: malformed `plugin` attribute input
   --> $DIR/malformed-plugin-2.rs:2:1
    |
 LL | #![plugin="bleh"]
-   | ^^^^^^^^^^^^^^^^^ help: must be of the form: `#[plugin(name)]`
+   | ^^^^^^^^^^^^^^^^^ help: must be of the form: `#![plugin(name)]`
 
 warning: use of deprecated attribute `plugin`: compiler plugins are deprecated. See https://github.com/rust-lang/rust/pull/64675
   --> $DIR/malformed-plugin-2.rs:2:1

src/test/ui/modules/path-invalid-form.rs

@@ -0,0 +1,4 @@
+#[path = 123]  //~ ERROR malformed `path` attribute
+mod foo;
+
+fn main() {}

src/test/ui/modules/path-invalid-form.stderr

@@ -0,0 +1,8 @@
+error: malformed `path` attribute input
+  --> $DIR/path-invalid-form.rs:1:1
+   |
+LL | #[path = 123]
+   | ^^^^^^^^^^^^^ help: must be of the form: `#[path = "file"]`
+
+error: aborting due to previous error
+

src/test/ui/modules/path-macro.rs

@@ -0,0 +1,8 @@
+macro_rules! foo {
+    () => {"bar.rs"};
+}
+
+#[path = foo!()] //~ ERROR malformed `path` attribute
+mod abc;
+
+fn main() {}

src/test/ui/modules/path-macro.stderr

@@ -0,0 +1,8 @@
+error: malformed `path` attribute input
+  --> $DIR/path-macro.rs:5:1
+   |
+LL | #[path = foo!()]
+   | ^^^^^^^^^^^^^^^^ help: must be of the form: `#[path = "file"]`
+
+error: aborting due to previous error
+

src/test/ui/no_crate_type.stderr

@@ -2,7 +2,7 @@ error: malformed `crate_type` attribute input
   --> $DIR/no_crate_type.rs:2:1
    |
 LL | #![crate_type]
-   | ^^^^^^^^^^^^^^ help: must be of the form: `#[crate_type = "bin|lib|..."]`
+   | ^^^^^^^^^^^^^^ help: must be of the form: `#![crate_type = "bin|lib|..."]`
 
 error: aborting due to previous error
 

src/test/ui/recursion_limit/invalid_digit_type.rs

@@ -0,0 +1,3 @@
+#![recursion_limit = 123] //~ ERROR malformed `recursion_limit` attribute
+
+fn main() {}

src/test/ui/recursion_limit/invalid_digit_type.stderr

@@ -0,0 +1,8 @@
+error: malformed `recursion_limit` attribute input
+  --> $DIR/invalid_digit_type.rs:1:1
+   |
+LL | #![recursion_limit = 123]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#![recursion_limit = "N"]`
+
+error: aborting due to previous error
+

src/test/ui/recursion_limit/invalid_macro.rs

@@ -0,0 +1,7 @@
+#![recursion_limit = foo!()] //~ ERROR malformed `recursion_limit` attribute
+
+macro_rules! foo {
+    () => {"128"};
+}
+
+fn main() {}

src/test/ui/recursion_limit/invalid_macro.stderr

@@ -0,0 +1,8 @@
+error: malformed `recursion_limit` attribute input
+  --> $DIR/invalid_macro.rs:1:1
+   |
+LL | #![recursion_limit = foo!()]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: must be of the form: `#![recursion_limit = "N"]`
+
+error: aborting due to previous error
+