improper_ctypes lint should check types referenced in C ABI function definitions

For example, this compiles with no warnings or errors:

#![feature(lang_items)]
#![crate_type="staticlib"]
#![allow(missing_copy_implementations)]
#![deny(improper_ctypes)]
#![no_std]

pub struct Blah {
    pub x: i32
}

#[no_mangle]
pub extern "C" fn yikes(b: Blah) -> i32 {
    b.x
}

#[lang = "copy"] trait Copy {}
#[lang = "sized"] trait Sized {}
#[lang = "stack_exhausted"] extern fn stack_exhausted() {}
#[lang = "eh_personality"] extern fn eh_personality() {}
#[lang = "panic_fmt"] fn panic_fmt() -> ! { loop {} }

Even though Blah lacks the #[repr(C)] attribute.

To clarify: foreign functions (fn inside an extern block) are checked. Regular functions with a C ABI are not.

This is still an issue. It just came up in a Stack Overflow question, with something roughly equivalent to this:

#[no_mangle]
pub extern fn print(s: String) {
    println!("{}", s);
}

This produces no warnings. For context, the user was trying to bind to this function from C#, which went about as well as you'd expect.

@DanielKeep

@DanielKeep

Its even worse here - this is an extern "Rust" function, and these have their own ABI.

@arielb1

@arielb1 https://github.com/rust-lang/rust/blob/master/src/doc/trpl/ffi.md#calling-rust-code-from-c seems to imply that a pub extern fn something() {} uses the C calling convention by default.

Edit: and the reference seems to agree: https://github.com/rust-lang/rust/blob/master/src/doc/reference.md#extern-functions

Continues to be a problem for people new to FFI:

#[no_mangle]
pub extern "C" fn hello(cs: CString) -> CString {
  // ...
}

The OP states:

Well, my reasoning is that if I can pass int32 back and forth + if CString can easily go in and out (commented code), then it should work.

If this generated a warning, there's more of a chance people might not do this.

@nagisa

Should we start some kind of RFC for this?

/cc @nagisa from comment

I have not... it's just been sitting in a tab, waiting, lonely. If someone wants to steal it, please feel free!

@varkor

@varkor @shepmaster I had a go at implementing the instructions in issue-19834-improper-ctypes-in-extern-C-fn, but quickly ran into problems:

I had to #[allow(improper_ctypes)] on a bunch of places within the libproc_macro, libstd, libpanic_abort and libpanic_unwind.
typeck currently only prohibits generics on foreign functions, so by allowing improper_ctypes on regular functions (that have extern "C"), you quickly run into something like ICE: src/librustc_lint/types.rs:858: unexpected type in foreign function: T #65035 - I added basic support for generics to the improper_ctypes lint so I could #[allow(improper_ctypes)] away the remaining errors.

The branch doesn't fail any tests at the moment. Do either of you think it's worth continuing with this and opening a PR?

Neither of these problems is a showstopper in my opinion, please continue and open a PR. Also, I have comments about both of the issues you identified and how you addressed them but it would probably best to discuss that on the PR.

@rkruppe opened as #65134

Rollup merge of rust-lang#72700 - davidtwco:issue-66220-improper-ctyp…

Closing, extern fns are handled in #72700.

Navigation Menu

Repository navigation

improper_ctypes lint should check types referenced in C ABI function definitions #19834

18 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

improper_ctypes lint should check types referenced in C ABI function definitions #19834

Description

Activity

tomjakubowski commented on Dec 20, 2014

DanielKeep commented on Sep 28, 2015

arielb1 commented on Sep 28, 2015

TimNN commented on Sep 28, 2015

shepmaster commented on Sep 16, 2016

shepmaster commented on Sep 16, 2016

18 remaining items

shepmaster commented on Jun 3, 2019

davidtwco commented on Oct 5, 2019

hanna-kruppe commented on Oct 5, 2019

davidtwco commented on Oct 5, 2019

davidtwco commented on Jul 2, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions