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

improper_ctypes_definitions lint #72700

Merged
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
2 changes: 2 additions & 0 deletions src/liballoc/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,13 @@
//! pub struct Foo;
//!
//! #[no_mangle]
//! #[allow(improper_ctypes_definitions)]
//! pub extern "C" fn foo_new() -> Box<Foo> {
//! Box::new(Foo)
//! }
//!
//! #[no_mangle]
//! #[allow(improper_ctypes_definitions)]
//! pub extern "C" fn foo_delete(_: Option<Box<Foo>>) {}
//! ```
//!
Expand Down
1 change: 1 addition & 0 deletions src/libpanic_abort/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use core::any::Any;

#[rustc_std_internal_symbol]
#[cfg_attr(not(bootstrap), allow(improper_ctypes_definitions))]
pub unsafe extern "C" fn __rust_panic_cleanup(_: *mut u8) -> *mut (dyn Any + Send + 'static) {
unreachable!()
}
Expand Down
1 change: 1 addition & 0 deletions src/libpanic_unwind/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ extern "C" {
mod dwarf;

#[rustc_std_internal_symbol]
#[cfg_attr(not(bootstrap), allow(improper_ctypes_definitions))]
pub unsafe extern "C" fn __rust_panic_cleanup(payload: *mut u8) -> *mut (dyn Any + Send + 'static) {
Box::into_raw(imp::cleanup(payload))
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ macro_rules! late_lint_mod_passes {
$args,
[
HardwiredLints: HardwiredLints,
ImproperCTypes: ImproperCTypes,
ImproperCTypesDeclarations: ImproperCTypesDeclarations,
ImproperCTypesDefinitions: ImproperCTypesDefinitions,
VariantSizeDifferences: VariantSizeDifferences,
BoxPointers: BoxPointers,
PathStatements: PathStatements,
Expand Down
120 changes: 95 additions & 25 deletions src/librustc_lint/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt, TypeFoldable};
use rustc_span::source_map;
use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_span::{Span, DUMMY_SP};
use rustc_target::abi::{Integer, LayoutOf, TagEncoding, VariantIdx, Variants};
use rustc_target::spec::abi::Abi;

Expand Down Expand Up @@ -498,10 +498,24 @@ declare_lint! {
"proper use of libc types in foreign modules"
}

declare_lint_pass!(ImproperCTypes => [IMPROPER_CTYPES]);
declare_lint_pass!(ImproperCTypesDeclarations => [IMPROPER_CTYPES]);

declare_lint! {
IMPROPER_CTYPES_DEFINITIONS,
Warn,
"proper use of libc types in foreign item definitions"
}

declare_lint_pass!(ImproperCTypesDefinitions => [IMPROPER_CTYPES_DEFINITIONS]);

enum ImproperCTypesMode {
Declarations,
Definitions,
}

struct ImproperCTypesVisitor<'a, 'tcx> {
cx: &'a LateContext<'a, 'tcx>,
mode: ImproperCTypesMode,
}

enum FfiResult<'tcx> {
Expand Down Expand Up @@ -804,27 +818,32 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
help: Some("consider using a struct instead".into()),
},

ty::RawPtr(ty::TypeAndMut { ty, .. }) | ty::Ref(_, ty, _)
if {
matches!(self.mode, ImproperCTypesMode::Definitions)
&& ty.is_sized(self.cx.tcx.at(DUMMY_SP), self.cx.param_env)
} =>
{
FfiSafe
}

ty::RawPtr(ty::TypeAndMut { ty, .. }) | ty::Ref(_, 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() {
Abi::Rust | Abi::RustIntrinsic | Abi::PlatformIntrinsic | Abi::RustCall => {
return FfiUnsafe {
ty,
reason: "this function pointer has Rust-specific calling convention"
if self.is_internal_abi(sig.abi()) {
return FfiUnsafe {
ty,
reason: "this function pointer has Rust-specific calling convention".into(),
help: Some(
"consider using an `extern fn(...) -> ...` \
function pointer instead"
.into(),
help: Some(
"consider using an `extern fn(...) -> ...` \
function pointer instead"
.into(),
),
};
}
_ => {}
),
};
}

let sig = cx.erase_late_bound_regions(&sig);
Expand Down Expand Up @@ -857,15 +876,23 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
FfiUnsafe { ty, reason: "opaque types have no C equivalent".into(), help: None }
}

// `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.
ty::Param(..) | ty::Projection(..)
if matches!(self.mode, ImproperCTypesMode::Definitions) =>
{
FfiSafe
}

ty::Param(..)
| ty::Projection(..)
| ty::Infer(..)
| ty::Bound(..)
| ty::Error(_)
| ty::Closure(..)
| ty::Generator(..)
| ty::GeneratorWitness(..)
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
| ty::Placeholder(..)
| ty::Projection(..)
| ty::FnDef(..) => bug!("unexpected type in foreign function: {:?}", ty),
}
}
Expand All @@ -877,9 +904,20 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
note: &str,
help: Option<&str>,
) {
self.cx.struct_span_lint(IMPROPER_CTYPES, sp, |lint| {
let mut diag =
lint.build(&format!("`extern` block uses type `{}`, which is not FFI-safe", ty));
let lint = match self.mode {
ImproperCTypesMode::Declarations => IMPROPER_CTYPES,
ImproperCTypesMode::Definitions => IMPROPER_CTYPES_DEFINITIONS,
};

self.cx.struct_span_lint(lint, sp, |lint| {
let item_description = match self.mode {
ImproperCTypesMode::Declarations => "block",
ImproperCTypesMode::Definitions => "fn",
};
let mut diag = lint.build(&format!(
"`extern` {} uses type `{}`, which is not FFI-safe",
item_description, ty
));
diag.span_label(sp, "not FFI-safe");
if let Some(help) = help {
diag.help(help);
Expand Down Expand Up @@ -947,7 +985,7 @@ 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);
let ty = self.cx.tcx.normalize_erasing_regions(self.cx.param_env, ty);

// C doesn't really support passing arrays by value - the only way to pass an array by value
// is through a struct. So, first test that the top level isn't an array, and then
Expand Down Expand Up @@ -997,15 +1035,22 @@ 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, true, false);
}

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 {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImproperCTypesDeclarations {
fn check_foreign_item(&mut self, cx: &LateContext<'_, '_>, it: &hir::ForeignItem<'_>) {
let mut vis = ImproperCTypesVisitor { cx };
let mut vis = ImproperCTypesVisitor { cx, mode: ImproperCTypesMode::Declarations };
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);
Expand All @@ -1019,6 +1064,31 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImproperCTypes {
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImproperCTypesDefinitions {
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, mode: ImproperCTypesMode::Definitions };
if !vis.is_internal_abi(abi) {
vis.check_foreign_fn(hir_id, decl);
}
}
}

declare_lint_pass!(VariantSizeDifferences => [VARIANT_SIZE_DIFFERENCES]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VariantSizeDifferences {
Expand Down
1 change: 1 addition & 0 deletions src/librustc_llvm/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub struct RustString {

/// Appending to a Rust string -- used by RawRustStringOstream.
#[no_mangle]
#[cfg_attr(not(bootstrap), allow(improper_ctypes_definitions))]
pub unsafe extern "C" fn LLVMRustStringWriteImpl(
sr: &RustString,
ptr: *const c_char,
Expand Down
1 change: 1 addition & 0 deletions src/libstd/sys/sgx/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ unsafe extern "C" fn tcs_init(secondary: bool) {
// able to specify this
#[cfg(not(test))]
#[no_mangle]
#[allow(improper_ctypes_definitions)]
extern "C" fn entry(p1: u64, p2: u64, p3: u64, secondary: bool, p4: u64, p5: u64) -> (u64, u64) {
// FIXME: how to support TLS in library mode?
let tls = Box::new(tls::Tls::new());
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/abi/abi-sysv64-register-usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub struct LargeStruct(i64, i64, i64, i64, i64, i64, i64, i64);

#[cfg(target_arch = "x86_64")]
#[inline(never)]
#[allow(improper_ctypes_definitions)]
pub extern "sysv64" fn large_struct_by_val(mut foo: LargeStruct) -> LargeStruct {
foo.0 *= 1;
foo.1 *= 2;
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/align-with-extern-c-fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#[repr(align(16))]
pub struct A(i64);

#[allow(improper_ctypes_definitions)]
pub extern "C" fn foo(x: A) {}

fn main() {
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/issues/issue-16441.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
struct Empty;

// This used to cause an ICE
#[allow(improper_ctypes_definitions)]
extern "C" fn ice(_a: Empty) {}

fn main() {
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/issues/issue-26997.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub struct Foo {
}

impl Foo {
#[allow(improper_ctypes_definitions)]
pub extern fn foo_new() -> Foo {
Foo { x: 21, y: 33 }
}
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/issues/issue-28600.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ struct Test;
impl Test {
#[allow(dead_code)]
#[allow(unused_variables)]
#[allow(improper_ctypes_definitions)]
pub extern fn test(val: &str) {

}
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/issues/issue-38763.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
pub struct Foo(i128);

#[no_mangle]
#[allow(improper_ctypes_definitions)]
pub extern "C" fn foo(x: Foo) -> Foo { x }

fn main() {
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/issues/issue-51907.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ trait Foo {

struct Bar;
impl Foo for Bar {
#[allow(improper_ctypes_definitions)]
extern fn borrow(&self) {}
#[allow(improper_ctypes_definitions)]
extern fn take(self: Box<Self>) {}
}

Expand Down
Loading