From fb6647c626f06b43095f5723249e4f357aea251c Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Mon, 11 Nov 2019 19:55:49 +0200 Subject: [PATCH 1/2] Add FFI raw array lint --- src/librustc_lint/types.rs | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index 9a4e981081fcf..71b234964c6ae 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -600,6 +600,23 @@ fn is_repr_nullable_ptr<'tcx>( } impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { + + /// Check if the type is array and emit an unsafe type lint. + fn check_for_array_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { + if let ty::Array(..) = ty.kind { + self.emit_ffi_unsafe_type_lint( + ty, + sp, + "passing raw arrays by value is not FFI-safe", + Some("consider passing a pointer to the array"), + ); + true + } else { + false + } + } + + /// Checks if the given type is "ffi-safe" (has a stable, well-defined /// representation which can be exported to C code). fn check_type_for_ffi(&self, @@ -834,7 +851,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::RawPtr(ty::TypeAndMut { ty, .. }) | ty::Ref(_, ty, _) => self.check_type_for_ffi(cache, ty), - ty::Array(ty, _) => self.check_type_for_ffi(cache, ty), + ty::Array(inner_ty, _) => self.check_type_for_ffi(cache, inner_ty), ty::FnPtr(sig) => { match sig.abi() { @@ -946,7 +963,7 @@ 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_static: 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) { @@ -957,6 +974,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // 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); + // C doesn't really support passing arrays by value. + // The only way to pass an array by value is through a struct. + // So we first test that the top level isn't an array, + // and then recursively check the types inside. + if !is_static && self.check_for_array_ty(sp, ty) { + return; + } match self.check_type_for_ffi(&mut FxHashSet::default(), ty) { FfiResult::FfiSafe => {} @@ -975,13 +999,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { 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, false); } 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, false); } } } @@ -989,7 +1013,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); } } From b3666b64738980579f05eec0cfae43f917f74a29 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Mon, 11 Nov 2019 19:56:45 +0200 Subject: [PATCH 2/2] Update tests for raw array in FFI lint --- src/test/ui/lint/lint-ctypes.rs | 7 +++++++ src/test/ui/lint/lint-ctypes.stderr | 27 ++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/test/ui/lint/lint-ctypes.rs b/src/test/ui/lint/lint-ctypes.rs index e20503a395c89..a439a1f339aea 100644 --- a/src/test/ui/lint/lint-ctypes.rs +++ b/src/test/ui/lint/lint-ctypes.rs @@ -65,6 +65,10 @@ extern { pub fn transparent_i128(p: TransparentI128); //~ ERROR: uses type `i128` pub fn transparent_str(p: TransparentStr); //~ ERROR: uses type `str` pub fn transparent_fn(p: TransparentBadFn); //~ ERROR: uses type `std::boxed::Box` + pub fn raw_array(arr: [u8; 8]); //~ ERROR: uses type `[u8; 8]` + + pub static static_u128_type: u128; //~ ERROR: uses type `u128` + pub static static_u128_array_type: [u128; 16]; //~ ERROR: uses type `u128` pub fn good3(fptr: Option); pub fn good4(aptr: &[u8; 4 as usize]); @@ -83,6 +87,9 @@ extern { pub fn good17(p: TransparentCustomZst); #[allow(improper_ctypes)] pub fn good18(_: &String); + pub fn good20(arr: *const [u8; 8]); + pub static good21: [u8; 8]; + } #[allow(improper_ctypes)] diff --git a/src/test/ui/lint/lint-ctypes.stderr b/src/test/ui/lint/lint-ctypes.stderr index e533a767b317f..e6bb49afb880f 100644 --- a/src/test/ui/lint/lint-ctypes.stderr +++ b/src/test/ui/lint/lint-ctypes.stderr @@ -197,5 +197,30 @@ LL | pub 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: aborting due to 20 previous errors +error: `extern` block uses type `[u8; 8]`, which is not FFI-safe + --> $DIR/lint-ctypes.rs:68:27 + | +LL | pub fn raw_array(arr: [u8; 8]); + | ^^^^^^^ not FFI-safe + | + = help: consider passing a pointer to the array + = note: passing raw arrays by value is not FFI-safe + +error: `extern` block uses type `u128`, which is not FFI-safe + --> $DIR/lint-ctypes.rs:70:34 + | +LL | pub static static_u128_type: u128; + | ^^^^ 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.rs:71:40 + | +LL | pub static static_u128_array_type: [u128; 16]; + | ^^^^^^^^^^ not FFI-safe + | + = note: 128-bit integers don't currently have a known stable ABI + +error: aborting due to 23 previous errors