diff options
| -rw-r--r-- | src/librustc_lint/builtin.rs | 69 | ||||
| -rw-r--r-- | src/librustc_lint/types.rs | 31 | ||||
| -rw-r--r-- | src/test/ui/lint/clashing-extern-fn.stderr | 62 |
3 files changed, 137 insertions, 25 deletions
diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index a45817beea1..47d1c7ba6f1 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -2155,14 +2155,16 @@ impl ClashingExternDeclarations { let a_kind = &a.kind; let b_kind = &b.kind; + use rustc_target::abi::LayoutOf; + let compare_layouts = |a, b| { + let a_layout = &cx.layout_of(a).unwrap().layout.abi; + let b_layout = &cx.layout_of(b).unwrap().layout.abi; + let result = a_layout == b_layout; + result + }; + match (a_kind, b_kind) { - (Adt(..), Adt(..)) => { - // Adts are pretty straightforward: just compare the layouts. - use rustc_target::abi::LayoutOf; - let a_layout = cx.layout_of(a).unwrap().layout; - let b_layout = cx.layout_of(b).unwrap().layout; - a_layout == b_layout - } + (Adt(..), Adt(..)) => compare_layouts(a, b), (Array(a_ty, a_const), Array(b_ty, b_const)) => { // For arrays, we also check the constness of the type. a_const.val == b_const.val @@ -2179,10 +2181,10 @@ impl ClashingExternDeclarations { a_mut == b_mut && Self::structurally_same_type(cx, a_ty, b_ty) } (FnDef(..), FnDef(..)) => { - // As we don't compare regions, skip_binder is fine. let a_poly_sig = a.fn_sig(tcx); let b_poly_sig = b.fn_sig(tcx); + // As we don't compare regions, skip_binder is fine. let a_sig = a_poly_sig.skip_binder(); let b_sig = b_poly_sig.skip_binder(); @@ -2210,7 +2212,56 @@ impl ClashingExternDeclarations { | (Opaque(..), Opaque(..)) => false, // These definitely should have been caught above. (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => unreachable!(), - _ => false, + + // Disjoint kinds. + (_, _) => { + // First, check if the conversion is FFI-safe. This can be so if the type is an + // enum with a non-null field (see improper_ctypes). + let is_primitive_or_pointer = + |ty: Ty<'tcx>| ty.is_primitive() || matches!(ty.kind, RawPtr(..)); + if (is_primitive_or_pointer(a) || is_primitive_or_pointer(b)) + && !(is_primitive_or_pointer(a) && is_primitive_or_pointer(b)) + && (matches!(a_kind, Adt(..)) || matches!(b_kind, Adt(..))) + /* ie, 1 adt and 1 primitive */ + { + let (primitive_ty, adt_ty) = + if is_primitive_or_pointer(a) { (a, b) } else { (b, a) }; + // First, check that the Adt is FFI-safe to use. + use crate::types::{ImproperCTypesMode, ImproperCTypesVisitor}; + let vis = + ImproperCTypesVisitor { cx, mode: ImproperCTypesMode::Declarations }; + + if let Adt(def, substs) = adt_ty.kind { + let repr_nullable = vis.is_repr_nullable_ptr(adt_ty, def, substs); + if let Some(safe_ty) = repr_nullable { + let safe_ty_layout = &cx.layout_of(safe_ty).unwrap(); + let primitive_ty_layout = &cx.layout_of(primitive_ty).unwrap(); + + use rustc_target::abi::Abi::*; + match (&safe_ty_layout.abi, &primitive_ty_layout.abi) { + (Scalar(safe), Scalar(primitive)) => { + // The two types are safe to convert between if `safe` is + // the non-zero version of `primitive`. + use std::ops::RangeInclusive; + + let safe_range: &RangeInclusive<_> = &safe.valid_range; + let primitive_range: &RangeInclusive<_> = + &primitive.valid_range; + + return primitive_range.end() == safe_range.end() + // This check works for both signed and unsigned types due to wraparound. + && *safe_range.start() == 1 + && *primitive_range.start() == 0; + } + _ => {} + } + } + } + } + // Otherwise, just compare the layouts. This may be underapproximate, but at + // the very least, will stop reads into uninitialised memory. + compare_layouts(a, b) + } } } } diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index 1affc9457b8..3d1080bea4c 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -509,14 +509,14 @@ declare_lint! { declare_lint_pass!(ImproperCTypesDefinitions => [IMPROPER_CTYPES_DEFINITIONS]); -enum ImproperCTypesMode { +crate enum ImproperCTypesMode { Declarations, Definitions, } -struct ImproperCTypesVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - mode: ImproperCTypesMode, +crate struct ImproperCTypesVisitor<'a, 'tcx> { + crate cx: &'a LateContext<'tcx>, + crate mode: ImproperCTypesMode, } enum FfiResult<'tcx> { @@ -562,17 +562,18 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } } - /// Check if this enum can be safely exported based on the "nullable pointer optimization". - /// Currently restricted to function pointers, boxes, references, `core::num::NonZero*`, - /// `core::ptr::NonNull`, and `#[repr(transparent)]` newtypes. - fn is_repr_nullable_ptr( + /// Check if this enum can be safely exported based on the "nullable pointer optimization". If + /// the type is it is, return the known non-null field type, else None. Currently restricted + /// to function pointers, boxes, references, `core::num::NonZero*`, `core::ptr::NonNull`, and + /// `#[repr(transparent)]` newtypes. + crate fn is_repr_nullable_ptr( &self, ty: Ty<'tcx>, ty_def: &'tcx ty::AdtDef, substs: SubstsRef<'tcx>, - ) -> bool { + ) -> Option<Ty<'tcx>> { if ty_def.variants.len() != 2 { - return false; + return None; } let get_variant_fields = |index| &ty_def.variants[VariantIdx::new(index)].fields; @@ -582,16 +583,16 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } else if variant_fields[1].is_empty() { &variant_fields[0] } else { - return false; + return None; }; if fields.len() != 1 { - return false; + return None; } let field_ty = fields[0].ty(self.cx.tcx, substs); if !self.ty_is_known_nonnull(field_ty) { - return false; + return None; } // At this point, the field's type is known to be nonnull and the parent enum is @@ -603,7 +604,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { bug!("improper_ctypes: Option nonnull optimization not applied?"); } - true + Some(field_ty) } /// Check if the type is array and emit an unsafe type lint. @@ -753,7 +754,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // discriminant. if !def.repr.c() && !def.repr.transparent() && def.repr.int.is_none() { // Special-case types like `Option<extern fn()>`. - if !self.is_repr_nullable_ptr(ty, def, substs) { + if self.is_repr_nullable_ptr(ty, def, substs).is_none() { return FfiUnsafe { ty, reason: "enum has no representation hint".into(), diff --git a/src/test/ui/lint/clashing-extern-fn.stderr b/src/test/ui/lint/clashing-extern-fn.stderr index a2aaaba7173..49a9d044a2f 100644 --- a/src/test/ui/lint/clashing-extern-fn.stderr +++ b/src/test/ui/lint/clashing-extern-fn.stderr @@ -105,5 +105,65 @@ LL | fn draw_point(p: Point); = note: expected `unsafe extern "C" fn(sameish_members::a::Point)` found `unsafe extern "C" fn(sameish_members::b::Point)` -warning: 8 warnings emitted +warning: `missing_return_type` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:208:13 + | +LL | fn missing_return_type() -> usize; + | ---------------------------------- `missing_return_type` previously declared here +... +LL | fn missing_return_type(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn() -> usize` + found `unsafe extern "C" fn()` + +warning: `non_zero_usize` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:226:13 + | +LL | fn non_zero_usize() -> core::num::NonZeroUsize; + | ----------------------------------------------- `non_zero_usize` previously declared here +... +LL | fn non_zero_usize() -> usize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn() -> std::num::NonZeroUsize` + found `unsafe extern "C" fn() -> usize` + +warning: `non_null_ptr` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:228:13 + | +LL | fn non_null_ptr() -> core::ptr::NonNull<usize>; + | ----------------------------------------------- `non_null_ptr` previously declared here +... +LL | fn non_null_ptr() -> *const usize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn() -> std::ptr::NonNull<usize>` + found `unsafe extern "C" fn() -> *const usize` + +warning: `option_non_zero_usize_incorrect` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:254:13 + | +LL | fn option_non_zero_usize_incorrect() -> usize; + | ---------------------------------------------- `option_non_zero_usize_incorrect` previously declared here +... +LL | fn option_non_zero_usize_incorrect() -> isize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn() -> usize` + found `unsafe extern "C" fn() -> isize` + +warning: `option_non_null_ptr_incorrect` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:256:13 + | +LL | fn option_non_null_ptr_incorrect() -> *const usize; + | --------------------------------------------------- `option_non_null_ptr_incorrect` previously declared here +... +LL | fn option_non_null_ptr_incorrect() -> *const isize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn() -> *const usize` + found `unsafe extern "C" fn() -> *const isize` + +warning: 13 warnings emitted |