From ecb551f5cbc510a33cf1dca99f2b5780ffa46efe Mon Sep 17 00:00:00 2001 From: David Wood Date: Sat, 5 Oct 2019 15:52:46 +0100 Subject: [PATCH 1/4] improper_ctypes: `extern "C"` fns --- src/libpanic_abort/lib.rs | 1 + src/libpanic_unwind/lib.rs | 1 + src/librustc_codegen_llvm/back/write.rs | 1 + src/librustc_codegen_llvm/llvm/mod.rs | 1 + src/librustc_lint/types.rs | 67 +++-- src/test/ui/abi/abi-sysv64-register-usage.rs | 1 + src/test/ui/align-with-extern-c-fn.rs | 1 + src/test/ui/issues/issue-16441.rs | 1 + src/test/ui/issues/issue-26997.rs | 1 + src/test/ui/issues/issue-28600.rs | 1 + src/test/ui/issues/issue-38763.rs | 1 + src/test/ui/issues/issue-51907.rs | 2 + src/test/ui/lint/lint-ctypes-fn.rs | 186 ++++++++++++++ src/test/ui/lint/lint-ctypes-fn.stderr | 247 +++++++++++++++++++ src/test/ui/mir/mir_cast_fn_ret.rs | 2 + src/test/ui/mir/mir_codegen_calls.rs | 1 + 16 files changed, 493 insertions(+), 22 deletions(-) create mode 100644 src/test/ui/lint/lint-ctypes-fn.rs create mode 100644 src/test/ui/lint/lint-ctypes-fn.stderr diff --git a/src/libpanic_abort/lib.rs b/src/libpanic_abort/lib.rs index 5509f47bc8858..bc9dd2edaf211 100644 --- a/src/libpanic_abort/lib.rs +++ b/src/libpanic_abort/lib.rs @@ -21,6 +21,7 @@ // Rust's "try" function, but if we're aborting on panics we just call the // function as there's nothing else we need to do here. #[rustc_std_internal_symbol] +#[allow(improper_ctypes)] pub unsafe extern fn __rust_maybe_catch_panic(f: fn(*mut u8), data: *mut u8, _data_ptr: *mut usize, diff --git a/src/libpanic_unwind/lib.rs b/src/libpanic_unwind/lib.rs index 2089a02083c59..2e845bfb28c11 100644 --- a/src/libpanic_unwind/lib.rs +++ b/src/libpanic_unwind/lib.rs @@ -74,6 +74,7 @@ mod windows; // hairy and tightly coupled, for more information see the compiler's // implementation of this. #[no_mangle] +#[allow(improper_ctypes)] pub unsafe extern "C" fn __rust_maybe_catch_panic(f: fn(*mut u8), data: *mut u8, data_ptr: *mut usize, diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 52f3a1cbb5c30..6f6c02618ba17 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -239,6 +239,7 @@ impl<'a> Drop for DiagnosticHandlers<'a> { } } +#[allow(improper_ctypes)] unsafe extern "C" fn report_inline_asm(cgcx: &CodegenContext, msg: &str, cookie: c_uint) { diff --git a/src/librustc_codegen_llvm/llvm/mod.rs b/src/librustc_codegen_llvm/llvm/mod.rs index 57815933af02b..e49462674b3c2 100644 --- a/src/librustc_codegen_llvm/llvm/mod.rs +++ b/src/librustc_codegen_llvm/llvm/mod.rs @@ -87,6 +87,7 @@ pub struct RustString { } /// Appending to a Rust string -- used by RawRustStringOstream. +#[allow(improper_ctypes)] #[no_mangle] pub unsafe extern "C" fn LLVMRustStringWriteImpl(sr: &RustString, ptr: *const c_char, diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index aa6dfa50dddf3..cea57fc6293cd 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -4,7 +4,7 @@ use rustc::hir::{ExprKind, Node}; use crate::hir::def_id::DefId; use rustc::hir::lowering::is_range_literal; use rustc::ty::subst::SubstsRef; -use rustc::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt}; +use rustc::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt, TypeFoldable}; use rustc::ty::layout::{self, IntegerExt, LayoutOf, VariantIdx, SizeSkeleton}; use rustc::{lint, util}; use rustc_index::vec::Idx; @@ -835,16 +835,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::Array(ty, _) => self.check_type_for_ffi(cache, ty), ty::FnPtr(sig) => { - match sig.abi() { - Abi::Rust | Abi::RustIntrinsic | Abi::PlatformIntrinsic | Abi::RustCall => { - return FfiUnsafe { - ty, - reason: "this function pointer has Rust-specific calling convention", - help: Some("consider using an `extern fn(...) -> ...` \ - function pointer instead"), - } - } - _ => {} + if self.is_internal_abi(sig.abi()) { + return FfiUnsafe { + ty, + reason: "this function pointer has Rust-specific calling convention", + help: Some("consider using an `extern fn(...) -> ...` \ + function pointer instead"), + }; } let sig = cx.erase_late_bound_regions(&sig); @@ -871,7 +868,10 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::Foreign(..) => FfiSafe, - ty::Param(..) | + // `extern "C" fn` functions can have type parameters, which may or may not be FFI-safe, + // so they are currently ignored for the purposes of this lint, see #65134. + ty::Param(..) | ty::Projection(..) => FfiSafe, + ty::Infer(..) | ty::Bound(..) | ty::Error | @@ -880,7 +880,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::GeneratorWitness(..) | ty::Placeholder(..) | ty::UnnormalizedProjection(..) | - ty::Projection(..) | ty::Opaque(..) | ty::FnDef(..) => bug!("unexpected type in foreign function: {:?}", ty), } @@ -912,8 +911,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { - use crate::rustc::ty::TypeFoldable; - struct ProhibitOpaqueTypes<'tcx> { ty: Option>, }; @@ -952,10 +949,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { 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); - + let ty = self.cx.tcx.normalize_erasing_regions(self.cx.param_env, ty); match self.check_type_for_ffi(&mut FxHashSet::default(), ty) { FfiResult::FfiSafe => {} FfiResult::FfiPhantom(ty) => { @@ -989,15 +983,21 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { let ty = self.cx.tcx.type_of(def_id); self.check_type_for_ffi_and_report_errors(span, ty); } + + fn is_internal_abi(&self, abi: Abi) -> bool { + if let Abi::Rust | Abi::RustCall | Abi::RustIntrinsic | Abi::PlatformIntrinsic = abi { + true + } else { + false + } + } } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImproperCTypes { fn check_foreign_item(&mut self, cx: &LateContext<'_, '_>, it: &hir::ForeignItem) { let mut vis = ImproperCTypesVisitor { cx }; let abi = cx.tcx.hir().get_foreign_abi(it.hir_id); - if let Abi::Rust | Abi::RustCall | Abi::RustIntrinsic | Abi::PlatformIntrinsic = abi { - // Don't worry about types in internal ABIs. - } else { + if !vis.is_internal_abi(abi) { match it.kind { hir::ForeignItemKind::Fn(ref decl, _, _) => { vis.check_foreign_fn(it.hir_id, decl); @@ -1009,6 +1009,29 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImproperCTypes { } } } + + fn check_fn( + &mut self, + cx: &LateContext<'a, 'tcx>, + kind: hir::intravisit::FnKind<'tcx>, + decl: &'tcx hir::FnDecl, + _: &'tcx hir::Body, + _: Span, + hir_id: hir::HirId, + ) { + use hir::intravisit::FnKind; + + let abi = match kind { + FnKind::ItemFn(_, _, header, ..) => (header.abi), + FnKind::Method(_, sig, ..) => (sig.header.abi), + _ => return, + }; + + let mut vis = ImproperCTypesVisitor { cx }; + if !vis.is_internal_abi(abi) { + vis.check_foreign_fn(hir_id, decl); + } + } } declare_lint_pass!(VariantSizeDifferences => [VARIANT_SIZE_DIFFERENCES]); diff --git a/src/test/ui/abi/abi-sysv64-register-usage.rs b/src/test/ui/abi/abi-sysv64-register-usage.rs index 0c7e2d906b7c1..88d89fba260aa 100644 --- a/src/test/ui/abi/abi-sysv64-register-usage.rs +++ b/src/test/ui/abi/abi-sysv64-register-usage.rs @@ -33,6 +33,7 @@ pub extern "sysv64" fn all_the_registers(rdi: i64, rsi: i64, rdx: i64, // this struct contains 8 i64's, while only 6 can be passed in registers. #[cfg(target_arch = "x86_64")] +#[repr(C)] #[derive(PartialEq, Eq, Debug)] pub struct LargeStruct(i64, i64, i64, i64, i64, i64, i64, i64); diff --git a/src/test/ui/align-with-extern-c-fn.rs b/src/test/ui/align-with-extern-c-fn.rs index 09abe4fbf7e09..b336ed76940cc 100644 --- a/src/test/ui/align-with-extern-c-fn.rs +++ b/src/test/ui/align-with-extern-c-fn.rs @@ -10,6 +10,7 @@ #[repr(align(16))] pub struct A(i64); +#[allow(improper_ctypes)] pub extern "C" fn foo(x: A) {} fn main() { diff --git a/src/test/ui/issues/issue-16441.rs b/src/test/ui/issues/issue-16441.rs index bae3813f9da95..1086aeb305542 100644 --- a/src/test/ui/issues/issue-16441.rs +++ b/src/test/ui/issues/issue-16441.rs @@ -5,6 +5,7 @@ struct Empty; // This used to cause an ICE +#[allow(improper_ctypes)] extern "C" fn ice(_a: Empty) {} fn main() { diff --git a/src/test/ui/issues/issue-26997.rs b/src/test/ui/issues/issue-26997.rs index a2b32a13678a5..bcf378e1f2968 100644 --- a/src/test/ui/issues/issue-26997.rs +++ b/src/test/ui/issues/issue-26997.rs @@ -6,6 +6,7 @@ pub struct Foo { } impl Foo { + #[allow(improper_ctypes)] pub extern fn foo_new() -> Foo { Foo { x: 21, y: 33 } } diff --git a/src/test/ui/issues/issue-28600.rs b/src/test/ui/issues/issue-28600.rs index 05c4050b03a43..d51611af49fbd 100644 --- a/src/test/ui/issues/issue-28600.rs +++ b/src/test/ui/issues/issue-28600.rs @@ -4,6 +4,7 @@ struct Test; impl Test { + #[allow(improper_ctypes)] #[allow(dead_code)] #[allow(unused_variables)] pub extern fn test(val: &str) { diff --git a/src/test/ui/issues/issue-38763.rs b/src/test/ui/issues/issue-38763.rs index 6e6de09225f57..c84846bc4e874 100644 --- a/src/test/ui/issues/issue-38763.rs +++ b/src/test/ui/issues/issue-38763.rs @@ -4,6 +4,7 @@ #[repr(C)] pub struct Foo(i128); +#[allow(improper_ctypes)] #[no_mangle] pub extern "C" fn foo(x: Foo) -> Foo { x } diff --git a/src/test/ui/issues/issue-51907.rs b/src/test/ui/issues/issue-51907.rs index 3691fe1911774..1946611a17c2f 100644 --- a/src/test/ui/issues/issue-51907.rs +++ b/src/test/ui/issues/issue-51907.rs @@ -6,7 +6,9 @@ trait Foo { struct Bar; impl Foo for Bar { + #[allow(improper_ctypes)] extern fn borrow(&self) {} + #[allow(improper_ctypes)] extern fn take(self: Box) {} } diff --git a/src/test/ui/lint/lint-ctypes-fn.rs b/src/test/ui/lint/lint-ctypes-fn.rs new file mode 100644 index 0000000000000..2daac70fedf0a --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-fn.rs @@ -0,0 +1,186 @@ +#![feature(rustc_private)] + +#![allow(private_in_public)] +#![deny(improper_ctypes)] + +extern crate libc; + +use std::default::Default; +use std::marker::PhantomData; + +trait Mirror { type It: ?Sized; } + +impl Mirror for T { type It = Self; } + +#[repr(C)] +pub struct StructWithProjection(*mut ::It); + +#[repr(C)] +pub struct StructWithProjectionAndLifetime<'a>( + &'a mut as Mirror>::It +); + +pub type I32Pair = (i32, i32); + +#[repr(C)] +pub struct ZeroSize; + +pub type RustFn = fn(); + +pub type RustBadRet = extern fn() -> Box; + +pub type CVoidRet = (); + +pub struct Foo; + +#[repr(transparent)] +pub struct TransparentI128(i128); + +#[repr(transparent)] +pub struct TransparentStr(&'static str); + +#[repr(transparent)] +pub struct TransparentBadFn(RustBadRet); + +#[repr(transparent)] +pub struct TransparentInt(u32); + +#[repr(transparent)] +pub struct TransparentRef<'a>(&'a TransparentInt); + +#[repr(transparent)] +pub struct TransparentLifetime<'a>(*const u8, PhantomData<&'a ()>); + +#[repr(transparent)] +pub struct TransparentUnit(f32, PhantomData); + +#[repr(transparent)] +pub struct TransparentCustomZst(i32, ZeroSize); + +#[repr(C)] +pub struct ZeroSizeWithPhantomData(PhantomData); + +pub extern "C" fn ptr_type1(size: *const Foo) { } +//~^ ERROR: uses type `Foo` + +pub extern "C" fn ptr_type2(size: *const Foo) { } +//~^ ERROR: uses type `Foo` + +pub extern "C" fn slice_type(p: &[u32]) { } +//~^ ERROR: uses type `[u32]` + +pub extern "C" fn str_type(p: &str) { } +//~^ ERROR: uses type `str` + +pub extern "C" fn box_type(p: Box) { } +//~^ ERROR uses type `std::boxed::Box` + +pub extern "C" fn char_type(p: char) { } +//~^ ERROR uses type `char` + +pub extern "C" fn i128_type(p: i128) { } +//~^ ERROR uses type `i128` + +pub extern "C" fn u128_type(p: u128) { } +//~^ ERROR uses type `u128` + +pub extern "C" fn tuple_type(p: (i32, i32)) { } +//~^ ERROR uses type `(i32, i32)` + +pub extern "C" fn tuple_type2(p: I32Pair) { } +//~^ ERROR uses type `(i32, i32)` + +pub extern "C" fn zero_size(p: ZeroSize) { } +//~^ ERROR uses type `ZeroSize` + +pub extern "C" fn zero_size_phantom(p: ZeroSizeWithPhantomData) { } +//~^ ERROR uses type `ZeroSizeWithPhantomData` + +pub extern "C" fn zero_size_phantom_toplevel() -> PhantomData { +//~^ ERROR uses type `std::marker::PhantomData` + Default::default() +} + +pub extern "C" fn fn_type(p: RustFn) { } +//~^ ERROR uses type `fn()` + +pub extern "C" fn fn_type2(p: fn()) { } +//~^ ERROR uses type `fn()` + +pub extern "C" fn fn_contained(p: RustBadRet) { } +//~^ ERROR: uses type `std::boxed::Box` + +pub extern "C" fn transparent_i128(p: TransparentI128) { } +//~^ ERROR: uses type `i128` + +pub extern "C" fn transparent_str(p: TransparentStr) { } +//~^ ERROR: uses type `str` + +pub extern "C" fn transparent_fn(p: TransparentBadFn) { } +//~^ ERROR: uses type `std::boxed::Box` + +pub extern "C" fn good3(fptr: Option) { } + +pub extern "C" fn good4(aptr: &[u8; 4 as usize]) { } + +pub extern "C" fn good5(s: StructWithProjection) { } + +pub extern "C" fn good6(s: StructWithProjectionAndLifetime) { } + +pub extern "C" fn good7(fptr: extern fn() -> ()) { } + +pub extern "C" fn good8(fptr: extern fn() -> !) { } + +pub extern "C" fn good9() -> () { } + +pub extern "C" fn good10() -> CVoidRet { } + +pub extern "C" fn good11(size: isize) { } + +pub extern "C" fn good12(size: usize) { } + +pub extern "C" fn good13(n: TransparentInt) { } + +pub extern "C" fn good14(p: TransparentRef) { } + +pub extern "C" fn good15(p: TransparentLifetime) { } + +pub extern "C" fn good16(p: TransparentUnit) { } + +pub extern "C" fn good17(p: TransparentCustomZst) { } + +#[allow(improper_ctypes)] +pub extern "C" fn good18(_: &String) { } + +#[cfg(not(target_arch = "wasm32"))] +pub extern "C" fn good1(size: *const libc::c_int) { } + +#[cfg(not(target_arch = "wasm32"))] +pub extern "C" fn good2(size: *const libc::c_uint) { } + +pub extern "C" fn unused_generic1(size: *const Foo) { } +//~^ ERROR: uses type `Foo` + +pub extern "C" fn unused_generic2() -> PhantomData { +//~^ ERROR uses type `std::marker::PhantomData` + Default::default() +} + +pub extern "C" fn used_generic1(x: T) { } + +pub extern "C" fn used_generic2(x: T, size: *const Foo) { } +//~^ ERROR: uses type `Foo` + +pub extern "C" fn used_generic3() -> T { + Default::default() +} + +pub extern "C" fn used_generic4(x: Vec) { } +//~^ ERROR: uses type `std::vec::Vec` + +pub extern "C" fn used_generic5() -> Vec { +//~^ ERROR: uses type `std::vec::Vec` + Default::default() +} + +fn main() {} diff --git a/src/test/ui/lint/lint-ctypes-fn.stderr b/src/test/ui/lint/lint-ctypes-fn.stderr new file mode 100644 index 0000000000000..c0ce1a1cfb9d6 --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-fn.stderr @@ -0,0 +1,247 @@ +error: `extern` block uses type `Foo`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:63:35 + | +LL | pub extern "C" fn ptr_type1(size: *const Foo) { } + | ^^^^^^^^^^ not FFI-safe + | +note: lint level defined here + --> $DIR/lint-ctypes-fn.rs:4: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/lint-ctypes-fn.rs:34:1 + | +LL | pub struct Foo; + | ^^^^^^^^^^^^^^^ + +error: `extern` block uses type `Foo`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:66:35 + | +LL | pub extern "C" fn ptr_type2(size: *const Foo) { } + | ^^^^^^^^^^ 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/lint-ctypes-fn.rs:34:1 + | +LL | pub struct Foo; + | ^^^^^^^^^^^^^^^ + +error: `extern` block uses type `[u32]`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:69:33 + | +LL | pub extern "C" fn slice_type(p: &[u32]) { } + | ^^^^^^ not FFI-safe + | + = help: consider using a raw pointer instead + = note: slices have no C equivalent + +error: `extern` block uses type `str`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:72:31 + | +LL | pub extern "C" fn str_type(p: &str) { } + | ^^^^ not FFI-safe + | + = help: consider using `*const u8` and a length instead + = note: string slices have no C equivalent + +error: `extern` block uses type `std::boxed::Box`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:75:31 + | +LL | pub extern "C" fn box_type(p: Box) { } + | ^^^^^^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct + = note: this struct has unspecified layout + +error: `extern` block uses type `char`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:78:32 + | +LL | pub extern "C" fn char_type(p: char) { } + | ^^^^ not FFI-safe + | + = help: consider using `u32` or `libc::wchar_t` instead + = note: the `char` type has no C equivalent + +error: `extern` block uses type `i128`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:81:32 + | +LL | pub extern "C" fn i128_type(p: i128) { } + | ^^^^ not FFI-safe + | + = note: 128-bit integers don't currently have a known stable ABI + +error: `extern` block uses type `u128`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:84:32 + | +LL | pub extern "C" fn u128_type(p: u128) { } + | ^^^^ not FFI-safe + | + = note: 128-bit integers don't currently have a known stable ABI + +error: `extern` block uses type `(i32, i32)`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:87:33 + | +LL | pub extern "C" fn tuple_type(p: (i32, i32)) { } + | ^^^^^^^^^^ not FFI-safe + | + = help: consider using a struct instead + = note: tuples have unspecified layout + +error: `extern` block uses type `(i32, i32)`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:90:34 + | +LL | pub extern "C" fn tuple_type2(p: I32Pair) { } + | ^^^^^^^ not FFI-safe + | + = help: consider using a struct instead + = note: tuples have unspecified layout + +error: `extern` block uses type `ZeroSize`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:93:32 + | +LL | pub extern "C" fn zero_size(p: ZeroSize) { } + | ^^^^^^^^ not FFI-safe + | + = help: consider adding a member to this struct + = note: this struct has no fields +note: type defined here + --> $DIR/lint-ctypes-fn.rs:26:1 + | +LL | pub struct ZeroSize; + | ^^^^^^^^^^^^^^^^^^^^ + +error: `extern` block uses type `ZeroSizeWithPhantomData`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:96:40 + | +LL | pub extern "C" fn zero_size_phantom(p: ZeroSizeWithPhantomData) { } + | ^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe + | + = note: composed only of `PhantomData` +note: type defined here + --> $DIR/lint-ctypes-fn.rs:61:1 + | +LL | pub struct ZeroSizeWithPhantomData(PhantomData); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: `extern` block uses type `std::marker::PhantomData`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:99:51 + | +LL | pub extern "C" fn zero_size_phantom_toplevel() -> PhantomData { + | ^^^^^^^^^^^^^^^^^ not FFI-safe + | + = note: composed only of `PhantomData` + +error: `extern` block uses type `fn()`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:104:30 + | +LL | pub extern "C" fn fn_type(p: RustFn) { } + | ^^^^^^ not FFI-safe + | + = help: consider using an `extern fn(...) -> ...` function pointer instead + = note: this function pointer has Rust-specific calling convention + +error: `extern` block uses type `fn()`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:107:31 + | +LL | pub extern "C" fn fn_type2(p: fn()) { } + | ^^^^ not FFI-safe + | + = help: consider using an `extern fn(...) -> ...` function pointer instead + = note: this function pointer has Rust-specific calling convention + +error: `extern` block uses type `std::boxed::Box`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:110:35 + | +LL | pub extern "C" fn fn_contained(p: RustBadRet) { } + | ^^^^^^^^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct + = note: this struct has unspecified layout + +error: `extern` block uses type `i128`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:113:39 + | +LL | pub extern "C" fn transparent_i128(p: TransparentI128) { } + | ^^^^^^^^^^^^^^^ not FFI-safe + | + = note: 128-bit integers don't currently have a known stable ABI + +error: `extern` block uses type `str`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:116:38 + | +LL | pub extern "C" fn transparent_str(p: TransparentStr) { } + | ^^^^^^^^^^^^^^ not FFI-safe + | + = help: consider using `*const u8` and a length instead + = note: string slices have no C equivalent + +error: `extern` block uses type `std::boxed::Box`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:119:37 + | +LL | pub extern "C" fn transparent_fn(p: TransparentBadFn) { } + | ^^^^^^^^^^^^^^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct + = note: this struct has unspecified layout + +error: `extern` block uses type `Foo`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:161:44 + | +LL | pub extern "C" fn unused_generic1(size: *const Foo) { } + | ^^^^^^^^^^ 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/lint-ctypes-fn.rs:34:1 + | +LL | pub struct Foo; + | ^^^^^^^^^^^^^^^ + +error: `extern` block uses type `std::marker::PhantomData`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:164:43 + | +LL | pub extern "C" fn unused_generic2() -> PhantomData { + | ^^^^^^^^^^^^^^^^^ not FFI-safe + | + = note: composed only of `PhantomData` + +error: `extern` block uses type `Foo`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:171:48 + | +LL | pub extern "C" fn used_generic2(x: T, size: *const Foo) { } + | ^^^^^^^^^^ 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/lint-ctypes-fn.rs:34:1 + | +LL | pub struct Foo; + | ^^^^^^^^^^^^^^^ + +error: `extern` block uses type `std::vec::Vec`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:178:39 + | +LL | pub extern "C" fn used_generic4(x: Vec) { } + | ^^^^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct + = note: this struct has unspecified layout + +error: `extern` block uses type `std::vec::Vec`, which is not FFI-safe + --> $DIR/lint-ctypes-fn.rs:181:41 + | +LL | pub extern "C" fn used_generic5() -> Vec { + | ^^^^^^ not FFI-safe + | + = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct + = note: this struct has unspecified layout + +error: aborting due to 24 previous errors + diff --git a/src/test/ui/mir/mir_cast_fn_ret.rs b/src/test/ui/mir/mir_cast_fn_ret.rs index 69fd64c1c092c..6b37ac3704038 100644 --- a/src/test/ui/mir/mir_cast_fn_ret.rs +++ b/src/test/ui/mir/mir_cast_fn_ret.rs @@ -1,8 +1,10 @@ // run-pass +#[allow(improper_ctypes)] pub extern "C" fn tuple2() -> (u16, u8) { (1, 2) } +#[allow(improper_ctypes)] pub extern "C" fn tuple3() -> (u8, u8, u8) { (1, 2, 3) } diff --git a/src/test/ui/mir/mir_codegen_calls.rs b/src/test/ui/mir/mir_codegen_calls.rs index fc0db03e3a968..c45629eddff01 100644 --- a/src/test/ui/mir/mir_codegen_calls.rs +++ b/src/test/ui/mir/mir_codegen_calls.rs @@ -74,6 +74,7 @@ fn test8() -> isize { Two::two() } +#[allow(improper_ctypes)] extern fn simple_extern(x: u32, y: (u32, u32)) -> u32 { x + y.0 * y.1 } From c12d52d878782848515acb8d84bf29e85988370d Mon Sep 17 00:00:00 2001 From: David Wood Date: Sat, 26 Oct 2019 20:36:51 +0100 Subject: [PATCH 2/4] codegen_llvm: remove unnecessary "extern C" Signed-off-by: David Wood --- src/librustc_codegen_llvm/back/write.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 6f6c02618ba17..bfba1c3ecff37 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -239,10 +239,7 @@ impl<'a> Drop for DiagnosticHandlers<'a> { } } -#[allow(improper_ctypes)] -unsafe extern "C" fn report_inline_asm(cgcx: &CodegenContext, - msg: &str, - cookie: c_uint) { +fn report_inline_asm(cgcx: &CodegenContext, msg: &str, cookie: c_uint) { cgcx.diag_emitter.inline_asm_error(cookie as u32, msg.to_owned()); } From 7f715453ee8fc2445180206ae6cafc9b4ee29e7b Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 27 Oct 2019 14:05:38 +0000 Subject: [PATCH 3/4] improper ctypes: adjust lint msg for extern fns Signed-off-by: David Wood --- src/librustc_lint/types.rs | 37 +++++++++++++------- src/test/ui/lint/lint-ctypes-fn.stderr | 48 +++++++++++++------------- 2 files changed, 49 insertions(+), 36 deletions(-) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index cea57fc6293cd..f2ad400d5dd37 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -891,11 +891,16 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { sp: Span, note: &str, help: Option<&str>, + is_foreign_item: bool, ) { let mut diag = self.cx.struct_span_lint( IMPROPER_CTYPES, sp, - &format!("`extern` block uses type `{}`, which is not FFI-safe", ty), + &format!( + "`extern` {} uses type `{}`, which is not FFI-safe", + if is_foreign_item { "block" } else { "fn" }, + ty, + ), ); diag.span_label(sp, "not FFI-safe"); if let Some(help) = help { @@ -910,7 +915,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { diag.emit(); } - fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { + fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>, is_foreign_item: bool) -> bool { struct ProhibitOpaqueTypes<'tcx> { ty: Option>, }; @@ -934,6 +939,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { sp, "opaque types have no C equivalent", None, + is_foreign_item, ); true } else { @@ -941,10 +947,15 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } } - fn check_type_for_ffi_and_report_errors(&mut self, sp: Span, ty: Ty<'tcx>) { + fn check_type_for_ffi_and_report_errors( + &mut self, + sp: Span, + ty: Ty<'tcx>, + is_foreign_item: bool, + ) { // 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) { + if self.check_for_opaque_ty(sp, ty, is_foreign_item) { // We've already emitted an error due to an opaque type. return; } @@ -953,27 +964,29 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { match self.check_type_for_ffi(&mut FxHashSet::default(), ty) { FfiResult::FfiSafe => {} FfiResult::FfiPhantom(ty) => { - self.emit_ffi_unsafe_type_lint(ty, sp, "composed only of `PhantomData`", None); + self.emit_ffi_unsafe_type_lint( + ty, sp, "composed only of `PhantomData`", None, is_foreign_item); } FfiResult::FfiUnsafe { ty, reason, help } => { - self.emit_ffi_unsafe_type_lint(ty, sp, reason, help); + self.emit_ffi_unsafe_type_lint( + ty, sp, reason, help, is_foreign_item); } } } - fn check_foreign_fn(&mut self, id: hir::HirId, decl: &hir::FnDecl) { + fn check_foreign_fn(&mut self, id: hir::HirId, decl: &hir::FnDecl, is_foreign_item: bool) { let def_id = self.cx.tcx.hir().local_def_id(id); let sig = self.cx.tcx.fn_sig(def_id); let sig = self.cx.tcx.erase_late_bound_regions(&sig); for (input_ty, input_hir) in sig.inputs().iter().zip(&decl.inputs) { - self.check_type_for_ffi_and_report_errors(input_hir.span, input_ty); + self.check_type_for_ffi_and_report_errors(input_hir.span, input_ty, is_foreign_item); } if let hir::Return(ref ret_hir) = decl.output { let ret_ty = sig.output(); if !ret_ty.is_unit() { - self.check_type_for_ffi_and_report_errors(ret_hir.span, ret_ty); + self.check_type_for_ffi_and_report_errors(ret_hir.span, ret_ty, is_foreign_item); } } } @@ -981,7 +994,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { fn check_foreign_static(&mut self, id: hir::HirId, span: Span) { let def_id = self.cx.tcx.hir().local_def_id(id); let ty = self.cx.tcx.type_of(def_id); - self.check_type_for_ffi_and_report_errors(span, ty); + self.check_type_for_ffi_and_report_errors(span, ty, true); } fn is_internal_abi(&self, abi: Abi) -> bool { @@ -1000,7 +1013,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImproperCTypes { if !vis.is_internal_abi(abi) { match it.kind { hir::ForeignItemKind::Fn(ref decl, _, _) => { - vis.check_foreign_fn(it.hir_id, decl); + vis.check_foreign_fn(it.hir_id, decl, true); } hir::ForeignItemKind::Static(ref ty, _) => { vis.check_foreign_static(it.hir_id, ty.span); @@ -1029,7 +1042,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImproperCTypes { let mut vis = ImproperCTypesVisitor { cx }; if !vis.is_internal_abi(abi) { - vis.check_foreign_fn(hir_id, decl); + vis.check_foreign_fn(hir_id, decl, false); } } } diff --git a/src/test/ui/lint/lint-ctypes-fn.stderr b/src/test/ui/lint/lint-ctypes-fn.stderr index c0ce1a1cfb9d6..59bd6bfc5afd2 100644 --- a/src/test/ui/lint/lint-ctypes-fn.stderr +++ b/src/test/ui/lint/lint-ctypes-fn.stderr @@ -1,4 +1,4 @@ -error: `extern` block uses type `Foo`, which is not FFI-safe +error: `extern` fn uses type `Foo`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:63:35 | LL | pub extern "C" fn ptr_type1(size: *const Foo) { } @@ -17,7 +17,7 @@ note: type defined here LL | pub struct Foo; | ^^^^^^^^^^^^^^^ -error: `extern` block uses type `Foo`, which is not FFI-safe +error: `extern` fn uses type `Foo`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:66:35 | LL | pub extern "C" fn ptr_type2(size: *const Foo) { } @@ -31,7 +31,7 @@ note: type defined here LL | pub struct Foo; | ^^^^^^^^^^^^^^^ -error: `extern` block uses type `[u32]`, which is not FFI-safe +error: `extern` fn uses type `[u32]`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:69:33 | LL | pub extern "C" fn slice_type(p: &[u32]) { } @@ -40,7 +40,7 @@ LL | pub extern "C" fn slice_type(p: &[u32]) { } = help: consider using a raw pointer instead = note: slices have no C equivalent -error: `extern` block uses type `str`, which is not FFI-safe +error: `extern` fn uses type `str`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:72:31 | LL | pub extern "C" fn str_type(p: &str) { } @@ -49,7 +49,7 @@ LL | pub extern "C" fn str_type(p: &str) { } = help: consider using `*const u8` and a length instead = note: string slices have no C equivalent -error: `extern` block uses type `std::boxed::Box`, which is not FFI-safe +error: `extern` fn uses type `std::boxed::Box`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:75:31 | LL | pub extern "C" fn box_type(p: Box) { } @@ -58,7 +58,7 @@ LL | pub extern "C" fn box_type(p: Box) { } = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout -error: `extern` block uses type `char`, which is not FFI-safe +error: `extern` fn uses type `char`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:78:32 | LL | pub extern "C" fn char_type(p: char) { } @@ -67,7 +67,7 @@ LL | pub extern "C" fn char_type(p: char) { } = help: consider using `u32` or `libc::wchar_t` instead = note: the `char` type has no C equivalent -error: `extern` block uses type `i128`, which is not FFI-safe +error: `extern` fn uses type `i128`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:81:32 | LL | pub extern "C" fn i128_type(p: i128) { } @@ -75,7 +75,7 @@ LL | pub extern "C" fn i128_type(p: i128) { } | = note: 128-bit integers don't currently have a known stable ABI -error: `extern` block uses type `u128`, which is not FFI-safe +error: `extern` fn uses type `u128`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:84:32 | LL | pub extern "C" fn u128_type(p: u128) { } @@ -83,7 +83,7 @@ LL | pub extern "C" fn u128_type(p: u128) { } | = note: 128-bit integers don't currently have a known stable ABI -error: `extern` block uses type `(i32, i32)`, which is not FFI-safe +error: `extern` fn uses type `(i32, i32)`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:87:33 | LL | pub extern "C" fn tuple_type(p: (i32, i32)) { } @@ -92,7 +92,7 @@ LL | pub extern "C" fn tuple_type(p: (i32, i32)) { } = help: consider using a struct instead = note: tuples have unspecified layout -error: `extern` block uses type `(i32, i32)`, which is not FFI-safe +error: `extern` fn uses type `(i32, i32)`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:90:34 | LL | pub extern "C" fn tuple_type2(p: I32Pair) { } @@ -101,7 +101,7 @@ LL | pub extern "C" fn tuple_type2(p: I32Pair) { } = help: consider using a struct instead = note: tuples have unspecified layout -error: `extern` block uses type `ZeroSize`, which is not FFI-safe +error: `extern` fn uses type `ZeroSize`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:93:32 | LL | pub extern "C" fn zero_size(p: ZeroSize) { } @@ -115,7 +115,7 @@ note: type defined here LL | pub struct ZeroSize; | ^^^^^^^^^^^^^^^^^^^^ -error: `extern` block uses type `ZeroSizeWithPhantomData`, which is not FFI-safe +error: `extern` fn uses type `ZeroSizeWithPhantomData`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:96:40 | LL | pub extern "C" fn zero_size_phantom(p: ZeroSizeWithPhantomData) { } @@ -128,7 +128,7 @@ note: type defined here LL | pub struct ZeroSizeWithPhantomData(PhantomData); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: `extern` block uses type `std::marker::PhantomData`, which is not FFI-safe +error: `extern` fn uses type `std::marker::PhantomData`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:99:51 | LL | pub extern "C" fn zero_size_phantom_toplevel() -> PhantomData { @@ -136,7 +136,7 @@ LL | pub extern "C" fn zero_size_phantom_toplevel() -> PhantomData { | = note: composed only of `PhantomData` -error: `extern` block uses type `fn()`, which is not FFI-safe +error: `extern` fn uses type `fn()`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:104:30 | LL | pub extern "C" fn fn_type(p: RustFn) { } @@ -145,7 +145,7 @@ LL | pub extern "C" fn fn_type(p: RustFn) { } = help: consider using an `extern fn(...) -> ...` function pointer instead = note: this function pointer has Rust-specific calling convention -error: `extern` block uses type `fn()`, which is not FFI-safe +error: `extern` fn uses type `fn()`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:107:31 | LL | pub extern "C" fn fn_type2(p: fn()) { } @@ -154,7 +154,7 @@ LL | pub extern "C" fn fn_type2(p: fn()) { } = help: consider using an `extern fn(...) -> ...` function pointer instead = note: this function pointer has Rust-specific calling convention -error: `extern` block uses type `std::boxed::Box`, which is not FFI-safe +error: `extern` fn uses type `std::boxed::Box`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:110:35 | LL | pub extern "C" fn fn_contained(p: RustBadRet) { } @@ -163,7 +163,7 @@ LL | pub extern "C" fn fn_contained(p: RustBadRet) { } = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout -error: `extern` block uses type `i128`, which is not FFI-safe +error: `extern` fn uses type `i128`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:113:39 | LL | pub extern "C" fn transparent_i128(p: TransparentI128) { } @@ -171,7 +171,7 @@ LL | pub extern "C" fn transparent_i128(p: TransparentI128) { } | = note: 128-bit integers don't currently have a known stable ABI -error: `extern` block uses type `str`, which is not FFI-safe +error: `extern` fn uses type `str`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:116:38 | LL | pub extern "C" fn transparent_str(p: TransparentStr) { } @@ -180,7 +180,7 @@ LL | pub extern "C" fn transparent_str(p: TransparentStr) { } = help: consider using `*const u8` and a length instead = note: string slices have no C equivalent -error: `extern` block uses type `std::boxed::Box`, which is not FFI-safe +error: `extern` fn uses type `std::boxed::Box`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:119:37 | LL | pub extern "C" fn transparent_fn(p: TransparentBadFn) { } @@ -189,7 +189,7 @@ LL | pub extern "C" fn transparent_fn(p: TransparentBadFn) { } = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout -error: `extern` block uses type `Foo`, which is not FFI-safe +error: `extern` fn uses type `Foo`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:161:44 | LL | pub extern "C" fn unused_generic1(size: *const Foo) { } @@ -203,7 +203,7 @@ note: type defined here LL | pub struct Foo; | ^^^^^^^^^^^^^^^ -error: `extern` block uses type `std::marker::PhantomData`, which is not FFI-safe +error: `extern` fn uses type `std::marker::PhantomData`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:164:43 | LL | pub extern "C" fn unused_generic2() -> PhantomData { @@ -211,7 +211,7 @@ LL | pub extern "C" fn unused_generic2() -> PhantomData { | = note: composed only of `PhantomData` -error: `extern` block uses type `Foo`, which is not FFI-safe +error: `extern` fn uses type `Foo`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:171:48 | LL | pub extern "C" fn used_generic2(x: T, size: *const Foo) { } @@ -225,7 +225,7 @@ note: type defined here LL | pub struct Foo; | ^^^^^^^^^^^^^^^ -error: `extern` block uses type `std::vec::Vec`, which is not FFI-safe +error: `extern` fn uses type `std::vec::Vec`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:178:39 | LL | pub extern "C" fn used_generic4(x: Vec) { } @@ -234,7 +234,7 @@ LL | pub extern "C" fn used_generic4(x: Vec) { } = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout -error: `extern` block uses type `std::vec::Vec`, which is not FFI-safe +error: `extern` fn uses type `std::vec::Vec`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:181:41 | LL | pub extern "C" fn used_generic5() -> Vec { From a2f3d5cb070a4a35a8b254de21f87152a8dcb808 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 28 Oct 2019 18:51:16 +0000 Subject: [PATCH 4/4] tests: add `#[repr(C)]` to test Signed-off-by: David Wood --- src/test/ui/align-with-extern-c-fn.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/ui/align-with-extern-c-fn.rs b/src/test/ui/align-with-extern-c-fn.rs index b336ed76940cc..8ba90225c6c7f 100644 --- a/src/test/ui/align-with-extern-c-fn.rs +++ b/src/test/ui/align-with-extern-c-fn.rs @@ -7,10 +7,9 @@ #![feature(repr_align)] -#[repr(align(16))] +#[repr(align(16), C)] pub struct A(i64); -#[allow(improper_ctypes)] pub extern "C" fn foo(x: A) {} fn main() {