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/lib.rs

@@ -339,6 +339,7 @@ fn register_builtins(store: &mut LintStore) {
         "improper_c_boundaries",
         IMPROPER_C_CALLBACKS,
         IMPROPER_C_FN_DEFINITIONS,
+        IMPROPER_C_VAR_DEFINITIONS,
         IMPROPER_CTYPES
     );
 

compiler/rustc_lint/src/types.rs

@@ -12,7 +12,8 @@ use {rustc_ast as ast, rustc_hir as hir};
 
 mod improper_ctypes; // these filed do the implementation for ImproperCTypesDefinitions,ImproperCTypesDeclarations
 pub(crate) use improper_ctypes::{
-    IMPROPER_C_CALLBACKS, IMPROPER_C_FN_DEFINITIONS, IMPROPER_CTYPES, ImproperCTypesLint,
+    IMPROPER_C_CALLBACKS, IMPROPER_C_FN_DEFINITIONS, IMPROPER_C_VAR_DEFINITIONS, IMPROPER_CTYPES,
+    ImproperCTypesLint,
 };
 
 use crate::lints::{

compiler/rustc_lint/src/types/improper_ctypes.rs

@@ -79,6 +79,37 @@ declare_lint! {
     "proper use of libc types in foreign item definitions"
 }
 
+declare_lint! {
+    /// The `improper_c_var_definitions` lint detects incorrect use of
+    /// [`no_mangle`] and [`export_name`] static variable definitions.
+    /// (In other words, static variables accessible by name by foreign code.)
+    ///
+    /// [`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
+    /// # #[unsafe(no_mangle)]
+    /// # #[used]
+    /// static mut PLUGIN_ABI_MIN_VERSION: &'static str = "0.0.5";
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// The compiler has several checks to verify that types used in
+    /// static variables exposed to foreign code are safe and follow
+    /// certain rules to ensure proper compatibility with the foreign interfaces.
+    /// This lint is issued when it detects a probable mistake in a definition.
+    /// The lint usually should provide a description of the issue,
+    /// along with possibly a hint on how to resolve it.
+    pub(crate) IMPROPER_C_VAR_DEFINITIONS,
+    Warn,
+    "proper use of libc types in foreign-reachable static variable definitions"
+}
+
 declare_lint! {
     /// The `improper_c_callbacks` lint detects incorrect use of
     /// [`extern` function] pointers.
@@ -165,6 +196,7 @@ declare_lint! {
 declare_lint_pass!(ImproperCTypesLint => [
     IMPROPER_CTYPES,
     IMPROPER_C_FN_DEFINITIONS,
+    IMPROPER_C_VAR_DEFINITIONS,
     IMPROPER_C_CALLBACKS,
     USES_POWER_ALIGNMENT,
 ]);
@@ -316,6 +348,8 @@ enum CItemKind {
     ImportedExtern,
     /// `extern "C"` function definitions, to be used elsewhere -> IMPROPER_C_FN_DEFINITIONS,
     ExportedFunction,
+    /// `no_mangle`/`export_name` static variables, assumed to be used from across an FFI boundary,
+    ExportedStatic,
     /// `extern "C"` function pointers -> IMPROPER_C_CALLBACKS,
     Callback,
 }
@@ -691,6 +725,8 @@ struct VisitorState {
 impl PersistentStateFlags {
     // 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 =
@@ -792,6 +828,12 @@ impl VisitorState {
         Self::entry_point(PersistentStateFlags::STATIC_TY)
     }
 
+    /// Get the proper visitor state for a locally-defined static variable's type
+    #[inline]
+    fn static_var_def() -> Self {
+        Self::entry_point(PersistentStateFlags::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.persistent_flags.contains(PersistentStateFlags::STATIC);
@@ -1716,6 +1758,14 @@ 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::OwnerId, span: Span) {
+        let ty = cx.tcx.type_of(id).instantiate_identity();
+        let mut visitor = ImproperCTypesVisitor::new(cx);
+        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,
@@ -1816,11 +1866,13 @@ impl<'tcx> ImproperCTypesLint {
         let lint = match fn_mode {
             CItemKind::ImportedExtern => IMPROPER_CTYPES,
             CItemKind::ExportedFunction => IMPROPER_C_FN_DEFINITIONS,
+            CItemKind::ExportedStatic => IMPROPER_C_VAR_DEFINITIONS,
             CItemKind::Callback => IMPROPER_C_CALLBACKS,
         };
         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() {
@@ -1888,6 +1940,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.owner_id, ty.span);
+                    }
+                }
             }
             // See `check_fn` for declarations, `check_foreign_items` for definitions in extern blocks
             hir::ItemKind::Fn { .. } => {}

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

@@ -5,7 +5,7 @@
 
 #![allow(private_interfaces)]
 #![deny(improper_ctypes, improper_c_callbacks)]
-#![deny(improper_c_fn_definitions)]
+#![deny(improper_c_fn_definitions, improper_c_var_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

@@ -208,5 +208,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 20 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:8:36
+   |
+LL | #![deny(improper_c_fn_definitions, improper_c_var_definitions)]
+   |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 21 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];