Skip to content

Commit 949e166

Browse files
zacklukemzacklukem
committed
Guarantee result nonzero for FFI
Implements the required comipler changes for result_ffi_guarantees (rust-lang#3391) Allows `Result<T, E>` to be used in extern "C" functions where one of (T, E) is a non-zero type and one of (T, E) is a zero sized type. e.g. `Result<NonZeroI32, ()>` Adds test cases to ensure that the abi for Result and Option types with nonzero representation remains consistent
1 parent f357475 commit 949e166

File tree

4 files changed

+309
-68
lines changed

4 files changed

+309
-68
lines changed

‎compiler/rustc_lint/src/types.rs

+55-9
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,14 @@ use rustc_data_structures::fx::FxHashSet;
1515
use rustc_errors::DiagnosticMessage;
1616
use rustc_hir as hir;
1717
use rustc_hir::{is_range_literal, Expr, ExprKind, Node};
18-
use rustc_middle::ty::layout::{IntegerExt, LayoutOf, SizeSkeleton};
19-
use rustc_middle::ty::subst::SubstsRef;
2018
use rustc_middle::ty::{
2119
self, AdtKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt,
2220
};
21+
use rustc_middle::ty::{
22+
layout::{IntegerExt, LayoutOf, SizeSkeleton},
23+
VariantDef,
24+
};
25+
use rustc_middle::ty::{subst::SubstsRef, FieldDef};
2326
use rustc_span::def_id::LocalDefId;
2427
use rustc_span::source_map;
2528
use rustc_span::symbol::sym;
@@ -770,8 +773,26 @@ pub(crate) fn repr_nullable_ptr<'tcx>(
770773
debug!("is_repr_nullable_ptr(cx, ty = {:?})", ty);
771774
if let ty::Adt(ty_def, substs) = ty.kind() {
772775
let field_ty = match &ty_def.variants().raw[..] {
773-
[var_one, var_two] => match (&var_one.fields.raw[..], &var_two.fields.raw[..]) {
776+
[variant1, variant2] => match (&variant1.fields.raw[..], &variant2.fields.raw[..]) {
777+
// If one variant is empty and the other is a single field (e.g. Option<T>)
774778
([], [field]) | ([field], []) => field.ty(cx.tcx, substs),
779+
// If one variant has a ZST and the other has a single field (e.g. Result<T, ()> or Result<(), T>)
780+
([field1], [field2]) => {
781+
let is_zst = |field: &FieldDef, variant: &VariantDef| {
782+
let param_env = cx.tcx.param_env(variant.def_id);
783+
let field_ty = field.ty(cx.tcx, substs);
784+
cx.tcx
785+
.layout_of(param_env.and(field_ty))
786+
.map_or(false, |layout| layout.is_zst())
787+
};
788+
if is_zst(field1, variant1) {
789+
field2.ty(cx.tcx, substs)
790+
} else if is_zst(field2, variant2) {
791+
field1.ty(cx.tcx, substs)
792+
} else {
793+
return None;
794+
}
795+
}
775796
_ => return None,
776797
},
777798
_ => return None,
@@ -832,9 +853,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
832853
cache: &mut FxHashSet<Ty<'tcx>>,
833854
field: &ty::FieldDef,
834855
substs: SubstsRef<'tcx>,
856+
allow_unit_type: bool,
835857
) -> FfiResult<'tcx> {
836858
let field_ty = field.ty(self.cx.tcx, substs);
837-
if field_ty.has_opaque_types() {
859+
if field_ty.is_unit() && allow_unit_type {
860+
FfiResult::FfiSafe
861+
} else if field_ty.has_opaque_types() {
838862
self.check_type_for_ffi(cache, field_ty)
839863
} else {
840864
let field_ty = self.cx.tcx.normalize_erasing_regions(self.cx.param_env, field_ty);
@@ -850,14 +874,15 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
850874
def: ty::AdtDef<'tcx>,
851875
variant: &ty::VariantDef,
852876
substs: SubstsRef<'tcx>,
877+
allow_unit_type: bool,
853878
) -> FfiResult<'tcx> {
854879
use FfiResult::*;
855880

856881
let transparent_safety = def.repr().transparent().then(|| {
857882
// Can assume that at most one field is not a ZST, so only check
858883
// that field's type for FFI-safety.
859884
if let Some(field) = transparent_newtype_field(self.cx.tcx, variant) {
860-
return self.check_field_type_for_ffi(cache, field, substs);
885+
return self.check_field_type_for_ffi(cache, field, substs, allow_unit_type);
861886
} else {
862887
// All fields are ZSTs; this means that the type should behave
863888
// like (), which is FFI-unsafe... except if all fields are PhantomData,
@@ -869,7 +894,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
869894
// actually safe.
870895
let mut all_phantom = !variant.fields.is_empty();
871896
for field in &variant.fields {
872-
match self.check_field_type_for_ffi(cache, &field, substs) {
897+
match self.check_field_type_for_ffi(cache, &field, substs, allow_unit_type) {
873898
FfiSafe => {
874899
all_phantom = false;
875900
}
@@ -967,25 +992,39 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
967992
};
968993
}
969994

970-
self.check_variant_for_ffi(cache, ty, def, def.non_enum_variant(), substs)
995+
self.check_variant_for_ffi(
996+
cache,
997+
ty,
998+
def,
999+
def.non_enum_variant(),
1000+
substs,
1001+
false,
1002+
)
9711003
}
9721004
AdtKind::Enum => {
9731005
if def.variants().is_empty() {
9741006
// Empty enums are okay... although sort of useless.
9751007
return FfiSafe;
9761008
}
9771009

1010+
// We need to keep track if this enum is a represented as a nullable
1011+
// pointer (or nonzero integer). This is because when we have an enum
1012+
// such as Result<(), NonZeroU32>, we allow this to compile since the FFI
1013+
// representation of this is as a u32
1014+
let mut is_repr_nullable_ptr = false;
9781015
// Check for a repr() attribute to specify the size of the
9791016
// discriminant.
9801017
if !def.repr().c() && !def.repr().transparent() && def.repr().int.is_none()
9811018
{
982-
// Special-case types like `Option<extern fn()>`.
1019+
// Special-case types like `Option<extern fn()>` and `Result<(), NonZeroU32>`.
9831020
if repr_nullable_ptr(self.cx, ty, self.mode).is_none() {
9841021
return FfiUnsafe {
9851022
ty,
9861023
reason: fluent::lint_improper_ctypes_enum_repr_reason,
9871024
help: Some(fluent::lint_improper_ctypes_enum_repr_help),
9881025
};
1026+
} else {
1027+
is_repr_nullable_ptr = true;
9891028
}
9901029
}
9911030

@@ -1008,7 +1047,14 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
10081047
};
10091048
}
10101049

1011-
match self.check_variant_for_ffi(cache, ty, def, variant, substs) {
1050+
match self.check_variant_for_ffi(
1051+
cache,
1052+
ty,
1053+
def,
1054+
variant,
1055+
substs,
1056+
is_repr_nullable_ptr,
1057+
) {
10121058
FfiSafe => (),
10131059
r => return r,
10141060
}

‎tests/ui/abi/nonzero-option-result-ffi.rs

+125
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
// run-pass
2+
3+
use std::mem;
4+
use std::num::*;
5+
6+
macro_rules! define_with_type {
7+
($namespace: ident, $ty: ty, $abi_ty: ty) => {
8+
mod $namespace {
9+
#[allow(unused)]
10+
use super::*;
11+
12+
#[inline(never)]
13+
pub extern "C" fn option(is_some: bool, value: $ty) -> Option<$ty> {
14+
if is_some {
15+
Some(value)
16+
} else {
17+
None
18+
}
19+
}
20+
21+
#[inline(never)]
22+
pub extern "C" fn result_pos(is_ok: bool, value: $ty) -> Result<$ty, ()> {
23+
if is_ok {
24+
Ok(value)
25+
} else {
26+
Err(())
27+
}
28+
}
29+
30+
#[inline(never)]
31+
pub extern "C" fn result_neg(is_ok: bool, value: $ty) -> Result<(), $ty> {
32+
if is_ok {
33+
Ok(())
34+
} else {
35+
Err(value)
36+
}
37+
}
38+
39+
#[inline(never)]
40+
pub extern "C" fn option_param(value: Option<$ty>) -> $abi_ty {
41+
unsafe { mem::transmute(value) }
42+
}
43+
44+
#[inline(never)]
45+
pub extern "C" fn result_param_pos(value: Result<$ty, ()>) -> $abi_ty {
46+
unsafe { mem::transmute(value) }
47+
}
48+
49+
#[inline(never)]
50+
pub extern "C" fn result_param_neg(value: Result<(), $ty>) -> $abi_ty {
51+
unsafe { mem::transmute(value) }
52+
}
53+
}
54+
};
55+
}
56+
57+
define_with_type!(nonzero_i8, NonZeroI8, i8);
58+
define_with_type!(nonzero_i16, NonZeroI16, i16);
59+
define_with_type!(nonzero_i32, NonZeroI32, i32);
60+
define_with_type!(nonzero_i64, NonZeroI64, i64);
61+
define_with_type!(nonzero_u8, NonZeroU8, u8);
62+
define_with_type!(nonzero_u16, NonZeroU16, u16);
63+
define_with_type!(nonzero_u32, NonZeroU32, u32);
64+
define_with_type!(nonzero_u64, NonZeroU64, u64);
65+
define_with_type!(nonzero_usize, NonZeroUsize, usize);
66+
define_with_type!(nonzero_ref, &'static i32, *const i32);
67+
68+
pub fn main() {
69+
macro_rules! test_with_type {
70+
(
71+
$namespace: ident,
72+
$ty: ty,
73+
$abi_ty: ty,
74+
$in_value: expr,
75+
$out_value: expr,
76+
$null_value:expr
77+
) => {
78+
let in_value: $ty = $in_value;
79+
let out_value: $abi_ty = $out_value;
80+
let null_value: $abi_ty = $null_value;
81+
unsafe {
82+
let f: extern "C" fn(bool, $ty) -> $abi_ty =
83+
mem::transmute($namespace::option as extern "C" fn(bool, $ty) -> Option<$ty>);
84+
assert_eq!(f(true, in_value), out_value);
85+
assert_eq!(f(false, in_value), null_value);
86+
}
87+
88+
unsafe {
89+
let f: extern "C" fn(bool, $ty) -> $abi_ty = mem::transmute(
90+
$namespace::result_pos as extern "C" fn(bool, $ty) -> Result<$ty, ()>,
91+
);
92+
assert_eq!(f(true, in_value), out_value);
93+
assert_eq!(f(false, in_value), null_value);
94+
}
95+
96+
unsafe {
97+
let f: extern "C" fn(bool, $ty) -> $abi_ty = mem::transmute(
98+
$namespace::result_neg as extern "C" fn(bool, $ty) -> Result<(), $ty>,
99+
);
100+
assert_eq!(f(false, in_value), out_value);
101+
assert_eq!(f(true, in_value), null_value);
102+
}
103+
104+
assert_eq!($namespace::option_param(Some(in_value)), out_value);
105+
assert_eq!($namespace::option_param(None), null_value);
106+
107+
assert_eq!($namespace::result_param_pos(Ok(in_value)), out_value);
108+
assert_eq!($namespace::result_param_pos(Err(())), null_value);
109+
110+
assert_eq!($namespace::result_param_neg(Err(in_value)), out_value);
111+
assert_eq!($namespace::result_param_neg(Ok(())), null_value);
112+
};
113+
}
114+
test_with_type!(nonzero_i8, NonZeroI8, i8, NonZeroI8::new(123).unwrap(), 123, 0);
115+
test_with_type!(nonzero_i16, NonZeroI16, i16, NonZeroI16::new(1234).unwrap(), 1234, 0);
116+
test_with_type!(nonzero_i32, NonZeroI32, i32, NonZeroI32::new(1234).unwrap(), 1234, 0);
117+
test_with_type!(nonzero_i64, NonZeroI64, i64, NonZeroI64::new(1234).unwrap(), 1234, 0);
118+
test_with_type!(nonzero_u8, NonZeroU8, u8, NonZeroU8::new(123).unwrap(), 123, 0);
119+
test_with_type!(nonzero_u16, NonZeroU16, u16, NonZeroU16::new(1234).unwrap(), 1234, 0);
120+
test_with_type!(nonzero_u32, NonZeroU32, u32, NonZeroU32::new(1234).unwrap(), 1234, 0);
121+
test_with_type!(nonzero_u64, NonZeroU64, u64, NonZeroU64::new(1234).unwrap(), 1234, 0);
122+
test_with_type!(nonzero_usize, NonZeroUsize, usize, NonZeroUsize::new(1234).unwrap(), 1234, 0);
123+
static FOO: i32 = 0xDEADBEE;
124+
test_with_type!(nonzero_ref, &'static i32, *const i32, &FOO, &FOO, std::ptr::null());
125+
}

‎tests/ui/lint/lint-ctypes-enum.rs

+60-31
Original file line numberDiff line numberDiff line change
@@ -56,37 +56,66 @@ union TransparentUnion<T: Copy> {
5656
struct Rust<T>(T);
5757

5858
extern "C" {
59-
fn zf(x: Z);
60-
fn uf(x: U); //~ ERROR `extern` block uses type `U`
61-
fn bf(x: B); //~ ERROR `extern` block uses type `B`
62-
fn tf(x: T); //~ ERROR `extern` block uses type `T`
63-
fn repr_c(x: ReprC);
64-
fn repr_u8(x: U8);
65-
fn repr_isize(x: Isize);
66-
fn option_ref(x: Option<&'static u8>);
67-
fn option_fn(x: Option<extern "C" fn()>);
68-
fn nonnull(x: Option<std::ptr::NonNull<u8>>);
69-
fn unique(x: Option<std::ptr::Unique<u8>>);
70-
fn nonzero_u8(x: Option<num::NonZeroU8>);
71-
fn nonzero_u16(x: Option<num::NonZeroU16>);
72-
fn nonzero_u32(x: Option<num::NonZeroU32>);
73-
fn nonzero_u64(x: Option<num::NonZeroU64>);
74-
fn nonzero_u128(x: Option<num::NonZeroU128>);
75-
//~^ ERROR `extern` block uses type `u128`
76-
fn nonzero_usize(x: Option<num::NonZeroUsize>);
77-
fn nonzero_i8(x: Option<num::NonZeroI8>);
78-
fn nonzero_i16(x: Option<num::NonZeroI16>);
79-
fn nonzero_i32(x: Option<num::NonZeroI32>);
80-
fn nonzero_i64(x: Option<num::NonZeroI64>);
81-
fn nonzero_i128(x: Option<num::NonZeroI128>);
82-
//~^ ERROR `extern` block uses type `i128`
83-
fn nonzero_isize(x: Option<num::NonZeroIsize>);
84-
fn transparent_struct(x: Option<TransparentStruct<num::NonZeroU8>>);
85-
fn transparent_enum(x: Option<TransparentEnum<num::NonZeroU8>>);
86-
fn transparent_union(x: Option<TransparentUnion<num::NonZeroU8>>);
87-
//~^ ERROR `extern` block uses type
88-
fn repr_rust(x: Option<Rust<num::NonZeroU8>>); //~ ERROR `extern` block uses type
89-
fn no_result(x: Result<(), num::NonZeroI32>); //~ ERROR `extern` block uses type
59+
fn zf(x: Z);
60+
fn uf(x: U); //~ ERROR `extern` block uses type `U`
61+
fn bf(x: B); //~ ERROR `extern` block uses type `B`
62+
fn tf(x: T); //~ ERROR `extern` block uses type `T`
63+
fn repr_c(x: ReprC);
64+
fn repr_u8(x: U8);
65+
fn repr_isize(x: Isize);
66+
fn option_ref(x: Option<&'static u8>);
67+
fn option_fn(x: Option<extern "C" fn()>);
68+
fn nonnull(x: Option<std::ptr::NonNull<u8>>);
69+
fn unique(x: Option<std::ptr::Unique<u8>>);
70+
fn nonzero_u8(x: Option<num::NonZeroU8>);
71+
fn nonzero_u16(x: Option<num::NonZeroU16>);
72+
fn nonzero_u32(x: Option<num::NonZeroU32>);
73+
fn nonzero_u64(x: Option<num::NonZeroU64>);
74+
fn nonzero_u128(x: Option<num::NonZeroU128>);
75+
//~^ ERROR `extern` block uses type `u128`
76+
fn nonzero_usize(x: Option<num::NonZeroUsize>);
77+
fn nonzero_i8(x: Option<num::NonZeroI8>);
78+
fn nonzero_i16(x: Option<num::NonZeroI16>);
79+
fn nonzero_i32(x: Option<num::NonZeroI32>);
80+
fn nonzero_i64(x: Option<num::NonZeroI64>);
81+
fn nonzero_i128(x: Option<num::NonZeroI128>);
82+
//~^ ERROR `extern` block uses type `i128`
83+
fn result_nonzero_u8(x: Result<(), num::NonZeroU8>);
84+
fn result_nonzero_u16(x: Result<(), num::NonZeroU16>);
85+
fn result_nonzero_u32(x: Result<(), num::NonZeroU32>);
86+
fn result_nonzero_u64(x: Result<(), num::NonZeroU64>);
87+
fn result_nonzero_u128(x: Result<(), num::NonZeroU128>);
88+
//~^ ERROR `extern` block uses type `u128`
89+
fn result_nonzero_usize(x: Result<(), num::NonZeroUsize>);
90+
fn result_nonzero_i8(x: Result<(), num::NonZeroI8>);
91+
fn result_nonzero_i16(x: Result<(), num::NonZeroI16>);
92+
fn result_nonzero_i32(x: Result<(), num::NonZeroI32>);
93+
fn result_nonzero_i64(x: Result<(), num::NonZeroI64>);
94+
fn result_nonzero_i128(x: Result<(), num::NonZeroI128>);
95+
//~^ ERROR `extern` block uses type `i128`
96+
fn left_result_nonzero_u8(x: Result<num::NonZeroU8, ()>);
97+
fn left_result_nonzero_u16(x: Result<num::NonZeroU16, ()>);
98+
fn left_result_nonzero_u32(x: Result<num::NonZeroU32, ()>);
99+
fn left_result_nonzero_u64(x: Result<num::NonZeroU64, ()>);
100+
fn left_result_nonzero_u128(x: Result<num::NonZeroU128, ()>);
101+
//~^ ERROR `extern` block uses type `u128`
102+
fn left_result_nonzero_usize(x: Result<num::NonZeroUsize, ()>);
103+
fn left_result_nonzero_i8(x: Result<num::NonZeroI8, ()>);
104+
fn left_result_nonzero_i16(x: Result<num::NonZeroI16, ()>);
105+
fn left_result_nonzero_i32(x: Result<num::NonZeroI32, ()>);
106+
fn left_result_nonzero_i64(x: Result<num::NonZeroI64, ()>);
107+
fn left_result_nonzero_i128(x: Result<num::NonZeroI128, ()>);
108+
//~^ ERROR `extern` block uses type `i128`
109+
fn result_sized(x: Result<i32, num::NonZeroI128>);
110+
//~^ ERROR `extern` block uses type `Result<i32, NonZeroI128>`
111+
fn left_result_sized(x: Result<num::NonZeroI128, i32>);
112+
//~^ ERROR `extern` block uses type `Result<NonZeroI128, i32>`
113+
fn nonzero_isize(x: Option<num::NonZeroIsize>);
114+
fn transparent_struct(x: Option<TransparentStruct<num::NonZeroU8>>);
115+
fn transparent_enum(x: Option<TransparentEnum<num::NonZeroU8>>);
116+
fn transparent_union(x: Option<TransparentUnion<num::NonZeroU8>>);
117+
//~^ ERROR `extern` block uses type
118+
fn repr_rust(x: Option<Rust<num::NonZeroU8>>); //~ ERROR `extern` block uses type
90119
}
91120

92121
pub fn main() {}

0 commit comments

Comments
 (0)