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

Add by-value arrays to improper_ctypes lint #66305

Merged
merged 2 commits into from
Nov 28, 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
34 changes: 29 additions & 5 deletions src/librustc_lint/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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) {
Expand All @@ -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 => {}
Expand All @@ -975,21 +999,21 @@ 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);
}
}
}

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

Expand Down
7 changes: 7 additions & 0 deletions src/test/ui/lint/lint-ctypes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u32>`
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<extern fn()>);
pub fn good4(aptr: &[u8; 4 as usize]);
Expand All @@ -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)]
Expand Down
27 changes: 26 additions & 1 deletion src/test/ui/lint/lint-ctypes.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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