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.

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

#[repr(C)] and Drop causes undefined behavior in FFI code #18380

Closed
mahkoh opened this issue Oct 28, 2014 · 6 comments
Closed

#[repr(C)] and Drop causes undefined behavior in FFI code #18380

mahkoh opened this issue Oct 28, 2014 · 6 comments
Labels
A-FFI Area: Foreign function interface (FFI) A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. P-medium Medium priority

Comments

@mahkoh
Copy link
Contributor

mahkoh commented Oct 28, 2014

extern {
    fn f(x: *const X);
}

#[repr(C)]
struct X {
    i: i32,
}

impl Drop for X {
    fn drop(&mut self) { }
}

fn main() {
    let x = [X { i: 0 }, X { i: 0 }];
    println!("{}", std::mem::size_of_val(&x));
    unsafe { f(x.as_ptr()); }
}

Prints 16.

@bstrie bstrie added I-wrong A-FFI Area: Foreign function interface (FFI) labels Oct 28, 2014
@huonw huonw added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Nov 4, 2014
@huonw
Copy link
Member

huonw commented Nov 4, 2014

It seems like the improper_ctypes lint should check for repr(C) on the RHS of a Drop impl. "warning: implementing Drop changes the representation of the type, conflicting with repr(C)".

(It should presumably also check for #[unsafe_no_drop_flag] and not warn if it exists.)

@mahkoh
Copy link
Contributor Author

mahkoh commented Jan 12, 2015

This has to be fixed before 1.0.

@pnkfelix
Copy link
Member

pnkfelix commented Feb 3, 2015

nominating; we are unlikely to implement non-zeroing dynamic drop before the 1.0 release (and certainly not before the 1.0 beta). So we probably need to address tis in some way, perhaps a lint as @huonw suggests.

Nominating for 1.0 polish.

@brson brson added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Feb 5, 2015
@brson
Copy link
Contributor

brson commented Feb 5, 2015

Adding a lint is easy.

@nikomatsakis
Copy link
Contributor

This is to some extent a dup of #5016

@pnkfelix
Copy link
Member

pnkfelix commented Feb 5, 2015

We won't object to someone adding a lint (which would be temporary assuming nonzeroing drop lands eventually).

But it also wont block the release.

P-high, not 1.0.

@pnkfelix pnkfelix added P-medium Medium priority and removed I-nominated labels Feb 5, 2015
@mahkoh mahkoh closed this as completed Apr 11, 2015
@rust-lang rust-lang locked and limited conversation to collaborators Apr 11, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-FFI Area: Foreign function interface (FFI) A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

6 participants