Skip to content

Commit 9dfb1d0

Browse files
committed
lint ImproperCTypes: redo handling of pattern types [...]
- also fix a couple of thorny typos in/around types.rs::is_outer_optionlike_around_ty() and subsequently fix library and tests
1 parent b73c51f commit 9dfb1d0

File tree

13 files changed

+570
-78
lines changed

13 files changed

+570
-78
lines changed

compiler/rustc_lint/messages.ftl

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,8 +415,10 @@ lint_improper_ctypes_only_phantomdata = composed only of `PhantomData`
415415
416416
lint_improper_ctypes_opaque = opaque types have no C equivalent
417417
418-
lint_improper_ctypes_pat_intrange_help = consider using the base type instead
419-
lint_improper_ctypes_pat_intrange_reason = integers constrained to a given range cannot have their value be provided by non-rust code
418+
lint_improper_ctypes_pat_int1_help = consider using the base type instead, or wrapping `{$ty}` in an `Option<_>`
419+
lint_improper_ctypes_pat_int1_reason = integer-pattern types with one disallowed value and no `Option` wrapping cannot have their value be provided by non-rust code
420+
lint_improper_ctypes_pat_int2_help = consider using the base type instead
421+
lint_improper_ctypes_pat_int2_reason = integer-pattern types with more than one disallowed value cannot have their value be provided by non-rust code
420422
421423
lint_improper_ctypes_ptr_validity_help = consider using a raw pointer, or wrapping `{$ty}` in an `Option<_>`
422424
lint_improper_ctypes_ptr_validity_reason =

compiler/rustc_lint/src/foreign_modules.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,14 +366,14 @@ fn structurally_same_type_impl<'tcx>(
366366
// An Adt and a primitive or pointer type. This can be FFI-safe if non-null
367367
// enum layout optimisation is being applied.
368368
(ty::Adt(..) | ty::Pat(..), _) if is_primitive_or_pointer(b) => {
369-
if let Some(a_inner) = types::repr_nullable_ptr(tcx, typing_env, a) {
369+
if let Some(a_inner) = types::repr_nullable_ptr(tcx, typing_env, a, false) {
370370
a_inner == b
371371
} else {
372372
false
373373
}
374374
}
375375
(_, ty::Adt(..) | ty::Pat(..)) if is_primitive_or_pointer(a) => {
376-
if let Some(b_inner) = types::repr_nullable_ptr(tcx, typing_env, b) {
376+
if let Some(b_inner) = types::repr_nullable_ptr(tcx, typing_env, b, false) {
377377
b_inner == a
378378
} else {
379379
false

compiler/rustc_lint/src/types.rs

Lines changed: 204 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
use std::iter;
22

3-
use rustc_abi::{BackendRepr, TagEncoding, Variants, WrappingRange};
3+
use rustc_abi::{BackendRepr, Size, TagEncoding, Variants, WrappingRange};
44
use rustc_hir::{Expr, ExprKind, HirId, LangItem};
55
use rustc_middle::bug;
66
use rustc_middle::ty::layout::{LayoutOf, SizeSkeleton};
7-
use rustc_middle::ty::{self, AdtKind, Ty, TyCtxt, TypeVisitableExt};
7+
use rustc_middle::ty::{self, AdtKind, Const, ScalarInt, Ty, TyCtxt, TypeVisitableExt};
88
use rustc_session::{declare_lint, declare_lint_pass, impl_lint_pass};
99
use rustc_span::{Span, Symbol, sym};
1010
use tracing::debug;
@@ -868,13 +868,14 @@ fn is_niche_optimization_candidate<'tcx>(
868868
}
869869

870870
/// Check if this enum can be safely exported based on the "nullable pointer optimization". If it
871-
/// can, return the type that `ty` can be safely converted to, otherwise return `None`.
871+
/// can, return the type that `ty` can be safely converted to/from, otherwise return `None`.
872872
/// Currently restricted to function pointers, boxes, references, `core::num::NonZero`,
873-
/// `core::ptr::NonNull`, and `#[repr(transparent)]` newtypes.
873+
/// `core::ptr::NonNull`, `#[repr(transparent)]` newtypes, and int-range pattern types.
874874
pub(crate) fn repr_nullable_ptr<'tcx>(
875875
tcx: TyCtxt<'tcx>,
876876
typing_env: ty::TypingEnv<'tcx>,
877877
ty: Ty<'tcx>,
878+
checked_conversion_is_from: bool,
878879
) -> Option<Ty<'tcx>> {
879880
debug!("is_repr_nullable_ptr(tcx, ty = {:?})", ty);
880881
match ty.kind() {
@@ -899,6 +900,20 @@ pub(crate) fn repr_nullable_ptr<'tcx>(
899900
_ => return None,
900901
};
901902

903+
if let ty::Pat(base, pat) = field_ty.kind() {
904+
return if let Some(disallowed) = get_pat_disallowed_value_count(*pat) {
905+
if disallowed != 1 && checked_conversion_is_from {
906+
// if there are values not taken into account by the optionlike Enum
907+
// then we can't safely convert from the base type, only the pattern type
908+
Some(field_ty)
909+
} else {
910+
get_nullable_type_from_pat(tcx, typing_env, *base, *pat)
911+
}
912+
} else {
913+
None
914+
};
915+
}
916+
902917
if !ty_is_known_nonnull(tcx, typing_env, field_ty) {
903918
return None;
904919
}
@@ -935,11 +950,191 @@ pub(crate) fn repr_nullable_ptr<'tcx>(
935950
}
936951
None
937952
}
938-
ty::Pat(base, pat) => get_nullable_type_from_pat(tcx, typing_env, *base, *pat),
953+
ty::Pat(base, pat) => {
954+
if checked_conversion_is_from && get_pat_disallowed_value_count(*pat).is_some() {
955+
// if there are values not taken into account by the pattern (the usual case)
956+
// then we can't safely convert from the base type
957+
None
958+
} else {
959+
get_nullable_type_from_pat(tcx, typing_env, *base, *pat)
960+
}
961+
}
939962
_ => None,
940963
}
941964
}
942965

966+
/// return the number of disallowed values in a pattern type
967+
/// note that Some(0) actually maps to 2^128 rather than 0
968+
pub(crate) fn get_pat_disallowed_value_count<'tcx>(pat: ty::Pattern<'tcx>) -> Option<u128> {
969+
// note the logic in this function assumes that signed ints use one's complement representation,
970+
// which I believe is a requirement for rust
971+
972+
/// find numeric metadata on a pair of range bounds
973+
/// if None, assume that there are no bounds specified
974+
/// and that this is a usize. in other words, all values are allowed
975+
fn unwrap_start_end<'tcx>(
976+
start: Const<'tcx>,
977+
end: Const<'tcx>,
978+
) -> (bool, Size, ScalarInt, ScalarInt) {
979+
let usable_bound = match (start.try_to_value(), end.try_to_value()) {
980+
(Some(ty), _) | (_, Some(ty)) => ty,
981+
(None, None) => bug!(
982+
"pattern range should have at least one defined value: {:?} - {:?}",
983+
start,
984+
end,
985+
),
986+
};
987+
let usable_size = usable_bound.valtree.unwrap_leaf().size();
988+
let is_signed = match usable_bound.ty.kind() {
989+
ty::Int(_) => true,
990+
ty::Uint(_) | ty::Char => false,
991+
kind @ _ => bug!("unexpected non-scalar base for pattern bounds: {:?}", kind),
992+
};
993+
994+
let end = match end.try_to_value() {
995+
Some(end) => end.valtree.unwrap_leaf(),
996+
None => {
997+
let max_val = if is_signed {
998+
usable_size.signed_int_max() as u128
999+
} else {
1000+
usable_size.unsigned_int_max()
1001+
};
1002+
ScalarInt::try_from_uint(max_val, usable_size).unwrap()
1003+
}
1004+
};
1005+
let start = match start.try_to_value() {
1006+
Some(start) => start.valtree.unwrap_leaf(),
1007+
None => {
1008+
let min_val = if is_signed {
1009+
(usable_size.signed_int_min() as u128) & usable_size.unsigned_int_max()
1010+
} else {
1011+
0_u128
1012+
};
1013+
ScalarInt::try_from_uint(min_val, usable_size).unwrap()
1014+
}
1015+
};
1016+
(is_signed, usable_size, start, end)
1017+
}
1018+
1019+
match *pat {
1020+
ty::PatternKind::Range { start, end } => {
1021+
let (is_signed, scalar_size, start, end) = unwrap_start_end(start, end);
1022+
let (scalar_min, scalar_max) = if is_signed {
1023+
(
1024+
(scalar_size.signed_int_min() as u128) & scalar_size.unsigned_int_max(),
1025+
scalar_size.signed_int_max() as u128,
1026+
)
1027+
} else {
1028+
(0, scalar_size.unsigned_int_max())
1029+
};
1030+
1031+
if (start.to_bits(scalar_size), end.to_bits(scalar_size)) == (scalar_min, scalar_max) {
1032+
return None;
1033+
}
1034+
1035+
// note: allow overflow here because negative values are allowed in the scalars represented here
1036+
let allowed_value_count_minus1 =
1037+
u128::overflowing_sub(end.to_bits(scalar_size), start.to_bits(scalar_size)).0
1038+
& scalar_size.unsigned_int_max();
1039+
let disallowed_value_count =
1040+
u128::overflowing_sub(scalar_size.unsigned_int_max(), allowed_value_count_minus1).0;
1041+
Some(disallowed_value_count)
1042+
}
1043+
ty::PatternKind::Or(patterns) => {
1044+
// first, get a simplified an sorted view of the ranges
1045+
let (is_signed, scalar_size, mut ranges) = {
1046+
let (is_signed, size, start, end) = match &*patterns[0] {
1047+
ty::PatternKind::Range { start, end } => unwrap_start_end(*start, *end),
1048+
ty::PatternKind::Or(_) => bug!("recursive \"or\" patterns?"),
1049+
};
1050+
(is_signed, size, vec![(start, end)])
1051+
};
1052+
let scalar_max = if is_signed {
1053+
scalar_size.signed_int_max() as u128
1054+
} else {
1055+
scalar_size.unsigned_int_max()
1056+
};
1057+
ranges.reserve(patterns.len() - 1);
1058+
for pat in patterns.iter().skip(1) {
1059+
match *pat {
1060+
ty::PatternKind::Range { start, end } => {
1061+
let (is_this_signed, this_scalar_size, start, end) =
1062+
unwrap_start_end(start, end);
1063+
assert_eq!(is_signed, is_this_signed);
1064+
assert_eq!(scalar_size, this_scalar_size);
1065+
ranges.push((start, end))
1066+
}
1067+
ty::PatternKind::Or(_) => bug!("recursive \"or\" patterns?"),
1068+
}
1069+
}
1070+
ranges.sort_by_key(|(start, _end)| {
1071+
let is_positive =
1072+
if is_signed { start.to_bits(scalar_size) <= scalar_max } else { true };
1073+
(is_positive, start.to_bits(scalar_size))
1074+
});
1075+
1076+
// then, range per range, look at the sizes of the gaps left in between
1077+
// (`prev_tail` is the highest value currently accounted for by the ranges,
1078+
// unless the first range has not been dealt with yet)
1079+
let mut prev_tail = scalar_max;
1080+
let mut disallowed_value_count = 0_u128;
1081+
let mut only_had_overlaps = true;
1082+
1083+
for (range_i, (start, end)) in ranges.into_iter().enumerate() {
1084+
let (start, end) = (start.to_bits(scalar_size), end.to_bits(scalar_size));
1085+
1086+
// if the start of the current range is lower
1087+
// than the current-highest-range-end, ...
1088+
let current_range_overlap =
1089+
if is_signed && prev_tail > scalar_max && start <= scalar_max {
1090+
false
1091+
} else if start <= u128::overflowing_add(prev_tail, 1).0 {
1092+
range_i > 0 // no overlap possible when dealing with the first range
1093+
} else {
1094+
false
1095+
};
1096+
if current_range_overlap {
1097+
// update the curent-highest-range-end, if the current range has a higher end
1098+
if is_signed {
1099+
if prev_tail > scalar_max && end <= scalar_max {
1100+
prev_tail = end;
1101+
} else if prev_tail <= scalar_max && end > scalar_max {
1102+
// nothing to do here
1103+
} else {
1104+
// prev_tail and end have the same sign
1105+
prev_tail = u128::max(prev_tail, end)
1106+
}
1107+
} else {
1108+
// prev_tail and end have the same sign
1109+
prev_tail = u128::max(prev_tail, end)
1110+
}
1111+
} else {
1112+
// no range overlap: first, add the newfound disallowed values to the count
1113+
only_had_overlaps = false;
1114+
let new_gap = u128::overflowing_sub(
1115+
start,
1116+
u128::overflowing_add(prev_tail, 1).0 & scalar_size.unsigned_int_max(),
1117+
)
1118+
.0 & scalar_size.unsigned_int_max();
1119+
disallowed_value_count =
1120+
u128::overflowing_add(disallowed_value_count, new_gap).0;
1121+
prev_tail = end;
1122+
}
1123+
}
1124+
if prev_tail != scalar_max {
1125+
disallowed_value_count = u128::overflowing_add(
1126+
disallowed_value_count,
1127+
u128::overflowing_sub(scalar_max, prev_tail).0,
1128+
)
1129+
.0;
1130+
only_had_overlaps = false;
1131+
}
1132+
1133+
if only_had_overlaps { None } else { Some(disallowed_value_count) }
1134+
}
1135+
}
1136+
}
1137+
9431138
fn get_nullable_type_from_pat<'tcx>(
9441139
tcx: TyCtxt<'tcx>,
9451140
typing_env: ty::TypingEnv<'tcx>,
@@ -969,19 +1164,19 @@ fn is_outer_optionlike_around_ty<'tcx>(
9691164
// That outer_ty is an enum, that this enum doesn't have a defined discriminant representation,
9701165
// and the the outer_ty's size is that of ty.
9711166
if let ty::Adt(def, _) = outer_ty.kind() {
972-
if !matches!(def.adt_kind(), AdtKind::Enum)
1167+
if (!matches!(def.adt_kind(), AdtKind::Enum))
9731168
|| def.repr().c()
9741169
|| def.repr().transparent()
975-
|| def.repr().int.is_none()
1170+
|| def.repr().int.is_some()
9761171
{
9771172
false
9781173
} else {
9791174
let (tcx, typing_env) = (cx.tcx, cx.typing_env());
9801175

9811176
// see the insides of super::repr_nullable_ptr()
982-
let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, typing_env).ok();
1177+
let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, typing_env);
9831178
match (compute_size_skeleton(ty), compute_size_skeleton(outer_ty)) {
984-
(Some(sk1), Some(sk2)) => sk1.same_size(sk2),
1179+
(Ok(sk1), Ok(sk2)) => sk1.same_size(sk2),
9851180
_ => false,
9861181
}
9871182
}

0 commit comments

Comments
 (0)