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.

Sign up for GitHub

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

Jump to bottom

improper_ctypes lint doesn't detect cross-crate un-ffi-safe structs #20098

Closed
tomjakubowski opened this issue Dec 21, 2014 · 4 comments
Closed

improper_ctypes lint doesn't detect cross-crate un-ffi-safe structs #20098

tomjakubowski opened this issue Dec 21, 2014 · 4 comments
Labels
A-FFI Area: Foreign function interface (FFI) A-lint Area: Lints (warnings about flaws in source code) such as unused_mut.

Comments

@tomjakubowski
Copy link
Contributor

For example, it accepts this:

#![crate_type="lib"]
#![deny(improper_ctypes)]

extern "C" {
    pub fn foo(x: String);
}

Even though String is not FFI safe.
@tomjakubowski
Copy link
Contributor Author

I think this is fixable by just turning the def::DefTy(..) pattern in check_def to def::DefTy(..) | def::DefStruct(..). Patch for this and #19834 incoming.

@huonw huonw added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-FFI Area: Foreign function interface (FFI) labels Dec 21, 2014
@tomjakubowski tomjakubowski changed the title improper_ctypes lint doesn't detect cross-crate un-ffi-safe types improper_ctypes lint doesn't detect cross-crate un-ffi-safe structs Dec 24, 2014
@tomjakubowski tomjakubowski mentioned this issue Dec 29, 2014
Closed
tomjakubowski added a commit to tomjakubowski/rust that referenced this issue Dec 29, 2014
@tomjakubowski
Make the improper_ctypes lint more robust
5ec3ee9 
It now checks extern fns in addition to foreign fns and checks `DefStruct`
defs as well.

There is room for improvement: for example, I believe that pointers
should be safe in extern fn signatures regardless of the pointee's
representation.

The `FIXME` on `rust_begin_unwind_fmt` is because I don't think it
should be, or needs to be, C ABI (cc @eddyb) and it takes a problematic
`fmt::Arguments` structure by value.

Fix rust-lang#20098, fix rust-lang#19834
@tomjakubowski
Copy link
Contributor Author

This and #19834 are still serious and might be masking bugs in programs exporting a native interface. I'll see if I can revive my PR from a few months ago.

@tomjakubowski tomjakubowski mentioned this issue Jul 22, 2015
Merged
@TimNN
Copy link
Contributor

TimNN commented Dec 20, 2015

This seems to be fixed in the current stable: http://is.gd/Ew2jCI

@alexcrichton
Copy link
Member

Yay, thanks @TimNN!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-FFI Area: Foreign function interface (FFI) A-lint Area: Lints (warnings about flaws in source code) such as unused_mut.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make the improper_ctypes lint more robust tomjakubowski/rust
4 participants
@alexcrichton @tomjakubowski @TimNN @huonw