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 b0fa774566736..9ecd95f424f8b 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 91857dd227ddd..adfe243430f34 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 f9f9799d3e4f2..f38b9c5834dec 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 9c6519ea4bb24..b746e543a98da 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 f38b9c5834dec..663dfbb4d13a0 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 7aeb5a49a1b58..aa20a4d4c653a 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 bc3c12eaf981b..394b98f89712a 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 c37d34c8784bb..6d2f3a614ec97 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 bc911c85ddb29..8b61a4b609fc0 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 845e18df11b54..8195d0e486369 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 2b7f5694f0206..0abe7613f1493 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`.