Skip to content

Implement pattern type ffi checks #136193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,6 @@ lint_improper_ctypes_only_phantomdata = composed only of `PhantomData`

lint_improper_ctypes_opaque = opaque types have no C equivalent

lint_improper_ctypes_pat_help = consider using the base type instead

lint_improper_ctypes_pat_reason = pattern 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
Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_lint/src/foreign_modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,7 @@ fn structurally_same_type_impl<'tcx>(
if let ty::Adt(def, args) = *ty.kind() {
let is_transparent = def.repr().transparent();
let is_non_null = types::nonnull_optimization_guaranteed(tcx, def);
debug!(
"non_transparent_ty({:?}) -- type is transparent? {}, type is non-null? {}",
ty, is_transparent, is_non_null
);
debug!(?ty, is_transparent, is_non_null);
if is_transparent && !is_non_null {
debug_assert_eq!(def.variants().len(), 1);
let v = &def.variant(FIRST_VARIANT);
Expand Down Expand Up @@ -378,14 +375,14 @@ fn structurally_same_type_impl<'tcx>(

// An Adt and a primitive or pointer type. This can be FFI-safe if non-null
// enum layout optimisation is being applied.
(Adt(..), _) if is_primitive_or_pointer(b) => {
(Adt(..) | Pat(..), _) if is_primitive_or_pointer(b) => {
if let Some(a_inner) = types::repr_nullable_ptr(tcx, typing_env, a, ckind) {
a_inner == b
} else {
false
}
}
(_, Adt(..)) if is_primitive_or_pointer(a) => {
(_, Adt(..) | Pat(..)) if is_primitive_or_pointer(a) => {
if let Some(b_inner) = types::repr_nullable_ptr(tcx, typing_env, b, ckind) {
b_inner == a
} else {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#![feature(let_chains)]
#![feature(rustc_attrs)]
#![feature(rustdoc_internals)]
#![feature(try_blocks)]
#![warn(unreachable_pub)]
// tidy-alphabetical-end

Expand Down
148 changes: 91 additions & 57 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,37 @@ fn ty_is_known_nonnull<'tcx>(
.filter_map(|variant| transparent_newtype_field(tcx, variant))
.any(|field| ty_is_known_nonnull(tcx, typing_env, field.ty(tcx, args), mode))
}
ty::Pat(base, pat) => {
ty_is_known_nonnull(tcx, typing_env, *base, mode)
|| Option::unwrap_or_default(
try {
match **pat {
ty::PatternKind::Range { start, end, include_end } => {
match (start, end) {
(Some(start), None) => {
start.try_to_value()?.try_to_bits(tcx, typing_env)? > 0
}
(Some(start), Some(end)) => {
let start =
start.try_to_value()?.try_to_bits(tcx, typing_env)?;
let end =
end.try_to_value()?.try_to_bits(tcx, typing_env)?;

if include_end {
// This also works for negative numbers, as we just need
// to ensure we aren't wrapping over zero.
start > 0 && end >= start
} else {
start > 0 && end > start
}
}
_ => false,
}
}
}
},
)
}
_ => false,
}
}
Expand Down Expand Up @@ -907,9 +938,8 @@ fn get_nullable_type<'tcx>(
};
return get_nullable_type(tcx, typing_env, inner_field_ty);
}
ty::Int(ty) => Ty::new_int(tcx, ty),
ty::Uint(ty) => Ty::new_uint(tcx, ty),
ty::RawPtr(ty, mutbl) => Ty::new_ptr(tcx, ty, mutbl),
ty::Pat(base, ..) => return get_nullable_type(tcx, typing_env, base),
ty::Int(_) | ty::Uint(_) | ty::RawPtr(..) => ty,
// As these types are always non-null, the nullable equivalent of
// `Option<T>` of these types are their raw pointer counterparts.
ty::Ref(_region, ty, mutbl) => Ty::new_ptr(tcx, ty, mutbl),
Expand Down Expand Up @@ -965,63 +995,69 @@ pub(crate) fn repr_nullable_ptr<'tcx>(
ckind: CItemKind,
) -> Option<Ty<'tcx>> {
debug!("is_repr_nullable_ptr(tcx, ty = {:?})", ty);
if let ty::Adt(ty_def, args) = ty.kind() {
let field_ty = match &ty_def.variants().raw[..] {
[var_one, var_two] => match (&var_one.fields.raw[..], &var_two.fields.raw[..]) {
([], [field]) | ([field], []) => field.ty(tcx, args),
([field1], [field2]) => {
let ty1 = field1.ty(tcx, args);
let ty2 = field2.ty(tcx, args);

if is_niche_optimization_candidate(tcx, typing_env, ty1) {
ty2
} else if is_niche_optimization_candidate(tcx, typing_env, ty2) {
ty1
} else {
return None;
match ty.kind() {
ty::Adt(ty_def, args) => {
let field_ty = match &ty_def.variants().raw[..] {
[var_one, var_two] => match (&var_one.fields.raw[..], &var_two.fields.raw[..]) {
([], [field]) | ([field], []) => field.ty(tcx, args),
([field1], [field2]) => {
let ty1 = field1.ty(tcx, args);
let ty2 = field2.ty(tcx, args);

if is_niche_optimization_candidate(tcx, typing_env, ty1) {
ty2
} else if is_niche_optimization_candidate(tcx, typing_env, ty2) {
ty1
} else {
return None;
}
}
}
_ => return None,
},
_ => return None,
},
_ => return None,
};
};

if !ty_is_known_nonnull(tcx, typing_env, field_ty, ckind) {
return None;
}
if !ty_is_known_nonnull(tcx, typing_env, field_ty, ckind) {
return None;
}

// At this point, the field's type is known to be nonnull and the parent enum is Option-like.
// If the computed size for the field and the enum are different, the nonnull optimization isn't
// being applied (and we've got a problem somewhere).
let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, typing_env).ok();
if !compute_size_skeleton(ty)?.same_size(compute_size_skeleton(field_ty)?) {
bug!("improper_ctypes: Option nonnull optimization not applied?");
}
// At this point, the field's type is known to be nonnull and the parent enum is Option-like.
// If the computed size for the field and the enum are different, the nonnull optimization isn't
// being applied (and we've got a problem somewhere).
let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, typing_env).ok();
if !compute_size_skeleton(ty)?.same_size(compute_size_skeleton(field_ty)?) {
bug!("improper_ctypes: Option nonnull optimization not applied?");
}

// Return the nullable type this Option-like enum can be safely represented with.
let field_ty_layout = tcx.layout_of(typing_env.as_query_input(field_ty));
if field_ty_layout.is_err() && !field_ty.has_non_region_param() {
bug!("should be able to compute the layout of non-polymorphic type");
}
// Return the nullable type this Option-like enum can be safely represented with.
let field_ty_layout = tcx.layout_of(typing_env.as_query_input(field_ty));
if field_ty_layout.is_err() && !field_ty.has_non_region_param() {
bug!("should be able to compute the layout of non-polymorphic type");
}

let field_ty_abi = &field_ty_layout.ok()?.backend_repr;
if let BackendRepr::Scalar(field_ty_scalar) = field_ty_abi {
match field_ty_scalar.valid_range(&tcx) {
WrappingRange { start: 0, end }
if end == field_ty_scalar.size(&tcx).unsigned_int_max() - 1 =>
{
return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
}
WrappingRange { start: 1, .. } => {
return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
}
WrappingRange { start, end } => {
unreachable!("Unhandled start and end range: ({}, {})", start, end)
}
};
let field_ty_abi = &field_ty_layout.ok()?.backend_repr;
if let BackendRepr::Scalar(field_ty_scalar) = field_ty_abi {
match field_ty_scalar.valid_range(&tcx) {
WrappingRange { start: 0, end }
if end == field_ty_scalar.size(&tcx).unsigned_int_max() - 1 =>
{
return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
}
WrappingRange { start: 1, .. } => {
return Some(get_nullable_type(tcx, typing_env, field_ty).unwrap());
}
WrappingRange { start, end } => {
unreachable!("Unhandled start and end range: ({}, {})", start, end)
}
};
}
None
}
ty::Pat(base, pat) => match **pat {
ty::PatternKind::Range { .. } => get_nullable_type(tcx, typing_env, *base),
},
_ => None,
}
None
}

impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
Expand Down Expand Up @@ -1256,11 +1292,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
help: Some(fluent::lint_improper_ctypes_char_help),
},

ty::Pat(..) => FfiUnsafe {
ty,
reason: fluent::lint_improper_ctypes_pat_reason,
help: Some(fluent::lint_improper_ctypes_pat_help),
},
// It's just extra invariants on the type that you need to uphold,
// but only the base type is relevant for being representable in FFI.
ty::Pat(base, ..) => self.check_type_for_ffi(acc, base),

ty::Int(ty::IntTy::I128) | ty::Uint(ty::UintTy::U128) => {
FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_128bit, help: None }
Expand Down
32 changes: 31 additions & 1 deletion tests/ui/lint/clashing-extern-fn.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//@ check-pass
//@ aux-build:external_extern_fn.rs
#![crate_type = "lib"]

#![feature(pattern_type_macro, pattern_types)]
mod redeclared_different_signature {
mod a {
extern "C" {
Expand Down Expand Up @@ -490,3 +490,33 @@ mod hidden_niche {
}
}
}

mod pattern_types {
mod a {
use std::pat::pattern_type;
#[repr(transparent)]
struct NonZeroUsize(pattern_type!(usize is 1..));
extern "C" {
fn pt_non_zero_usize() -> pattern_type!(usize is 1..);
fn pt_non_zero_usize_opt() -> Option<pattern_type!(usize is 1..)>;
fn pt_non_zero_usize_opt_full_range() -> Option<pattern_type!(usize is 0..)>;
//~^ WARN not FFI-safe
fn pt_non_null_ptr() -> pattern_type!(usize is 1..);
fn pt_non_zero_usize_wrapper() -> NonZeroUsize;
fn pt_non_zero_usize_wrapper_opt() -> Option<NonZeroUsize>;
}
}
mod b {
extern "C" {
// If there's a clash in either of these cases you're either gaining an incorrect
// invariant that the value is non-zero, or you're missing out on that invariant. Both
// cases are warning for, from both a caller-convenience and optimisation perspective.
fn pt_non_zero_usize() -> usize;
fn pt_non_zero_usize_opt() -> usize;
fn pt_non_null_ptr() -> *const ();
//~^ WARN `pt_non_null_ptr` redeclared with a different signature
fn pt_non_zero_usize_wrapper() -> usize;
fn pt_non_zero_usize_wrapper_opt() -> usize;
}
}
}
23 changes: 22 additions & 1 deletion tests/ui/lint/clashing-extern-fn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ LL | fn hidden_niche_unsafe_cell() -> Option<UnsafeCell<NonZero<usiz
= help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
= note: enum has no representation hint

warning: `extern` block uses type `Option<(usize) is 0..=>`, which is not FFI-safe
--> $DIR/clashing-extern-fn.rs:502:54
|
LL | fn pt_non_zero_usize_opt_full_range() -> Option<pattern_type!(usize is 0..)>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
= help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
= note: enum has no representation hint

warning: `clash` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:13:13
|
Expand Down Expand Up @@ -258,5 +267,17 @@ LL | fn hidden_niche_unsafe_cell() -> Option<UnsafeCell<NonZero<usiz
= note: expected `unsafe extern "C" fn() -> usize`
found `unsafe extern "C" fn() -> Option<UnsafeCell<NonZero<usize>>>`

warning: 22 warnings emitted
warning: `pt_non_null_ptr` redeclared with a different signature
--> $DIR/clashing-extern-fn.rs:516:13
|
LL | fn pt_non_null_ptr() -> pattern_type!(usize is 1..);
| ---------------------------------------------------- `pt_non_null_ptr` previously declared here
...
LL | fn pt_non_null_ptr() -> *const ();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration
|
= note: expected `unsafe extern "C" fn() -> (usize) is 1..=`
found `unsafe extern "C" fn() -> *const ()`

warning: 24 warnings emitted

1 change: 1 addition & 0 deletions tests/ui/lint/lint-ctypes-enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ extern "C" {
fn option_transparent_union(x: Option<TransparentUnion<num::NonZero<u8>>>);
//~^ ERROR `extern` block uses type
fn option_repr_rust(x: Option<Rust<num::NonZero<u8>>>); //~ ERROR `extern` block uses type
fn option_u8(x: Option<u8>); //~ ERROR `extern` block uses type

fn result_ref_t(x: Result<&'static u8, ()>);
fn result_fn_t(x: Result<extern "C" fn(), ()>);
Expand Down
Loading
Loading