Skip to content

Commit de4ab83

Browse files
niacdoialniacdoial
committed
ImproperCTypes: add tests
- ensure proper coverage of as many edge cases in the type checking as possible - test for an issue that was fixed by this commit chain - understand where `[allow(improper_c*)]` needs to be to take effect (current behaviour not ideal, one should be able to flag a struct definitition as safe anyway)
1 parent f48dc94 commit de4ab83

File tree

6 files changed

+967
-0
lines changed

6 files changed

+967
-0
lines changed

‎tests/ui/lint/improper_ctypes/allow_improper_ctypes.rs‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
#![deny(improper_ctypes, improper_c_fn_definitions)]
2+
#![deny(improper_c_callbacks)]
3+
4+
//@ aux-build: extern_crate_types.rs
5+
//@ compile-flags:--extern extern_crate_types
6+
extern crate extern_crate_types as ext_crate;
7+
8+
// ////////////////////////////////////////////////////////
9+
// first, the same bank of types as in the extern crate
10+
11+
// FIXME: maybe re-introduce improper_ctype_definitions (ctype singular)
12+
// as a way to mark ADTs as "let's ignore that they are not actually FFI-unsafe"
13+
14+
#[repr(C)]
15+
struct SafeStruct (i32);
16+
17+
#[repr(C)]
18+
struct UnsafeStruct (String);
19+
20+
#[repr(C)]
21+
//#[allow(improper_ctype_definitions)]
22+
struct AllowedUnsafeStruct (String);
23+
24+
// refs are only unsafe if the value comes from the other side of the FFI boundary
25+
// due to the non-null assumption
26+
// (technically there are also assumptions about non-dandling, alignment,
27+
// aliasing, lifetimes, etc...)
28+
// the lint is not raised here, but will be if used in the wrong place
29+
#[repr(C)]
30+
struct UnsafeFromForeignStruct<'a> (&'a u32);
31+
32+
#[repr(C)]
33+
//#[allow(improper_ctype_definitions)]
34+
struct AllowedUnsafeFromForeignStruct<'a> (&'a u32);
35+
36+
37+
type SafeFnPtr = extern "C" fn(i32)->i32;
38+
39+
type UnsafeFnPtr = extern "C" fn((i32, i32))->i32;
40+
//~^ ERROR: `extern` callback uses type `(i32, i32)`
41+
42+
43+
// for now, let's not lint on the nonzero assumption,
44+
// because:
45+
// - we don't know if the callback is rust-callee-foreign-caller or the other way around
46+
// - having to cast around function signatures to get function pointers
47+
// would be an awful experience
48+
// so, let's assume that the unsafety in this fnptr
49+
// will be pointed out indirectly by a lint elsewhere
50+
// (note: there's one case where the error would be missed altogether:
51+
// a rust-caller,non-rust-callee callback where the fnptr
52+
// is given as an argument to a rust-callee,non-rust-caller
53+
// FFI boundary)
54+
#[allow(improper_c_callbacks)]
55+
type AllowedUnsafeFnPtr = extern "C" fn(&[i32])->i32;
56+
57+
type UnsafeRustCalleeFnPtr = extern "C" fn(i32)->&'static i32;
58+
59+
#[allow(improper_c_callbacks)]
60+
type AllowedUnsafeRustCalleeFnPtr = extern "C" fn(i32)->&'static i32;
61+
62+
type UnsafeForeignCalleeFnPtr = extern "C" fn(&i32);
63+
64+
#[allow(improper_c_callbacks)]
65+
type AllowedUnsafeForeignCalleeFnPtr = extern "C" fn(&i32);
66+
67+
68+
// ////////////////////////////////////////////////////////
69+
// then, some functions that use them
70+
71+
static INT: u32 = 42;
72+
73+
#[allow(improper_c_fn_definitions)]
74+
extern "C" fn fn1a(e: &String) -> &str {&*e}
75+
extern "C" fn fn1u(e: &String) -> &str {&*e}
76+
//~^ ERROR: `extern` fn uses type `&str`
77+
// | FIXME: not warning about the &String feels wrong, but it's behind a FFI-safe reference so...
78+
79+
#[allow(improper_c_fn_definitions)]
80+
extern "C" fn fn2a(e: UnsafeStruct) {}
81+
extern "C" fn fn2u(e: UnsafeStruct) {}
82+
//~^ ERROR: `extern` fn uses type `UnsafeStruct`
83+
#[allow(improper_c_fn_definitions)]
84+
extern "C" fn fn2oa(e: ext_crate::UnsafeStruct) {}
85+
extern "C" fn fn2ou(e: ext_crate::UnsafeStruct) {}
86+
//~^ ERROR: `extern` fn uses type `ext_crate::UnsafeStruct`
87+
88+
#[allow(improper_c_fn_definitions)]
89+
extern "C" fn fn3a(e: AllowedUnsafeStruct) {}
90+
extern "C" fn fn3u(e: AllowedUnsafeStruct) {}
91+
//~^ ERROR: `extern` fn uses type `AllowedUnsafeStruct`
92+
// ^^ FIXME: ...ideally the lint should not trigger here
93+
#[allow(improper_c_fn_definitions)]
94+
extern "C" fn fn3oa(e: ext_crate::AllowedUnsafeStruct) {}
95+
extern "C" fn fn3ou(e: ext_crate::AllowedUnsafeStruct) {}
96+
//~^ ERROR: `extern` fn uses type `ext_crate::AllowedUnsafeStruct`
97+
// ^^ FIXME: ...ideally the lint should not trigger here
98+
99+
#[allow(improper_c_fn_definitions)]
100+
extern "C" fn fn4a(e: UnsafeFromForeignStruct) {}
101+
extern "C" fn fn4u(e: UnsafeFromForeignStruct) {}
102+
#[allow(improper_c_fn_definitions)]
103+
extern "C" fn fn4oa(e: ext_crate::UnsafeFromForeignStruct) {}
104+
extern "C" fn fn4ou(e: ext_crate::UnsafeFromForeignStruct) {}
105+
// the block above might become unsafe if/once we lint on the value assumptions of types
106+
107+
#[allow(improper_c_fn_definitions)]
108+
extern "C" fn fn5a() -> UnsafeFromForeignStruct<'static> { UnsafeFromForeignStruct(&INT)}
109+
extern "C" fn fn5u() -> UnsafeFromForeignStruct<'static> { UnsafeFromForeignStruct(&INT)}
110+
#[allow(improper_c_fn_definitions)]
111+
extern "C" fn fn5oa() -> ext_crate::UnsafeFromForeignStruct<'static> {
112+
ext_crate::UnsafeFromForeignStruct(&INT)
113+
}
114+
extern "C" fn fn5ou() -> ext_crate::UnsafeFromForeignStruct<'static> {
115+
ext_crate::UnsafeFromForeignStruct(&INT)
116+
}
117+
118+
#[allow(improper_c_fn_definitions)]
119+
extern "C" fn fn6a() -> AllowedUnsafeFromForeignStruct<'static> {
120+
AllowedUnsafeFromForeignStruct(&INT)
121+
}
122+
extern "C" fn fn6u() -> AllowedUnsafeFromForeignStruct<'static> {
123+
AllowedUnsafeFromForeignStruct(&INT)
124+
}
125+
#[allow(improper_c_fn_definitions)]
126+
extern "C" fn fn6oa() -> ext_crate::AllowedUnsafeFromForeignStruct<'static> {
127+
ext_crate::AllowedUnsafeFromForeignStruct(&INT)
128+
}
129+
extern "C" fn fn6ou() -> ext_crate::AllowedUnsafeFromForeignStruct<'static> {
130+
ext_crate::AllowedUnsafeFromForeignStruct(&INT)
131+
}
132+
133+
// ////////////////////////////////////////////////////////
134+
// special cases: struct-in-fnptr and fnptr-in-struct
135+
136+
#[repr(C)]
137+
struct FakeVTable<A>{
138+
make_new: extern "C" fn() -> A,
139+
combine: extern "C" fn(&[A]) -> A,
140+
//~^ ERROR: `extern` callback uses type `&[A]`
141+
drop: extern "C" fn(A),
142+
something_else: (A, usize),
143+
}
144+
145+
type FakeVTableMaker = extern "C" fn() -> FakeVTable<u32>;
146+
//~^ ERROR: `extern` callback uses type `FakeVTable<u32>`
147+
148+
#[repr(C)]
149+
#[allow(improper_c_callbacks)]
150+
struct FakeVTableAllowed<A>{
151+
make_new: extern "C" fn() -> A,
152+
combine: extern "C" fn(&[A]) -> A,
153+
drop: extern "C" fn(A),
154+
something_else: (A, usize),
155+
}
156+
157+
#[allow(improper_c_callbacks)]
158+
type FakeVTableMakerAllowed = extern "C" fn() -> FakeVTable<u32>;
159+
160+
fn main(){}

‎tests/ui/lint/improper_ctypes/allow_improper_ctypes.stderr‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
error: `extern` callback uses type `(i32, i32)`, which is not FFI-safe
2+
--> $DIR/allow_improper_ctypes.rs:39:20
3+
|
4+
LL | type UnsafeFnPtr = extern "C" fn((i32, i32))->i32;
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
6+
|
7+
= note: the function pointer to `extern "C" fn((i32, i32)) -> i32` is FFI-unsafe due to `(i32, i32)`
8+
= help: consider using a struct instead
9+
= note: tuples have unspecified layout
10+
note: the lint level is defined here
11+
--> $DIR/allow_improper_ctypes.rs:2:9
12+
|
13+
LL | #![deny(improper_c_callbacks)]
14+
| ^^^^^^^^^^^^^^^^^^^^
15+
16+
error: `extern` fn uses type `&str`, which is not FFI-safe
17+
--> $DIR/allow_improper_ctypes.rs:75:35
18+
|
19+
LL | extern "C" fn fn1u(e: &String) -> &str {&*e}
20+
| ^^^^ not FFI-safe
21+
|
22+
= help: consider using `*const u8` and a length instead
23+
= note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer
24+
note: the lint level is defined here
25+
--> $DIR/allow_improper_ctypes.rs:1:26
26+
|
27+
LL | #![deny(improper_ctypes, improper_c_fn_definitions)]
28+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
29+
30+
error: `extern` fn uses type `UnsafeStruct`, which is not FFI-safe
31+
--> $DIR/allow_improper_ctypes.rs:81:23
32+
|
33+
LL | extern "C" fn fn2u(e: UnsafeStruct) {}
34+
| ^^^^^^^^^^^^ not FFI-safe
35+
|
36+
= note: this struct/enum/union (`UnsafeStruct`) is FFI-unsafe due to a `String` field
37+
note: the type is defined here
38+
--> $DIR/allow_improper_ctypes.rs:18:1
39+
|
40+
LL | struct UnsafeStruct (String);
41+
| ^^^^^^^^^^^^^^^^^^^
42+
= help: consider adding a `#[repr(C)]` (not `#[repr(C,packed)]`) or `#[repr(transparent)]` attribute to `String`
43+
= note: `String` has unspecified layout
44+
45+
error: `extern` fn uses type `ext_crate::UnsafeStruct`, which is not FFI-safe
46+
--> $DIR/allow_improper_ctypes.rs:85:24
47+
|
48+
LL | extern "C" fn fn2ou(e: ext_crate::UnsafeStruct) {}
49+
| ^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
50+
|
51+
= note: this struct/enum/union (`ext_crate::UnsafeStruct`) is FFI-unsafe due to a `String` field
52+
= help: consider adding a `#[repr(C)]` (not `#[repr(C,packed)]`) or `#[repr(transparent)]` attribute to `String`
53+
= note: `String` has unspecified layout
54+
55+
error: `extern` fn uses type `AllowedUnsafeStruct`, which is not FFI-safe
56+
--> $DIR/allow_improper_ctypes.rs:90:23
57+
|
58+
LL | extern "C" fn fn3u(e: AllowedUnsafeStruct) {}
59+
| ^^^^^^^^^^^^^^^^^^^ not FFI-safe
60+
|
61+
= note: this struct/enum/union (`AllowedUnsafeStruct`) is FFI-unsafe due to a `String` field
62+
note: the type is defined here
63+
--> $DIR/allow_improper_ctypes.rs:22:1
64+
|
65+
LL | struct AllowedUnsafeStruct (String);
66+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
67+
= help: consider adding a `#[repr(C)]` (not `#[repr(C,packed)]`) or `#[repr(transparent)]` attribute to `String`
68+
= note: `String` has unspecified layout
69+
70+
error: `extern` fn uses type `ext_crate::AllowedUnsafeStruct`, which is not FFI-safe
71+
--> $DIR/allow_improper_ctypes.rs:95:24
72+
|
73+
LL | extern "C" fn fn3ou(e: ext_crate::AllowedUnsafeStruct) {}
74+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
75+
|
76+
= note: this struct/enum/union (`ext_crate::AllowedUnsafeStruct`) is FFI-unsafe due to a `String` field
77+
= help: consider adding a `#[repr(C)]` (not `#[repr(C,packed)]`) or `#[repr(transparent)]` attribute to `String`
78+
= note: `String` has unspecified layout
79+
80+
error: `extern` callback uses type `&[A]`, which is not FFI-safe
81+
--> $DIR/allow_improper_ctypes.rs:139:14
82+
|
83+
LL | combine: extern "C" fn(&[A]) -> A,
84+
| ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
85+
|
86+
= note: the function pointer to `for<'a> extern "C" fn(&'a [A]) -> A` is FFI-unsafe due to `&[A]`
87+
= help: consider using a raw pointer to the slice's first element (and a length) instead
88+
= note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer
89+
90+
error: `extern` callback uses type `FakeVTable<u32>`, which is not FFI-safe
91+
--> $DIR/allow_improper_ctypes.rs:145:24
92+
|
93+
LL | type FakeVTableMaker = extern "C" fn() -> FakeVTable<u32>;
94+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
95+
|
96+
= note: the function pointer to `extern "C" fn() -> FakeVTable<u32>` is FFI-unsafe due to `FakeVTable<u32>`
97+
= note: this struct/enum/union (`FakeVTable<u32>`) is FFI-unsafe due to a `(u32, usize)` field
98+
note: the type is defined here
99+
--> $DIR/allow_improper_ctypes.rs:137:1
100+
|
101+
LL | struct FakeVTable<A>{
102+
| ^^^^^^^^^^^^^^^^^^^^
103+
= help: consider using a struct instead
104+
= note: tuples have unspecified layout
105+
106+
error: aborting due to 8 previous errors
107+

‎tests/ui/lint/improper_ctypes/auxiliary/extern_crate_types.rs‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/// a bank of types (structs, function pointers) that are safe or unsafe for whatever reason,
2+
/// with or without said unsafety being explicitely ignored
3+
4+
#[repr(C)]
5+
pub struct SafeStruct (pub i32);
6+
7+
#[repr(C)]
8+
pub struct UnsafeStruct (pub String);
9+
10+
#[repr(C)]
11+
//#[allow(improper_ctype_definitions)]
12+
pub struct AllowedUnsafeStruct (pub String);
13+
14+
// refs are only unsafe if the value comes from the other side of the FFI boundary
15+
// due to the non-null assumption
16+
// (technically there are also assumptions about non-dandling, alignment, aliasing,
17+
// lifetimes, etc...)
18+
#[repr(C)]
19+
pub struct UnsafeFromForeignStruct<'a> (pub &'a u32);
20+
21+
#[repr(C)]
22+
//#[allow(improper_ctype_definitions)]
23+
pub struct AllowedUnsafeFromForeignStruct<'a> (pub &'a u32);
24+
25+
26+
pub type SafeFnPtr = extern "C" fn(i32)->i32;
27+
28+
pub type UnsafeFnPtr = extern "C" fn((i32,i32))->i32;
29+
30+
#[allow(improper_c_callbacks)]
31+
pub type AllowedUnsafeFnPtr = extern "C" fn(&[i32])->i32;
32+
33+
pub type UnsafeRustCalleeFnPtr = extern "C" fn(i32)->&'static i32;
34+
35+
#[allow(improper_c_callbacks)]
36+
pub type AllowedUnsafeRustCalleeFnPtr = extern "C" fn(i32)->&'static i32;
37+
38+
pub type UnsafeForeignCalleeFnPtr = extern "C" fn(&i32);
39+
40+
#[allow(improper_c_callbacks)]
41+
pub type AllowedUnsafeForeignCalleeFnPtr = extern "C" fn(&i32);
42+
43+
44+
// ////////////////////////////////////
45+
/// types used in specific issue-based tests that need extern-crate types
46+
47+
#[repr(C)]
48+
#[non_exhaustive]
49+
pub struct NonExhaustiveStruct {
50+
pub field: u8
51+
}
52+
53+
extern "C" {
54+
pub fn nonexhaustivestruct_create() -> *mut NonExhaustiveStruct;
55+
pub fn nonexhaustivestruct_destroy(s: *mut NonExhaustiveStruct);
56+
}

‎tests/ui/lint/improper_ctypes/issue-nonexhaust-ptr.rs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//@ check-pass
2+
#![deny(improper_ctypes)]
3+
4+
//@ aux-build: extern_crate_types.rs
5+
//@ compile-flags:--extern extern_crate_types
6+
extern crate extern_crate_types as ext_crate;
7+
8+
9+
// Issue: https://github.com/rust-lang/rust/issues/132699
10+
// FFI-safe pointers to nonexhaustive structs should be FFI-safe too
11+
12+
// BEGIN: this is the exact same code as in ext_crate, to compare the lints
13+
#[repr(C)]
14+
#[non_exhaustive]
15+
pub struct OtherNonExhaustiveStruct {
16+
pub field: u8
17+
}
18+
19+
extern "C" {
20+
pub fn othernonexhaustivestruct_create() -> *mut OtherNonExhaustiveStruct;
21+
pub fn othernonexhaustivestruct_destroy(s: *mut OtherNonExhaustiveStruct);
22+
}
23+
// END
24+
25+
use ext_crate::NonExhaustiveStruct;
26+
27+
extern "C" {
28+
pub fn use_struct(s: *mut NonExhaustiveStruct);
29+
}
30+
31+
fn main() {}

0 commit comments

Comments
 (0)