Skip to content

Commit 7f18720

Browse files
oli-obkoli-obk
committed
Taint attributes when validating them causes errors
1 parent 2ad5514 commit 7f18720

File tree

11 files changed

+132
-111
lines changed

11 files changed

+132
-111
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1883,9 +1883,14 @@ pub enum LitKind {
18831883

18841884
impl LitKind {
18851885
pub fn str(&self) -> Option<Symbol> {
1886+
self.try_str().ok()?
1887+
}
1888+
1889+
pub fn try_str(&self) -> Result<Option<Symbol>, ErrorGuaranteed> {
18861890
match *self {
1887-
LitKind::Str(s, _) => Some(s),
1888-
_ => None,
1891+
LitKind::Str(s, _) => Ok(Some(s)),
1892+
LitKind::Err(guar) => Err(guar),
1893+
_ => Ok(None),
18891894
}
18901895
}
18911896

@@ -2827,6 +2832,15 @@ pub enum AttrKind {
28272832
DocComment(CommentKind, Symbol),
28282833
}
28292834

2835+
impl AttrKind {
2836+
pub fn normal_mut(&mut self) -> &mut AttrItem {
2837+
match self {
2838+
AttrKind::Normal(normal) => &mut normal.item,
2839+
AttrKind::DocComment(..) => panic!("unexpected doc comment"),
2840+
}
2841+
}
2842+
}
2843+
28302844
#[derive(Clone, Encodable, Decodable, Debug)]
28312845
pub struct NormalAttr {
28322846
pub item: AttrItem,

‎compiler/rustc_ast/src/attr/mod.rs

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use crate::util::comments;
1414
use crate::util::literal::escape_string_symbol;
1515
use rustc_index::bit_set::GrowableBitSet;
1616
use rustc_span::symbol::{sym, Ident, Symbol};
17-
use rustc_span::Span;
17+
use rustc_span::{ErrorGuaranteed, Span};
1818
use smallvec::{smallvec, SmallVec};
1919
use std::iter;
2020
use std::sync::atomic::{AtomicU32, Ordering};
@@ -144,9 +144,13 @@ impl Attribute {
144144
}
145145

146146
pub fn value_str(&self) -> Option<Symbol> {
147+
self.try_value_str().ok()?
148+
}
149+
150+
pub fn try_value_str(&self) -> Result<Option<Symbol>, ErrorGuaranteed> {
147151
match &self.kind {
148-
AttrKind::Normal(normal) => normal.item.value_str(),
149-
AttrKind::DocComment(..) => None,
152+
AttrKind::Normal(normal) => normal.item.try_value_str(),
153+
AttrKind::DocComment(..) => Ok(None),
150154
}
151155
}
152156

@@ -236,9 +240,13 @@ impl AttrItem {
236240
}
237241

238242
fn value_str(&self) -> Option<Symbol> {
243+
self.try_value_str().ok()?
244+
}
245+
246+
fn try_value_str(&self) -> Result<Option<Symbol>, ErrorGuaranteed> {
239247
match &self.args {
240248
AttrArgs::Eq(_, args) => args.value_str(),
241-
AttrArgs::Delimited(_) | AttrArgs::Empty => None,
249+
AttrArgs::Delimited(_) | AttrArgs::Empty => Ok(None),
242250
}
243251
}
244252

@@ -257,15 +265,17 @@ impl AttrItem {
257265
}
258266

259267
impl AttrArgsEq {
260-
fn value_str(&self) -> Option<Symbol> {
268+
fn value_str(&self) -> Result<Option<Symbol>, ErrorGuaranteed> {
261269
match self {
262270
AttrArgsEq::Ast(expr) => match expr.kind {
263-
ExprKind::Lit(token_lit) => {
264-
LitKind::from_token_lit(token_lit).ok().and_then(|lit| lit.str())
265-
}
266-
_ => None,
271+
ExprKind::Lit(token_lit) => LitKind::from_token_lit(token_lit)
272+
.ok()
273+
.and_then(|lit| lit.try_str().transpose())
274+
.transpose(),
275+
ExprKind::Err(guar) => Err(guar),
276+
_ => Ok(None),
267277
},
268-
AttrArgsEq::Hir(lit) => lit.kind.str(),
278+
AttrArgsEq::Hir(lit) => lit.kind.try_str(),
269279
}
270280
}
271281
}

‎compiler/rustc_ast_lowering/src/lib.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -949,14 +949,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
949949
&& let Ok(lit) = MetaItemLit::from_token_lit(token_lit, expr.span)
950950
{
951951
lit
952-
} else {
953-
let guar = self.dcx().has_errors().unwrap();
952+
} else if let ExprKind::Err(guar) = expr.kind {
954953
MetaItemLit {
955954
symbol: kw::Empty,
956955
suffix: None,
957956
kind: LitKind::Err(guar),
958957
span: DUMMY_SP,
959958
}
959+
} else {
960+
span_bug!(*eq_span, "eq attrs can only be literals, but got {expr:#?}")
960961
};
961962
AttrArgs::Eq(*eq_span, AttrArgsEq::Hir(lit))
962963
}

‎compiler/rustc_expand/src/config.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute], crate_name: Symbol) -
123123
features
124124
}
125125

126-
pub fn pre_configure_attrs(sess: &Session, attrs: &[Attribute]) -> ast::AttrVec {
126+
pub fn pre_configure_attrs(sess: &Session, attrs: &mut [Attribute]) -> ast::AttrVec {
127127
let strip_unconfigured = StripUnconfigured {
128128
sess,
129129
features: None,
@@ -133,7 +133,9 @@ pub fn pre_configure_attrs(sess: &Session, attrs: &[Attribute]) -> ast::AttrVec
133133
attrs
134134
.iter()
135135
.flat_map(|attr| strip_unconfigured.process_cfg_attr(attr))
136-
.take_while(|attr| !is_cfg(attr) || strip_unconfigured.cfg_true(attr).0)
136+
.map_while(|mut attr| {
137+
(!is_cfg(&attr) || strip_unconfigured.cfg_true(&mut attr).0).then_some(attr)
138+
})
137139
.collect()
138140
}
139141

@@ -150,7 +152,9 @@ macro_rules! configure {
150152
impl<'a> StripUnconfigured<'a> {
151153
pub fn configure<T: HasAttrs + HasTokens>(&self, mut node: T) -> Option<T> {
152154
self.process_cfg_attrs(&mut node);
153-
self.in_cfg(node.attrs()).then(|| {
155+
let mut in_cfg = false;
156+
node.visit_attrs(|attrs| in_cfg = self.in_cfg(attrs));
157+
in_cfg.then(|| {
154158
self.try_configure_tokens(&mut node);
155159
node
156160
})
@@ -189,7 +193,7 @@ impl<'a> StripUnconfigured<'a> {
189193
AttrTokenTree::AttrsTarget(mut target) => {
190194
target.attrs.flat_map_in_place(|attr| self.process_cfg_attr(&attr));
191195

192-
if self.in_cfg(&target.attrs) {
196+
if self.in_cfg(&mut target.attrs) {
193197
target.tokens = LazyAttrTokenStream::new(
194198
self.configure_tokens(&target.tokens.to_attr_token_stream()),
195199
);
@@ -363,11 +367,11 @@ impl<'a> StripUnconfigured<'a> {
363367
}
364368

365369
/// Determines if a node with the given attributes should be included in this configuration.
366-
fn in_cfg(&self, attrs: &[Attribute]) -> bool {
367-
attrs.iter().all(|attr| !is_cfg(attr) || self.cfg_true(attr).0)
370+
fn in_cfg(&self, attrs: &mut [Attribute]) -> bool {
371+
attrs.iter_mut().all(|attr| !is_cfg(attr) || self.cfg_true(attr).0)
368372
}
369373

370-
pub(crate) fn cfg_true(&self, attr: &Attribute) -> (bool, Option<MetaItem>) {
374+
pub(crate) fn cfg_true(&self, attr: &mut Attribute) -> (bool, Option<MetaItem>) {
371375
let meta_item = match validate_attr::parse_meta(&self.sess.psess, attr) {
372376
Ok(meta_item) => meta_item,
373377
Err(_guar) => {

‎compiler/rustc_expand/src/expand.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ use rustc_span::symbol::{sym, Ident};
3737
use rustc_span::{ErrorGuaranteed, FileName, LocalExpnId, Span};
3838

3939
use smallvec::SmallVec;
40-
use std::ops::Deref;
40+
use std::ops::{Deref, DerefMut};
4141
use std::path::PathBuf;
4242
use std::rc::Rc;
4343
use std::{iter, mem};
@@ -699,7 +699,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
699699
}
700700
_ => unreachable!(),
701701
},
702-
InvocationKind::Attr { attr, pos, mut item, derives } => match ext {
702+
InvocationKind::Attr { mut attr, pos, mut item, derives } => match ext {
703703
SyntaxExtensionKind::Attr(expander) => {
704704
self.gate_proc_macro_input(&item);
705705
self.gate_proc_macro_attr_item(span, &item);
@@ -742,7 +742,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
742742
}
743743
}
744744
SyntaxExtensionKind::LegacyAttr(expander) => {
745-
match validate_attr::parse_meta(&self.cx.sess.psess, &attr) {
745+
match validate_attr::parse_meta(&self.cx.sess.psess, &mut attr) {
746746
Ok(meta) => {
747747
let items = match expander.expand(self.cx, span, &meta, item, false) {
748748
ExpandResult::Ready(items) => items,
@@ -1079,7 +1079,7 @@ enum AddSemicolon {
10791079
/// of functionality used by `InvocationCollector`.
10801080
trait InvocationCollectorNode: HasAttrs + HasNodeId + Sized {
10811081
type OutputTy = SmallVec<[Self; 1]>;
1082-
type AttrsTy: Deref<Target = [ast::Attribute]> = ast::AttrVec;
1082+
type AttrsTy: Deref<Target = [ast::Attribute]> + DerefMut = ast::AttrVec;
10831083
type ItemKind = ItemKind;
10841084
const KIND: AstFragmentKind;
10851085
fn to_annotatable(self) -> Annotatable;
@@ -1873,9 +1873,9 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
18731873
// Detect use of feature-gated or invalid attributes on macro invocations
18741874
// since they will not be detected after macro expansion.
18751875
#[allow(rustc::untranslatable_diagnostic)] // FIXME: make this translatable
1876-
fn check_attributes(&self, attrs: &[ast::Attribute], call: &ast::MacCall) {
1876+
fn check_attributes(&self, attrs: &mut [ast::Attribute], call: &ast::MacCall) {
18771877
let features = self.cx.ecfg.features;
1878-
let mut attrs = attrs.iter().peekable();
1878+
let mut attrs = attrs.iter_mut().peekable();
18791879
let mut span: Option<Span> = None;
18801880
while let Some(attr) = attrs.next() {
18811881
rustc_ast_passes::feature_gate::check_attribute(attr, self.cx.sess, features);
@@ -1918,10 +1918,10 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
19181918
fn expand_cfg_true(
19191919
&mut self,
19201920
node: &mut impl HasAttrs,
1921-
attr: ast::Attribute,
1921+
mut attr: ast::Attribute,
19221922
pos: usize,
19231923
) -> (bool, Option<ast::MetaItem>) {
1924-
let (res, meta_item) = self.cfg().cfg_true(&attr);
1924+
let (res, meta_item) = self.cfg().cfg_true(&mut attr);
19251925
if res {
19261926
// FIXME: `cfg(TRUE)` attributes do not currently remove themselves during expansion,
19271927
// and some tools like rustdoc and clippy rely on that. Find a way to remove them
@@ -1978,8 +1978,8 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
19781978
}
19791979
},
19801980
None if node.is_mac_call() => {
1981-
let (mac, attrs, add_semicolon) = node.take_mac_call();
1982-
self.check_attributes(&attrs, &mac);
1981+
let (mac, mut attrs, add_semicolon) = node.take_mac_call();
1982+
self.check_attributes(&mut attrs, &mac);
19831983
let mut res = self.collect_bang(mac, Node::KIND).make_ast::<Node>();
19841984
Node::post_flat_map_node_collect_bang(&mut res, add_semicolon);
19851985
res
@@ -2058,8 +2058,8 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
20582058
None if node.is_mac_call() => {
20592059
visit_clobber(node, |node| {
20602060
// Do not clobber unless it's actually a macro (uncommon case).
2061-
let (mac, attrs, _) = node.take_mac_call();
2062-
self.check_attributes(&attrs, &mac);
2061+
let (mac, mut attrs, _) = node.take_mac_call();
2062+
self.check_attributes(&mut attrs, &mac);
20632063
self.collect_bang(mac, Node::KIND).make_ast::<Node>()
20642064
})
20652065
}

‎compiler/rustc_expand/src/module.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::errors::{
44
};
55
use rustc_ast::ptr::P;
66
use rustc_ast::{token, AttrVec, Attribute, Inline, Item, ModSpans};
7-
use rustc_errors::{Diag, ErrorGuaranteed};
7+
use rustc_errors::{Diag, ErrorGuaranteed, FatalError};
88
use rustc_parse::validate_attr;
99
use rustc_parse::{new_parser_from_file, unwrap_or_emit_fatal};
1010
use rustc_session::parse::ParseSess;
@@ -177,7 +177,7 @@ fn mod_file_path_from_attr(
177177
) -> Option<PathBuf> {
178178
// Extract path string from first `#[path = "path_string"]` attribute.
179179
let first_path = attrs.iter().find(|at| at.has_name(sym::path))?;
180-
let Some(path_sym) = first_path.value_str() else {
180+
let Some(path_sym) = first_path.try_value_str().unwrap_or_else(|_| FatalError.raise()) else {
181181
// This check is here mainly to catch attempting to use a macro,
182182
// such as #[path = concat!(...)]. This isn't currently supported
183183
// because otherwise the InvocationCollector would need to defer

‎compiler/rustc_interface/src/passes.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use rustc_codegen_ssa::traits::CodegenBackend;
88
use rustc_data_structures::parallel;
99
use rustc_data_structures::steal::Steal;
1010
use rustc_data_structures::sync::{AppendOnlyIndexVec, FreezeLock, Lrc, OnceLock, WorkerLocal};
11+
use rustc_errors::FatalError;
1112
use rustc_expand::base::{ExtCtxt, LintStoreExpand};
1213
use rustc_feature::Features;
1314
use rustc_fs_util::try_canonicalize;
@@ -658,7 +659,7 @@ pub(crate) fn create_global_ctxt<'tcx>(
658659
&sess.opts.unstable_opts.crate_attr,
659660
);
660661

661-
let pre_configured_attrs = rustc_expand::config::pre_configure_attrs(sess, &krate.attrs);
662+
let pre_configured_attrs = rustc_expand::config::pre_configure_attrs(sess, &mut krate.attrs);
662663

663664
// parse `#[crate_name]` even if `--crate-name` was passed, to make sure it matches.
664665
let crate_name = find_crate_name(sess, &pre_configured_attrs);
@@ -1045,10 +1046,10 @@ pub(crate) fn start_codegen<'tcx>(
10451046
}
10461047

10471048
fn get_recursion_limit(krate_attrs: &[ast::Attribute], sess: &Session) -> Limit {
1048-
if let Some(attr) = krate_attrs
1049-
.iter()
1050-
.find(|attr| attr.has_name(sym::recursion_limit) && attr.value_str().is_none())
1051-
{
1049+
if let Some(attr) = krate_attrs.iter().find(|attr| {
1050+
attr.has_name(sym::recursion_limit)
1051+
&& attr.try_value_str().unwrap_or_else(|_| FatalError.raise()).is_none()
1052+
}) {
10521053
// This is here mainly to check for using a macro, such as
10531054
// #![recursion_limit = foo!()]. That is not supported since that
10541055
// would require expanding this while in the middle of expansion,

‎compiler/rustc_interface/src/util.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use rustc_ast as ast;
33
use rustc_codegen_ssa::traits::CodegenBackend;
44
#[cfg(parallel_compiler)]
55
use rustc_data_structures::sync;
6+
use rustc_errors::FatalError;
67
use rustc_metadata::{load_symbol_from_dylib, DylibError};
78
use rustc_middle::ty::CurrentGcx;
89
use rustc_parse::validate_attr;
@@ -392,7 +393,7 @@ pub(crate) fn check_attr_crate_type(
392393
// Unconditionally collect crate types from attributes to make them used
393394
for a in attrs.iter() {
394395
if a.has_name(sym::crate_type) {
395-
if let Some(n) = a.value_str() {
396+
if let Some(n) = a.try_value_str().unwrap_or_else(|_| FatalError.raise()) {
396397
if categorize_crate_type(n).is_some() {
397398
return;
398399
}

0 commit comments

Comments
 (0)