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

Rollup merge of #72890 - davidtwco:issue-66202-normalize-and-transparent-improper-ctypes, r=varkor

Browse Source 
improper ctypes: normalize return types and transparent structs

Fixes #66202.

See each commit individually (except the first which adds a test) for more detailed explanations on the changes made.

In summary, this PR ensures that return types are normalized before being checked for FFI-safety, and that transparent newtype wrappers are FFI-safe if the type being wrapped is FFI-safe (often true previously, but not if, after substitution, all types in a transparent newtype were zero sized).
This commit is contained in:
Dylan DPC 2020-06-10 11:03:40 +02:00 committed by GitHub
commit fda594e6ae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 105 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
src/librustc_lint/types.rs
View File

@ -6,7 +6,6 @@ use rustc_attr as attr;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::{is_range_literal, ExprKind, Node};
use rustc_index::vec::Idx;
use rustc_middle::mir::interpret::{sign_extend, truncate};
@ -511,10 +510,6 @@ enum FfiResult<'tcx> {
FfiUnsafe { ty: Ty<'tcx>, reason: &'static str, help: Option<&'static str> },
}
fn is_zst<'tcx>(tcx: TyCtxt<'tcx>, did: DefId, ty: Ty<'tcx>) -> bool {
tcx.layout_of(tcx.param_env(did).and(ty)).map(|layout| layout.is_zst()).unwrap_or(false)
}
fn ty_is_known_nonnull<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool {
match ty.kind {
ty::FnPtr(_) => true,
@ -523,7 +518,7 @@ fn ty_is_known_nonnull<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool {
for field in field_def.all_fields() {
let field_ty =
tcx.normalize_erasing_regions(ParamEnv::reveal_all(), field.ty(tcx, substs));
if is_zst(tcx, field.did, field_ty) {
if field_ty.is_zst(tcx, field.did) {
continue;
}
@ -653,32 +648,43 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
};
}
// We can't completely trust repr(C) and repr(transparent) markings;
// make sure the fields are actually safe.
let mut all_phantom = true;
for field in &def.non_enum_variant().fields {
let field_ty = cx.normalize_erasing_regions(
ParamEnv::reveal_all(),
field.ty(cx, substs),
);
// repr(transparent) types are allowed to have arbitrary ZSTs, not just
// PhantomData -- skip checking all ZST fields
if def.repr.transparent() && is_zst(cx, field.did, field_ty) {
continue;
if def.repr.transparent() {
// Can assume that only one field is not a ZST, so only check
// that field's type for FFI-safety.
if let Some(field) =
def.transparent_newtype_field(cx, self.cx.param_env)
{
let field_ty = cx.normalize_erasing_regions(
self.cx.param_env,
field.ty(cx, substs),
);
self.check_type_for_ffi(cache, field_ty)
} else {
FfiSafe
}
let r = self.check_type_for_ffi(cache, field_ty);
match r {
FfiSafe => {
all_phantom = false;
}
FfiPhantom(..) => {}
FfiUnsafe { .. } => {
return r;
} else {
// We can't completely trust repr(C) markings; make sure the fields are
// actually safe.
let mut all_phantom = true;
for field in &def.non_enum_variant().fields {
let field_ty = cx.normalize_erasing_regions(
self.cx.param_env,
field.ty(cx, substs),
);
let r = self.check_type_for_ffi(cache, field_ty);
match r {
FfiSafe => {
all_phantom = false;
}
FfiPhantom(..) => {}
FfiUnsafe { .. } => {
return r;
}
}
}
}
if all_phantom { FfiPhantom(ty) } else { FfiSafe }
if all_phantom { FfiPhantom(ty) } else { FfiSafe }
}
}
AdtKind::Union => {
if !def.repr.c() && !def.repr.transparent() {
@ -708,7 +714,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
);
// repr(transparent) types are allowed to have arbitrary ZSTs, not just
// PhantomData -- skip checking all ZST fields.
if def.repr.transparent() && is_zst(cx, field.did, field_ty) {
if def.repr.transparent() && field_ty.is_zst(cx, field.did) {
continue;
}
let r = self.check_type_for_ffi(cache, field_ty);
@ -774,7 +780,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
);
// repr(transparent) types are allowed to have arbitrary ZSTs, not
// just PhantomData -- skip checking all ZST fields.
if def.repr.transparent() && is_zst(cx, field.did, field_ty) {
if def.repr.transparent() && field_ty.is_zst(cx, field.did) {
continue;
}
let r = self.check_type_for_ffi(cache, field_ty);
@ -946,7 +952,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
}
}
fn check_type_for_ffi_and_report_errors(&mut self, sp: Span, ty: Ty<'tcx>, is_static: bool) {
fn check_type_for_ffi_and_report_errors(
&mut self,
sp: Span,
ty: Ty<'tcx>,
is_static: bool,
is_return_type: bool,
) {
// We have to check for opaque types before `normalize_erasing_regions`,
// which will replace opaque types with their underlying concrete type.
if self.check_for_opaque_ty(sp, ty) {
@ -957,19 +969,29 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
// it is only OK to use this function because extern fns cannot have
// any generic types right now:
let ty = self.cx.tcx.normalize_erasing_regions(ParamEnv::reveal_all(), ty);
// C doesn't really support passing arrays by value.
// The only way to pass an array by value is through a struct.
// So we first test that the top level isn't an array,
// and then recursively check the types inside.
// C doesn't really support passing arrays by value - the only way to pass an array by value
// is through a struct. So, first test that the top level isn't an array, and then
// recursively check the types inside.
if !is_static && self.check_for_array_ty(sp, ty) {
return;
}
// Don't report FFI errors for unit return types. This check exists here, and not in
// `check_foreign_fn` (where it would make more sense) so that normalization has definitely
// happened.
if is_return_type && ty.is_unit() {
return;
}
match self.check_type_for_ffi(&mut FxHashSet::default(), ty) {
FfiResult::FfiSafe => {}
FfiResult::FfiPhantom(ty) => {
self.emit_ffi_unsafe_type_lint(ty, sp, "composed only of `PhantomData`", None);
}
// If `ty` is a `repr(transparent)` newtype, and the non-zero-sized type is a generic
// argument, which after substitution, is `()`, then this branch can be hit.
FfiResult::FfiUnsafe { ty, .. } if is_return_type && ty.is_unit() => return,
FfiResult::FfiUnsafe { ty, reason, help } => {
self.emit_ffi_unsafe_type_lint(ty, sp, reason, help);
}
@ -982,21 +1004,19 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
let sig = self.cx.tcx.erase_late_bound_regions(&sig);
for (input_ty, input_hir) in sig.inputs().iter().zip(decl.inputs) {
self.check_type_for_ffi_and_report_errors(input_hir.span, input_ty, false);
self.check_type_for_ffi_and_report_errors(input_hir.span, input_ty, false, false);
}
if let hir::FnRetTy::Return(ref ret_hir) = decl.output {
let ret_ty = sig.output();
if !ret_ty.is_unit() {
self.check_type_for_ffi_and_report_errors(ret_hir.span, ret_ty, false);
}
self.check_type_for_ffi_and_report_errors(ret_hir.span, ret_ty, false, true);
}
}
fn check_foreign_static(&mut self, id: hir::HirId, span: Span) {
let def_id = self.cx.tcx.hir().local_def_id(id);
let ty = self.cx.tcx.type_of(def_id);
self.check_type_for_ffi_and_report_errors(span, ty, true);
self.check_type_for_ffi_and_report_errors(span, ty, true, false);
}
}

23
src/librustc_middle/ty/mod.rs
View File

@ -2390,6 +2390,29 @@ impl<'tcx> AdtDef {
pub fn sized_constraint(&self, tcx: TyCtxt<'tcx>) -> &'tcx [Ty<'tcx>] {
tcx.adt_sized_constraint(self.did).0
}
/// `repr(transparent)` structs can have a single non-ZST field, this function returns that
/// field.
pub fn transparent_newtype_field(
&self,
tcx: TyCtxt<'tcx>,
param_env: ParamEnv<'tcx>,
) -> Option<&FieldDef> {
assert!(self.is_struct() && self.repr.transparent());
for field in &self.non_enum_variant().fields {
let field_ty = tcx.normalize_erasing_regions(
param_env,
field.ty(tcx, InternalSubsts::identity_for_item(tcx, self.did)),
);
if !field_ty.is_zst(tcx, self.did) {
return Some(field);
}
}
None
}
}
impl<'tcx> FieldDef {

5
src/librustc_middle/ty/sty.rs
View File

@ -2186,6 +2186,11 @@ impl<'tcx> TyS<'tcx> {
}
}
}
/// Is this a zero-sized type?
pub fn is_zst(&'tcx self, tcx: TyCtxt<'tcx>, did: DefId) -> bool {
tcx.layout_of(tcx.param_env(did).and(self)).map(|layout| layout.is_zst()).unwrap_or(false)
}
}
/// Typed constant value.

17
src/test/ui/lint/lint-ctypes-66202.rs Normal file
View File

@ -0,0 +1,17 @@
// check-pass
#![deny(improper_ctypes)]
// This test checks that return types are normalized before being checked for FFI-safety, and that
// transparent newtype wrappers are FFI-safe if the type being wrapped is FFI-safe.
#[repr(transparent)]
pub struct W<T>(T);
extern "C" {
pub fn bare() -> ();
pub fn normalize() -> <() as ToOwned>::Owned;
pub fn transparent() -> W<()>;
}
fn main() {}