ImproperCTypes: change cstr linting

niacdoial · niacdoial · commit b5f34fcb9135 · 2025-09-06T17:18:00.000+02:00
another user-visible change: change the messaging and help around
CStr/CString lints
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
@@ -363,8 +363,17 @@ lint_improper_ctypes_char_help = consider using `u32` or `libc::wchar_t` instead
 
 lint_improper_ctypes_char_reason = the `char` type has no C equivalent
 
-lint_improper_ctypes_cstr_help =
-    consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+lint_improper_ctypes_cstr_help_const =
+    consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` or `CString::as_ptr()`
+lint_improper_ctypes_cstr_help_mut =
+    consider passing a `*mut std::ffi::c_char` instead, and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
+lint_improper_ctypes_cstr_help_owned =
+    consider passing a `*const std::ffi::c_char` or `*mut std::ffi::c_char` instead,
+    and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
+    (note that `CString::into_raw()`'s output must not be `libc::free()`'d)
+lint_improper_ctypes_cstr_help_unknown =
+    consider passing a `*const std::ffi::c_char` or `*mut std::ffi::c_char` instead,
+    and use (depending on the use case) `CStr::as_ptr()`, `CString::into_raw()` then `CString::from_raw()`, or a dedicated buffer
 lint_improper_ctypes_cstr_reason = `CStr`/`CString` do not have a guaranteed layout
 
 lint_improper_ctypes_dyn = trait objects have no C equivalent
@@ -386,8 +395,8 @@ lint_improper_ctypes_opaque = opaque types have no C equivalent
 lint_improper_ctypes_slice_help = consider using a raw pointer instead
 
 lint_improper_ctypes_slice_reason = slices have no C equivalent
-lint_improper_ctypes_str_help = consider using `*const u8` and a length instead
 
+lint_improper_ctypes_str_help = consider using `*const u8` and a length instead
 lint_improper_ctypes_str_reason = string slices have no C equivalent
 lint_improper_ctypes_struct_fieldless_help = consider adding a member to this struct
 
@@ -409,6 +418,10 @@ lint_improper_ctypes_union_layout_help = consider adding a `#[repr(C)]` or `#[re
 lint_improper_ctypes_union_layout_reason = this union has unspecified layout
 lint_improper_ctypes_union_non_exhaustive = this union is non-exhaustive
 
+lint_improper_ctypes_unsized_box = this box for an unsized type contains metadata, which makes it incompatible with a C pointer
+lint_improper_ctypes_unsized_ptr = this pointer to an unsized type contains metadata, which makes it incompatible with a C pointer
+lint_improper_ctypes_unsized_ref = this reference to an unsized type contains metadata, which makes it incompatible with a C pointer
+
 lint_incomplete_include =
     include macro expected single expression in source
 
diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs
@@ -281,7 +281,6 @@ impl<'tcx> FfiResult<'tcx> {
     /// If the FfiUnsafe variant, 'wraps' all reasons,
     /// creating new `FfiUnsafeReason`s, putting the originals as their `inner` fields.
     /// Otherwise, keep unchanged.
-    #[expect(unused)]
     fn wrap_all(self, ty: Ty<'tcx>, note: DiagMessage, help: Option<DiagMessage>) -> Self {
         match self {
             Self::FfiUnsafe(this) => {
@@ -534,7 +533,6 @@ struct ImproperCTypesVisitor<'a, 'tcx> {
 
     /// The original type being checked, before we recursed
     /// to any other types it contains.
-    base_ty: Ty<'tcx>,
     base_fn_mode: CItemKind,
 }
 
@@ -551,13 +549,8 @@ impl<'a, 'tcx, 'v> Drop for ImproperCTypesVisitorDepthGuard<'a, 'tcx, 'v> {
 }
 
 impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
-    fn new(cx: &'a LateContext<'tcx>, base_ty: Ty<'tcx>, base_fn_mode: CItemKind) -> Self {
-        Self {
-            cx,
-            base_ty,
-            base_fn_mode,
-            recursion_limiter: RefCell::new((FxHashSet::default(), 0)),
-        }
+    fn new(cx: &'a LateContext<'tcx>, base_fn_mode: CItemKind) -> Self {
+        Self { cx, base_fn_mode, recursion_limiter: RefCell::new((FxHashSet::default(), 0)) }
     }
 
     /// Protect against infinite recursion, for example
@@ -596,6 +589,36 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
         }
     }
 
+    /// Return the right help for Cstring and Cstr-linked unsafety.
+    fn visit_cstr(&self, outer_ty: Option<Ty<'tcx>>, ty: Ty<'tcx>) -> FfiResult<'tcx> {
+        debug_assert!(matches!(ty.kind(), ty::Adt(def, _)
+            if matches!(
+                self.cx.tcx.get_diagnostic_name(def.did()),
+                Some(sym::cstring_type | sym::cstr_type)
+            )
+        ));
+
+        let help = if let Some(outer_ty) = outer_ty {
+            match outer_ty.kind() {
+                ty::Ref(..) | ty::RawPtr(..) => {
+                    if outer_ty.is_mutable_ptr() {
+                        fluent::lint_improper_ctypes_cstr_help_mut
+                    } else {
+                        fluent::lint_improper_ctypes_cstr_help_const
+                    }
+                }
+                ty::Adt(..) if outer_ty.boxed_ty().is_some() => {
+                    fluent::lint_improper_ctypes_cstr_help_owned
+                }
+                _ => fluent::lint_improper_ctypes_cstr_help_unknown,
+            }
+        } else {
+            fluent::lint_improper_ctypes_cstr_help_owned
+        };
+
+        FfiResult::new_with_reason(ty, fluent::lint_improper_ctypes_cstr_reason, Some(help))
+    }
+
     /// Checks if the given indirection (box,ref,pointer) is "ffi-safe".
     fn visit_indirection(
         &self,
@@ -607,6 +630,35 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
         use FfiResult::*;
         let tcx = self.cx.tcx;
 
+        if let ty::Adt(def, _) = inner_ty.kind() {
+            if let Some(diag_name @ (sym::cstring_type | sym::cstr_type)) =
+                tcx.get_diagnostic_name(def.did())
+            {
+                // we have better error messages when checking for C-strings directly
+                let mut cstr_res = self.visit_cstr(Some(ty), inner_ty); // always unsafe with one depth-one reason.
+
+                // Cstr pointer have metadata, CString is Sized
+                if diag_name == sym::cstr_type {
+                    // we need to override the "type" part of `cstr_res`'s only FfiResultReason
+                    // so it says that it's the use of the indirection that is unsafe
+                    match cstr_res {
+                        FfiResult::FfiUnsafe(ref mut reasons) => {
+                            reasons.first_mut().unwrap().reason.ty = ty;
+                        }
+                        _ => unreachable!(),
+                    }
+                    let note = match indirection_type {
+                        IndirectionType::RawPtr => fluent::lint_improper_ctypes_unsized_ptr,
+                        IndirectionType::Ref => fluent::lint_improper_ctypes_unsized_ref,
+                        IndirectionType::Box => fluent::lint_improper_ctypes_unsized_box,
+                    };
+                    return cstr_res.wrap_all(ty, note, None);
+                } else {
+                    return cstr_res;
+                }
+            }
+        }
+
         match indirection_type {
             IndirectionType::Box => {
                 // FIXME(ctypes): this logic is broken, but it still fits the current tests
@@ -830,13 +882,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                         // but function *pointers* don't seem to have the same no-unsized-parameters requirement to compile
                         if let Some(sym::cstring_type | sym::cstr_type) =
                             tcx.get_diagnostic_name(def.did())
-                            && !self.base_ty.is_mutable_ptr()
                         {
-                            return FfiResult::new_with_reason(
-                                ty,
-                                fluent::lint_improper_ctypes_cstr_reason,
-                                Some(fluent::lint_improper_ctypes_cstr_help),
-                            );
+                            return self.visit_cstr(outer_ty, ty);
                         }
                         self.visit_struct_or_union(state, ty, def, args)
                     }
@@ -1144,7 +1191,7 @@ impl<'tcx> ImproperCTypesLint {
 
         let all_types = iter::zip(visitor.tys.drain(..), visitor.spans.drain(..));
         for (fn_ptr_ty, span) in all_types {
-            let visitor = ImproperCTypesVisitor::new(cx, fn_ptr_ty, fn_mode);
+            let visitor = ImproperCTypesVisitor::new(cx, fn_mode);
             // FIXME(ctypes): make a check_for_fnptr
             let ffi_res = visitor.check_type(state, fn_ptr_ty);
 
@@ -1194,7 +1241,7 @@ impl<'tcx> ImproperCTypesLint {
 
     fn check_foreign_static(&self, cx: &LateContext<'tcx>, id: hir::OwnerId, span: Span) {
         let ty = cx.tcx.type_of(id).instantiate_identity();
-        let visitor = ImproperCTypesVisitor::new(cx, ty, CItemKind::ImportedExtern);
+        let visitor = ImproperCTypesVisitor::new(cx, CItemKind::ImportedExtern);
         let ffi_res = visitor.check_type(VisitorState::STATIC_TY, ty);
         self.process_ffi_result(cx, span, ffi_res, CItemKind::ImportedExtern);
     }
@@ -1212,14 +1259,14 @@ impl<'tcx> ImproperCTypesLint {
 
         for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) {
             let visit_state = VisitorState::argument_from_fnmode(fn_mode);
-            let visitor = ImproperCTypesVisitor::new(cx, *input_ty, fn_mode);
+            let visitor = ImproperCTypesVisitor::new(cx, fn_mode);
             let ffi_res = visitor.check_type(visit_state, *input_ty);
             self.process_ffi_result(cx, input_hir.span, ffi_res, fn_mode);
         }
 
         if let hir::FnRetTy::Return(ret_hir) = decl.output {
             let visit_state = VisitorState::return_from_fnmode(fn_mode);
-            let visitor = ImproperCTypesVisitor::new(cx, sig.output(), fn_mode);
+            let visitor = ImproperCTypesVisitor::new(cx, fn_mode);
             let ffi_res = visitor.check_type(visit_state, sig.output());
             self.process_ffi_result(cx, ret_hir.span, ffi_res, fn_mode);
         }
diff --git a/tests/ui/lint/improper_ctypes/lint-cstr.rs b/tests/ui/lint/improper_ctypes/lint-cstr.rs
@@ -6,31 +6,35 @@ use std::ffi::{CStr, CString};
 extern "C" {
     fn take_cstr(s: CStr);
     //~^ ERROR `extern` block uses type `CStr`, which is not FFI-safe
-    //~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+    //~| HELP consider passing a `*const std::ffi::c_char` or `*mut std::ffi::c_char` instead
     fn take_cstr_ref(s: &CStr);
-    //~^ ERROR `extern` block uses type `CStr`, which is not FFI-safe
+    //~^ ERROR `extern` block uses type `&CStr`, which is not FFI-safe
     //~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
     fn take_cstring(s: CString);
     //~^ ERROR `extern` block uses type `CString`, which is not FFI-safe
-    //~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+    //~| HELP consider passing a `*const std::ffi::c_char` or `*mut std::ffi::c_char` instead
     fn take_cstring_ref(s: &CString);
     //~^ ERROR `extern` block uses type `CString`, which is not FFI-safe
     //~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
 
-    fn no_special_help_for_mut_cstring(s: *mut CString);
+    fn take_cstring_ptr_mut(s: *mut CString);
     //~^ ERROR `extern` block uses type `CString`, which is not FFI-safe
-    //~| HELP consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
+    //~| HELP consider passing a `*mut std::ffi::c_char` instead, and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
 
-    fn no_special_help_for_mut_cstring_ref(s: &mut CString);
+    fn take_cstring_ref_mut(s: &mut CString);
     //~^ ERROR `extern` block uses type `CString`, which is not FFI-safe
-    //~| HELP consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
+    //~| HELP consider passing a `*mut std::ffi::c_char` instead, and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
 }
 
 extern "C" fn rust_take_cstr_ref(s: &CStr) {}
-//~^ ERROR `extern` fn uses type `CStr`, which is not FFI-safe
+//~^ ERROR `extern` fn uses type `&CStr`, which is not FFI-safe
 //~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
 extern "C" fn rust_take_cstring(s: CString) {}
 //~^ ERROR `extern` fn uses type `CString`, which is not FFI-safe
-//~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
-extern "C" fn rust_no_special_help_for_mut_cstring(s: *mut CString) {}
-extern "C" fn rust_no_special_help_for_mut_cstring_ref(s: &mut CString) {}
+//~| HELP consider passing a `*const std::ffi::c_char` or `*mut std::ffi::c_char` instead
+extern "C" fn rust_take_cstring_ptr_mut(s: *mut CString) {}
+//~^ ERROR `extern` fn uses type `CString`, which is not FFI-safe
+//~| HELP consider passing a `*mut std::ffi::c_char` instead, and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
+extern "C" fn rust_take_cstring_ref_mut(s: &mut CString) {}
+//~^ ERROR `extern` fn uses type `CString`, which is not FFI-safe
+//~| HELP consider passing a `*mut std::ffi::c_char` instead, and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
diff --git a/tests/ui/lint/improper_ctypes/lint-cstr.stderr b/tests/ui/lint/improper_ctypes/lint-cstr.stderr
@@ -4,21 +4,24 @@ error: `extern` block uses type `CStr`, which is not FFI-safe
 LL |     fn take_cstr(s: CStr);
    |                     ^^^^ not FFI-safe
    |
-   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+   = help: consider passing a `*const std::ffi::c_char` or `*mut std::ffi::c_char` instead,
+           and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
+           (note that `CString::into_raw()`'s output must not be `libc::free()`'d)
    = note: `CStr`/`CString` do not have a guaranteed layout
 note: the lint level is defined here
   --> $DIR/lint-cstr.rs:2:9
    |
 LL | #![deny(improper_ctypes, improper_c_fn_definitions, improper_c_callbacks)]
    |         ^^^^^^^^^^^^^^^
 
-error: `extern` block uses type `CStr`, which is not FFI-safe
+error: `extern` block uses type `&CStr`, which is not FFI-safe
   --> $DIR/lint-cstr.rs:10:25
    |
 LL |     fn take_cstr_ref(s: &CStr);
    |                         ^^^^^ not FFI-safe
    |
-   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+   = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer
+   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` or `CString::as_ptr()`
    = note: `CStr`/`CString` do not have a guaranteed layout
 
 error: `extern` block uses type `CString`, which is not FFI-safe
@@ -27,7 +30,9 @@ error: `extern` block uses type `CString`, which is not FFI-safe
 LL |     fn take_cstring(s: CString);
    |                        ^^^^^^^ not FFI-safe
    |
-   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+   = help: consider passing a `*const std::ffi::c_char` or `*mut std::ffi::c_char` instead,
+           and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
+           (note that `CString::into_raw()`'s output must not be `libc::free()`'d)
    = note: `CStr`/`CString` do not have a guaranteed layout
 
 error: `extern` block uses type `CString`, which is not FFI-safe
@@ -36,34 +41,35 @@ error: `extern` block uses type `CString`, which is not FFI-safe
 LL |     fn take_cstring_ref(s: &CString);
    |                            ^^^^^^^^ not FFI-safe
    |
-   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` or `CString::as_ptr()`
    = note: `CStr`/`CString` do not have a guaranteed layout
 
 error: `extern` block uses type `CString`, which is not FFI-safe
-  --> $DIR/lint-cstr.rs:20:43
+  --> $DIR/lint-cstr.rs:20:32
    |
-LL |     fn no_special_help_for_mut_cstring(s: *mut CString);
-   |                                           ^^^^^^^^^^^^ not FFI-safe
+LL |     fn take_cstring_ptr_mut(s: *mut CString);
+   |                                ^^^^^^^^^^^^ not FFI-safe
    |
-   = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
-   = note: this struct has unspecified layout
+   = help: consider passing a `*mut std::ffi::c_char` instead, and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
+   = note: `CStr`/`CString` do not have a guaranteed layout
 
 error: `extern` block uses type `CString`, which is not FFI-safe
-  --> $DIR/lint-cstr.rs:24:47
+  --> $DIR/lint-cstr.rs:24:32
    |
-LL |     fn no_special_help_for_mut_cstring_ref(s: &mut CString);
-   |                                               ^^^^^^^^^^^^ not FFI-safe
+LL |     fn take_cstring_ref_mut(s: &mut CString);
+   |                                ^^^^^^^^^^^^ not FFI-safe
    |
-   = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
-   = note: this struct has unspecified layout
+   = help: consider passing a `*mut std::ffi::c_char` instead, and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
+   = note: `CStr`/`CString` do not have a guaranteed layout
 
-error: `extern` fn uses type `CStr`, which is not FFI-safe
+error: `extern` fn uses type `&CStr`, which is not FFI-safe
   --> $DIR/lint-cstr.rs:29:37
    |
 LL | extern "C" fn rust_take_cstr_ref(s: &CStr) {}
    |                                     ^^^^^ not FFI-safe
    |
-   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+   = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer
+   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` or `CString::as_ptr()`
    = note: `CStr`/`CString` do not have a guaranteed layout
 note: the lint level is defined here
   --> $DIR/lint-cstr.rs:2:26
@@ -77,8 +83,28 @@ error: `extern` fn uses type `CString`, which is not FFI-safe
 LL | extern "C" fn rust_take_cstring(s: CString) {}
    |                                    ^^^^^^^ not FFI-safe
    |
-   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+   = help: consider passing a `*const std::ffi::c_char` or `*mut std::ffi::c_char` instead,
+           and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
+           (note that `CString::into_raw()`'s output must not be `libc::free()`'d)
+   = note: `CStr`/`CString` do not have a guaranteed layout
+
+error: `extern` fn uses type `CString`, which is not FFI-safe
+  --> $DIR/lint-cstr.rs:35:44
+   |
+LL | extern "C" fn rust_take_cstring_ptr_mut(s: *mut CString) {}
+   |                                            ^^^^^^^^^^^^ not FFI-safe
+   |
+   = help: consider passing a `*mut std::ffi::c_char` instead, and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
+   = note: `CStr`/`CString` do not have a guaranteed layout
+
+error: `extern` fn uses type `CString`, which is not FFI-safe
+  --> $DIR/lint-cstr.rs:38:44
+   |
+LL | extern "C" fn rust_take_cstring_ref_mut(s: &mut CString) {}
+   |                                            ^^^^^^^^^^^^ not FFI-safe
+   |
+   = help: consider passing a `*mut std::ffi::c_char` instead, and use `CString::into_raw()` then `CString::from_raw()` or a dedicated buffer
    = note: `CStr`/`CString` do not have a guaranteed layout
 
-error: aborting due to 8 previous errors
+error: aborting due to 10 previous errors
 
