Skip to content

Strengthen validation of FFI attributes #107257

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 1, 2023
Merged
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
50 changes: 3 additions & 47 deletions compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_error_messages/locales/en-US/passes.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
35 changes: 35 additions & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(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
Expand Down Expand Up @@ -1171,6 +1174,38 @@ impl CheckAttrVisitor<'_> {
}
}

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;
}
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, attr_span: Span, target: Target) -> bool {
if target == Target::ForeignFn {
true
} else {
self.tcx.sess.emit_err(errors::FfiConstInvalidTarget { attr_span });
false
}
}

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 });
false
}
}

/// Warns against some misuses of `#[must_use]`
fn check_must_use(&self, hir_id: HirId, attr: &Attribute, target: Target) -> bool {
if !matches!(
Expand Down
28 changes: 28 additions & 0 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions tests/ui/ffi_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
14 changes: 13 additions & 1 deletion tests/ui/ffi_const.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
10 changes: 10 additions & 0 deletions tests/ui/ffi_pure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
14 changes: 13 additions & 1 deletion tests/ui/ffi_pure.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
10 changes: 10 additions & 0 deletions tests/ui/ffi_returns_twice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
14 changes: 13 additions & 1 deletion tests/ui/ffi_returns_twice.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`.