ImproperCTypes: also check 'exported' static variables

Added the missing case for FFI-exposed pieces of code: static variables
with the `no_mangle` or `export_name` annotations.
This adds a new lint, which is managed by the rest of the ImproperCTypes
architecture.

Author: niacdoial

compiler/rustc_lint/src/types/improper_ctypes.rs

@@ -57,27 +57,32 @@ declare_lint! {
 
 declare_lint! {
     /// The `improper_ctypes_definitions` lint detects incorrect use of
-    /// [`extern` function] definitions.
-    /// (In other words, functions to be used by foreign code.)
+    /// [`extern` function] definitions and [`no_mangle`] / [`export_name`] static variable definitions.
+    /// (In other words, functions and global variables to be used by foreign code.)
     ///
     /// [`extern` function]: https://doc.rust-lang.org/reference/items/functions.html#extern-function-qualifier
+    /// [`no_mangle`]: https://doc.rust-lang.org/stable/reference/abi.html#the-no_mangle-attribute
+    /// [`export_name`]: https://doc.rust-lang.org/stable/reference/abi.html#the-export_name-attribute
     ///
     /// ### Example
     ///
     /// ```rust
     /// # #![allow(unused)]
     /// pub extern "C" fn str_type(p: &str) { }
+    /// # #[used]
+    /// # #[unsafe(no_mangle)]
+    /// static PLUGIN_ABI_MIN_VERSION: &'static str = "0.0.5";
     /// ```
     ///
     /// {{produces}}
     ///
     /// ### Explanation
     ///
-    /// There are many parameter and return types that may be specified in an
-    /// `extern` function that are not compatible with the given ABI. This
-    /// lint is an alert that these types should not be used. The lint usually
-    /// should provide a description of the issue, along with possibly a hint
-    /// on how to resolve it.
+    /// There are many types that may be specified at interfaces exposed to foreign code,
+    /// but are not follow the rules to ensure proper ABI compatibility.
+    /// This lint is issued when a mistake is detected.
+    /// The lint usually should provide a description of the issue,
+    /// along with possibly a hint on how to resolve it.
     pub(crate) IMPROPER_CTYPES_DEFINITIONS,
     Warn,
     "proper use of libc types in foreign item definitions"
@@ -282,6 +287,9 @@ enum CItemKind {
     ExportedFunction,
     /// `extern "C"` function pointers -> also IMPROPER_CTYPES,
     Callback,
+    /// `no_mangle`/`export_name` static variables, assumed to be used from across an FFI boundary,
+    /// -> also IMPROPER_CTYPES_DEFINITIONS
+    ExportedStatic,
 }
 
 /// Annotates whether we are in the context of a function's argument types or return type.
@@ -694,6 +702,8 @@ struct VisitorState {
 impl RootUseFlags {
     // The values that can be set.
     const STATIC_TY: Self = Self::STATIC;
+    const EXPORTED_STATIC_TY: Self =
+        Self::from_bits(Self::STATIC.bits() | Self::DEFINED.bits()).unwrap();
     const ARGUMENT_TY_IN_DEFINITION: Self =
         Self::from_bits(Self::FUNC.bits() | Self::DEFINED.bits()).unwrap();
     const RETURN_TY_IN_DEFINITION: Self =
@@ -734,6 +744,9 @@ impl VisitorState {
             (CItemKind::ExportedFunction, FnPos::Arg) => RootUseFlags::ARGUMENT_TY_IN_DEFINITION,
             (CItemKind::ImportedExtern, FnPos::Arg) => RootUseFlags::ARGUMENT_TY_IN_DECLARATION,
             (CItemKind::Callback, FnPos::Arg) => RootUseFlags::ARGUMENT_TY_IN_FNPTR,
+            (CItemKind::ExportedStatic, _) => bug!(
+                "VisitorState::entry_point_from_fnmode() should not be used for static variables!"
+            ),
         };
         Self::entry_point(p_flags)
     }
@@ -743,6 +756,11 @@ impl VisitorState {
         Self::entry_point(RootUseFlags::STATIC_TY)
     }
 
+    /// Get the proper visitor state for a locally-defined static variable's type
+    fn static_var_def() -> Self {
+        Self::entry_point(RootUseFlags::EXPORTED_STATIC_TY)
+    }
+
     /// Whether the type is used as the type of a static variable.
     fn is_direct_in_static(&self) -> bool {
         let ret = self.root_use_flags.contains(RootUseFlags::STATIC);
@@ -1632,6 +1650,15 @@ impl<'tcx> ImproperCTypesLint {
         self.process_ffi_result(cx, span, ffi_res, CItemKind::ImportedExtern);
     }
 
+    /// Check that a `#[no_mangle]`/`#[export_name = _]` static variable is of a ffi-safe type.
+    fn check_exported_static(&self, cx: &LateContext<'tcx>, id: hir::HirId, span: Span) {
+        let ty = cx.tcx.type_of(id.owner).instantiate_identity();
+        let mod_id = cx.tcx.parent_module(id).to_def_id();
+        let mut visitor = ImproperCTypesVisitor::new(cx, mod_id);
+        let ffi_res = visitor.check_type(VisitorState::static_var_def(), ty);
+        self.process_ffi_result(cx, span, ffi_res, CItemKind::ExportedStatic);
+    }
+
     /// Check if a function's argument types and result type are "ffi-safe".
     fn check_foreign_fn(
         &mut self,
@@ -1738,10 +1765,13 @@ impl<'tcx> ImproperCTypesLint {
             // Internally, we treat this differently, but at the end of the day
             // their linting needs to be enabled/disabled alongside that of "FFI-imported" items.
             CItemKind::Callback => IMPROPER_CTYPES,
+            // Same thing with static variables, which are "FFI-exported"
+            CItemKind::ExportedStatic => IMPROPER_CTYPES_DEFINITIONS,
         };
         let desc = match fn_mode {
             CItemKind::ImportedExtern => "`extern` block",
             CItemKind::ExportedFunction => "`extern` fn",
+            CItemKind::ExportedStatic => "foreign-code-reachable static",
             CItemKind::Callback => "`extern` callback",
         };
         for reason in reasons.iter_mut() {
@@ -1785,7 +1815,14 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesLint {
                 let mir_sig = cx.tcx.fn_sig(it.owner_id.def_id).instantiate_identity();
                 let mod_id = cx.tcx.parent_module_from_def_id(it.owner_id.def_id).to_def_id();
                 if !abi.is_rustic_abi() {
-                    self.check_foreign_fn(cx, CItemKind::ImportedExtern, mir_sig, sig.decl, mod_id, 0);
+                    self.check_foreign_fn(
+                        cx,
+                        CItemKind::ImportedExtern,
+                        mir_sig,
+                        sig.decl,
+                        mod_id,
+                        0,
+                    );
                 }
             }
             hir::ForeignItemKind::Static(ty, _, _) if !abi.is_rustic_abi() => {
@@ -1805,6 +1842,25 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesLint {
                     ty,
                     cx.tcx.type_of(item.owner_id).instantiate_identity(),
                 );
+
+                // FIXME: cx.tcx.has_attr no worky
+                // if matches!(item.kind, hir::ItemKind::Static(..))
+                //     && (cx.tcx.has_attr(item.owner_id, sym::no_mangle)
+                //         || cx.tcx.has_attr(item.owner_id, sym::export_name))
+                if matches!(item.kind, hir::ItemKind::Static(..)) {
+                    let is_exported_static = cx.tcx.get_all_attrs(item.owner_id).iter().any(|x| {
+                        matches!(
+                            x,
+                            hir::Attribute::Parsed(
+                                hir::attrs::AttributeKind::NoMangle(_)
+                                    | hir::attrs::AttributeKind::ExportName { .. }
+                            )
+                        )
+                    });
+                    if is_exported_static {
+                        self.check_exported_static(cx, item.hir_id(), ty.span);
+                    }
+                }
             }
             // See `check_fn` for declarations, `check_foreign_items` for definitions in extern blocks
             hir::ItemKind::Fn { .. } => {}
@@ -1887,7 +1943,14 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesLint {
                 if !sig.header.abi.is_rustic_abi() {
                     let mir_sig = cx.tcx.fn_sig(local_id).instantiate_identity();
                     let mod_id = cx.tcx.parent_module_from_def_id(local_id).to_def_id();
-                    self.check_foreign_fn(cx, CItemKind::ExportedFunction, mir_sig, sig.decl, mod_id, 0);
+                    self.check_foreign_fn(
+                        cx,
+                        CItemKind::ExportedFunction,
+                        mir_sig,
+                        sig.decl,
+                        mod_id,
+                        0,
+                    );
                 }
             }
             hir::TraitItemKind::Type(_, ty_maybe) => {

src/tools/miri/tests/fail/function_calls/exported_symbol_wrong_type.rs

@@ -1,4 +1,5 @@
 #[no_mangle]
+#[allow(improper_c_var_definitions)]
 static FOO: () = ();
 
 fn main() {

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

@@ -4,7 +4,7 @@
 #![feature(pattern_type_macro)]
 
 #![allow(private_interfaces)]
-#![deny(improper_ctypes)]
+#![deny(improper_ctypes, improper_ctypes_definitions)]
 
 use std::cell::UnsafeCell;
 use std::marker::PhantomData;
@@ -138,6 +138,15 @@ extern "C" {
     pub fn good19(_: &String);
 }
 
+static DEFAULT_U32: u32 = 42;
+#[no_mangle]
+static EXPORTED_STATIC: &u32 = &DEFAULT_U32;
+#[no_mangle]
+static EXPORTED_STATIC_BAD: &'static str = "is this reaching you, plugin?";
+//~^ ERROR: uses type `&str`
+#[export_name="EXPORTED_STATIC_MUT_BUT_RENAMED"]
+static mut EXPORTED_STATIC_MUT: &u32 = &DEFAULT_U32;
+
 #[cfg(not(target_arch = "wasm32"))]
 extern "C" {
     pub fn good1(size: *const c_int);

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

@@ -9,7 +9,7 @@ LL |     pub fn slice_type(p: &[u32]);
 note: the lint level is defined here
   --> $DIR/lint-ctypes.rs:7:9
    |
-LL | #![deny(improper_ctypes)]
+LL | #![deny(improper_ctypes, improper_ctypes_definitions)]
    |         ^^^^^^^^^^^^^^^
 
 error: `extern` block uses type `&str`, which is not FFI-safe
@@ -215,5 +215,19 @@ LL |     pub fn no_niche_b(b: Option<UnsafeCell<&i32>>);
    = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
    = note: enum has no representation hint
 
-error: aborting due to 21 previous errors
+error: foreign-code-reachable static uses type `&str`, which is not FFI-safe
+  --> $DIR/lint-ctypes.rs:145:29
+   |
+LL | static EXPORTED_STATIC_BAD: &'static str = "is this reaching you, plugin?";
+   |                             ^^^^^^^^^^^^ not FFI-safe
+   |
+   = help: consider using `*const u8` and a length instead
+   = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer
+note: the lint level is defined here
+  --> $DIR/lint-ctypes.rs:7:26
+   |
+LL | #![deny(improper_ctypes, improper_ctypes_definitions)]
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 22 previous errors
 

tests/ui/statics/nested-allocations-dont-inherit-codegen-attrs.rs

@@ -1,5 +1,7 @@
 //@ build-pass
 
+#![allow(improper_c_var_definitions)]
+
 // Make sure that the nested static allocation for `FOO` doesn't inherit `no_mangle`.
 #[no_mangle]
 pub static mut FOO: &mut [i32] = &mut [42];