Rollup merge of #127581 - fmease:fix-crate_name-validation, r=bjorn3

Fix crate name validation

Reject macro calls inside attribute `#![crate_name]` like in `#![crate_name = concat!("na", "me")]`.

Prior to #117584, the result of the expansion (here: `"name"`) would actually be properly picked up by the compiler and used as the crate name. However since #117584 / on master, we extract the "value" (i.e., the *literal* string literal) of the `#![crate_name]` much earlier in the pipeline way before macro expansion and **skip**/**ignore** any `#![crate_name]`s "assigned to" a macro call. See also #122001.

T-lang has ruled to reject `#![crate_name = MACRO!(...)]` outright very similar to other built-in attributes whose value we need early like `#![crate_type]`. See accepted FCP: #122001 (comment).

Note that the check as implemented in this PR is even more "aggressive" compared to the one of `#![crate_type]` by running as early as possible in order to reject `#![crate_name = MACRO!(...)]` even in "non-normal" executions of `rustc`, namely on *print requests* (e.g., `--print=crate-name` and `--print=file-names`). If I were to move the validation step a bit further back close to the `#![crate_type]` one, `--print=crate-name` (etc.) would *not* exit fatally with an error in this kind of situation but happily report an incorrect crate name (i.e., the "crate name" as if `#![crate_name]` didn't exist / deduced from other sources like `--crate-name` or the file name) which would match the behavior on master. Again, see also #122001.

I'm mentioning this explicitly because I'm not sure if it was that clear in the FCP'ed issue. I argue that my current approach is the most reasonable one. I know (from reading the code and from past experiments) that various print requests are still quite broken (mostly lack of validation).

To the best of my knowledge, there's no print request whose output references/contains a crate *type*, so there's no "inherent need" to move `#![crate_type]`'s validation to happen earlier.

---

Fixes #122001.

https://github.com/rust-lang/rust/labels/relnotes: Compatibility. Breaking change.

Author: matthiaskrgr

Diff for: compiler/rustc_driver_impl/src/lib.rs

@@ -667,11 +667,12 @@ fn print_crate_info(
                     return Compilation::Continue;
                 };
                 let t_outputs = rustc_interface::util::build_output_filenames(attrs, sess);
-                let id = rustc_session::output::find_crate_name(sess, attrs);
+                let crate_name = passes::get_crate_name(sess, attrs);
                 let crate_types = collect_crate_types(sess, attrs);
                 for &style in &crate_types {
-                    let fname =
-                        rustc_session::output::filename_for_input(sess, style, id, &t_outputs);
+                    let fname = rustc_session::output::filename_for_input(
+                        sess, style, crate_name, &t_outputs,
+                    );
                     println_info!("{}", fname.as_path().file_name().unwrap().to_string_lossy());
                 }
             }
@@ -680,8 +681,7 @@ fn print_crate_info(
                     // no crate attributes, print out an error and exit
                     return Compilation::Continue;
                 };
-                let id = rustc_session::output::find_crate_name(sess, attrs);
-                println_info!("{id}");
+                println_info!("{}", passes::get_crate_name(sess, attrs));
             }
             Cfg => {
                 let mut cfgs = sess

Diff for: compiler/rustc_expand/src/module.rs

@@ -183,12 +183,12 @@ pub(crate) fn mod_file_path_from_attr(
     let first_path = attrs.iter().find(|at| at.has_name(sym::path))?;
     let Some(path_sym) = first_path.value_str() else {
         // 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
+        // such as `#[path = concat!(...)]`. This isn't 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 during semantic analysis via
+        // `TyCtxt::check_mod_attrs`), 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.psess, first_path, sym::path);
     };

Diff for: compiler/rustc_incremental/src/persist/fs.rs

@@ -117,8 +117,9 @@ use rustc_data_structures::{base_n, flock};
 use rustc_fs_util::{LinkOrCopy, link_or_copy, try_canonicalize};
 use rustc_middle::bug;
 use rustc_session::config::CrateType;
-use rustc_session::output::{collect_crate_types, find_crate_name};
+use rustc_session::output::collect_crate_types;
 use rustc_session::{Session, StableCrateId};
+use rustc_span::Symbol;
 use tracing::debug;
 
 use crate::errors;
@@ -211,7 +212,7 @@ pub fn in_incr_comp_dir(incr_comp_session_dir: &Path, file_name: &str) -> PathBu
 /// The garbage collection will take care of it.
 ///
 /// [`rustc_interface::queries::dep_graph`]: ../../rustc_interface/struct.Queries.html#structfield.dep_graph
-pub(crate) fn prepare_session_directory(sess: &Session) {
+pub(crate) fn prepare_session_directory(sess: &Session, crate_name: Symbol) {
     if sess.opts.incremental.is_none() {
         return;
     }
@@ -221,7 +222,7 @@ pub(crate) fn prepare_session_directory(sess: &Session) {
     debug!("prepare_session_directory");
 
     // {incr-comp-dir}/{crate-name-and-disambiguator}
-    let crate_dir = crate_path(sess);
+    let crate_dir = crate_path(sess, crate_name);
     debug!("crate-dir: {}", crate_dir.display());
     create_dir(sess, &crate_dir, "crate");
 
@@ -594,10 +595,9 @@ fn string_to_timestamp(s: &str) -> Result<SystemTime, &'static str> {
     Ok(UNIX_EPOCH + duration)
 }
 
-fn crate_path(sess: &Session) -> PathBuf {
+fn crate_path(sess: &Session, crate_name: Symbol) -> PathBuf {
     let incr_dir = sess.opts.incremental.as_ref().unwrap().clone();
 
-    let crate_name = find_crate_name(sess, &[]);
     let crate_types = collect_crate_types(sess, &[]);
     let stable_crate_id = StableCrateId::new(
         crate_name,

Diff for: compiler/rustc_incremental/src/persist/load.rs

@@ -11,6 +11,7 @@ use rustc_serialize::Decodable;
 use rustc_serialize::opaque::MemDecoder;
 use rustc_session::Session;
 use rustc_session::config::IncrementalStateAssertion;
+use rustc_span::Symbol;
 use tracing::{debug, warn};
 
 use super::data::*;
@@ -203,9 +204,9 @@ pub fn load_query_result_cache(sess: &Session) -> Option<OnDiskCache> {
 
 /// Setups the dependency graph by loading an existing graph from disk and set up streaming of a
 /// new graph to an incremental session directory.
-pub fn setup_dep_graph(sess: &Session) -> DepGraph {
+pub fn setup_dep_graph(sess: &Session, crate_name: Symbol) -> DepGraph {
     // `load_dep_graph` can only be called after `prepare_session_directory`.
-    prepare_session_directory(sess);
+    prepare_session_directory(sess, crate_name);
 
     let res = sess.opts.build_dep_graph().then(|| load_dep_graph(sess));
 

Diff for: compiler/rustc_interface/messages.ftl

@@ -6,6 +6,10 @@ interface_abi_required_feature_issue = for more information, see issue #116344 <
 interface_cant_emit_mir =
     could not emit MIR: {$error}
 
+interface_crate_name_does_not_match = `--crate-name` and `#[crate_name]` are required to match, but `{$crate_name}` != `{$attr_crate_name}`
+
+interface_crate_name_invalid = crate names cannot start with a `-`, but `{$crate_name}` has a leading hyphen
+
 interface_emoji_identifier =
     identifiers cannot contain emoji: `{$ident}`
 

Diff for: compiler/rustc_interface/src/errors.rs

@@ -4,6 +4,21 @@ use std::path::Path;
 use rustc_macros::Diagnostic;
 use rustc_span::{Span, Symbol};
 
+#[derive(Diagnostic)]
+#[diag(interface_crate_name_does_not_match)]
+pub(crate) struct CrateNameDoesNotMatch {
+    #[primary_span]
+    pub(crate) span: Span,
+    pub(crate) crate_name: Symbol,
+    pub(crate) attr_crate_name: Symbol,
+}
+
+#[derive(Diagnostic)]
+#[diag(interface_crate_name_invalid)]
+pub(crate) struct CrateNameInvalid<'a> {
+    pub(crate) crate_name: &'a str,
+}
+
 #[derive(Diagnostic)]
 #[diag(interface_ferris_identifier)]
 pub struct FerrisIdentifier {

Diff for: compiler/rustc_interface/src/passes.rs

@@ -28,10 +28,12 @@ use rustc_passes::{abi_test, input_stats, layout_test};
 use rustc_resolve::Resolver;
 use rustc_session::config::{CrateType, Input, OutFileName, OutputFilenames, OutputType};
 use rustc_session::cstore::Untracked;
-use rustc_session::output::{collect_crate_types, filename_for_input, find_crate_name};
+use rustc_session::output::{collect_crate_types, filename_for_input};
 use rustc_session::search_paths::PathKind;
 use rustc_session::{Limit, Session};
-use rustc_span::{ErrorGuaranteed, FileName, SourceFileHash, SourceFileHashAlgorithm, Symbol, sym};
+use rustc_span::{
+    ErrorGuaranteed, FileName, SourceFileHash, SourceFileHashAlgorithm, Span, Symbol, sym,
+};
 use rustc_target::spec::PanicStrategy;
 use rustc_trait_selection::traits;
 use tracing::{info, instrument};
@@ -725,8 +727,7 @@ pub fn create_and_enter_global_ctxt<T, F: for<'tcx> FnOnce(TyCtxt<'tcx>) -> T>(
 
     let pre_configured_attrs = rustc_expand::config::pre_configure_attrs(sess, &krate.attrs);
 
-    // parse `#[crate_name]` even if `--crate-name` was passed, to make sure it matches.
-    let crate_name = find_crate_name(sess, &pre_configured_attrs);
+    let crate_name = get_crate_name(sess, &pre_configured_attrs);
     let crate_types = collect_crate_types(sess, &pre_configured_attrs);
     let stable_crate_id = StableCrateId::new(
         crate_name,
@@ -735,7 +736,7 @@ pub fn create_and_enter_global_ctxt<T, F: for<'tcx> FnOnce(TyCtxt<'tcx>) -> T>(
         sess.cfg_version,
     );
     let outputs = util::build_output_filenames(&pre_configured_attrs, sess);
-    let dep_graph = setup_dep_graph(sess);
+    let dep_graph = setup_dep_graph(sess, crate_name);
 
     let cstore =
         FreezeLock::new(Box::new(CStore::new(compiler.codegen_backend.metadata_loader())) as _);
@@ -1080,23 +1081,85 @@ pub(crate) 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())
+/// Compute and validate the crate name.
+pub fn get_crate_name(sess: &Session, krate_attrs: &[ast::Attribute]) -> Symbol {
+    // We validate *all* occurrences of `#![crate_name]`, pick the first find and
+    // if a crate name was passed on the command line via `--crate-name` we enforce
+    // that they match.
+    // We perform the validation step here instead of later to ensure it gets run
+    // in all code paths that require the crate name very early on, namely before
+    // macro expansion.
+
+    let attr_crate_name =
+        validate_and_find_value_str_builtin_attr(sym::crate_name, sess, krate_attrs);
+
+    let validate = |name, span| {
+        rustc_session::output::validate_crate_name(sess, name, span);
+        name
+    };
+
+    if let Some(crate_name) = &sess.opts.crate_name {
+        let crate_name = Symbol::intern(crate_name);
+        if let Some((attr_crate_name, span)) = attr_crate_name
+            && attr_crate_name != crate_name
+        {
+            sess.dcx().emit_err(errors::CrateNameDoesNotMatch {
+                span,
+                crate_name,
+                attr_crate_name,
+            });
+        }
+        return validate(crate_name, None);
+    }
+
+    if let Some((crate_name, span)) = attr_crate_name {
+        return validate(crate_name, Some(span));
+    }
+
+    if let Input::File(ref path) = sess.io.input
+        && let Some(file_stem) = path.file_stem().and_then(|s| s.to_str())
     {
-        // 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.psess,
-            attr,
-            sym::recursion_limit,
-        );
+        if file_stem.starts_with('-') {
+            sess.dcx().emit_err(errors::CrateNameInvalid { crate_name: file_stem });
+        } else {
+            return validate(Symbol::intern(&file_stem.replace('-', "_")), None);
+        }
     }
+
+    sym::rust_out
+}
+
+fn get_recursion_limit(krate_attrs: &[ast::Attribute], sess: &Session) -> Limit {
+    // We don't permit macro calls inside of the attribute (e.g., #![recursion_limit = `expand!()`])
+    // because that would require expanding this while in the middle of expansion, which needs to
+    // know the limit before expanding.
+    let _ = validate_and_find_value_str_builtin_attr(sym::recursion_limit, sess, krate_attrs);
     rustc_middle::middle::limits::get_recursion_limit(krate_attrs, sess)
 }
+
+/// Validate *all* occurrences of the given "[value-str]" built-in attribute and return the first find.
+///
+/// This validator is intended for built-in attributes whose value needs to be known very early
+/// during compilation (namely, before macro expansion) and it mainly exists to reject macro calls
+/// inside of the attributes, such as in `#![name = expand!()]`. Normal attribute validation happens
+/// during semantic analysis via [`TyCtxt::check_mod_attrs`] which happens *after* macro expansion
+/// when such macro calls (here: `expand`) have already been expanded and we can no longer check for
+/// their presence.
+///
+/// [value-str]: ast::Attribute::value_str
+fn validate_and_find_value_str_builtin_attr(
+    name: Symbol,
+    sess: &Session,
+    krate_attrs: &[ast::Attribute],
+) -> Option<(Symbol, Span)> {
+    let mut result = None;
+    // Validate *all* relevant attributes, not just the first occurrence.
+    for attr in ast::attr::filter_by_name(krate_attrs, name) {
+        let Some(value) = attr.value_str() else {
+            validate_attr::emit_fatal_malformed_builtin_attribute(&sess.psess, attr, name)
+        };
+        // Choose the first occurrence as our result.
+        result.get_or_insert((value, attr.span));
+    }
+    result
+}

Diff for: compiler/rustc_interface/src/util.rs

@@ -433,11 +433,11 @@ 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 = 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
+                // caught during semantic analysis via `TyCtxt::check_mod_attrs`,
+                // 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.psess,

Diff for: compiler/rustc_session/messages.ftl

@@ -8,12 +8,8 @@ session_cannot_mix_and_match_sanitizers = `-Zsanitizer={$first}` is incompatible
 session_cli_feature_diagnostic_help =
     add `-Zcrate-attr="feature({$feature})"` to the command-line options to enable
 
-session_crate_name_does_not_match = `--crate-name` and `#[crate_name]` are required to match, but `{$s}` != `{$name}`
-
 session_crate_name_empty = crate name must not be empty
 
-session_crate_name_invalid = crate names cannot start with a `-`, but `{$s}` has a leading hyphen
-
 session_embed_source_insufficient_dwarf_version = `-Zembed-source=y` requires at least `-Z dwarf-version=5` but DWARF version is {$dwarf_version}
 
 session_embed_source_requires_debug_info = `-Zembed-source=y` requires debug information to be enabled
@@ -52,8 +48,8 @@ session_instrumentation_not_supported = {$us} instrumentation is not supported f
 session_int_literal_too_large = integer literal is too large
     .note = value exceeds limit of `{$limit}`
 
-session_invalid_character_in_create_name = invalid character `{$character}` in crate name: `{$crate_name}`
-session_invalid_character_in_create_name_help = you can either pass `--crate-name` on the command line or add `#![crate_name="…"]` to set the crate name
+session_invalid_character_in_crate_name = invalid character {$character} in crate name: `{$crate_name}`
+    .help = you can either pass `--crate-name` on the command line or add `#![crate_name = "…"]` to set the crate name
 
 session_invalid_float_literal_suffix = invalid suffix `{$suffix}` for float literal
     .label = invalid suffix `{$suffix}`

Diff for: compiler/rustc_session/src/errors.rs

@@ -212,21 +212,6 @@ pub(crate) struct FileWriteFail<'a> {
     pub(crate) err: String,
 }
 
-#[derive(Diagnostic)]
-#[diag(session_crate_name_does_not_match)]
-pub(crate) struct CrateNameDoesNotMatch {
-    #[primary_span]
-    pub(crate) span: Span,
-    pub(crate) s: Symbol,
-    pub(crate) name: Symbol,
-}
-
-#[derive(Diagnostic)]
-#[diag(session_crate_name_invalid)]
-pub(crate) struct CrateNameInvalid<'a> {
-    pub(crate) s: &'a str,
-}
-
 #[derive(Diagnostic)]
 #[diag(session_crate_name_empty)]
 pub(crate) struct CrateNameEmpty {
@@ -235,20 +220,14 @@ pub(crate) struct CrateNameEmpty {
 }
 
 #[derive(Diagnostic)]
-#[diag(session_invalid_character_in_create_name)]
+#[diag(session_invalid_character_in_crate_name)]
 pub(crate) struct InvalidCharacterInCrateName {
     #[primary_span]
     pub(crate) span: Option<Span>,
     pub(crate) character: char,
     pub(crate) crate_name: Symbol,
-    #[subdiagnostic]
-    pub(crate) crate_name_help: Option<InvalidCrateNameHelp>,
-}
-
-#[derive(Subdiagnostic)]
-pub(crate) enum InvalidCrateNameHelp {
-    #[help(session_invalid_character_in_create_name_help)]
-    AddCrateName,
+    #[help]
+    pub(crate) help: Option<()>,
 }
 
 #[derive(Subdiagnostic)]