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

Commit b335ec9

Browse files
Flying-Toastjieyouxu
Flying-Toast
and
jieyouxu
committed
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`. Co-authored-by: Jieyou Xu <[email protected]>
1 parent 93ea767 commit b335ec9

File tree

7 files changed

+172
-21
lines changed

7 files changed

+172
-21
lines changed

‎compiler/rustc_lint/messages.ftl

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,11 @@ lint_improper_ctypes_box = box cannot be represented as a single pointer
361361
lint_improper_ctypes_char_help = consider using `u32` or `libc::wchar_t` instead
362362
363363
lint_improper_ctypes_char_reason = the `char` type has no C equivalent
364+
365+
lint_improper_ctypes_cstr_help =
366+
consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
367+
lint_improper_ctypes_cstr_reason = `CStr`/`CString` do not have a guaranteed layout
368+
364369
lint_improper_ctypes_dyn = trait objects have no C equivalent
365370
366371
lint_improper_ctypes_enum_repr_help =

‎compiler/rustc_lint/src/types.rs

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -984,6 +984,14 @@ struct ImproperCTypesVisitor<'a, 'tcx> {
984984
mode: CItemKind,
985985
}
986986

987+
/// Accumulator for recursive ffi type checking
988+
struct CTypesVisitorState<'tcx> {
989+
cache: FxHashSet<Ty<'tcx>>,
990+
/// The original type being checked, before we recursed
991+
/// to any other types it contains.
992+
base_ty: Ty<'tcx>,
993+
}
994+
987995
enum FfiResult<'tcx> {
988996
FfiSafe,
989997
FfiPhantom(Ty<'tcx>),
@@ -1212,7 +1220,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12121220
/// Checks if the given field's type is "ffi-safe".
12131221
fn check_field_type_for_ffi(
12141222
&self,
1215-
cache: &mut FxHashSet<Ty<'tcx>>,
1223+
acc: &mut CTypesVisitorState<'tcx>,
12161224
field: &ty::FieldDef,
12171225
args: GenericArgsRef<'tcx>,
12181226
) -> FfiResult<'tcx> {
@@ -1222,13 +1230,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12221230
.tcx
12231231
.try_normalize_erasing_regions(self.cx.param_env, field_ty)
12241232
.unwrap_or(field_ty);
1225-
self.check_type_for_ffi(cache, field_ty)
1233+
self.check_type_for_ffi(acc, field_ty)
12261234
}
12271235

12281236
/// Checks if the given `VariantDef`'s field types are "ffi-safe".
12291237
fn check_variant_for_ffi(
12301238
&self,
1231-
cache: &mut FxHashSet<Ty<'tcx>>,
1239+
acc: &mut CTypesVisitorState<'tcx>,
12321240
ty: Ty<'tcx>,
12331241
def: ty::AdtDef<'tcx>,
12341242
variant: &ty::VariantDef,
@@ -1238,7 +1246,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12381246
let transparent_with_all_zst_fields = if def.repr().transparent() {
12391247
if let Some(field) = transparent_newtype_field(self.cx.tcx, variant) {
12401248
// Transparent newtypes have at most one non-ZST field which needs to be checked..
1241-
match self.check_field_type_for_ffi(cache, field, args) {
1249+
match self.check_field_type_for_ffi(acc, field, args) {
12421250
FfiUnsafe { ty, .. } if ty.is_unit() => (),
12431251
r => return r,
12441252
}
@@ -1256,7 +1264,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12561264
// We can't completely trust `repr(C)` markings, so make sure the fields are actually safe.
12571265
let mut all_phantom = !variant.fields.is_empty();
12581266
for field in &variant.fields {
1259-
all_phantom &= match self.check_field_type_for_ffi(cache, field, args) {
1267+
all_phantom &= match self.check_field_type_for_ffi(acc, field, args) {
12601268
FfiSafe => false,
12611269
// `()` fields are FFI-safe!
12621270
FfiUnsafe { ty, .. } if ty.is_unit() => false,
@@ -1276,7 +1284,11 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12761284

12771285
/// Checks if the given type is "ffi-safe" (has a stable, well-defined
12781286
/// representation which can be exported to C code).
1279-
fn check_type_for_ffi(&self, cache: &mut FxHashSet<Ty<'tcx>>, ty: Ty<'tcx>) -> FfiResult<'tcx> {
1287+
fn check_type_for_ffi(
1288+
&self,
1289+
acc: &mut CTypesVisitorState<'tcx>,
1290+
ty: Ty<'tcx>,
1291+
) -> FfiResult<'tcx> {
12801292
use FfiResult::*;
12811293

12821294
let tcx = self.cx.tcx;
@@ -1285,7 +1297,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
12851297
// `struct S(*mut S);`.
12861298
// FIXME: A recursion limit is necessary as well, for irregular
12871299
// recursive types.
1288-
if !cache.insert(ty) {
1300+
if !acc.cache.insert(ty) {
12891301
return FfiSafe;
12901302
}
12911303

@@ -1307,6 +1319,17 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
13071319
}
13081320
match def.adt_kind() {
13091321
AdtKind::Struct | AdtKind::Union => {
1322+
if let Some(sym::cstring_type | sym::cstr_type) =
1323+
tcx.get_diagnostic_name(def.did())
1324+
&& !acc.base_ty.is_mutable_ptr()
1325+
{
1326+
return FfiUnsafe {
1327+
ty,
1328+
reason: fluent::lint_improper_ctypes_cstr_reason,
1329+
help: Some(fluent::lint_improper_ctypes_cstr_help),
1330+
};
1331+
}
1332+
13101333
if !def.repr().c() && !def.repr().transparent() {
13111334
return FfiUnsafe {
13121335
ty,
@@ -1353,7 +1376,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
13531376
};
13541377
}
13551378

1356-
self.check_variant_for_ffi(cache, ty, def, def.non_enum_variant(), args)
1379+
self.check_variant_for_ffi(acc, ty, def, def.non_enum_variant(), args)
13571380
}
13581381
AdtKind::Enum => {
13591382
if def.variants().is_empty() {
@@ -1377,7 +1400,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
13771400
if let Some(ty) =
13781401
repr_nullable_ptr(self.cx.tcx, self.cx.param_env, ty, self.mode)
13791402
{
1380-
return self.check_type_for_ffi(cache, ty);
1403+
return self.check_type_for_ffi(acc, ty);
13811404
}
13821405

13831406
return FfiUnsafe {
@@ -1398,7 +1421,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
13981421
};
13991422
}
14001423

1401-
match self.check_variant_for_ffi(cache, ty, def, variant, args) {
1424+
match self.check_variant_for_ffi(acc, ty, def, variant, args) {
14021425
FfiSafe => (),
14031426
r => return r,
14041427
}
@@ -1468,9 +1491,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
14681491
FfiSafe
14691492
}
14701493

1471-
ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => self.check_type_for_ffi(cache, ty),
1494+
ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => self.check_type_for_ffi(acc, ty),
14721495

1473-
ty::Array(inner_ty, _) => self.check_type_for_ffi(cache, inner_ty),
1496+
ty::Array(inner_ty, _) => self.check_type_for_ffi(acc, inner_ty),
14741497

14751498
ty::FnPtr(sig) => {
14761499
if self.is_internal_abi(sig.abi()) {
@@ -1483,7 +1506,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
14831506

14841507
let sig = tcx.instantiate_bound_regions_with_erased(sig);
14851508
for arg in sig.inputs() {
1486-
match self.check_type_for_ffi(cache, *arg) {
1509+
match self.check_type_for_ffi(acc, *arg) {
14871510
FfiSafe => {}
14881511
r => return r,
14891512
}
@@ -1494,7 +1517,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
14941517
return FfiSafe;
14951518
}
14961519

1497-
self.check_type_for_ffi(cache, ret_ty)
1520+
self.check_type_for_ffi(acc, ret_ty)
14981521
}
14991522

15001523
ty::Foreign(..) => FfiSafe,
@@ -1617,7 +1640,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
16171640
return;
16181641
}
16191642

1620-
match self.check_type_for_ffi(&mut FxHashSet::default(), ty) {
1643+
let mut acc = CTypesVisitorState { cache: FxHashSet::default(), base_ty: ty };
1644+
match self.check_type_for_ffi(&mut acc, ty) {
16211645
FfiResult::FfiSafe => {}
16221646
FfiResult::FfiPhantom(ty) => {
16231647
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
@@ -671,6 +671,7 @@ symbols! {
671671
crate_visibility_modifier,
672672
crt_dash_static: "crt-static",
673673
csky_target_feature,
674+
cstr_type,
674675
cstring_type,
675676
ctlz,
676677
ctlz_nonzero,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ use crate::{fmt, intrinsics, ops, slice, str};
9191
/// [str]: prim@str "str"
9292
#[derive(PartialEq, Eq, Hash)]
9393
#[stable(feature = "core_c_str", since = "1.64.0")]
94+
#[rustc_diagnostic_item = "cstr_type"]
9495
#[rustc_has_incoherent_inherent_impls]
9596
#[lang = "CStr"]
9697
// `fn from` in `impl From<&CStr> for Box<CStr>` current implementation relies

‎tests/ui/extern/extern-C-non-FFI-safe-arg-ice-52334.stderr

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,21 @@
1-
warning: `extern` fn uses type `[i8 or u8 (arch dependant)]`, which is not FFI-safe
1+
warning: `extern` fn uses type `CStr`, which is not FFI-safe
22
--> $DIR/extern-C-non-FFI-safe-arg-ice-52334.rs:7:12
33
|
44
LL | type Foo = extern "C" fn(::std::ffi::CStr);
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
66
|
7-
= help: consider using a raw pointer instead
8-
= note: slices have no C equivalent
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
99
= note: `#[warn(improper_ctypes_definitions)]` on by default
1010

11-
warning: `extern` block uses type `[i8 or u8 (arch dependant)]`, which is not FFI-safe
11+
warning: `extern` block uses type `CStr`, which is not FFI-safe
1212
--> $DIR/extern-C-non-FFI-safe-arg-ice-52334.rs:10:18
1313
|
1414
LL | fn meh(blah: Foo);
1515
| ^^^ not FFI-safe
1616
|
17-
= help: consider using a raw pointer instead
18-
= note: slices have no C equivalent
17+
= help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
18+
= note: `CStr`/`CString` do not have a guaranteed layout
1919
= note: `#[warn(improper_ctypes)]` on by default
2020

2121
warning: 2 warnings emitted

‎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)