Skip to content
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

Forbid opaque types in `extern "C"` blocks #64359

Merged
merged 4 commits into from Sep 12, 2019
Merged
Changes from 3 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -624,7 +624,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
AdtKind::Struct => {
if !def.repr.c() && !def.repr.transparent() {
return FfiUnsafe {
ty: ty,
ty,
reason: "this struct has unspecified layout",
help: Some("consider adding a `#[repr(C)]` or \
`#[repr(transparent)]` attribute to this struct"),
@@ -633,7 +633,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {

if def.non_enum_variant().fields.is_empty() {
return FfiUnsafe {
ty: ty,
ty,
reason: "this struct has no fields",
help: Some("consider adding a member to this struct"),
};
@@ -669,7 +669,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
AdtKind::Union => {
if !def.repr.c() && !def.repr.transparent() {
return FfiUnsafe {
ty: ty,
ty,
reason: "this union has unspecified layout",
help: Some("consider adding a `#[repr(C)]` or \
`#[repr(transparent)]` attribute to this union"),
@@ -678,7 +678,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {

if def.non_enum_variant().fields.is_empty() {
return FfiUnsafe {
ty: ty,
ty,
reason: "this union has no fields",
help: Some("consider adding a field to this union"),
};
@@ -721,7 +721,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
// Special-case types like `Option<extern fn()>`.
if !is_repr_nullable_ptr(cx, ty, def, substs) {
return FfiUnsafe {
ty: ty,
ty,
reason: "enum has no representation hint",
help: Some("consider adding a `#[repr(C)]`, \
`#[repr(transparent)]`, or integer `#[repr(...)]` \
@@ -750,7 +750,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
}
FfiPhantom(..) => {
return FfiUnsafe {
ty: ty,
ty,
reason: "this enum contains a PhantomData field",
help: None,
};
@@ -764,13 +764,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
}

ty::Char => FfiUnsafe {
ty: ty,
ty,
reason: "the `char` type has no C equivalent",
help: Some("consider using `u32` or `libc::wchar_t` instead"),
},

ty::Int(ast::IntTy::I128) | ty::Uint(ast::UintTy::U128) => FfiUnsafe {
ty: ty,
ty,
reason: "128-bit integers don't currently have a known stable ABI",
help: None,
},
@@ -779,25 +779,25 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
ty::Bool | ty::Int(..) | ty::Uint(..) | ty::Float(..) | ty::Never => FfiSafe,

ty::Slice(_) => FfiUnsafe {
ty: ty,
ty,
reason: "slices have no C equivalent",
help: Some("consider using a raw pointer instead"),
},

ty::Dynamic(..) => FfiUnsafe {
ty: ty,
ty,
reason: "trait objects have no C equivalent",
help: None,
},

ty::Str => FfiUnsafe {
ty: ty,
ty,
reason: "string slices have no C equivalent",
help: Some("consider using `*const u8` and a length instead"),
},

ty::Tuple(..) => FfiUnsafe {
ty: ty,
ty,
reason: "tuples have unspecified layout",
help: Some("consider using a struct instead"),
},
@@ -811,7 +811,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
match sig.abi() {
Abi::Rust | Abi::RustIntrinsic | Abi::PlatformIntrinsic | Abi::RustCall => {
return FfiUnsafe {
ty: ty,
ty,
reason: "this function pointer has Rust-specific calling convention",
help: Some("consider using an `extern fn(...) -> ...` \
function pointer instead"),
@@ -855,11 +855,44 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
ty::UnnormalizedProjection(..) |
ty::Projection(..) |
ty::Opaque(..) |
ty::FnDef(..) => bug!("Unexpected type in foreign function"),
ty::FnDef(..) => bug!("unexpected type in foreign function: {:?}", ty),
}
}

fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool {
use crate::rustc::ty::TypeFoldable;

struct ProhibitOpaqueTypes<'a, 'tcx> {
cx: &'a LateContext<'a, 'tcx>,
sp: Span,
};

impl<'a, 'tcx> ty::fold::TypeVisitor<'tcx> for ProhibitOpaqueTypes<'a, 'tcx> {
fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool {
if let ty::Opaque(..) = ty.sty {
self.cx.span_lint(IMPROPER_CTYPES,
This conversation was marked as resolved by varkor

This comment has been minimized.

Copy link
@estebank

estebank Sep 10, 2019

Contributor

We have two other similar emitters of IMPROPER_CTYPES, could we deduplicate them a bit?

self.sp,
&format!("`extern` block uses type `{}` which is not FFI-safe: \
opaque types have no C equivalent", ty));
This conversation was marked as resolved by varkor

This comment has been minimized.

Copy link
@estebank

estebank Sep 10, 2019

Contributor

Could we make this line a note separate from the main message? (And a main label not FFI-safe while we are at it).

true
} else {
ty.super_visit_with(self)
}
}
}

let mut visitor = ProhibitOpaqueTypes { cx: self.cx, sp };
ty.visit_with(&mut visitor)
}

fn check_type_for_ffi_and_report_errors(&mut self, sp: Span, ty: Ty<'tcx>) {
// We have to check for opaque types before `normalize_erasing_regions`,
// which will replace opaque types with their underlying concrete type.
if self.check_for_opaque_ty(sp, ty) {
// We've already emitted an error due to an opaque type.
return;
}

// it is only OK to use this function because extern fns cannot have
// any generic types right now:
let ty = self.cx.tcx.normalize_erasing_regions(ParamEnv::reveal_all(), ty);
@@ -870,7 +903,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
self.cx.span_lint(IMPROPER_CTYPES,
sp,
&format!("`extern` block uses type `{}` which is not FFI-safe: \
composed only of PhantomData", ty));
composed only of `PhantomData`", ty));
}
FfiResult::FfiUnsafe { ty: unsafe_ty, reason, help } => {
let msg = format!("`extern` block uses type `{}` which is not FFI-safe: {}",
@@ -1,7 +1,7 @@
#![deny(improper_ctypes)]
#![feature(rustc_private)]

#![allow(private_in_public)]
#![deny(improper_ctypes)]

extern crate libc;

@@ -55,9 +55,9 @@ extern {
pub fn tuple_type(p: (i32, i32)); //~ ERROR uses type `(i32, i32)`
pub fn tuple_type2(p: I32Pair); //~ ERROR uses type `(i32, i32)`
pub fn zero_size(p: ZeroSize); //~ ERROR struct has no fields
pub fn zero_size_phantom(p: ZeroSizeWithPhantomData); //~ ERROR composed only of PhantomData
pub fn zero_size_phantom(p: ZeroSizeWithPhantomData); //~ ERROR composed only of `PhantomData`
pub fn zero_size_phantom_toplevel()
-> ::std::marker::PhantomData<bool>; //~ ERROR: composed only of PhantomData
-> ::std::marker::PhantomData<bool>; //~ ERROR: composed only of `PhantomData`
pub fn fn_type(p: RustFn); //~ ERROR function pointer has Rust-specific
pub fn fn_type2(p: fn()); //~ ERROR function pointer has Rust-specific
pub fn fn_contained(p: RustBadRet); //~ ERROR: uses type `std::boxed::Box<u32>`
@@ -5,7 +5,7 @@ LL | pub fn ptr_type1(size: *const Foo);
| ^^^^^^^^^^
|
note: lint level defined here
--> $DIR/lint-ctypes.rs:1:9
--> $DIR/lint-ctypes.rs:4:9
|
LL | #![deny(improper_ctypes)]
| ^^^^^^^^^^^^^^^
@@ -108,13 +108,13 @@ note: type defined here
LL | pub struct ZeroSize;
| ^^^^^^^^^^^^^^^^^^^^

error: `extern` block uses type `ZeroSizeWithPhantomData` which is not FFI-safe: composed only of PhantomData
error: `extern` block uses type `ZeroSizeWithPhantomData` which is not FFI-safe: composed only of `PhantomData`
--> $DIR/lint-ctypes.rs:58:33
|
LL | pub fn zero_size_phantom(p: ZeroSizeWithPhantomData);
| ^^^^^^^^^^^^^^^^^^^^^^^

error: `extern` block uses type `std::marker::PhantomData<bool>` which is not FFI-safe: composed only of PhantomData
error: `extern` block uses type `std::marker::PhantomData<bool>` which is not FFI-safe: composed only of `PhantomData`
--> $DIR/lint-ctypes.rs:60:12
|
LL | -> ::std::marker::PhantomData<bool>;
@@ -0,0 +1,16 @@
#![feature(type_alias_impl_trait)]

#![deny(improper_ctypes)]

type A = impl Fn();

pub fn ret_closure() -> A {
|| {}
}

extern "C" {
pub fn a(_: A);
//~^ ERROR `extern` block uses type `A` which is not FFI-safe: opaque types have no C equivalent
}

fn main() {}
@@ -0,0 +1,14 @@
error: `extern` block uses type `A` which is not FFI-safe: opaque types have no C equivalent
--> $DIR/opaque-ty-ffi-unsafe.rs:12:17
|
LL | pub fn a(_: A);
| ^
|
note: lint level defined here
--> $DIR/opaque-ty-ffi-unsafe.rs:3:9
|
LL | #![deny(improper_ctypes)]
| ^^^^^^^^^^^^^^^

error: aborting due to previous error

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.