Skip to content

Commit 30fd25b

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 41aa64b commit 30fd25b

File tree

8 files changed

+1013
-18
lines changed

8 files changed

+1013
-18
lines changed

‎tests/ui/lint/improper-ctypes/allow-improper-ctypes.rs‎

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

‎tests/ui/lint/improper-ctypes/allow-improper-ctypes.stderr‎

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

‎tests/ui/lint/improper-ctypes/auxiliary/extern_crate_types.rs‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
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+
#[repr(C)]
54+
#[non_exhaustive]
55+
pub enum NonExhaustiveEnum {
56+
variant(u8),
57+
}
58+
59+
#[repr(C)]
60+
#[non_exhaustive]
61+
pub enum NonExhaustiveCEnum {
62+
variant1,
63+
variant2(()),
64+
}
65+
66+
#[repr(C)]
67+
pub enum NonExhaustiveEnumVariant {
68+
variant1,
69+
#[non_exhaustive]
70+
variant2((u32,)),
71+
}
72+
73+
extern "C" {
74+
pub fn nonexhaustivestruct_create() -> *mut NonExhaustiveStruct;
75+
pub fn nonexhaustivestruct_destroy(s: *mut NonExhaustiveStruct);
76+
pub fn nonexhaustiveenum_create() -> *mut NonExhaustiveEnum;
77+
pub fn nonexhaustiveenum_destroy(s: *mut NonExhaustiveEnum);
78+
pub fn nonexhaustivecenum_create() -> *mut NonExhaustiveCEnum;
79+
pub fn nonexhaustivecenum_destroy(s: *mut NonExhaustiveCEnum);
80+
pub fn nonexhaustiveenumvariant_create() -> *mut NonExhaustiveEnumVariant;
81+
pub fn nonexhaustiveenumvariant_destroy(s: *mut NonExhaustiveEnumVariant);
82+
83+
pub fn nonexhaustivestruct_onstack() -> NonExhaustiveStruct;
84+
pub fn nonexhaustivestruct_owned() -> NonExhaustiveStruct;
85+
}

‎tests/ui/lint/improper-ctypes/lint-nonexhaustive.rs‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
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+
// Properly deal with non_exhaustive types:
9+
// the thorny logic is expressed in https://github.com/rust-lang/rust/issues/44109#issuecomment-537583344
10+
// and its linked comments
11+
//
12+
13+
// Issue: https://github.com/rust-lang/rust/issues/132699
14+
// FFI-safe pointers to nonexhaustive structs should be FFI-safe too
15+
16+
// BEGIN: this is the exact same code as in ext_crate, to compare the lints
17+
#[repr(C)]
18+
#[non_exhaustive]
19+
pub struct OtherNonExhaustiveStruct {
20+
pub field: u8
21+
}
22+
23+
extern "C" {
24+
pub fn othernonexhaustivestruct_create() -> *mut OtherNonExhaustiveStruct;
25+
pub fn othernonexhaustivestruct_destroy(s: *mut OtherNonExhaustiveStruct);
26+
}
27+
// END
28+
29+
use ext_crate::NonExhaustiveStruct;
30+
31+
extern "C" {
32+
pub fn use_struct(s: *mut NonExhaustiveStruct);
33+
}
34+
35+
fn main() {}

0 commit comments

Comments
 (0)