OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Don't emit clashing decl lint for FFI-safe enums.

Browse Source 
An example of an FFI-safe enum conversion is when converting
Option<NonZeroUsize> to usize. Because the Some value must be non-zero,
rustc can use 0 to represent the None variant, making this conversion is
safe. Furthermore, it can be relied on (and removing this optimisation
already would be a breaking change).
This commit is contained in:
jumbatm 2020-07-03 20:14:05 +10:00
parent 4da72f5387
commit 3eaead7d51
3 changed files with 137 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
src/librustc_lint/builtin.rs
View File

@ -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)
}
}
}
}

31
src/librustc_lint/types.rs
View File

@ -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(),

62
src/test/ui/lint/clashing-extern-fn.stderr
View File

@ -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