ImproperCTypes: change cstr linting

niacdoial · niacdoial · commit 84d388ec7664 · 2025-09-21T12:21:28.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,10 @@ 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, converting to/from `{$ty}` as needed
+lint_improper_ctypes_cstr_help_mut =
+    consider passing a `*mut std::ffi::c_char` instead, converting to/from `{$ty}` as needed
 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 +388,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 +411,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
@@ -350,7 +350,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) => {
@@ -705,6 +704,18 @@ impl VisitorState {
     fn is_field(&self) -> bool {
         self.ephemeral_flags.contains(EphemeralStateFlags::IN_ADT)
     }
+
+    /// Whether the current type is behind a pointer
+    #[inline]
+    fn is_pointee(&self) -> bool {
+        self.ephemeral_flags.contains(EphemeralStateFlags::IN_PTR)
+    }
+
+    /// Whether the current type is behind a pointer that doesn't allow mutating this
+    #[inline]
+    fn is_nonmut_pointee(&self) -> bool {
+        self.is_pointee() && !self.ephemeral_flags.contains(EphemeralStateFlags::PTR_MUT)
+    }
 }
 
 /// Visitor used to recursively traverse MIR types and evaluate FFI-safety.
@@ -717,13 +728,30 @@ struct ImproperCTypesVisitor<'a, 'tcx> {
     cache: FxHashSet<Ty<'tcx>>,
     /// The original type being checked, before we recursed
     /// to any other types it contains.
-    base_ty: Ty<'tcx>,
     base_fn_mode: CItemKind,
 }
 
 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, cache: FxHashSet::default() }
+    fn new(cx: &'a LateContext<'tcx>, base_fn_mode: CItemKind) -> Self {
+        Self { cx, base_fn_mode, cache: FxHashSet::default() }
+    }
+
+    /// Return the right help for Cstring and Cstr-linked unsafety.
+    fn visit_cstr(&mut self, state: VisitorState, 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 state.is_nonmut_pointee() {
+            fluent::lint_improper_ctypes_cstr_help_const
+        } else {
+            fluent::lint_improper_ctypes_cstr_help_mut
+        };
+
+        FfiResult::new_with_reason(ty, fluent::lint_improper_ctypes_cstr_reason, Some(help))
     }
 
     /// Checks if the given indirection (box,ref,pointer) is "ffi-safe".
@@ -737,6 +765,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(state.get_next(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_kind {
+                        IndirectionKind::RawPtr => fluent::lint_improper_ctypes_unsized_ptr,
+                        IndirectionKind::Ref => fluent::lint_improper_ctypes_unsized_ref,
+                        IndirectionKind::Box => fluent::lint_improper_ctypes_unsized_box,
+                    };
+                    return cstr_res.wrap_all(ty, note, None);
+                } else {
+                    return cstr_res;
+                }
+            }
+        }
+
         match indirection_kind {
             IndirectionKind::Box => {
                 // FIXME(ctypes): this logic is broken, but it still fits the current tests:
@@ -977,13 +1034,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
                     AdtKind::Struct | AdtKind::Union => {
                         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(state, ty);
                         }
                         self.visit_struct_or_union(state, ty, def, args)
                     }
@@ -1232,7 +1284,7 @@ impl<'tcx> ImproperCTypesLint {
             iter::zip(visitor.tys.drain(..), visitor.spans.drain(..)),
         );
         for (depth, (fn_ptr_ty, span)) in all_types {
-            let mut visitor = ImproperCTypesVisitor::new(cx, fn_ptr_ty, fn_mode);
+            let mut visitor = ImproperCTypesVisitor::new(cx, fn_mode);
             let bridge_state = VisitorState { depth, ..state };
             // FIXME(ctypes): make a check_for_fnptr
             let ffi_res = visitor.check_type(bridge_state, fn_ptr_ty);
@@ -1283,7 +1335,7 @@ impl<'tcx> ImproperCTypesLint {
 
     fn check_foreign_static(&mut self, cx: &LateContext<'tcx>, id: hir::OwnerId, span: Span) {
         let ty = cx.tcx.type_of(id).instantiate_identity();
-        let mut visitor = ImproperCTypesVisitor::new(cx, ty, CItemKind::ImportedExtern);
+        let mut visitor = ImproperCTypesVisitor::new(cx, CItemKind::ImportedExtern);
         let ffi_res = visitor.check_type(VisitorState::static_var(), ty);
         self.process_ffi_result(cx, span, ffi_res, CItemKind::ImportedExtern);
     }
@@ -1301,14 +1353,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 mut visitor = ImproperCTypesVisitor::new(cx, *input_ty, fn_mode);
+            let mut 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 mut visitor = ImproperCTypesVisitor::new(cx, sig.output(), fn_mode);
+            let mut 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/extern/extern-C-non-FFI-safe-arg-ice-52334.stderr b/tests/ui/extern/extern-C-non-FFI-safe-arg-ice-52334.stderr
@@ -4,7 +4,7 @@ warning: `extern` callback uses type `CStr`, which is not FFI-safe
 LL | type Foo = extern "C" fn(::std::ffi::CStr);
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
    |
-   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+   = help: consider passing a `*mut std::ffi::c_char` instead, converting to/from `CStr` as needed
    = note: `CStr`/`CString` do not have a guaranteed layout
    = note: `#[warn(improper_c_callbacks)]` (part of `#[warn(improper_c_boundaries)]`) on by default
 
@@ -14,7 +14,7 @@ warning: `extern` block uses type `CStr`, which is not FFI-safe
 LL |     fn meh(blah: Foo);
    |                  ^^^ not FFI-safe
    |
-   = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()`
+   = help: consider passing a `*mut std::ffi::c_char` instead, converting to/from `CStr` as needed
    = note: `CStr`/`CString` do not have a guaranteed layout
    = note: `#[warn(improper_ctypes)]` (part of `#[warn(improper_c_boundaries)]`) on by default
 
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 `*mut std::ffi::c_char` instead, converting to/from `CStr` as needed
     fn take_cstr_ref(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()`
+    //~^ ERROR `extern` block uses type `&CStr`, which is not FFI-safe
+    //~| HELP consider passing a `*const std::ffi::c_char` instead, converting to/from `&CStr` as needed
     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 `*mut std::ffi::c_char` instead, converting to/from `CString` as needed
     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()`
+    //~| HELP consider passing a `*const std::ffi::c_char` instead, converting to/from `CString` as needed
 
-    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, converting to/from `CString` as needed
 
-    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, converting to/from `CString` as needed
 }
 
 extern "C" fn rust_take_cstr_ref(s: &CStr) {}
-//~^ 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()`
+//~^ ERROR `extern` fn uses type `&CStr`, which is not FFI-safe
+//~| HELP consider passing a `*const std::ffi::c_char` instead, converting to/from `&CStr` as needed
 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 `*mut std::ffi::c_char` instead, converting to/from `CString` as needed
+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, converting to/from `CString` as needed
+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, converting to/from `CString` as needed
diff --git a/tests/ui/lint/improper-ctypes/lint-cstr.stderr b/tests/ui/lint/improper-ctypes/lint-cstr.stderr
@@ -4,21 +4,22 @@ 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 `*mut std::ffi::c_char` instead, converting to/from `CStr` as needed
    = 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, converting to/from `&CStr` as needed
    = note: `CStr`/`CString` do not have a guaranteed layout
 
 error: `extern` block uses type `CString`, which is not FFI-safe
@@ -27,7 +28,7 @@ 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 `*mut std::ffi::c_char` instead, converting to/from `CString` as needed
    = note: `CStr`/`CString` do not have a guaranteed layout
 
 error: `extern` block uses type `CString`, which is not FFI-safe
@@ -36,34 +37,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, converting to/from `CString` as needed
    = 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, converting to/from `CString` as needed
+   = 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, converting to/from `CString` as needed
+   = 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, converting to/from `&CStr` as needed
    = note: `CStr`/`CString` do not have a guaranteed layout
 note: the lint level is defined here
   --> $DIR/lint-cstr.rs:2:26
@@ -77,8 +79,26 @@ 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 `*mut std::ffi::c_char` instead, converting to/from `CString` as needed
+   = 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, converting to/from `CString` as needed
+   = 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, converting to/from `CString` as needed
    = note: `CStr`/`CString` do not have a guaranteed layout
 
-error: aborting due to 8 previous errors
+error: aborting due to 10 previous errors
 
