Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate builtin attributes for macro args. #88680

Merged
merged 2 commits into from
Sep 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion compiler/rustc_expand/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down
30 changes: 25 additions & 5 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
16 changes: 15 additions & 1 deletion compiler/rustc_interface/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
) {
Expand Down Expand Up @@ -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
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
// 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,
);
}
}
}
Expand Down
133 changes: 73 additions & 60 deletions compiler/rustc_parse/src/validate_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -91,73 +91,86 @@ 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) {
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;
if template.word {
first = false;
let code = format!("#[{}]", name);
msg.push_str(&format!("`{}`", &code));
suggestions.push(code);
}
if let Some(descr) = template.list {
if !first {
msg.push_str(" or ");
}
first = false;
let code = format!("#[{}({})]", name, descr);
msg.push_str(&format!("`{}`", &code));
suggestions.push(code);
}
if let Some(descr) = template.name_value_str {
if !first {
msg.push_str(" or ");
}
let code = format!("#[{} = \"{}\"]", 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,
);
} else {
sess.span_diagnostic
.struct_span_err(meta.span, &error_msg)
.span_suggestions(
meta.span,
if suggestions.len() == 1 {
"must be of the form"
} else {
"the following are the possible correct uses"
},
suggestions.into_iter(),
Applicability::HasPlaceholders,
)
.emit();
}
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!("#{}[{}]", inner, name);
msg.push_str(&format!("`{}`", &code));
suggestions.push(code);
}
if let Some(descr) = template.list {
if !first {
msg.push_str(" or ");
}
first = false;
let code = format!("#{}[{}({})]", inner, name, descr);
msg.push_str(&format!("`{}`", &code));
suggestions.push(code);
}
if let Some(descr) = template.name_value_str {
if !first {
msg.push_str(" or ");
}
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, attr.span, ast::CRATE_NODE_ID, &msg);
} else {
sess.span_diagnostic
.struct_span_err(attr.span, &error_msg)
.span_suggestions(
attr.span,
if suggestions.len() == 1 {
"must be of the form"
} else {
"the following are the possible correct uses"
},
suggestions.into_iter(),
Applicability::HasPlaceholders,
)
.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()
}
4 changes: 2 additions & 2 deletions src/test/ui/attributes/register-attr-tool-fail.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

4 changes: 2 additions & 2 deletions src/test/ui/gated-bad-feature.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 7 additions & 0 deletions src/test/ui/invalid/invalid-crate-type-macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#![crate_type = foo!()] //~ ERROR malformed `crate_type` attribute

macro_rules! foo {
() => {"rlib"};
}

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/invalid/invalid-crate-type-macro.stderr
Original file line number Diff line number Diff line change
@@ -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

2 changes: 1 addition & 1 deletion src/test/ui/invalid_crate_type_syntax.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

2 changes: 1 addition & 1 deletion src/test/ui/lint/lint-malformed.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/malformed/malformed-plugin-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/malformed/malformed-plugin-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/modules/path-invalid-form.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#[path = 123] //~ ERROR malformed `path` attribute
mod foo;

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/modules/path-invalid-form.stderr
Original file line number Diff line number Diff line change
@@ -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

8 changes: 8 additions & 0 deletions src/test/ui/modules/path-macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
macro_rules! foo {
() => {"bar.rs"};
}

#[path = foo!()] //~ ERROR malformed `path` attribute
mod abc;

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/modules/path-macro.stderr
Original file line number Diff line number Diff line change
@@ -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

Loading