From 75f058dbfd1c02daa191efb6eb53dc91fda8add6 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 17 Sep 2021 13:08:56 -0700 Subject: [PATCH] Check for macros in built-in attributes that don't support them. --- compiler/rustc_expand/src/module.rs | 21 ++++++++++++- compiler/rustc_interface/src/passes.rs | 30 +++++++++++++++---- compiler/rustc_interface/src/util.rs | 16 +++++++++- compiler/rustc_parse/src/validate_attr.rs | 14 ++++++++- .../ui/invalid/invalid-crate-type-macro.rs | 7 +++++ .../invalid/invalid-crate-type-macro.stderr | 8 +++++ src/test/ui/modules/path-invalid-form.rs | 4 +++ src/test/ui/modules/path-invalid-form.stderr | 8 +++++ src/test/ui/modules/path-macro.rs | 8 +++++ src/test/ui/modules/path-macro.stderr | 8 +++++ .../ui/recursion_limit/invalid_digit_type.rs | 3 ++ .../recursion_limit/invalid_digit_type.stderr | 8 +++++ src/test/ui/recursion_limit/invalid_macro.rs | 7 +++++ .../ui/recursion_limit/invalid_macro.stderr | 8 +++++ 14 files changed, 142 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/invalid/invalid-crate-type-macro.rs create mode 100644 src/test/ui/invalid/invalid-crate-type-macro.stderr create mode 100644 src/test/ui/modules/path-invalid-form.rs create mode 100644 src/test/ui/modules/path-invalid-form.stderr create mode 100644 src/test/ui/modules/path-macro.rs create mode 100644 src/test/ui/modules/path-macro.stderr create mode 100644 src/test/ui/recursion_limit/invalid_digit_type.rs create mode 100644 src/test/ui/recursion_limit/invalid_digit_type.stderr create mode 100644 src/test/ui/recursion_limit/invalid_macro.rs create mode 100644 src/test/ui/recursion_limit/invalid_macro.stderr diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 60d653ac8b6f9..1c0b2a9b48761 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/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 { // 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 diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index cb7529b527e8f..0a6d76c99b77a 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/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) +} diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 24975702b875d..a1d1b63c8fafe 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/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, + ); } } } diff --git a/compiler/rustc_parse/src/validate_attr.rs b/compiler/rustc_parse/src/validate_attr.rs index 6e798fc5c2096..2aa20d02c8830 100644 --- a/compiler/rustc_parse/src/validate_attr.rs +++ b/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; @@ -162,3 +162,15 @@ fn emit_malformed_attribute( .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() +} diff --git a/src/test/ui/invalid/invalid-crate-type-macro.rs b/src/test/ui/invalid/invalid-crate-type-macro.rs new file mode 100644 index 0000000000000..9ba5e79ba947a --- /dev/null +++ b/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() {} diff --git a/src/test/ui/invalid/invalid-crate-type-macro.stderr b/src/test/ui/invalid/invalid-crate-type-macro.stderr new file mode 100644 index 0000000000000..c196d4278a277 --- /dev/null +++ b/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 + diff --git a/src/test/ui/modules/path-invalid-form.rs b/src/test/ui/modules/path-invalid-form.rs new file mode 100644 index 0000000000000..713ef4a795d88 --- /dev/null +++ b/src/test/ui/modules/path-invalid-form.rs @@ -0,0 +1,4 @@ +#[path = 123] //~ ERROR malformed `path` attribute +mod foo; + +fn main() {} diff --git a/src/test/ui/modules/path-invalid-form.stderr b/src/test/ui/modules/path-invalid-form.stderr new file mode 100644 index 0000000000000..7e8aa44ef8be6 --- /dev/null +++ b/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 + diff --git a/src/test/ui/modules/path-macro.rs b/src/test/ui/modules/path-macro.rs new file mode 100644 index 0000000000000..ce2d1e2da2659 --- /dev/null +++ b/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() {} diff --git a/src/test/ui/modules/path-macro.stderr b/src/test/ui/modules/path-macro.stderr new file mode 100644 index 0000000000000..9a2e01ea26451 --- /dev/null +++ b/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 + diff --git a/src/test/ui/recursion_limit/invalid_digit_type.rs b/src/test/ui/recursion_limit/invalid_digit_type.rs new file mode 100644 index 0000000000000..e408109b3682c --- /dev/null +++ b/src/test/ui/recursion_limit/invalid_digit_type.rs @@ -0,0 +1,3 @@ +#![recursion_limit = 123] //~ ERROR malformed `recursion_limit` attribute + +fn main() {} diff --git a/src/test/ui/recursion_limit/invalid_digit_type.stderr b/src/test/ui/recursion_limit/invalid_digit_type.stderr new file mode 100644 index 0000000000000..6d1409bb39065 --- /dev/null +++ b/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 + diff --git a/src/test/ui/recursion_limit/invalid_macro.rs b/src/test/ui/recursion_limit/invalid_macro.rs new file mode 100644 index 0000000000000..7db67a8d16284 --- /dev/null +++ b/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() {} diff --git a/src/test/ui/recursion_limit/invalid_macro.stderr b/src/test/ui/recursion_limit/invalid_macro.stderr new file mode 100644 index 0000000000000..0189e99da3714 --- /dev/null +++ b/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 +