Auto merge of #108611 - davidtwco:issue-94223-external-abi-fn-ptr-in-internal-abi-fn, r=jackh726
lint/ctypes: ext. abi fn-ptr in internal abi fn Fixes #94223. - In the improper ctypes lint, instead of skipping functions with internal ABIs, check that the signature doesn't contain any fn-ptr types with external ABIs that aren't FFI-safe. - When computing the ABI for fn-ptr types, remove an `unwrap` that assumed FFI-safe types in foreign fn-ptr types. - I'm not certain that this is the correct approach.
This commit is contained in:
bors
commit
0ab38e95bb
@ -1379,7 +1379,29 @@ fn check_type_for_ffi_and_report_errors(
|
||||
}
|
||||
}
|
||||
|
||||
fn check_foreign_fn(&mut self, def_id: LocalDefId, decl: &hir::FnDecl<'_>) {
|
||||
/// Check if a function's argument types and result type are "ffi-safe".
|
||||
///
|
||||
/// For a external ABI function, argument types and the result type are walked to find fn-ptr
|
||||
/// types that have external ABIs, as these still need checked.
|
||||
fn check_fn(&mut self, def_id: LocalDefId, decl: &'tcx hir::FnDecl<'_>) {
|
||||
let sig = self.cx.tcx.fn_sig(def_id).subst_identity();
|
||||
let sig = self.cx.tcx.erase_late_bound_regions(sig);
|
||||
|
||||
for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) {
|
||||
for (fn_ptr_ty, span) in self.find_fn_ptr_ty_with_external_abi(input_hir, *input_ty) {
|
||||
self.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, false, false);
|
||||
}
|
||||
}
|
||||
|
||||
if let hir::FnRetTy::Return(ref ret_hir) = decl.output {
|
||||
for (fn_ptr_ty, span) in self.find_fn_ptr_ty_with_external_abi(ret_hir, sig.output()) {
|
||||
self.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, false, true);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Check if a function's argument types and result type are "ffi-safe".
|
||||
fn check_foreign_fn(&mut self, def_id: LocalDefId, decl: &'tcx hir::FnDecl<'_>) {
|
||||
let sig = self.cx.tcx.fn_sig(def_id).subst_identity();
|
||||
let sig = self.cx.tcx.erase_late_bound_regions(sig);
|
||||
|
||||
@ -1388,8 +1410,7 @@ fn check_foreign_fn(&mut self, def_id: LocalDefId, decl: &hir::FnDecl<'_>) {
|
||||
}
|
||||
|
||||
if let hir::FnRetTy::Return(ref ret_hir) = decl.output {
|
||||
let ret_ty = sig.output();
|
||||
self.check_type_for_ffi_and_report_errors(ret_hir.span, ret_ty, false, true);
|
||||
self.check_type_for_ffi_and_report_errors(ret_hir.span, sig.output(), false, true);
|
||||
}
|
||||
}
|
||||
|
||||
@ -1404,28 +1425,131 @@ fn is_internal_abi(&self, abi: SpecAbi) -> bool {
|
||||
SpecAbi::Rust | SpecAbi::RustCall | SpecAbi::RustIntrinsic | SpecAbi::PlatformIntrinsic
|
||||
)
|
||||
}
|
||||
|
||||
/// Find any fn-ptr types with external ABIs in `ty`.
|
||||
///
|
||||
/// For example, `Option<extern "C" fn()>` returns `extern "C" fn()`
|
||||
fn find_fn_ptr_ty_with_external_abi(
|
||||
&self,
|
||||
hir_ty: &hir::Ty<'tcx>,
|
||||
ty: Ty<'tcx>,
|
||||
) -> Vec<(Ty<'tcx>, Span)> {
|
||||
struct FnPtrFinder<'parent, 'a, 'tcx> {
|
||||
visitor: &'parent ImproperCTypesVisitor<'a, 'tcx>,
|
||||
spans: Vec<Span>,
|
||||
tys: Vec<Ty<'tcx>>,
|
||||
}
|
||||
|
||||
impl<'parent, 'a, 'tcx> hir::intravisit::Visitor<'_> for FnPtrFinder<'parent, 'a, 'tcx> {
|
||||
fn visit_ty(&mut self, ty: &'_ hir::Ty<'_>) {
|
||||
debug!(?ty);
|
||||
if let hir::TyKind::BareFn(hir::BareFnTy { abi, .. }) = ty.kind
|
||||
&& !self.visitor.is_internal_abi(*abi)
|
||||
{
|
||||
self.spans.push(ty.span);
|
||||
}
|
||||
|
||||
hir::intravisit::walk_ty(self, ty)
|
||||
}
|
||||
}
|
||||
|
||||
impl<'vis, 'a, 'tcx> ty::visit::TypeVisitor<TyCtxt<'tcx>> for FnPtrFinder<'vis, 'a, 'tcx> {
|
||||
type BreakTy = Ty<'tcx>;
|
||||
|
||||
fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
|
||||
if let ty::FnPtr(sig) = ty.kind() && !self.visitor.is_internal_abi(sig.abi()) {
|
||||
self.tys.push(ty);
|
||||
}
|
||||
|
||||
ty.super_visit_with(self)
|
||||
}
|
||||
}
|
||||
|
||||
let mut visitor = FnPtrFinder { visitor: &*self, spans: Vec::new(), tys: Vec::new() };
|
||||
ty.visit_with(&mut visitor);
|
||||
hir::intravisit::Visitor::visit_ty(&mut visitor, hir_ty);
|
||||
|
||||
iter::zip(visitor.tys.drain(..), visitor.spans.drain(..)).collect()
|
||||
}
|
||||
}
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDeclarations {
|
||||
fn check_foreign_item(&mut self, cx: &LateContext<'_>, it: &hir::ForeignItem<'_>) {
|
||||
fn check_foreign_item(&mut self, cx: &LateContext<'tcx>, it: &hir::ForeignItem<'tcx>) {
|
||||
let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Declaration };
|
||||
let abi = cx.tcx.hir().get_foreign_abi(it.hir_id());
|
||||
|
||||
if !vis.is_internal_abi(abi) {
|
||||
match it.kind {
|
||||
hir::ForeignItemKind::Fn(ref decl, _, _) => {
|
||||
vis.check_foreign_fn(it.owner_id.def_id, decl);
|
||||
}
|
||||
hir::ForeignItemKind::Static(ref ty, _) => {
|
||||
vis.check_foreign_static(it.owner_id, ty.span);
|
||||
}
|
||||
hir::ForeignItemKind::Type => (),
|
||||
match it.kind {
|
||||
hir::ForeignItemKind::Fn(ref decl, _, _) if !vis.is_internal_abi(abi) => {
|
||||
vis.check_foreign_fn(it.owner_id.def_id, decl);
|
||||
}
|
||||
hir::ForeignItemKind::Static(ref ty, _) if !vis.is_internal_abi(abi) => {
|
||||
vis.check_foreign_static(it.owner_id, ty.span);
|
||||
}
|
||||
hir::ForeignItemKind::Fn(ref decl, _, _) => vis.check_fn(it.owner_id.def_id, decl),
|
||||
hir::ForeignItemKind::Static(..) | hir::ForeignItemKind::Type => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ImproperCTypesDefinitions {
|
||||
fn check_ty_maybe_containing_foreign_fnptr<'tcx>(
|
||||
&mut self,
|
||||
cx: &LateContext<'tcx>,
|
||||
hir_ty: &'tcx hir::Ty<'_>,
|
||||
ty: Ty<'tcx>,
|
||||
) {
|
||||
let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition };
|
||||
for (fn_ptr_ty, span) in vis.find_fn_ptr_ty_with_external_abi(hir_ty, ty) {
|
||||
vis.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, true, false);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// `ImproperCTypesDefinitions` checks items outside of foreign items (e.g. stuff that isn't in
|
||||
/// `extern "C" { }` blocks):
|
||||
///
|
||||
/// - `extern "<abi>" fn` definitions are checked in the same way as the
|
||||
/// `ImproperCtypesDeclarations` visitor checks functions if `<abi>` is external (e.g. "C").
|
||||
/// - All other items which contain types (e.g. other functions, struct definitions, etc) are
|
||||
/// checked for extern fn-ptrs with external ABIs.
|
||||
impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDefinitions {
|
||||
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
|
||||
match item.kind {
|
||||
hir::ItemKind::Static(ty, ..)
|
||||
| hir::ItemKind::Const(ty, ..)
|
||||
| hir::ItemKind::TyAlias(ty, ..) => {
|
||||
self.check_ty_maybe_containing_foreign_fnptr(
|
||||
cx,
|
||||
ty,
|
||||
cx.tcx.type_of(item.owner_id).subst_identity(),
|
||||
);
|
||||
}
|
||||
// See `check_fn`..
|
||||
hir::ItemKind::Fn(..) => {}
|
||||
// See `check_field_def`..
|
||||
hir::ItemKind::Union(..) | hir::ItemKind::Struct(..) | hir::ItemKind::Enum(..) => {}
|
||||
// Doesn't define something that can contain a external type to be checked.
|
||||
hir::ItemKind::Impl(..)
|
||||
| hir::ItemKind::TraitAlias(..)
|
||||
| hir::ItemKind::Trait(..)
|
||||
| hir::ItemKind::OpaqueTy(..)
|
||||
| hir::ItemKind::GlobalAsm(..)
|
||||
| hir::ItemKind::ForeignMod { .. }
|
||||
| hir::ItemKind::Mod(..)
|
||||
| hir::ItemKind::Macro(..)
|
||||
| hir::ItemKind::Use(..)
|
||||
| hir::ItemKind::ExternCrate(..) => {}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_field_def(&mut self, cx: &LateContext<'tcx>, field: &'tcx hir::FieldDef<'tcx>) {
|
||||
self.check_ty_maybe_containing_foreign_fnptr(
|
||||
cx,
|
||||
field.ty,
|
||||
cx.tcx.type_of(field.def_id).subst_identity(),
|
||||
);
|
||||
}
|
||||
|
||||
fn check_fn(
|
||||
&mut self,
|
||||
cx: &LateContext<'tcx>,
|
||||
@ -1444,7 +1568,9 @@ fn check_fn(
|
||||
};
|
||||
|
||||
let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition };
|
||||
if !vis.is_internal_abi(abi) {
|
||||
if vis.is_internal_abi(abi) {
|
||||
vis.check_fn(id, decl);
|
||||
} else {
|
||||
vis.check_foreign_fn(id, decl);
|
||||
}
|
||||
}
|
||||
|
||||
@ -153,9 +153,9 @@ fn reg_component(cls: &[Option<Class>], i: &mut usize, size: Size) -> Option<Reg
|
||||
}
|
||||
}
|
||||
|
||||
fn cast_target(cls: &[Option<Class>], size: Size) -> CastTarget {
|
||||
fn cast_target(cls: &[Option<Class>], size: Size) -> Option<CastTarget> {
|
||||
let mut i = 0;
|
||||
let lo = reg_component(cls, &mut i, size).unwrap();
|
||||
let lo = reg_component(cls, &mut i, size)?;
|
||||
let offset = Size::from_bytes(8) * (i as u64);
|
||||
let mut target = CastTarget::from(lo);
|
||||
if size > offset {
|
||||
@ -164,7 +164,7 @@ fn cast_target(cls: &[Option<Class>], size: Size) -> CastTarget {
|
||||
}
|
||||
}
|
||||
assert_eq!(reg_component(cls, &mut i, Size::ZERO), None);
|
||||
target
|
||||
Some(target)
|
||||
}
|
||||
|
||||
const MAX_INT_REGS: usize = 6; // RDI, RSI, RDX, RCX, R8, R9
|
||||
@ -227,7 +227,9 @@ pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>)
|
||||
// split into sized chunks passed individually
|
||||
if arg.layout.is_aggregate() {
|
||||
let size = arg.layout.size;
|
||||
arg.cast_to(cast_target(cls, size))
|
||||
if let Some(cast_target) = cast_target(cls, size) {
|
||||
arg.cast_to(cast_target);
|
||||
}
|
||||
} else {
|
||||
arg.extend_integer_width_to(32);
|
||||
}
|
||||
|
||||
@ -1,5 +1,5 @@
|
||||
// run-pass
|
||||
#![allow(improper_ctypes)]
|
||||
#![allow(improper_ctypes, improper_ctypes_definitions)]
|
||||
|
||||
// ignore-wasm32-bare no libc to test ffi with
|
||||
|
||||
|
||||
8
tests/ui/abi/issue-94223.rs
Normal file
8
tests/ui/abi/issue-94223.rs
Normal file
@ -0,0 +1,8 @@
|
||||
// check-pass
|
||||
#![allow(improper_ctypes_definitions)]
|
||||
#![crate_type = "lib"]
|
||||
|
||||
// Check that computing the fn abi for `bad`, with a external ABI fn ptr that is not FFI-safe, does
|
||||
// not ICE.
|
||||
|
||||
pub fn bad(f: extern "C" fn([u8])) {}
|
||||
@ -1,5 +1,6 @@
|
||||
// run-pass
|
||||
|
||||
#![allow(improper_ctypes_definitions)]
|
||||
#![allow(non_camel_case_types)]
|
||||
#![allow(dead_code)]
|
||||
#![allow(unused_mut)]
|
||||
|
||||
42
tests/ui/lint/lint-ctypes-94223.rs
Normal file
42
tests/ui/lint/lint-ctypes-94223.rs
Normal file
@ -0,0 +1,42 @@
|
||||
#![crate_type = "lib"]
|
||||
#![deny(improper_ctypes_definitions)]
|
||||
|
||||
pub fn bad(f: extern "C" fn([u8])) {}
|
||||
//~^ ERROR `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
|
||||
pub fn bad_twice(f: Result<extern "C" fn([u8]), extern "C" fn([u8])>) {}
|
||||
//~^ ERROR `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
//~^^ ERROR `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
|
||||
struct BadStruct(extern "C" fn([u8]));
|
||||
//~^ ERROR `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
|
||||
enum BadEnum {
|
||||
A(extern "C" fn([u8])),
|
||||
//~^ ERROR `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
}
|
||||
|
||||
enum BadUnion {
|
||||
A(extern "C" fn([u8])),
|
||||
//~^ ERROR `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
}
|
||||
|
||||
type Foo = extern "C" fn([u8]);
|
||||
//~^ ERROR `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
|
||||
pub struct FfiUnsafe;
|
||||
|
||||
#[allow(improper_ctypes_definitions)]
|
||||
extern "C" fn f(_: FfiUnsafe) {
|
||||
unimplemented!()
|
||||
}
|
||||
|
||||
pub static BAD: extern "C" fn(FfiUnsafe) = f;
|
||||
//~^ ERROR `extern` fn uses type `FfiUnsafe`, which is not FFI-safe
|
||||
|
||||
pub static BAD_TWICE: Result<extern "C" fn(FfiUnsafe), extern "C" fn(FfiUnsafe)> = Ok(f);
|
||||
//~^ ERROR `extern` fn uses type `FfiUnsafe`, which is not FFI-safe
|
||||
//~^^ ERROR `extern` fn uses type `FfiUnsafe`, which is not FFI-safe
|
||||
|
||||
pub const BAD_CONST: extern "C" fn(FfiUnsafe) = f;
|
||||
//~^ ERROR `extern` fn uses type `FfiUnsafe`, which is not FFI-safe
|
||||
126
tests/ui/lint/lint-ctypes-94223.stderr
Normal file
126
tests/ui/lint/lint-ctypes-94223.stderr
Normal file
@ -0,0 +1,126 @@
|
||||
error: `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
--> $DIR/lint-ctypes-94223.rs:4:15
|
||||
|
|
||||
LL | pub fn bad(f: extern "C" fn([u8])) {}
|
||||
| ^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
||||
|
|
||||
= help: consider using a raw pointer instead
|
||||
= note: slices have no C equivalent
|
||||
note: the lint level is defined here
|
||||
--> $DIR/lint-ctypes-94223.rs:2:9
|
||||
|
|
||||
LL | #![deny(improper_ctypes_definitions)]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
--> $DIR/lint-ctypes-94223.rs:7:28
|
||||
|
|
||||
LL | pub fn bad_twice(f: Result<extern "C" fn([u8]), extern "C" fn([u8])>) {}
|
||||
| ^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
||||
|
|
||||
= help: consider using a raw pointer instead
|
||||
= note: slices have no C equivalent
|
||||
|
||||
error: `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
--> $DIR/lint-ctypes-94223.rs:7:49
|
||||
|
|
||||
LL | pub fn bad_twice(f: Result<extern "C" fn([u8]), extern "C" fn([u8])>) {}
|
||||
| ^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
||||
|
|
||||
= help: consider using a raw pointer instead
|
||||
= note: slices have no C equivalent
|
||||
|
||||
error: `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
--> $DIR/lint-ctypes-94223.rs:11:18
|
||||
|
|
||||
LL | struct BadStruct(extern "C" fn([u8]));
|
||||
| ^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
||||
|
|
||||
= help: consider using a raw pointer instead
|
||||
= note: slices have no C equivalent
|
||||
|
||||
error: `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
--> $DIR/lint-ctypes-94223.rs:15:7
|
||||
|
|
||||
LL | A(extern "C" fn([u8])),
|
||||
| ^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
||||
|
|
||||
= help: consider using a raw pointer instead
|
||||
= note: slices have no C equivalent
|
||||
|
||||
error: `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
--> $DIR/lint-ctypes-94223.rs:20:7
|
||||
|
|
||||
LL | A(extern "C" fn([u8])),
|
||||
| ^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
||||
|
|
||||
= help: consider using a raw pointer instead
|
||||
= note: slices have no C equivalent
|
||||
|
||||
error: `extern` fn uses type `[u8]`, which is not FFI-safe
|
||||
--> $DIR/lint-ctypes-94223.rs:24:12
|
||||
|
|
||||
LL | type Foo = extern "C" fn([u8]);
|
||||
| ^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
||||
|
|
||||
= help: consider using a raw pointer instead
|
||||
= note: slices have no C equivalent
|
||||
|
||||
error: `extern` fn uses type `FfiUnsafe`, which is not FFI-safe
|
||||
--> $DIR/lint-ctypes-94223.rs:34:17
|
||||
|
|
||||
LL | pub static BAD: extern "C" fn(FfiUnsafe) = f;
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
||||
|
|
||||
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
|
||||
= note: this struct has unspecified layout
|
||||
note: the type is defined here
|
||||
--> $DIR/lint-ctypes-94223.rs:27:1
|
||||
|
|
||||
LL | pub struct FfiUnsafe;
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: `extern` fn uses type `FfiUnsafe`, which is not FFI-safe
|
||||
--> $DIR/lint-ctypes-94223.rs:37:30
|
||||
|
|
||||
LL | pub static BAD_TWICE: Result<extern "C" fn(FfiUnsafe), extern "C" fn(FfiUnsafe)> = Ok(f);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
||||
|
|
||||
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
|
||||
= note: this struct has unspecified layout
|
||||
note: the type is defined here
|
||||
--> $DIR/lint-ctypes-94223.rs:27:1
|
||||
|
|
||||
LL | pub struct FfiUnsafe;
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: `extern` fn uses type `FfiUnsafe`, which is not FFI-safe
|
||||
--> $DIR/lint-ctypes-94223.rs:37:56
|
||||
|
|
||||
LL | pub static BAD_TWICE: Result<extern "C" fn(FfiUnsafe), extern "C" fn(FfiUnsafe)> = Ok(f);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
||||
|
|
||||
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
|
||||
= note: this struct has unspecified layout
|
||||
note: the type is defined here
|
||||
--> $DIR/lint-ctypes-94223.rs:27:1
|
||||
|
|
||||
LL | pub struct FfiUnsafe;
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: `extern` fn uses type `FfiUnsafe`, which is not FFI-safe
|
||||
--> $DIR/lint-ctypes-94223.rs:41:22
|
||||
|
|
||||
LL | pub const BAD_CONST: extern "C" fn(FfiUnsafe) = f;
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
||||
|
|
||||
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
|
||||
= note: this struct has unspecified layout
|
||||
note: the type is defined here
|
||||
--> $DIR/lint-ctypes-94223.rs:27:1
|
||||
|
|
||||
LL | pub struct FfiUnsafe;
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: aborting due to 11 previous errors
|
||||
|
||||
Loading…
Reference in New Issue
Block a user