Skip to content

Commit 7e15ea0

Browse files
carbotaniumancarbotaniuman
committed
Deny unsafe on more builtin attributes
1 parent fda509e commit 7e15ea0

File tree

11 files changed

+307
-58
lines changed

11 files changed

+307
-58
lines changed

‎compiler/rustc_builtin_macros/src/cfg_accessible.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,13 @@ impl MultiItemModifier for Expander {
4646
) -> ExpandResult<Vec<Annotatable>, Annotatable> {
4747
let template = AttributeTemplate { list: Some("path"), ..Default::default() };
4848
validate_attr::check_builtin_meta_item(
49+
&ecx.ecfg.features,
4950
&ecx.sess.psess,
5051
meta_item,
5152
ast::AttrStyle::Outer,
5253
sym::cfg_accessible,
5354
template,
55+
true,
5456
);
5557

5658
let Some(path) = validate_input(ecx, meta_item) else {

‎compiler/rustc_builtin_macros/src/derive.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,13 @@ impl MultiItemModifier for Expander {
3838
let template =
3939
AttributeTemplate { list: Some("Trait1, Trait2, ..."), ..Default::default() };
4040
validate_attr::check_builtin_meta_item(
41+
features,
4142
&sess.psess,
4243
meta_item,
4344
ast::AttrStyle::Outer,
4445
sym::derive,
4546
template,
47+
true,
4648
);
4749

4850
let mut resolutions = match &meta_item.kind {

‎compiler/rustc_builtin_macros/src/util.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@ pub(crate) fn check_builtin_macro_attribute(ecx: &ExtCtxt<'_>, meta_item: &MetaI
1414
// All the built-in macro attributes are "words" at the moment.
1515
let template = AttributeTemplate { word: true, ..Default::default() };
1616
validate_attr::check_builtin_meta_item(
17+
&ecx.ecfg.features,
1718
&ecx.sess.psess,
1819
meta_item,
1920
AttrStyle::Outer,
2021
name,
2122
template,
23+
true,
2224
);
2325
}
2426

‎compiler/rustc_expand/src/config.rs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use rustc_ast::NodeId;
1212
use rustc_ast::{self as ast, AttrStyle, Attribute, HasAttrs, HasTokens, MetaItem};
1313
use rustc_attr as attr;
1414
use rustc_data_structures::flat_map_in_place::FlatMapInPlace;
15-
use rustc_feature::Features;
15+
use rustc_feature::{AttributeSafety, Features};
1616
use rustc_feature::{ACCEPTED_FEATURES, REMOVED_FEATURES, UNSTABLE_FEATURES};
1717
use rustc_lint_defs::BuiltinLintDiag;
1818
use rustc_parse::validate_attr;
@@ -250,6 +250,13 @@ impl<'a> StripUnconfigured<'a> {
250250
/// is in the original source file. Gives a compiler error if the syntax of
251251
/// the attribute is incorrect.
252252
pub(crate) fn expand_cfg_attr(&self, attr: &Attribute, recursive: bool) -> Vec<Attribute> {
253+
validate_attr::check_attribute_safety(
254+
self.features.unwrap_or(&Features::default()),
255+
&self.sess.psess,
256+
AttributeSafety::Normal,
257+
&attr,
258+
);
259+
253260
let Some((cfg_predicate, expanded_attrs)) =
254261
rustc_parse::parse_cfg_attr(attr, &self.sess.psess)
255262
else {
@@ -372,6 +379,13 @@ impl<'a> StripUnconfigured<'a> {
372379
return (true, None);
373380
}
374381
};
382+
383+
validate_attr::deny_builtin_meta_unsafety(
384+
self.features.unwrap_or(&Features::default()),
385+
&self.sess.psess,
386+
&meta_item,
387+
);
388+
375389
(
376390
parse_cfg(&meta_item, self.sess).map_or(true, |meta_item| {
377391
attr::cfg_matches(meta_item, &self.sess, self.lint_node_id, self.features)

‎compiler/rustc_parse/src/validate_attr.rs‎

Lines changed: 92 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -26,76 +26,35 @@ pub fn check_attr(features: &Features, psess: &ParseSess, attr: &Attribute) {
2626
let attr_info = attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name));
2727
let attr_item = attr.get_normal_item();
2828

29-
let is_unsafe_attr = attr_info.is_some_and(|attr| attr.safety == AttributeSafety::Unsafe);
30-
31-
if features.unsafe_attributes {
32-
if is_unsafe_attr {
33-
if let ast::Safety::Default = attr_item.unsafety {
34-
let path_span = attr_item.path.span;
35-
36-
// If the `attr_item`'s span is not from a macro, then just suggest
37-
// wrapping it in `unsafe(...)`. Otherwise, we suggest putting the
38-
// `unsafe(`, `)` right after and right before the opening and closing
39-
// square bracket respectively.
40-
let diag_span = if attr_item.span().can_be_used_for_suggestions() {
41-
attr_item.span()
42-
} else {
43-
attr.span
44-
.with_lo(attr.span.lo() + BytePos(2))
45-
.with_hi(attr.span.hi() - BytePos(1))
46-
};
47-
48-
if attr.span.at_least_rust_2024() {
49-
psess.dcx().emit_err(errors::UnsafeAttrOutsideUnsafe {
50-
span: path_span,
51-
suggestion: errors::UnsafeAttrOutsideUnsafeSuggestion {
52-
left: diag_span.shrink_to_lo(),
53-
right: diag_span.shrink_to_hi(),
54-
},
55-
});
56-
} else {
57-
psess.buffer_lint(
58-
UNSAFE_ATTR_OUTSIDE_UNSAFE,
59-
path_span,
60-
ast::CRATE_NODE_ID,
61-
BuiltinLintDiag::UnsafeAttrOutsideUnsafe {
62-
attribute_name_span: path_span,
63-
sugg_spans: (diag_span.shrink_to_lo(), diag_span.shrink_to_hi()),
64-
},
65-
);
66-
}
67-
}
68-
} else {
69-
if let Safety::Unsafe(unsafe_span) = attr_item.unsafety {
70-
psess.dcx().emit_err(errors::InvalidAttrUnsafe {
71-
span: unsafe_span,
72-
name: attr_item.path.clone(),
73-
});
74-
}
75-
}
76-
}
29+
// All non-builtin attributes are considered safe
30+
let safety = attr_info.map(|x| x.safety).unwrap_or(AttributeSafety::Normal);
31+
check_attribute_safety(features, psess, safety, attr);
7732

7833
// Check input tokens for built-in and key-value attributes.
7934
match attr_info {
8035
// `rustc_dummy` doesn't have any restrictions specific to built-in attributes.
8136
Some(BuiltinAttribute { name, template, .. }) if *name != sym::rustc_dummy => {
8237
match parse_meta(psess, attr) {
83-
Ok(meta) => check_builtin_meta_item(psess, &meta, attr.style, *name, *template),
38+
// Don't check safety again, we just did that
39+
Ok(meta) => check_builtin_meta_item(
40+
features, psess, &meta, attr.style, *name, *template, false,
41+
),
8442
Err(err) => {
8543
err.emit();
8644
}
8745
}
8846
}
89-
_ if let AttrArgs::Eq(..) = attr_item.args => {
90-
// All key-value attributes are restricted to meta-item syntax.
91-
match parse_meta(psess, attr) {
92-
Ok(_) => {}
93-
Err(err) => {
94-
err.emit();
47+
_ => {
48+
if let AttrArgs::Eq(..) = attr_item.args {
49+
// All key-value attributes are restricted to meta-item syntax.
50+
match parse_meta(psess, attr) {
51+
Ok(_) => {}
52+
Err(err) => {
53+
err.emit();
54+
}
9555
}
9656
}
9757
}
98-
_ => {}
9958
}
10059
}
10160

@@ -198,12 +157,85 @@ fn is_attr_template_compatible(template: &AttributeTemplate, meta: &ast::MetaIte
198157
}
199158
}
200159

160+
pub fn check_attribute_safety(
161+
features: &Features,
162+
psess: &ParseSess,
163+
safety: AttributeSafety,
164+
attr: &Attribute,
165+
) {
166+
if features.unsafe_attributes {
167+
let attr_item = attr.get_normal_item();
168+
169+
if safety == AttributeSafety::Unsafe {
170+
if let ast::Safety::Default = attr_item.unsafety {
171+
let path_span = attr_item.path.span;
172+
173+
// If the `attr_item`'s span is not from a macro, then just suggest
174+
// wrapping it in `unsafe(...)`. Otherwise, we suggest putting the
175+
// `unsafe(`, `)` right after and right before the opening and closing
176+
// square bracket respectively.
177+
let diag_span = if attr_item.span().can_be_used_for_suggestions() {
178+
attr_item.span()
179+
} else {
180+
attr.span
181+
.with_lo(attr.span.lo() + BytePos(2))
182+
.with_hi(attr.span.hi() - BytePos(1))
183+
};
184+
185+
if attr.span.at_least_rust_2024() {
186+
psess.dcx().emit_err(errors::UnsafeAttrOutsideUnsafe {
187+
span: path_span,
188+
suggestion: errors::UnsafeAttrOutsideUnsafeSuggestion {
189+
left: diag_span.shrink_to_lo(),
190+
right: diag_span.shrink_to_hi(),
191+
},
192+
});
193+
} else {
194+
psess.buffer_lint(
195+
UNSAFE_ATTR_OUTSIDE_UNSAFE,
196+
path_span,
197+
ast::CRATE_NODE_ID,
198+
BuiltinLintDiag::UnsafeAttrOutsideUnsafe {
199+
attribute_name_span: path_span,
200+
sugg_spans: (diag_span.shrink_to_lo(), diag_span.shrink_to_hi()),
201+
},
202+
);
203+
}
204+
}
205+
} else {
206+
if let Safety::Unsafe(unsafe_span) = attr_item.unsafety {
207+
psess.dcx().emit_err(errors::InvalidAttrUnsafe {
208+
span: unsafe_span,
209+
name: attr_item.path.clone(),
210+
});
211+
}
212+
}
213+
}
214+
}
215+
216+
// Called by `check_builtin_meta_item` and code that manually denies
217+
// `unsafe(...)` in `cfg`
218+
pub fn deny_builtin_meta_unsafety(features: &Features, psess: &ParseSess, meta: &MetaItem) {
219+
// This only supports denying unsafety right now - making builtin attributes
220+
// support unsafety will requite us to thread the actual `Attribute` through
221+
// for the nice diagnostics.
222+
if features.unsafe_attributes {
223+
if let Safety::Unsafe(unsafe_span) = meta.unsafety {
224+
psess
225+
.dcx()
226+
.emit_err(errors::InvalidAttrUnsafe { span: unsafe_span, name: meta.path.clone() });
227+
}
228+
}
229+
}
230+
201231
pub fn check_builtin_meta_item(
232+
features: &Features,
202233
psess: &ParseSess,
203234
meta: &MetaItem,
204235
style: ast::AttrStyle,
205236
name: Symbol,
206237
template: AttributeTemplate,
238+
deny_unsafety: bool,
207239
) {
208240
// Some special attributes like `cfg` must be checked
209241
// before the generic check, so we skip them here.
@@ -212,6 +244,10 @@ pub fn check_builtin_meta_item(
212244
if !should_skip(name) && !is_attr_template_compatible(&template, &meta.kind) {
213245
emit_malformed_attribute(psess, style, meta.span, name, template);
214246
}
247+
248+
if deny_unsafety {
249+
deny_builtin_meta_unsafety(features, psess, meta);
250+
}
215251
}
216252

217253
fn emit_malformed_attribute(

‎tests/ui/attributes/unsafe/derive-unsafe-attributes.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,7 @@
33
#[derive(unsafe(Debug))] //~ ERROR: traits in `#[derive(...)]` don't accept `unsafe(...)`
44
struct Foo;
55

6+
#[unsafe(derive(Debug))] //~ ERROR: is not an unsafe attribute
7+
struct Bar;
8+
69
fn main() {}

‎tests/ui/attributes/unsafe/derive-unsafe-attributes.stderr‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,13 @@ error: traits in `#[derive(...)]` don't accept `unsafe(...)`
44
LL | #[derive(unsafe(Debug))]
55
| ^^^^^^
66

7-
error: aborting due to 1 previous error
7+
error: `derive` is not an unsafe attribute
8+
--> $DIR/derive-unsafe-attributes.rs:6:3
9+
|
10+
LL | #[unsafe(derive(Debug))]
11+
| ^^^^^^
12+
|
13+
= note: extraneous unsafe is not allowed in attributes
14+
15+
error: aborting due to 2 previous errors
816

‎tests/ui/attributes/unsafe/extraneous-unsafe-attributes.rs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//@ edition: 2024
2+
//@ compile-flags: -Zunstable-options
3+
#![feature(unsafe_attributes)]
4+
5+
#[unsafe(cfg(any()))] //~ ERROR: is not an unsafe attribute
6+
fn a() {}
7+
8+
#[unsafe(cfg_attr(any(), allow(dead_code)))] //~ ERROR: is not an unsafe attribute
9+
fn b() {}
10+
11+
#[unsafe(test)] //~ ERROR: is not an unsafe attribute
12+
fn aa() {}
13+
14+
#[unsafe(ignore = "test")] //~ ERROR: is not an unsafe attribute
15+
fn bb() {}
16+
17+
#[unsafe(should_panic(expected = "test"))] //~ ERROR: is not an unsafe attribute
18+
fn cc() {}
19+
20+
#[unsafe(macro_use)] //~ ERROR: is not an unsafe attribute
21+
mod inner {
22+
#[unsafe(macro_export)] //~ ERROR: is not an unsafe attribute
23+
macro_rules! m {
24+
() => {};
25+
}
26+
}
27+
28+
#[unsafe(used)] //~ ERROR: is not an unsafe attribute
29+
static FOO: usize = 0;
30+
31+
fn main() {}

‎tests/ui/attributes/unsafe/extraneous-unsafe-attributes.stderr‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
error: `cfg` is not an unsafe attribute
2+
--> $DIR/extraneous-unsafe-attributes.rs:5:3
3+
|
4+
LL | #[unsafe(cfg(any()))]
5+
| ^^^^^^
6+
|
7+
= note: extraneous unsafe is not allowed in attributes
8+
9+
error: `cfg_attr` is not an unsafe attribute
10+
--> $DIR/extraneous-unsafe-attributes.rs:8:3
11+
|
12+
LL | #[unsafe(cfg_attr(any(), allow(dead_code)))]
13+
| ^^^^^^
14+
|
15+
= note: extraneous unsafe is not allowed in attributes
16+
17+
error: `test` is not an unsafe attribute
18+
--> $DIR/extraneous-unsafe-attributes.rs:11:3
19+
|
20+
LL | #[unsafe(test)]
21+
| ^^^^^^
22+
|
23+
= note: extraneous unsafe is not allowed in attributes
24+
25+
error: `ignore` is not an unsafe attribute
26+
--> $DIR/extraneous-unsafe-attributes.rs:14:3
27+
|
28+
LL | #[unsafe(ignore = "test")]
29+
| ^^^^^^
30+
|
31+
= note: extraneous unsafe is not allowed in attributes
32+
33+
error: `should_panic` is not an unsafe attribute
34+
--> $DIR/extraneous-unsafe-attributes.rs:17:3
35+
|
36+
LL | #[unsafe(should_panic(expected = "test"))]
37+
| ^^^^^^
38+
|
39+
= note: extraneous unsafe is not allowed in attributes
40+
41+
error: `macro_use` is not an unsafe attribute
42+
--> $DIR/extraneous-unsafe-attributes.rs:20:3
43+
|
44+
LL | #[unsafe(macro_use)]
45+
| ^^^^^^
46+
|
47+
= note: extraneous unsafe is not allowed in attributes
48+
49+
error: `macro_export` is not an unsafe attribute
50+
--> $DIR/extraneous-unsafe-attributes.rs:22:7
51+
|
52+
LL | #[unsafe(macro_export)]
53+
| ^^^^^^
54+
|
55+
= note: extraneous unsafe is not allowed in attributes
56+
57+
error: `used` is not an unsafe attribute
58+
--> $DIR/extraneous-unsafe-attributes.rs:28:3
59+
|
60+
LL | #[unsafe(used)]
61+
| ^^^^^^
62+
|
63+
= note: extraneous unsafe is not allowed in attributes
64+
65+
error: aborting due to 8 previous errors
66+

0 commit comments

Comments
 (0)