Make meta-item API compatible with LocalInternedString::get soundness fix

petrochenkov · petrochenkov · commit db74efce6971 · 2019-03-17T17:04:58.000+03:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<F>(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<ReprAttr> {
 
                 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
@@ -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<Ident> {
         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<Symbol> {
@@ -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
diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs

Original file line number	Diff line number	Diff line change
`@@ -65,9 +65,9 @@ impl<'a, 'tcx> LibFeatureCollector<'a, 'tcx> {`
`65`	`65`	`for meta in metas {`
`66`	`66`	`if let Some(mi) = meta.meta_item() {`
`67`	`67`	// Find the `feature = ".."` meta-item.
`68`		`- match (mi.ident_str(), mi.value_str()) {`
`69`		`- (Some("feature"), val) => feature = val,`
`70`		`- (Some("since"), val) => since = val,`
	`68`	`+ match (mi.name_or_empty().get(), mi.value_str()) {`
	`69`	`+ ("feature", val) => feature = val,`
	`70`	`+ ("since", val) => since = val,`
`71`	`71`	`_ => {}`
`72`	`72`	`}`
`73`	`73`	`}`