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
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
113 changes: 82 additions & 31 deletions src/librustc_lint/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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"),
};
Expand Down Expand Up @@ -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"),
Expand All @@ -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"),
};
Expand Down Expand Up @@ -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(...)]` \
Expand Down Expand Up @@ -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,
};
Expand All @@ -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,
},
Expand All @@ -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"),
},
Expand All @@ -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"),
Expand Down Expand Up @@ -855,36 +855,87 @@ 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 emit_ffi_unsafe_type_lint(
&mut self,
ty: Ty<'tcx>,
sp: Span,
note: &str,
help: Option<&str>,
) {
let mut diag = self.cx.struct_span_lint(
IMPROPER_CTYPES,
sp,
&format!("`extern` block uses type `{}`, which is not FFI-safe", ty),
);
diag.span_label(sp, "not FFI-safe");
if let Some(help) = help {
diag.help(help);
}
diag.note(note);
if let ty::Adt(def, _) = ty.sty {
if let Some(sp) = self.cx.tcx.hir().span_if_local(def.did) {
diag.span_note(sp, "type defined here");
}
}
diag.emit();
}

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

struct ProhibitOpaqueTypes<'tcx> {
ty: Option<Ty<'tcx>>,
};

impl<'tcx> ty::fold::TypeVisitor<'tcx> for ProhibitOpaqueTypes<'tcx> {
fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool {
if let ty::Opaque(..) = ty.sty {
self.ty = Some(ty);
true
} else {
ty.super_visit_with(self)
}
}
}

let mut visitor = ProhibitOpaqueTypes { ty: None };
ty.visit_with(&mut visitor);
if let Some(ty) = visitor.ty {
self.emit_ffi_unsafe_type_lint(
ty,
sp,
"opaque types have no C equivalent",
None,
);
true
} else {
false
}
}

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

match self.check_type_for_ffi(&mut FxHashSet::default(), ty) {
FfiResult::FfiSafe => {}
FfiResult::FfiPhantom(ty) => {
self.cx.span_lint(IMPROPER_CTYPES,
sp,
&format!("`extern` block uses type `{}` which is not FFI-safe: \
composed only of PhantomData", ty));
self.emit_ffi_unsafe_type_lint(ty, sp, "composed only of `PhantomData`", None);
}
FfiResult::FfiUnsafe { ty: unsafe_ty, reason, help } => {
let msg = format!("`extern` block uses type `{}` which is not FFI-safe: {}",
unsafe_ty, reason);
let mut diag = self.cx.struct_span_lint(IMPROPER_CTYPES, sp, &msg);
if let Some(s) = help {
diag.help(s);
}
if let ty::Adt(def, _) = unsafe_ty.sty {
if let Some(sp) = self.cx.tcx.hir().span_if_local(def.did) {
diag.span_note(sp, "type defined here");
}
}
diag.emit();
FfiResult::FfiUnsafe { ty, reason, help } => {
self.emit_ffi_unsafe_type_lint(ty, sp, reason, help);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-14309.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ struct D {
}

extern "C" {
fn foo(x: A); //~ ERROR type `A` which is not FFI-safe
fn foo(x: A); //~ ERROR type `A`, which is not FFI-safe
fn bar(x: B); //~ ERROR type `A`
fn baz(x: C);
fn qux(x: A2); //~ ERROR type `A`
Expand Down
25 changes: 15 additions & 10 deletions src/test/ui/issues/issue-14309.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
error: `extern` block uses type `A` which is not FFI-safe: this struct has unspecified layout
error: `extern` block uses type `A`, which is not FFI-safe
--> $DIR/issue-14309.rs:30:15
|
LL | fn foo(x: A);
| ^
| ^ not FFI-safe
|
note: lint level defined here
--> $DIR/issue-14309.rs:1:9
|
LL | #![deny(improper_ctypes)]
| ^^^^^^^^^^^^^^^
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
= note: this struct has unspecified layout
note: type defined here
--> $DIR/issue-14309.rs:4:1
|
Expand All @@ -18,13 +19,14 @@ LL | | x: i32
LL | | }
| |_^

error: `extern` block uses type `A` which is not FFI-safe: this struct has unspecified layout
error: `extern` block uses type `A`, which is not FFI-safe
--> $DIR/issue-14309.rs:31:15
|
LL | fn bar(x: B);
| ^
| ^ not FFI-safe
|
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
= note: this struct has unspecified layout
note: type defined here
--> $DIR/issue-14309.rs:4:1
|
Expand All @@ -33,13 +35,14 @@ LL | | x: i32
LL | | }
| |_^

error: `extern` block uses type `A` which is not FFI-safe: this struct has unspecified layout
error: `extern` block uses type `A`, which is not FFI-safe
--> $DIR/issue-14309.rs:33:15
|
LL | fn qux(x: A2);
| ^^
| ^^ not FFI-safe
|
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
= note: this struct has unspecified layout
note: type defined here
--> $DIR/issue-14309.rs:4:1
|
Expand All @@ -48,13 +51,14 @@ LL | | x: i32
LL | | }
| |_^

error: `extern` block uses type `A` which is not FFI-safe: this struct has unspecified layout
error: `extern` block uses type `A`, which is not FFI-safe
--> $DIR/issue-14309.rs:34:16
|
LL | fn quux(x: B2);
| ^^
| ^^ not FFI-safe
|
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
= note: this struct has unspecified layout
note: type defined here
--> $DIR/issue-14309.rs:4:1
|
Expand All @@ -63,13 +67,14 @@ LL | | x: i32
LL | | }
| |_^

error: `extern` block uses type `A` which is not FFI-safe: this struct has unspecified layout
error: `extern` block uses type `A`, which is not FFI-safe
--> $DIR/issue-14309.rs:36:16
|
LL | fn fred(x: D);
| ^
| ^ not FFI-safe
|
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
= note: this struct has unspecified layout
note: type defined here
--> $DIR/issue-14309.rs:4:1
|
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-16250.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
pub struct Foo;

extern {
pub fn foo(x: (Foo)); //~ ERROR unspecified layout
pub fn foo(x: (Foo)); //~ ERROR `extern` block uses type `Foo`
}

fn main() {
Expand Down
5 changes: 3 additions & 2 deletions src/test/ui/issues/issue-16250.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: `extern` block uses type `Foo` which is not FFI-safe: this struct has unspecified layout
error: `extern` block uses type `Foo`, which is not FFI-safe
--> $DIR/issue-16250.rs:6:20
|
LL | pub fn foo(x: (Foo));
| ^^^
| ^^^ not FFI-safe
|
note: lint level defined here
--> $DIR/issue-16250.rs:1:9
Expand All @@ -11,6 +11,7 @@ LL | #![deny(warnings)]
| ^^^^^^^^
= note: `#[deny(improper_ctypes)]` implied by `#[deny(warnings)]`
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
= note: this struct has unspecified layout
note: type defined here
--> $DIR/issue-16250.rs:3:1
|
Expand Down
21 changes: 11 additions & 10 deletions src/test/ui/lint/lint-ctypes-enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,36 +36,37 @@ struct Rust<T>(T);

extern {
fn zf(x: Z);
fn uf(x: U); //~ ERROR enum has no representation hint
fn bf(x: B); //~ ERROR enum has no representation hint
fn tf(x: T); //~ ERROR enum has no representation hint
fn uf(x: U); //~ ERROR `extern` block uses type `U`
fn bf(x: B); //~ ERROR `extern` block uses type `B`
fn tf(x: T); //~ ERROR `extern` block uses type `T`
fn repr_c(x: ReprC);
fn repr_u8(x: U8);
fn repr_isize(x: Isize);
fn option_ref(x: Option<&'static u8>);
fn option_fn(x: Option<extern "C" fn()>);
fn nonnull(x: Option<std::ptr::NonNull<u8>>);
fn unique(x: Option<std::ptr::Unique<u8>>); //~ ERROR enum has no representation hint
fn unique(x: Option<std::ptr::Unique<u8>>);
//~^ ERROR `extern` block uses type `std::option::Option<std::ptr::Unique<u8>>`
fn nonzero_u8(x: Option<num::NonZeroU8>);
fn nonzero_u16(x: Option<num::NonZeroU16>);
fn nonzero_u32(x: Option<num::NonZeroU32>);
fn nonzero_u64(x: Option<num::NonZeroU64>);
fn nonzero_u128(x: Option<num::NonZeroU128>);
//~^ ERROR 128-bit integers don't currently have a known stable ABI
//~^ ERROR `extern` block uses type `u128`
fn nonzero_usize(x: Option<num::NonZeroUsize>);
fn nonzero_i8(x: Option<num::NonZeroI8>);
fn nonzero_i16(x: Option<num::NonZeroI16>);
fn nonzero_i32(x: Option<num::NonZeroI32>);
fn nonzero_i64(x: Option<num::NonZeroI64>);
fn nonzero_i128(x: Option<num::NonZeroI128>);
//~^ ERROR 128-bit integers don't currently have a known stable ABI
//~^ ERROR `extern` block uses type `i128`
fn nonzero_isize(x: Option<num::NonZeroIsize>);
fn transparent_struct(x: Option<TransparentStruct<num::NonZeroU8>>);
fn transparent_enum(x: Option<TransparentEnum<num::NonZeroU8>>);
fn transparent_union(x: Option<TransparentUnion<num::NonZeroU8>>);
//~^ ERROR enum has no representation hint
fn repr_rust(x: Option<Rust<num::NonZeroU8>>); //~ ERROR enum has no representation hint
fn no_result(x: Result<(), num::NonZeroI32>); //~ ERROR enum has no representation hint
//~^ ERROR `extern` block uses type
fn repr_rust(x: Option<Rust<num::NonZeroU8>>); //~ ERROR `extern` block uses type
fn no_result(x: Result<(), num::NonZeroI32>); //~ ERROR `extern` block uses type
}

pub fn main() { }
pub fn main() {}
Loading