Skip to content

Commit

Permalink
Auto merge of #65134 - davidtwco:issue-19834-improper-ctypes-in-exter…
Browse files Browse the repository at this point in the history
…n-C-fn, r=rkruppe

improper_ctypes: `extern "C"` fns

cc #19834. Fixes #65867.

This pull request implements the change [described in this comment](#19834 (comment)).

cc @rkruppe @varkor @shepmaster
  • Loading branch information
bors committed Nov 6, 2019
2 parents 61a551b + 49e2403 commit 3f0e164
Show file tree
Hide file tree
Showing 18 changed files with 521 additions and 37 deletions.
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),
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 @@ -240,9 +240,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());
}

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,
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,

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

0 comments on commit 3f0e164

Please sign in to comment.