From 05b7cc83707026de18dc49276db04a599781bbc2 Mon Sep 17 00:00:00 2001 From: inquisitivecrystal <22333129+inquisitivecrystal@users.noreply.github.com> Date: Tue, 24 Jan 2023 02:19:04 -0800 Subject: [PATCH 1/3] Move FFI attribute validation to `check_attr` --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 50 ++----------------- .../locales/en-US/passes.ftl | 12 +++++ compiler/rustc_passes/src/check_attr.rs | 35 +++++++++++++ compiler/rustc_passes/src/errors.rs | 28 +++++++++++ 4 files changed, 78 insertions(+), 47 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index b0fa7745667..9ecd95f424f 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -85,55 +85,11 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: DefId) -> CodegenFnAttrs { } else if attr.has_name(sym::rustc_allocator) { codegen_fn_attrs.flags |= CodegenFnAttrFlags::ALLOCATOR; } else if attr.has_name(sym::ffi_returns_twice) { - if tcx.is_foreign_item(did) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_RETURNS_TWICE; - } else { - // `#[ffi_returns_twice]` is only allowed `extern fn`s. - struct_span_err!( - tcx.sess, - attr.span, - E0724, - "`#[ffi_returns_twice]` may only be used on foreign functions" - ) - .emit(); - } + codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_RETURNS_TWICE; } else if attr.has_name(sym::ffi_pure) { - if tcx.is_foreign_item(did) { - if attrs.iter().any(|a| a.has_name(sym::ffi_const)) { - // `#[ffi_const]` functions cannot be `#[ffi_pure]` - struct_span_err!( - tcx.sess, - attr.span, - E0757, - "`#[ffi_const]` function cannot be `#[ffi_pure]`" - ) - .emit(); - } else { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_PURE; - } - } else { - // `#[ffi_pure]` is only allowed on foreign functions - struct_span_err!( - tcx.sess, - attr.span, - E0755, - "`#[ffi_pure]` may only be used on foreign functions" - ) - .emit(); - } + codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_PURE; } else if attr.has_name(sym::ffi_const) { - if tcx.is_foreign_item(did) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_CONST; - } else { - // `#[ffi_const]` is only allowed on foreign functions - struct_span_err!( - tcx.sess, - attr.span, - E0756, - "`#[ffi_const]` may only be used on foreign functions" - ) - .emit(); - } + codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_CONST; } else if attr.has_name(sym::rustc_nounwind) { codegen_fn_attrs.flags |= CodegenFnAttrFlags::NEVER_UNWIND; } else if attr.has_name(sym::rustc_reallocator) { diff --git a/compiler/rustc_error_messages/locales/en-US/passes.ftl b/compiler/rustc_error_messages/locales/en-US/passes.ftl index 91857dd227d..adfe243430f 100644 --- a/compiler/rustc_error_messages/locales/en-US/passes.ftl +++ b/compiler/rustc_error_messages/locales/en-US/passes.ftl @@ -182,6 +182,18 @@ passes_has_incoherent_inherent_impl = `rustc_has_incoherent_inherent_impls` attribute should be applied to types or traits. .label = only adts, extern types and traits are supported +passes_both_ffi_const_and_pure = + `#[ffi_const]` function cannot be `#[ffi_pure]` + +passes_ffi_pure_invalid_target = + `#[ffi_pure]` may only be used on foreign functions + +passes_ffi_const_invalid_target = + `#[ffi_const]` may only be used on foreign functions + +passes_ffi_returns_twice_invalid_target = + `#[ffi_returns_twice]` may only be used on foreign functions + passes_must_use_async = `must_use` attribute on `async` functions applies to the anonymous `Future` returned by the function, not the value within .label = this attribute does nothing, the `Future`s returned by async functions are already `must_use` diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index f9f9799d3e4..f38b9c5834d 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -150,6 +150,9 @@ impl CheckAttrVisitor<'_> { sym::rustc_has_incoherent_inherent_impls => { self.check_has_incoherent_inherent_impls(&attr, span, target) } + sym::ffi_pure => self.check_ffi_pure(hir_id, attr.span, attrs), + sym::ffi_const => self.check_ffi_const(hir_id, attr.span), + sym::ffi_returns_twice => self.check_ffi_returns_twice(hir_id, attr.span), sym::rustc_const_unstable | sym::rustc_const_stable | sym::unstable @@ -1171,6 +1174,38 @@ impl CheckAttrVisitor<'_> { } } + fn check_ffi_pure(&self, hir_id: HirId, attr_span: Span, attrs: &[Attribute]) -> bool { + if !self.tcx.is_foreign_item(self.tcx.hir().local_def_id(hir_id)) { + self.tcx.sess.emit_err(errors::FfiPureInvalidTarget { attr_span }); + return false; + } + if attrs.iter().any(|a| a.has_name(sym::ffi_const)) { + // `#[ffi_const]` functions cannot be `#[ffi_pure]` + self.tcx.sess.emit_err(errors::BothFfiConstAndPure { attr_span }); + false + } else { + true + } + } + + fn check_ffi_const(&self, hir_id: HirId, attr_span: Span) -> bool { + if self.tcx.is_foreign_item(self.tcx.hir().local_def_id(hir_id)) { + true + } else { + self.tcx.sess.emit_err(errors::FfiConstInvalidTarget { attr_span }); + false + } + } + + fn check_ffi_returns_twice(&self, hir_id: HirId, attr_span: Span) -> bool { + if self.tcx.is_foreign_item(self.tcx.hir().local_def_id(hir_id)) { + true + } else { + self.tcx.sess.emit_err(errors::FfiReturnsTwiceInvalidTarget { attr_span }); + false + } + } + /// Warns against some misuses of `#[must_use]` fn check_must_use(&self, hir_id: HirId, attr: &Attribute, target: Target) -> bool { if !matches!( diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 9c6519ea4bb..b746e543a98 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -347,6 +347,34 @@ pub struct HasIncoherentInherentImpl { pub span: Span, } +#[derive(Diagnostic)] +#[diag(passes_both_ffi_const_and_pure, code = "E0757")] +pub struct BothFfiConstAndPure { + #[primary_span] + pub attr_span: Span, +} + +#[derive(Diagnostic)] +#[diag(passes_ffi_pure_invalid_target, code = "E0755")] +pub struct FfiPureInvalidTarget { + #[primary_span] + pub attr_span: Span, +} + +#[derive(Diagnostic)] +#[diag(passes_ffi_const_invalid_target, code = "E0756")] +pub struct FfiConstInvalidTarget { + #[primary_span] + pub attr_span: Span, +} + +#[derive(Diagnostic)] +#[diag(passes_ffi_returns_twice_invalid_target, code = "E0724")] +pub struct FfiReturnsTwiceInvalidTarget { + #[primary_span] + pub attr_span: Span, +} + #[derive(LintDiagnostic)] #[diag(passes_must_use_async)] pub struct MustUseAsync { From 6e04e678dca99db95c28a3aa7091d4fa800dfb61 Mon Sep 17 00:00:00 2001 From: inquisitivecrystal <22333129+inquisitivecrystal@users.noreply.github.com> Date: Tue, 24 Jan 2023 02:54:00 -0800 Subject: [PATCH 2/3] Make sure FFI attrs aren't used on foreign statics Previously, we verified that FFI attrs were used on foreign items, but this allowed them on both foreign functions and foreign statics. This change only allows them on foreign functions. --- compiler/rustc_passes/src/check_attr.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index f38b9c5834d..663dfbb4d13 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -150,9 +150,9 @@ impl CheckAttrVisitor<'_> { sym::rustc_has_incoherent_inherent_impls => { self.check_has_incoherent_inherent_impls(&attr, span, target) } - sym::ffi_pure => self.check_ffi_pure(hir_id, attr.span, attrs), - sym::ffi_const => self.check_ffi_const(hir_id, attr.span), - sym::ffi_returns_twice => self.check_ffi_returns_twice(hir_id, attr.span), + sym::ffi_pure => self.check_ffi_pure(attr.span, attrs, target), + sym::ffi_const => self.check_ffi_const(attr.span, target), + sym::ffi_returns_twice => self.check_ffi_returns_twice(attr.span, target), sym::rustc_const_unstable | sym::rustc_const_stable | sym::unstable @@ -1174,8 +1174,8 @@ impl CheckAttrVisitor<'_> { } } - fn check_ffi_pure(&self, hir_id: HirId, attr_span: Span, attrs: &[Attribute]) -> bool { - if !self.tcx.is_foreign_item(self.tcx.hir().local_def_id(hir_id)) { + fn check_ffi_pure(&self, attr_span: Span, attrs: &[Attribute], target: Target) -> bool { + if target != Target::ForeignFn { self.tcx.sess.emit_err(errors::FfiPureInvalidTarget { attr_span }); return false; } @@ -1188,8 +1188,8 @@ impl CheckAttrVisitor<'_> { } } - fn check_ffi_const(&self, hir_id: HirId, attr_span: Span) -> bool { - if self.tcx.is_foreign_item(self.tcx.hir().local_def_id(hir_id)) { + fn check_ffi_const(&self, attr_span: Span, target: Target) -> bool { + if target == Target::ForeignFn { true } else { self.tcx.sess.emit_err(errors::FfiConstInvalidTarget { attr_span }); @@ -1197,8 +1197,8 @@ impl CheckAttrVisitor<'_> { } } - fn check_ffi_returns_twice(&self, hir_id: HirId, attr_span: Span) -> bool { - if self.tcx.is_foreign_item(self.tcx.hir().local_def_id(hir_id)) { + fn check_ffi_returns_twice(&self, attr_span: Span, target: Target) -> bool { + if target == Target::ForeignFn { true } else { self.tcx.sess.emit_err(errors::FfiReturnsTwiceInvalidTarget { attr_span }); From bc23e9aa4c78066043be246a47627746341480dd Mon Sep 17 00:00:00 2001 From: inquisitivecrystal <22333129+inquisitivecrystal@users.noreply.github.com> Date: Tue, 24 Jan 2023 04:11:00 -0800 Subject: [PATCH 3/3] Improve tests for FFI attr validation --- tests/ui/ffi_const.rs | 10 ++++++++++ tests/ui/ffi_const.stderr | 14 +++++++++++++- tests/ui/ffi_pure.rs | 10 ++++++++++ tests/ui/ffi_pure.stderr | 14 +++++++++++++- tests/ui/ffi_returns_twice.rs | 10 ++++++++++ tests/ui/ffi_returns_twice.stderr | 14 +++++++++++++- 6 files changed, 69 insertions(+), 3 deletions(-) diff --git a/tests/ui/ffi_const.rs b/tests/ui/ffi_const.rs index 7aeb5a49a1b..aa20a4d4c65 100644 --- a/tests/ui/ffi_const.rs +++ b/tests/ui/ffi_const.rs @@ -3,3 +3,13 @@ #[ffi_const] //~ ERROR `#[ffi_const]` may only be used on foreign functions pub fn foo() {} + +#[ffi_const] //~ ERROR `#[ffi_const]` may only be used on foreign functions +macro_rules! bar { + () => () +} + +extern "C" { + #[ffi_const] //~ ERROR `#[ffi_const]` may only be used on foreign functions + static INT: i32; +} diff --git a/tests/ui/ffi_const.stderr b/tests/ui/ffi_const.stderr index bc3c12eaf98..394b98f8971 100644 --- a/tests/ui/ffi_const.stderr +++ b/tests/ui/ffi_const.stderr @@ -4,6 +4,18 @@ error[E0756]: `#[ffi_const]` may only be used on foreign functions LL | #[ffi_const] | ^^^^^^^^^^^^ -error: aborting due to previous error +error[E0756]: `#[ffi_const]` may only be used on foreign functions + --> $DIR/ffi_const.rs:7:1 + | +LL | #[ffi_const] + | ^^^^^^^^^^^^ + +error[E0756]: `#[ffi_const]` may only be used on foreign functions + --> $DIR/ffi_const.rs:13:5 + | +LL | #[ffi_const] + | ^^^^^^^^^^^^ + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0756`. diff --git a/tests/ui/ffi_pure.rs b/tests/ui/ffi_pure.rs index c37d34c8784..6d2f3a614ec 100644 --- a/tests/ui/ffi_pure.rs +++ b/tests/ui/ffi_pure.rs @@ -3,3 +3,13 @@ #[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on foreign functions pub fn foo() {} + +#[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on foreign functions +macro_rules! bar { + () => () +} + +extern "C" { + #[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on foreign functions + static INT: i32; +} diff --git a/tests/ui/ffi_pure.stderr b/tests/ui/ffi_pure.stderr index bc911c85ddb..8b61a4b609f 100644 --- a/tests/ui/ffi_pure.stderr +++ b/tests/ui/ffi_pure.stderr @@ -4,6 +4,18 @@ error[E0755]: `#[ffi_pure]` may only be used on foreign functions LL | #[ffi_pure] | ^^^^^^^^^^^ -error: aborting due to previous error +error[E0755]: `#[ffi_pure]` may only be used on foreign functions + --> $DIR/ffi_pure.rs:7:1 + | +LL | #[ffi_pure] + | ^^^^^^^^^^^ + +error[E0755]: `#[ffi_pure]` may only be used on foreign functions + --> $DIR/ffi_pure.rs:13:5 + | +LL | #[ffi_pure] + | ^^^^^^^^^^^ + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0755`. diff --git a/tests/ui/ffi_returns_twice.rs b/tests/ui/ffi_returns_twice.rs index 845e18df11b..8195d0e4863 100644 --- a/tests/ui/ffi_returns_twice.rs +++ b/tests/ui/ffi_returns_twice.rs @@ -3,3 +3,13 @@ #[ffi_returns_twice] //~ ERROR `#[ffi_returns_twice]` may only be used on foreign functions pub fn foo() {} + +#[ffi_returns_twice] //~ ERROR `#[ffi_returns_twice]` may only be used on foreign functions +macro_rules! bar { + () => () +} + +extern "C" { + #[ffi_returns_twice] //~ ERROR `#[ffi_returns_twice]` may only be used on foreign functions + static INT: i32; +} diff --git a/tests/ui/ffi_returns_twice.stderr b/tests/ui/ffi_returns_twice.stderr index 2b7f5694f02..0abe7613f14 100644 --- a/tests/ui/ffi_returns_twice.stderr +++ b/tests/ui/ffi_returns_twice.stderr @@ -4,6 +4,18 @@ error[E0724]: `#[ffi_returns_twice]` may only be used on foreign functions LL | #[ffi_returns_twice] | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error[E0724]: `#[ffi_returns_twice]` may only be used on foreign functions + --> $DIR/ffi_returns_twice.rs:7:1 + | +LL | #[ffi_returns_twice] + | ^^^^^^^^^^^^^^^^^^^^ + +error[E0724]: `#[ffi_returns_twice]` may only be used on foreign functions + --> $DIR/ffi_returns_twice.rs:13:5 + | +LL | #[ffi_returns_twice] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0724`.