Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 0f5b39f

Browse files
borsbors
committed
Auto merge of rust-lang#120176 - Flying-Toast:cstr_in_ffi_lint, r=cjgillot
Add a special case for CStr/CString in the improper_ctypes lint Instead of saying to "consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct", we now tell users to "Use `*const ffi::c_char` instead, and pass the value from `CStr::as_ptr()`" when the type involved is a `CStr` or a `CString`. Inspired by a conversation on the #beginners Discord channel.
2 parents 378a43a + 4ea52f1 commit 0f5b39f

File tree

6 files changed

+165
-14
lines changed

6 files changed

+165
-14
lines changed

‎compiler/rustc_lint/messages.ftl‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,11 @@ lint_improper_ctypes_box = box cannot be represented as a single pointer
283283
lint_improper_ctypes_char_help = consider using `u32` or `libc::wchar_t` instead
284284
285285
lint_improper_ctypes_char_reason = the `char` type has no C equivalent
286+
287+
lint_improper_ctypes_cstr_help =
288+
consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
289+
lint_improper_ctypes_cstr_reason = `CStr`/`CString` do not have a guaranteed layout
290+
286291
lint_improper_ctypes_dyn = trait objects have no C equivalent
287292
288293
lint_improper_ctypes_enum_repr_help =

‎compiler/rustc_lint/src/types.rs‎

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -993,6 +993,14 @@ struct ImproperCTypesVisitor<'a, 'tcx> {
993993
mode: CItemKind,
994994
}
995995

996+
/// Accumulator for recursive ffi type checking
997+
struct CTypesVisitorState<'tcx> {
998+
cache: FxHashSet<Ty<'tcx>>,
999+
/// The original type being checked, before we recursed
1000+
/// to any other types it contains.
1001+
base_ty: Ty<'tcx>,
1002+
}
1003+
9961004
enum FfiResult<'tcx> {
9971005
FfiSafe,
9981006
FfiPhantom(Ty<'tcx>),
@@ -1179,7 +1187,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
11791187
/// Checks if the given field's type is "ffi-safe".
11801188
fn check_field_type_for_ffi(
11811189
&self,
1182-
cache: &mut FxHashSet<Ty<'tcx>>,
1190+
acc: &mut CTypesVisitorState<'tcx>,
11831191
field: &ty::FieldDef,
11841192
args: GenericArgsRef<'tcx>,
11851193
) -> FfiResult<'tcx> {
@@ -1189,13 +1197,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
11891197
.tcx
11901198
.try_normalize_erasing_regions(self.cx.param_env, field_ty)
11911199
.unwrap_or(field_ty);
1192-
self.check_type_for_ffi(cache, field_ty)
1200+
self.check_type_for_ffi(acc, field_ty)
11931201
}
11941202

11951203
/// Checks if the given `VariantDef`'s field types are "ffi-safe".
11961204
fn check_variant_for_ffi(
11971205
&self,
1198-
cache: &mut FxHashSet<Ty<'tcx>>,
1206+
acc: &mut CTypesVisitorState<'tcx>,
11991207
ty: Ty<'tcx>,
12001208
def: ty::AdtDef<'tcx>,
12011209
variant: &ty::VariantDef,
@@ -1206,7 +1214,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12061214
let transparent_with_all_zst_fields = if def.repr().transparent() {
12071215
if let Some(field) = transparent_newtype_field(self.cx.tcx, variant) {
12081216
// Transparent newtypes have at most one non-ZST field which needs to be checked..
1209-
match self.check_field_type_for_ffi(cache, field, args) {
1217+
match self.check_field_type_for_ffi(acc, field, args) {
12101218
FfiUnsafe { ty, .. } if ty.is_unit() => (),
12111219
r => return r,
12121220
}
@@ -1224,7 +1232,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12241232
// We can't completely trust `repr(C)` markings, so make sure the fields are actually safe.
12251233
let mut all_phantom = !variant.fields.is_empty();
12261234
for field in &variant.fields {
1227-
all_phantom &= match self.check_field_type_for_ffi(cache, field, args) {
1235+
all_phantom &= match self.check_field_type_for_ffi(acc, field, args) {
12281236
FfiSafe => false,
12291237
// `()` fields are FFI-safe!
12301238
FfiUnsafe { ty, .. } if ty.is_unit() => false,
@@ -1244,7 +1252,11 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12441252

12451253
/// Checks if the given type is "ffi-safe" (has a stable, well-defined
12461254
/// representation which can be exported to C code).
1247-
fn check_type_for_ffi(&self, cache: &mut FxHashSet<Ty<'tcx>>, ty: Ty<'tcx>) -> FfiResult<'tcx> {
1255+
fn check_type_for_ffi(
1256+
&self,
1257+
acc: &mut CTypesVisitorState<'tcx>,
1258+
ty: Ty<'tcx>,
1259+
) -> FfiResult<'tcx> {
12481260
use FfiResult::*;
12491261

12501262
let tcx = self.cx.tcx;
@@ -1253,7 +1265,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12531265
// `struct S(*mut S);`.
12541266
// FIXME: A recursion limit is necessary as well, for irregular
12551267
// recursive types.
1256-
if !cache.insert(ty) {
1268+
if !acc.cache.insert(ty) {
12571269
return FfiSafe;
12581270
}
12591271

@@ -1275,6 +1287,17 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12751287
}
12761288
match def.adt_kind() {
12771289
AdtKind::Struct | AdtKind::Union => {
1290+
if let Some(sym::cstring_type | sym::cstr_type) =
1291+
tcx.get_diagnostic_name(def.did())
1292+
&& !acc.base_ty.is_mutable_ptr()
1293+
{
1294+
return FfiUnsafe {
1295+
ty,
1296+
reason: fluent::lint_improper_ctypes_cstr_reason,
1297+
help: Some(fluent::lint_improper_ctypes_cstr_help),
1298+
};
1299+
}
1300+
12781301
if !def.repr().c() && !def.repr().transparent() {
12791302
return FfiUnsafe {
12801303
ty,
@@ -1321,7 +1344,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
13211344
};
13221345
}
13231346

1324-
self.check_variant_for_ffi(cache, ty, def, def.non_enum_variant(), args)
1347+
self.check_variant_for_ffi(acc, ty, def, def.non_enum_variant(), args)
13251348
}
13261349
AdtKind::Enum => {
13271350
if def.variants().is_empty() {
@@ -1364,7 +1387,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
13641387
};
13651388
}
13661389

1367-
match self.check_variant_for_ffi(cache, ty, def, variant, args) {
1390+
match self.check_variant_for_ffi(acc, ty, def, variant, args) {
13681391
FfiSafe => (),
13691392
r => return r,
13701393
}
@@ -1434,9 +1457,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
14341457
FfiSafe
14351458
}
14361459

1437-
ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => self.check_type_for_ffi(cache, ty),
1460+
ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => self.check_type_for_ffi(acc, ty),
14381461

1439-
ty::Array(inner_ty, _) => self.check_type_for_ffi(cache, inner_ty),
1462+
ty::Array(inner_ty, _) => self.check_type_for_ffi(acc, inner_ty),
14401463

14411464
ty::FnPtr(sig) => {
14421465
if self.is_internal_abi(sig.abi()) {
@@ -1449,7 +1472,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
14491472

14501473
let sig = tcx.instantiate_bound_regions_with_erased(sig);
14511474
for arg in sig.inputs() {
1452-
match self.check_type_for_ffi(cache, *arg) {
1475+
match self.check_type_for_ffi(acc, *arg) {
14531476
FfiSafe => {}
14541477
r => return r,
14551478
}
@@ -1460,7 +1483,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
14601483
return FfiSafe;
14611484
}
14621485

1463-
self.check_type_for_ffi(cache, ret_ty)
1486+
self.check_type_for_ffi(acc, ret_ty)
14641487
}
14651488

14661489
ty::Foreign(..) => FfiSafe,
@@ -1583,7 +1606,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
15831606
return;
15841607
}
15851608

1586-
match self.check_type_for_ffi(&mut FxHashSet::default(), ty) {
1609+
let mut acc = CTypesVisitorState { cache: FxHashSet::default(), base_ty: ty };
1610+
match self.check_type_for_ffi(&mut acc, ty) {
15871611
FfiResult::FfiSafe => {}
15881612
FfiResult::FfiPhantom(ty) => {
15891613
self.emit_ffi_unsafe_type_lint(

‎compiler/rustc_span/src/symbol.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,7 @@ symbols! {
648648
crate_visibility_modifier,
649649
crt_dash_static: "crt-static",
650650
csky_target_feature,
651+
cstr_type,
651652
cstring_type,
652653
ctlz,
653654
ctlz_nonzero,

‎library/core/src/ffi/c_str.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ use crate::str;
8888
/// [str]: prim@str "str"
8989
#[derive(Hash)]
9090
#[stable(feature = "core_c_str", since = "1.64.0")]
91+
#[rustc_diagnostic_item = "cstr_type"]
9192
#[rustc_has_incoherent_inherent_impls]
9293
#[lang = "CStr"]
9394
// `fn from` in `impl From<&CStr> for Box<CStr>` current implementation relies

‎tests/ui/lint/lint-ctypes-cstr.rs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
#![crate_type = "lib"]
2+
#![deny(improper_ctypes, improper_ctypes_definitions)]
3+
4+
use std::ffi::{CStr, CString};
5+
6+
extern "C" {
7+
fn take_cstr(s: CStr);
8+
//~^ ERROR `extern` block uses type `CStr`, which is not FFI-safe
9+
//~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
10+
fn take_cstr_ref(s: &CStr);
11+
//~^ ERROR `extern` block uses type `CStr`, which is not FFI-safe
12+
//~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
13+
fn take_cstring(s: CString);
14+
//~^ ERROR `extern` block uses type `CString`, which is not FFI-safe
15+
//~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
16+
fn take_cstring_ref(s: &CString);
17+
//~^ ERROR `extern` block uses type `CString`, which is not FFI-safe
18+
//~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
19+
20+
fn no_special_help_for_mut_cstring(s: *mut CString);
21+
//~^ ERROR `extern` block uses type `CString`, which is not FFI-safe
22+
//~| HELP consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
23+
24+
fn no_special_help_for_mut_cstring_ref(s: &mut CString);
25+
//~^ ERROR `extern` block uses type `CString`, which is not FFI-safe
26+
//~| HELP consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
27+
}
28+
29+
extern "C" fn rust_take_cstr_ref(s: &CStr) {}
30+
//~^ ERROR `extern` fn uses type `CStr`, which is not FFI-safe
31+
//~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
32+
extern "C" fn rust_take_cstring(s: CString) {}
33+
//~^ ERROR `extern` fn uses type `CString`, which is not FFI-safe
34+
//~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
35+
extern "C" fn rust_no_special_help_for_mut_cstring(s: *mut CString) {}
36+
extern "C" fn rust_no_special_help_for_mut_cstring_ref(s: &mut CString) {}

‎tests/ui/lint/lint-ctypes-cstr.stderr‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
error: `extern` block uses type `CStr`, which is not FFI-safe
2+
--> $DIR/lint-ctypes-cstr.rs:7:21
3+
|
4+
LL | fn take_cstr(s: CStr);
5+
| ^^^^ not FFI-safe
6+
|
7+
= help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
8+
= note: `CStr`/`CString` do not have a guaranteed layout
9+
note: the lint level is defined here
10+
--> $DIR/lint-ctypes-cstr.rs:2:9
11+
|
12+
LL | #![deny(improper_ctypes, improper_ctypes_definitions)]
13+
| ^^^^^^^^^^^^^^^
14+
15+
error: `extern` block uses type `CStr`, which is not FFI-safe
16+
--> $DIR/lint-ctypes-cstr.rs:10:25
17+
|
18+
LL | fn take_cstr_ref(s: &CStr);
19+
| ^^^^^ not FFI-safe
20+
|
21+
= help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
22+
= note: `CStr`/`CString` do not have a guaranteed layout
23+
24+
error: `extern` block uses type `CString`, which is not FFI-safe
25+
--> $DIR/lint-ctypes-cstr.rs:13:24
26+
|
27+
LL | fn take_cstring(s: CString);
28+
| ^^^^^^^ not FFI-safe
29+
|
30+
= help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
31+
= note: `CStr`/`CString` do not have a guaranteed layout
32+
33+
error: `extern` block uses type `CString`, which is not FFI-safe
34+
--> $DIR/lint-ctypes-cstr.rs:16:28
35+
|
36+
LL | fn take_cstring_ref(s: &CString);
37+
| ^^^^^^^^ not FFI-safe
38+
|
39+
= help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
40+
= note: `CStr`/`CString` do not have a guaranteed layout
41+
42+
error: `extern` block uses type `CString`, which is not FFI-safe
43+
--> $DIR/lint-ctypes-cstr.rs:20:43
44+
|
45+
LL | fn no_special_help_for_mut_cstring(s: *mut CString);
46+
| ^^^^^^^^^^^^ not FFI-safe
47+
|
48+
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
49+
= note: this struct has unspecified layout
50+
51+
error: `extern` block uses type `CString`, which is not FFI-safe
52+
--> $DIR/lint-ctypes-cstr.rs:24:47
53+
|
54+
LL | fn no_special_help_for_mut_cstring_ref(s: &mut CString);
55+
| ^^^^^^^^^^^^ not FFI-safe
56+
|
57+
= help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
58+
= note: this struct has unspecified layout
59+
60+
error: `extern` fn uses type `CStr`, which is not FFI-safe
61+
--> $DIR/lint-ctypes-cstr.rs:29:37
62+
|
63+
LL | extern "C" fn rust_take_cstr_ref(s: &CStr) {}
64+
| ^^^^^ not FFI-safe
65+
|
66+
= help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
67+
= note: `CStr`/`CString` do not have a guaranteed layout
68+
note: the lint level is defined here
69+
--> $DIR/lint-ctypes-cstr.rs:2:26
70+
|
71+
LL | #![deny(improper_ctypes, improper_ctypes_definitions)]
72+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
73+
74+
error: `extern` fn uses type `CString`, which is not FFI-safe
75+
--> $DIR/lint-ctypes-cstr.rs:32:36
76+
|
77+
LL | extern "C" fn rust_take_cstring(s: CString) {}
78+
| ^^^^^^^ not FFI-safe
79+
|
80+
= help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
81+
= note: `CStr`/`CString` do not have a guaranteed layout
82+
83+
error: aborting due to 8 previous errors
84+

0 commit comments

Comments
 (0)