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: extern "C" fns #65134

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
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 @@
// 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,
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 @@ -69,6 +69,7 @@ mod dwarf;
// 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),
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
data: *mut u8,
data_ptr: *mut usize,
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_codegen_llvm/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,7 @@ impl<'a> Drop for DiagnosticHandlers<'a> {
}
}

unsafe extern "C" fn report_inline_asm(cgcx: &CodegenContext<LlvmCodegenBackend>,
msg: &str,
cookie: c_uint) {
fn report_inline_asm(cgcx: &CodegenContext<LlvmCodegenBackend>, msg: &str, cookie: c_uint) {
cgcx.diag_emitter.inline_asm_error(cookie as u32, msg.to_owned());
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
1 change: 1 addition & 0 deletions src/librustc_codegen_llvm/llvm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
ptr: *const c_char,
Expand Down
102 changes: 69 additions & 33 deletions src/librustc_lint/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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,
davidtwco marked this conversation as resolved.
Show resolved Hide resolved

ty::Infer(..) |
ty::Bound(..) |
ty::Error |
Expand All @@ -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),
}
Expand All @@ -892,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 {
Expand All @@ -911,9 +915,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
diag.emit();
}

fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool {
use crate::rustc::ty::TypeFoldable;

fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>, is_foreign_item: bool) -> bool {
struct ProhibitOpaqueTypes<'tcx> {
ty: Option<Ty<'tcx>>,
};
Expand All @@ -937,70 +939,81 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
sp,
"opaque types have no C equivalent",
None,
is_foreign_item,
);
true
} else {
false
}
}

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

// 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) => {
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);
}
}
}

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 {
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);
vis.check_foreign_fn(it.hir_id, decl, true);
}
hir::ForeignItemKind::Static(ref ty, _) => {
vis.check_foreign_static(it.hir_id, ty.span);
Expand All @@ -1009,6 +1022,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, false);
}
}
}

declare_lint_pass!(VariantSizeDifferences => [VARIANT_SIZE_DIFFERENCES]);
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 @@ -53,6 +53,7 @@ unsafe extern "C" fn tcs_init(secondary: bool) {
// (main function exists). If this is a library, the crate author should be
// able to specify this
#[cfg(not(test))]
#[allow(improper_ctypes)]
#[no_mangle]
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?
Expand Down
3 changes: 3 additions & 0 deletions src/libstd/sys/sgx/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ const EINVAL: i32 = 22;

#[cfg(not(test))]
#[no_mangle]
#[allow(improper_ctypes)]
pub unsafe extern "C" fn __rust_rwlock_rdlock(p: *mut RWLock) -> i32 {
if p.is_null() {
return EINVAL;
Expand All @@ -181,6 +182,7 @@ pub unsafe extern "C" fn __rust_rwlock_rdlock(p: *mut RWLock) -> i32 {
}

#[cfg(not(test))]
#[allow(improper_ctypes)]
#[no_mangle]
pub unsafe extern "C" fn __rust_rwlock_wrlock(p: *mut RWLock) -> i32 {
if p.is_null() {
Expand All @@ -190,6 +192,7 @@ pub unsafe extern "C" fn __rust_rwlock_wrlock(p: *mut RWLock) -> i32 {
return 0;
}
#[cfg(not(test))]
#[allow(improper_ctypes)]
#[no_mangle]
pub unsafe extern "C" fn __rust_rwlock_unlock(p: *mut RWLock) -> i32 {
if p.is_null() {
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 @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/align-with-extern-c-fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#![feature(repr_align)]

#[repr(align(16))]
#[repr(align(16), C)]
pub struct A(i64);

pub extern "C" fn foo(x: A) {}
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)]
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)]
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 @@ -4,6 +4,7 @@
struct Test;

impl Test {
#[allow(improper_ctypes)]
#[allow(dead_code)]
#[allow(unused_variables)]
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 @@ -4,6 +4,7 @@
#[repr(C)]
pub struct Foo(i128);

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

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)]
extern fn borrow(&self) {}
#[allow(improper_ctypes)]
extern fn take(self: Box<Self>) {}
}

Expand Down
Loading