Skip to content

Commit

Permalink
Auto merge of #64359 - varkor:opaque-ty-in-extern, r=estebank
Browse files Browse the repository at this point in the history 
Forbid opaque types in `extern "C"` blocks

Fixes #64338.
  • Loading branch information
@bors
bors committed Sep 12, 2019
2 parents 28e85d7 + 9d71217 commit eb48d6b
Show file tree
Hide file tree
Showing 13 changed files with 268 additions and 135 deletions.
113 changes: 82 additions & 31 deletions src/librustc_lint/types.rs
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit eb48d6b

Please sign in to comment.