Skip to content
Open
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
46 changes: 45 additions & 1 deletion compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1202,6 +1202,48 @@ impl<'a> AstValidator<'a> {
self.visit_vis(vis);
self.visit_ident(ident);
}

// Check EII implementation attributes against an allowlist.
fn check_eii_impl_attrs(&self, attrs: &[Attribute], eii_impls: &[EiiImpl]) {

@chenyukang chenyukang Jul 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the checking to here in ast_validation, since macro expansion finished,
and leaves the final attributes, similar with
https://github.com/chenyukang/rust/blob/30cc6b46e8bb20318b05443cb80f1e50119ba971/compiler/rustc_ast_passes/src/ast_validation.rs#L522-L532

the old place where depends on HIR, HIR converts some attributes into variants without a uniform path or span.

View changes since the review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the current location is the best we can currently do.
Integrating this into the attribute parsing system somehow would be ideal, but I don't see a way to do this. Doing this after attribute parsing (like in the HIR) is not desirable since the spans are gone then.

if eii_impls.is_empty() {
return;
}

let allowed_attrs: &[Symbol] = &[
sym::allow,
sym::warn,
sym::deny,
sym::forbid,
sym::expect,
sym::doc,
sym::inline,
sym::cold,
sym::optimize,
sym::coverage,
sym::sanitize,
sym::must_use,
sym::deprecated,
];

for attr in attrs {
let AttrKind::Normal(normal) = &attr.kind else {
continue;
};
if attr.has_any_name(allowed_attrs) {
continue;
}

let attr_name = pprust::path_to_string(&normal.item.path);
for eii_impl in eii_impls {
self.dcx().emit_err(diagnostics::EiiImplAttributeNotSupported {
attr_span: attr.span,
attr_name: &attr_name,
eii_span: eii_impl.span,
eii_name: pprust::path_to_string(&eii_impl.eii_macro_path),
});
}
}
}
}

/// Checks that generic parameters are in the correct order,
Expand Down Expand Up @@ -1391,6 +1433,7 @@ impl Visitor<'_> for AstValidator<'_> {
for EiiImpl { eii_macro_path, .. } in eii_impls {
self.visit_path(eii_macro_path);
}
self.check_eii_impl_attrs(&item.attrs, eii_impls);

let is_intrinsic = item.attrs.iter().any(|a| a.has_name(sym::rustc_intrinsic));
if body.is_none() && !is_intrinsic && !self.is_sdylib_interface {
Expand Down Expand Up @@ -1566,8 +1609,9 @@ impl Visitor<'_> for AstValidator<'_> {

visit::walk_item(self, item);
}
ItemKind::Static(StaticItem { expr, safety, .. }) => {
ItemKind::Static(StaticItem { expr, safety, eii_impls, .. }) => {
self.check_item_safety(item.span, *safety);
self.check_eii_impl_attrs(&item.attrs, eii_impls);
if matches!(safety, Safety::Unsafe(_)) {
self.dcx().emit_err(diagnostics::UnsafeStatic { span: item.span });
}
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_ast_passes/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,17 @@ pub(crate) struct FnParamForbiddenAttr {
pub span: Span,
}

#[derive(Diagnostic)]
#[diag("`#[{$eii_name}]` is not allowed to have `#[{$attr_name}]`")]
pub(crate) struct EiiImplAttributeNotSupported<'a> {
#[primary_span]
pub attr_span: Span,
pub attr_name: &'a str,
pub eii_name: String,
#[label("`#[{$eii_name}]` is not allowed to have `#[{$attr_name}]`")]
pub eii_span: Span,
}

#[derive(Diagnostic)]
#[diag("`self` parameter is only allowed in associated functions")]
#[note("associated functions are those in `impl` or `trait` definitions")]
Expand Down
16 changes: 0 additions & 16 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -791,22 +791,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
sig_span: sig.span,
});
}

if let Some(impls) = find_attr!(attrs, EiiImpls(impls) => impls) {
let sig = self.tcx.hir_node(hir_id).fn_sig().unwrap();
for i in impls {
let name = match i.resolution {
EiiImplResolution::Macro(def_id) => self.tcx.item_name(def_id),
EiiImplResolution::Known(def_id) => self.tcx.item_name(def_id),
EiiImplResolution::Error(_eg) => continue,
};
self.dcx().emit_err(diagnostics::EiiWithTrackCaller {
attr_span,
name,
sig_span: sig.span,
});
}
}
}
_ => {}
}
Expand Down
10 changes: 0 additions & 10 deletions compiler/rustc_passes/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1077,16 +1077,6 @@ pub(crate) struct EiiImplRequiresUnsafeSuggestion {
pub right: Span,
}

#[derive(Diagnostic)]
#[diag("`#[{$name}]` is not allowed to have `#[track_caller]`")]
pub(crate) struct EiiWithTrackCaller {
#[primary_span]
pub attr_span: Span,
pub name: Symbol,
#[label("`#[{$name}]` is not allowed to have `#[track_caller]`")]
pub sig_span: Span,
}

#[derive(Diagnostic)]
#[diag("`#[{$name}]` {$kind} required, but not found")]
pub(crate) struct EiiWithoutImpl {
Expand Down
102 changes: 102 additions & 0 deletions tests/ui/eii/implementation-attribute-allowlist-issue-159015.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// EII implementations only accept attributes from a conservative allowlist.
// Regression test for #159015

//@ edition: 2024
//@ needs-asm-support

#![feature(coverage_attribute)]
#![feature(extern_item_impls)]
#![feature(optimize_attribute)]
#![feature(sanitize)]

#[eii]
fn allowed();

/// Sugared and explicit documentation attributes are both allowed.
#[allowed]
#[allow(dead_code)]
#[warn(unreachable_code)]
#[deny(unused_mut)]
#[forbid(unsafe_code)]
#[expect(unused_variables)]
#[cfg(all())]
#[doc = "An allowed EII implementation."]
#[cold]
#[optimize(none)]
#[coverage(off)]
#[sanitize(address = "off")]
#[must_use]
#[deprecated]
fn allowed_impl() {
let unused = ();
}

#[eii]
fn allowed_inline();

#[allowed_inline]
#[allow(unused_attributes)]
#[cfg_attr(all(), inline)]
fn allowed_inline_impl() {}

#[eii]
fn foo();

#[foo]
#[unsafe(no_mangle)]
//~^ ERROR `#[foo]` is not allowed to have `#[no_mangle]`
fn bar() {}

#[eii]
fn baz();

#[baz]
#[unsafe(export_name = "qux")]
//~^ ERROR `#[baz]` is not allowed to have `#[export_name]`
fn qux() {}

#[eii]
fn quux();

#[quux]
#[unsafe(link_section = "__TEXT,__text")]
//~^ ERROR `#[quux]` is not allowed to have `#[link_section]`
fn corge() {}

#[eii]
fn grault();

#[grault]
#[track_caller]
//~^ ERROR `#[grault]` is not allowed to have `#[track_caller]`
fn garply() {}

#[eii]
extern "C" fn naked_attr();

#[naked_attr]
#[unsafe(naked)]
//~^ ERROR `#[naked_attr]` is not allowed to have `#[naked]`
extern "C" fn naked_attr_impl() {
core::arch::naked_asm!("")
}

#[eii]
fn multiple_invalid_attrs();

#[multiple_invalid_attrs]
#[unsafe(no_mangle)]
//~^ ERROR `#[multiple_invalid_attrs]` is not allowed to have `#[no_mangle]`
#[track_caller]
//~^ ERROR `#[multiple_invalid_attrs]` is not allowed to have `#[track_caller]`
fn multiple_invalid_attrs_impl() {}

#[eii(static_eii)]
static STATIC_EII: u8;

#[static_eii]
#[used]
//~^ ERROR `#[static_eii]` is not allowed to have `#[used]`
static STATIC_EII_IMPL: u8 = 0;

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
error: `#[foo]` is not allowed to have `#[no_mangle]`
--> $DIR/implementation-attribute-allowlist-issue-159015.rs:46:1
|
LL | #[foo]
| ------ `#[foo]` is not allowed to have `#[no_mangle]`
LL | #[unsafe(no_mangle)]
| ^^^^^^^^^^^^^^^^^^^^

error: `#[baz]` is not allowed to have `#[export_name]`
--> $DIR/implementation-attribute-allowlist-issue-159015.rs:54:1
|
LL | #[baz]
| ------ `#[baz]` is not allowed to have `#[export_name]`
LL | #[unsafe(export_name = "qux")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: `#[quux]` is not allowed to have `#[link_section]`
--> $DIR/implementation-attribute-allowlist-issue-159015.rs:62:1
|
LL | #[quux]
| ------- `#[quux]` is not allowed to have `#[link_section]`
LL | #[unsafe(link_section = "__TEXT,__text")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: `#[grault]` is not allowed to have `#[track_caller]`
--> $DIR/implementation-attribute-allowlist-issue-159015.rs:70:1
|
LL | #[grault]
| --------- `#[grault]` is not allowed to have `#[track_caller]`
LL | #[track_caller]
| ^^^^^^^^^^^^^^^

error: `#[naked_attr]` is not allowed to have `#[naked]`
--> $DIR/implementation-attribute-allowlist-issue-159015.rs:78:1
|
LL | #[naked_attr]
| ------------- `#[naked_attr]` is not allowed to have `#[naked]`
LL | #[unsafe(naked)]
| ^^^^^^^^^^^^^^^^

error: `#[multiple_invalid_attrs]` is not allowed to have `#[no_mangle]`
--> $DIR/implementation-attribute-allowlist-issue-159015.rs:88:1
|
LL | #[multiple_invalid_attrs]
| ------------------------- `#[multiple_invalid_attrs]` is not allowed to have `#[no_mangle]`
LL | #[unsafe(no_mangle)]
| ^^^^^^^^^^^^^^^^^^^^

error: `#[multiple_invalid_attrs]` is not allowed to have `#[track_caller]`
--> $DIR/implementation-attribute-allowlist-issue-159015.rs:90:1
|
LL | #[multiple_invalid_attrs]
| ------------------------- `#[multiple_invalid_attrs]` is not allowed to have `#[track_caller]`
...
LL | #[track_caller]
| ^^^^^^^^^^^^^^^

error: `#[static_eii]` is not allowed to have `#[used]`
--> $DIR/implementation-attribute-allowlist-issue-159015.rs:98:1
|
LL | #[static_eii]
| ------------- `#[static_eii]` is not allowed to have `#[used]`
LL | #[used]
| ^^^^^^^

error: aborting due to 8 previous errors

3 changes: 1 addition & 2 deletions tests/ui/eii/track_caller_errors.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ error: `#[decl1]` is not allowed to have `#[track_caller]`
LL | #[track_caller]
| ^^^^^^^^^^^^^^^
LL | #[decl1]
LL | fn impl1(x: u64) {
| ---------------- `#[decl1]` is not allowed to have `#[track_caller]`
| -------- `#[decl1]` is not allowed to have `#[track_caller]`

error: aborting due to 1 previous error

Loading