From db74efce69711fcee03d3338afcbca67c27ceee8 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 17 Mar 2019 14:17:47 +0300 Subject: [PATCH] Make meta-item API compatible with `LocalInternedString::get` soundness fix --- src/librustc/hir/check_attr.rs | 12 +---- src/librustc/lint/levels.rs | 2 +- src/librustc/lint/mod.rs | 7 +-- src/librustc/middle/lib_features.rs | 6 +-- src/librustc/middle/stability.rs | 11 ++--- src/librustc/traits/on_unimplemented.rs | 4 +- .../persist/dirty_clean.rs | 4 +- src/librustc_lint/builtin.rs | 3 +- src/librustc_lint/unused.rs | 6 +-- src/librustc_passes/layout_test.rs | 5 +- src/librustc_plugin/load.rs | 12 ++--- src/librustdoc/core.rs | 12 ++--- src/librustdoc/html/render.rs | 14 +++--- src/libsyntax/attr/builtin.rs | 46 +++++++++---------- src/libsyntax/attr/mod.rs | 14 +++--- src/libsyntax/feature_gate.rs | 16 +++---- src/libsyntax_ext/deriving/generic/mod.rs | 7 +-- 17 files changed, 84 insertions(+), 97 deletions(-) diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs index 8602d159ba960..4c527f80d0f5d 100644 --- a/src/librustc/hir/check_attr.rs +++ b/src/librustc/hir/check_attr.rs @@ -177,16 +177,8 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { let mut is_transparent = false; for hint in &hints { - let name = if let Some(name) = hint.ident_str() { - name - } else { - // Invalid repr hint like repr(42). We don't check for unrecognized hints here - // (libsyntax does that), so just ignore it. - continue; - }; - - let (article, allowed_targets) = match name { - "C" | "align" => { + let (article, allowed_targets) = match hint.name_or_empty().get() { + name @ "C" | name @ "align" => { is_c |= name == "C"; if target != Target::Struct && target != Target::Union && diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs index 19e899ceb421f..2e5bd8add0cdc 100644 --- a/src/librustc/lint/levels.rs +++ b/src/librustc/lint/levels.rs @@ -194,7 +194,7 @@ impl<'a> LintLevelsBuilder<'a> { struct_span_err!(sess, span, E0452, "malformed lint attribute") }; for attr in attrs { - let level = match attr.ident_str().and_then(|name| Level::from_str(name)) { + let level = match Level::from_str(&attr.name_or_empty()) { None => continue, Some(lvl) => lvl, }; diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index b1ff66eb64fa6..f92e33582c4c3 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -723,12 +723,7 @@ pub fn struct_lint_level<'a>(sess: &'a Session, pub fn maybe_lint_level_root(tcx: TyCtxt<'_, '_, '_>, id: hir::HirId) -> bool { let attrs = tcx.hir().attrs_by_hir_id(id); - for attr in attrs { - if attr.ident_str().and_then(Level::from_str).is_some() { - return true; - } - } - false + attrs.iter().any(|attr| Level::from_str(&attr.name_or_empty()).is_some()) } fn lint_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, cnum: CrateNum) diff --git a/src/librustc/middle/lib_features.rs b/src/librustc/middle/lib_features.rs index 237b00db575a9..ee8dd9e58b5ff 100644 --- a/src/librustc/middle/lib_features.rs +++ b/src/librustc/middle/lib_features.rs @@ -65,9 +65,9 @@ impl<'a, 'tcx> LibFeatureCollector<'a, 'tcx> { for meta in metas { if let Some(mi) = meta.meta_item() { // Find the `feature = ".."` meta-item. - match (mi.ident_str(), mi.value_str()) { - (Some("feature"), val) => feature = val, - (Some("since"), val) => since = val, + match (mi.name_or_empty().get(), mi.value_str()) { + ("feature", val) => feature = val, + ("since", val) => since = val, _ => {} } } diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 2664d6eaa2857..e63665574b940 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -194,12 +194,11 @@ impl<'a, 'tcx: 'a> Annotator<'a, 'tcx> { } else { // Emit errors for non-staged-api crates. for attr in attrs { - if let Some(tag) = attr.ident_str() { - if tag == "unstable" || tag == "stable" || tag == "rustc_deprecated" { - attr::mark_used(attr); - self.tcx.sess.span_err(attr.span, "stability attributes may not be used \ - outside of the standard library"); - } + let name = attr.name_or_empty(); + if ["unstable", "stable", "rustc_deprecated"].contains(&name.get()) { + attr::mark_used(attr); + self.tcx.sess.span_err(attr.span, "stability attributes may not be used \ + outside of the standard library"); } } diff --git a/src/librustc/traits/on_unimplemented.rs b/src/librustc/traits/on_unimplemented.rs index d0acaf674ae74..2b286ee1b97fb 100644 --- a/src/librustc/traits/on_unimplemented.rs +++ b/src/librustc/traits/on_unimplemented.rs @@ -177,9 +177,9 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedDirective { for command in self.subcommands.iter().chain(Some(self)).rev() { if let Some(ref condition) = command.condition { if !attr::eval_condition(condition, &tcx.sess.parse_sess, &mut |c| { - c.ident_str().map_or(false, |name| { + c.ident().map_or(false, |ident| { options.contains(&( - name.to_string(), + ident.to_string(), c.value_str().map(|s| s.as_str().to_string()) )) }) diff --git a/src/librustc_incremental/persist/dirty_clean.rs b/src/librustc_incremental/persist/dirty_clean.rs index 5551cf6b3b604..0a43624c687bb 100644 --- a/src/librustc_incremental/persist/dirty_clean.rs +++ b/src/librustc_incremental/persist/dirty_clean.rs @@ -576,8 +576,8 @@ fn expect_associated_value(tcx: TyCtxt<'_, '_, '_>, item: &NestedMetaItem) -> as if let Some(value) = item.value_str() { value } else { - let msg = if let Some(name) = item.ident_str() { - format!("associated value expected for `{}`", name) + let msg = if let Some(ident) = item.ident() { + format!("associated value expected for `{}`", ident) } else { "expected an associated value".to_string() }; diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 80c5eeeeac345..37aa8b0db8be5 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -759,8 +759,9 @@ impl LintPass for DeprecatedAttr { impl EarlyLintPass for DeprecatedAttr { fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &ast::Attribute) { + let name = attr.name_or_empty(); for &&(n, _, _, ref g) in &self.depr_attrs { - if attr.ident_str() == Some(n) { + if name == n { if let &AttributeGate::Gated(Stability::Deprecated(link, suggestion), ref name, ref reason, diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 648cae30da6c8..ebbd27798419b 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -267,21 +267,21 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedAttributes { } } - let name = attr.ident_str(); + let name = attr.name_or_empty(); if !attr::is_used(attr) { debug!("Emitting warning for: {:?}", attr); cx.span_lint(UNUSED_ATTRIBUTES, attr.span, "unused attribute"); // Is it a builtin attribute that must be used at the crate level? let known_crate = BUILTIN_ATTRIBUTES.iter() .find(|&&(builtin, ty, ..)| { - name == Some(builtin) && ty == AttributeType::CrateLevel + name == builtin && ty == AttributeType::CrateLevel }) .is_some(); // Has a plugin registered this attribute as one that must be used at // the crate level? let plugin_crate = plugin_attributes.iter() - .find(|&&(ref x, t)| name == Some(x) && AttributeType::CrateLevel == t) + .find(|&&(ref x, t)| name == x.as_str() && AttributeType::CrateLevel == t) .is_some(); if known_crate || plugin_crate { let msg = match attr.style { diff --git a/src/librustc_passes/layout_test.rs b/src/librustc_passes/layout_test.rs index 373bcf7f0e2f3..6940f8f442ee9 100644 --- a/src/librustc_passes/layout_test.rs +++ b/src/librustc_passes/layout_test.rs @@ -53,8 +53,7 @@ impl<'a, 'tcx> VarianceTest<'a, 'tcx> { // The `..` are the names of fields to dump. let meta_items = attr.meta_item_list().unwrap_or_default(); for meta_item in meta_items { - let name = meta_item.ident_str().unwrap_or(""); - match name { + match meta_item.name_or_empty().get() { "abi" => { self.tcx .sess @@ -84,7 +83,7 @@ impl<'a, 'tcx> VarianceTest<'a, 'tcx> { ); } - _ => { + name => { self.tcx.sess.span_err( meta_item.span(), &format!("unrecognized field name `{}`", name), diff --git a/src/librustc_plugin/load.rs b/src/librustc_plugin/load.rs index bd11e0ce8023a..8b86bddb29f4e 100644 --- a/src/librustc_plugin/load.rs +++ b/src/librustc_plugin/load.rs @@ -56,12 +56,12 @@ pub fn load_plugins(sess: &Session, for plugin in plugins { // plugins must have a name and can't be key = value - match plugin.ident_str() { - Some(name) if !plugin.is_value_str() => { - let args = plugin.meta_item_list().map(ToOwned::to_owned); - loader.load_plugin(plugin.span(), name, args.unwrap_or_default()); - }, - _ => call_malformed_plugin_attribute(sess, attr.span), + let name = plugin.name_or_empty(); + if !name.is_empty() && !plugin.is_value_str() { + let args = plugin.meta_item_list().map(ToOwned::to_owned); + loader.load_plugin(plugin.span(), &name, args.unwrap_or_default()); + } else { + call_malformed_plugin_attribute(sess, attr.span); } } } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index dca6458c701c5..7a64773054234 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -521,21 +521,21 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt for attr in krate.module.as_ref().unwrap().attrs.lists("doc") { let diag = ctxt.sess().diagnostic(); - let name = attr.ident_str(); + let name = attr.name_or_empty(); if attr.is_word() { - if name == Some("no_default_passes") { + if name == "no_default_passes" { report_deprecated_attr("no_default_passes", diag); if default_passes == passes::DefaultPassOption::Default { default_passes = passes::DefaultPassOption::None; } } } else if let Some(value) = attr.value_str() { - let sink = match name { - Some("passes") => { + let sink = match name.get() { + "passes" => { report_deprecated_attr("passes = \"...\"", diag); &mut manual_passes }, - Some("plugins") => { + "plugins" => { report_deprecated_attr("plugins = \"...\"", diag); eprintln!("WARNING: #![doc(plugins = \"...\")] no longer functions; \ see CVE-2018-1000622"); @@ -548,7 +548,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt } } - if attr.is_word() && name == Some("document_private_items") { + if attr.is_word() && name == "document_private_items" { if default_passes == passes::DefaultPassOption::Default { default_passes = passes::DefaultPassOption::Private; } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index b3a816d17f56c..d6850ed3bb972 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -562,23 +562,23 @@ pub fn run(mut krate: clean::Crate, // going to emit HTML if let Some(attrs) = krate.module.as_ref().map(|m| &m.attrs) { for attr in attrs.lists("doc") { - match (attr.ident_str(), attr.value_str()) { - (Some("html_favicon_url"), Some(s)) => { + match (attr.name_or_empty().get(), attr.value_str()) { + ("html_favicon_url", Some(s)) => { scx.layout.favicon = s.to_string(); } - (Some("html_logo_url"), Some(s)) => { + ("html_logo_url", Some(s)) => { scx.layout.logo = s.to_string(); } - (Some("html_playground_url"), Some(s)) => { + ("html_playground_url", Some(s)) => { markdown::PLAYGROUND.with(|slot| { let name = krate.name.clone(); *slot.borrow_mut() = Some((Some(name), s.to_string())); }); } - (Some("issue_tracker_base_url"), Some(s)) => { + ("issue_tracker_base_url", Some(s)) => { scx.issue_tracker_base_url = Some(s.to_string()); } - (Some("html_no_source"), None) if attr.is_word() => { + ("html_no_source", None) if attr.is_word() => { scx.include_sources = false; } _ => {} @@ -3749,7 +3749,7 @@ fn render_attributes(w: &mut fmt::Formatter<'_>, it: &clean::Item) -> fmt::Resul let mut attrs = String::new(); for attr in &it.attrs.other_attrs { - if !attr.ident_str().map_or(false, |name| ATTRIBUTE_WHITELIST.contains(&name)) { + if !ATTRIBUTE_WHITELIST.contains(&attr.name_or_empty().get()) { continue; } if let Some(s) = render_attribute(&attr.meta().unwrap()) { diff --git a/src/libsyntax/attr/builtin.rs b/src/libsyntax/attr/builtin.rs index cdfb83c6e56c8..74c952b076cd7 100644 --- a/src/libsyntax/attr/builtin.rs +++ b/src/libsyntax/attr/builtin.rs @@ -222,9 +222,9 @@ fn find_stability_generic<'a, I>(sess: &ParseSess, )+ for meta in metas { if let Some(mi) = meta.meta_item() { - match mi.ident_str() { + match mi.name_or_empty().get() { $( - Some(stringify!($name)) + stringify!($name) => if !get(mi, &mut $name) { continue 'outer }, )+ _ => { @@ -252,7 +252,7 @@ fn find_stability_generic<'a, I>(sess: &ParseSess, } } - match meta.ident_str().expect("not a stability level") { + match meta.name_or_empty().get() { "rustc_deprecated" => { if rustc_depr.is_some() { span_err!(diagnostic, item_sp, E0540, @@ -306,10 +306,10 @@ fn find_stability_generic<'a, I>(sess: &ParseSess, let mut issue = None; for meta in metas { if let Some(mi) = meta.meta_item() { - match mi.ident_str() { - Some("feature") => if !get(mi, &mut feature) { continue 'outer }, - Some("reason") => if !get(mi, &mut reason) { continue 'outer }, - Some("issue") => if !get(mi, &mut issue) { continue 'outer }, + match mi.name_or_empty().get() { + "feature" => if !get(mi, &mut feature) { continue 'outer }, + "reason" => if !get(mi, &mut reason) { continue 'outer }, + "issue" => if !get(mi, &mut issue) { continue 'outer }, _ => { handle_errors( sess, @@ -377,10 +377,10 @@ fn find_stability_generic<'a, I>(sess: &ParseSess, for meta in metas { match meta { NestedMetaItem::MetaItem(mi) => { - match mi.ident_str() { - Some("feature") => + match mi.name_or_empty().get() { + "feature" => if !get(mi, &mut feature) { continue 'outer }, - Some("since") => + "since" => if !get(mi, &mut since) { continue 'outer }, _ => { handle_errors( @@ -532,14 +532,14 @@ pub fn eval_condition(cfg: &ast::MetaItem, sess: &ParseSess, eval: &mut F) // The unwraps below may look dangerous, but we've already asserted // that they won't fail with the loop above. - match cfg.ident_str() { - Some("any") => mis.iter().any(|mi| { + match cfg.name_or_empty().get() { + "any" => mis.iter().any(|mi| { eval_condition(mi.meta_item().unwrap(), sess, eval) }), - Some("all") => mis.iter().all(|mi| { + "all" => mis.iter().all(|mi| { eval_condition(mi.meta_item().unwrap(), sess, eval) }), - Some("not") => { + "not" => { if mis.len() != 1 { span_err!(sess.span_diagnostic, cfg.span, E0536, "expected 1 cfg-pattern"); return false; @@ -635,9 +635,9 @@ fn find_deprecation_generic<'a, I>(sess: &ParseSess, for meta in list { match meta { NestedMetaItem::MetaItem(mi) => { - match mi.ident_str() { - Some("since") => if !get(mi, &mut since) { continue 'outer }, - Some("note") => if !get(mi, &mut note) { continue 'outer }, + match mi.name_or_empty().get() { + "since" => if !get(mi, &mut since) { continue 'outer }, + "note" => if !get(mi, &mut note) { continue 'outer }, _ => { handle_errors( sess, @@ -729,12 +729,12 @@ pub fn find_repr_attrs(sess: &ParseSess, attr: &Attribute) -> Vec { let mut recognised = false; if item.is_word() { - let hint = match item.ident_str() { - Some("C") => Some(ReprC), - Some("packed") => Some(ReprPacked(1)), - Some("simd") => Some(ReprSimd), - Some("transparent") => Some(ReprTransparent), - name => name.and_then(|name| int_type_of_word(name)).map(ReprInt), + let hint = match item.name_or_empty().get() { + "C" => Some(ReprC), + "packed" => Some(ReprPacked(1)), + "simd" => Some(ReprSimd), + "transparent" => Some(ReprTransparent), + name => int_type_of_word(name).map(ReprInt), }; if let Some(h) = hint { diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs index 1a8faa43fff7c..c0bd5c79b1dd1 100644 --- a/src/libsyntax/attr/mod.rs +++ b/src/libsyntax/attr/mod.rs @@ -22,7 +22,7 @@ use crate::parse::parser::Parser; use crate::parse::{self, ParseSess, PResult}; use crate::parse::token::{self, Token}; use crate::ptr::P; -use crate::symbol::Symbol; +use crate::symbol::{keywords, LocalInternedString, Symbol}; use crate::ThinVec; use crate::tokenstream::{TokenStream, TokenTree, DelimSpan}; use crate::GLOBALS; @@ -89,8 +89,8 @@ impl NestedMetaItem { pub fn ident(&self) -> Option { self.meta_item().and_then(|meta_item| meta_item.ident()) } - pub fn ident_str(&self) -> Option<&str> { - self.ident().map(|name| name.as_str().get()) + pub fn name_or_empty(&self) -> LocalInternedString { + self.ident().unwrap_or(keywords::Invalid.ident()).name.as_str() } /// Gets the string value if self is a MetaItem and the MetaItem is a @@ -167,8 +167,8 @@ impl Attribute { None } } - pub fn ident_str(&self) -> Option<&str> { - self.ident().map(|name| name.as_str().get()) + pub fn name_or_empty(&self) -> LocalInternedString { + self.ident().unwrap_or(keywords::Invalid.ident()).name.as_str() } pub fn value_str(&self) -> Option { @@ -205,8 +205,8 @@ impl MetaItem { None } } - pub fn ident_str(&self) -> Option<&str> { - self.ident().map(|name| name.as_str().get()) + pub fn name_or_empty(&self) -> LocalInternedString { + self.ident().unwrap_or(keywords::Invalid.ident()).name.as_str() } // #[attribute(name = "value")] diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 9beaabb0cd58b..d1d0d79a705da 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -1341,9 +1341,9 @@ macro_rules! gate_feature { impl<'a> Context<'a> { fn check_attribute(&self, attr: &ast::Attribute, is_macro: bool) { debug!("check_attribute(attr = {:?})", attr); - let name = attr.ident_str(); + let name = attr.name_or_empty(); for &(n, ty, _template, ref gateage) in BUILTIN_ATTRIBUTES { - if name == Some(n) { + if name == n { if let Gated(_, name, desc, ref has_feature) = *gateage { if !attr.span.allows_unstable(name) { gate_feature_fn!( @@ -1373,7 +1373,7 @@ impl<'a> Context<'a> { } } if !attr::is_known(attr) { - if name.map_or(false, |name| name.starts_with("rustc_")) { + if name.starts_with("rustc_") { let msg = "unless otherwise specified, attributes with the prefix `rustc_` \ are reserved for internal compiler diagnostics"; gate_feature!(self, rustc_attrs, attr.span, msg); @@ -2054,12 +2054,12 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute], }; for mi in list { - let name = match mi.ident_str() { - Some(name) if mi.is_word() => name, - _ => continue, - }; + if !mi.is_word() { + continue; + } - if incomplete_features.iter().any(|f| *f == name) { + let name = mi.name_or_empty(); + if incomplete_features.iter().any(|f| name == *f) { span_handler.struct_span_warn( mi.span(), &format!( diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index 2bb98c1bf625c..c790e6ba7f17c 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -463,9 +463,10 @@ impl<'a> TraitDef<'a> { let mut attrs = newitem.attrs.clone(); attrs.extend(item.attrs .iter() - .filter(|a| a.ident_str().map_or(false, |name| { - ["allow", "warn", "deny", "forbid", "stable", "unstable"].contains(&name) - })) + .filter(|a| { + ["allow", "warn", "deny", "forbid", "stable", "unstable"] + .contains(&a.name_or_empty().get()) + }) .cloned()); push(Annotatable::Item(P(ast::Item { attrs: attrs, ..(*newitem).clone() }))) }