Skip to content

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

Closed
Closed
improper_ctypes lint should check types referenced in C ABI function definitions#19834
Assignees
shepmaster
Labels
A-FFIArea: Foreign function interface (FFI)A-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.C-bugCategory: This is a bug.E-mentorCall for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Description

@tomjakubowski
tomjakubowski
opened on Dec 14, 2014

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.

Activity

  1. jdm
    added
    A-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.
    I-wrong
    on Dec 14, 2014

  2. tomjakubowski commented on Dec 20, 2014

    @tomjakubowski
    tomjakubowski
    on Dec 20, 2014
    ContributorAuthor

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

  3. DanielKeep commented on Sep 28, 2015

    @DanielKeep
    DanielKeep
    on Sep 28, 2015
    Contributor

    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.

  4. arielb1 commented on Sep 28, 2015

    @arielb1
    arielb1
    on Sep 28, 2015
    Contributor

    @DanielKeep

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

  5. TimNN commented on Sep 28, 2015

    @TimNN
    TimNN
    on Sep 28, 2015
    Contributor

    @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

  6. shepmaster commented on Sep 16, 2016

    @shepmaster
    shepmaster
    on Sep 16, 2016
    Member

    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.

  7. shepmaster commented on Sep 16, 2016

    @shepmaster
    shepmaster
    on Sep 16, 2016
    Member

    Should we start some kind of RFC for this?

    /cc @nagisa from comment

  8. 18 remaining items

  9. shepmaster commented on Jun 3, 2019

    @shepmaster
    shepmaster
    on Jun 3, 2019
    Member

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

  10. davidtwco commented on Oct 5, 2019

    @davidtwco
    davidtwco
    on Oct 5, 2019
    Member

    @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?

  11. hanna-kruppe commented on Oct 5, 2019

    @hanna-kruppe
    hanna-kruppe
    on Oct 5, 2019
    Contributor

    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.

  12. davidtwco commented on Oct 5, 2019

    @davidtwco
    davidtwco
    on Oct 5, 2019
    Member

    @rkruppe opened as #65134

  13. Dylan-DPC-zz
    added a commit that references this issue on Jun 23, 2020

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

    24c460f
  14. Dylan-DPC-zz
    added 2 commits that reference this issue on Jun 25, 2020

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

    6940e7b

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

    67db7a2

  15. davidtwco commented on Jul 2, 2020

    @davidtwco
    davidtwco
    on Jul 2, 2020
    Member

    Closing, extern fns are handled in #72700.

  16. davidtwco
    closed this as completedon Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

A-FFIArea: Foreign function interface (FFI)A-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.C-bugCategory: This is a bug.E-mentorCall for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions