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

Auto merge of #73257 - davidtwco:issue-73249-improper-ctypes-projection, r=lcnr,varkor

Browse Source 
ty: projections in `transparent_newtype_field`

Fixes #73249.

This PR modifies `transparent_newtype_field` so that it handles
projections with generic parameters, where `normalize_erasing_regions`
would ICE.
This commit is contained in:
bors 2020-06-19 19:14:45 +00:00
commit 2d8bd9b74d
11 changed files with 299 additions and 159 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

240
src/librustc_lint/types.rs
View File

@ -11,7 +11,7 @@ use rustc_index::vec::Idx;
use rustc_middle::mir::interpret::{sign_extend, truncate};
use rustc_middle::ty::layout::{IntegerExt, SizeSkeleton};
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt};
use rustc_middle::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt, TypeFoldable};
use rustc_span::source_map;
use rustc_span::symbol::sym;
use rustc_span::Span;
@ -507,7 +507,7 @@ struct ImproperCTypesVisitor<'a, 'tcx> {
enum FfiResult<'tcx> {
FfiSafe,
FfiPhantom(Ty<'tcx>),
FfiUnsafe { ty: Ty<'tcx>, reason: &'static str, help: Option<&'static str> },
FfiUnsafe { ty: Ty<'tcx>, reason: String, help: Option<String> },
}
fn ty_is_known_nonnull<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool {
@ -597,6 +597,66 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
}
}
/// Checks if the given field's type is "ffi-safe".
fn check_field_type_for_ffi(
&self,
cache: &mut FxHashSet<Ty<'tcx>>,
field: &ty::FieldDef,
substs: SubstsRef<'tcx>,
) -> FfiResult<'tcx> {
let field_ty = field.ty(self.cx.tcx, substs);
if field_ty.has_opaque_types() {
self.check_type_for_ffi(cache, field_ty)
} else {
let field_ty = self.cx.tcx.normalize_erasing_regions(self.cx.param_env, field_ty);
self.check_type_for_ffi(cache, field_ty)
}
}
/// Checks if the given `VariantDef`'s field types are "ffi-safe".
fn check_variant_for_ffi(
&self,
cache: &mut FxHashSet<Ty<'tcx>>,
ty: Ty<'tcx>,
def: &ty::AdtDef,
variant: &ty::VariantDef,
substs: SubstsRef<'tcx>,
) -> FfiResult<'tcx> {
use FfiResult::*;
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) = variant.transparent_newtype_field(self.cx.tcx) {
self.check_field_type_for_ffi(cache, field, substs)
} else {
bug!("malformed transparent type");
}
} else {
// We can't completely trust repr(C) markings; make sure the fields are
// actually safe.
let mut all_phantom = !variant.fields.is_empty();
for field in &variant.fields {
match self.check_field_type_for_ffi(cache, &field, substs) {
FfiSafe => {
all_phantom = false;
}
FfiPhantom(..) if def.is_enum() => {
return FfiUnsafe {
ty,
reason: "this enum contains a PhantomData field".into(),
help: None,
};
}
FfiPhantom(..) => {}
r => return r,
}
}
if all_phantom { FfiPhantom(ty) } else { FfiSafe }
}
}
/// Checks if the given type is "ffi-safe" (has a stable, well-defined
/// representation which can be exported to C code).
fn check_type_for_ffi(&self, cache: &mut FxHashSet<Ty<'tcx>>, ty: Ty<'tcx>) -> FfiResult<'tcx> {
@ -618,15 +678,18 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
return FfiPhantom(ty);
}
match def.adt_kind() {
AdtKind::Struct => {
AdtKind::Struct | AdtKind::Union => {
let kind = if def.is_struct() { "struct" } else { "union" };
if !def.repr.c() && !def.repr.transparent() {
return FfiUnsafe {
ty,
reason: "this struct has unspecified layout",
help: Some(
reason: format!("this {} has unspecified layout", kind),
help: Some(format!(
"consider adding a `#[repr(C)]` or \
`#[repr(transparent)]` attribute to this struct",
),
`#[repr(transparent)]` attribute to this {}",
kind
)),
};
}
@ -635,7 +698,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
if is_non_exhaustive && !def.did.is_local() {
return FfiUnsafe {
ty,
reason: "this struct is non-exhaustive",
reason: format!("this {} is non-exhaustive", kind),
help: None,
};
}
@ -643,93 +706,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
if def.non_enum_variant().fields.is_empty() {
return FfiUnsafe {
ty,
reason: "this struct has no fields",
help: Some("consider adding a member to this struct"),
reason: format!("this {} has no fields", kind),
help: Some(format!("consider adding a member to this {}", kind)),
};
}
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
}
} 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 }
}
}
AdtKind::Union => {
if !def.repr.c() && !def.repr.transparent() {
return FfiUnsafe {
ty,
reason: "this union has unspecified layout",
help: Some(
"consider adding a `#[repr(C)]` or \
`#[repr(transparent)]` attribute to this union",
),
};
}
if def.non_enum_variant().fields.is_empty() {
return FfiUnsafe {
ty,
reason: "this union has no fields",
help: Some("consider adding a field to this union"),
};
}
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() && field_ty.is_zst(cx, field.did) {
continue;
}
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 }
self.check_variant_for_ffi(cache, ty, def, def.non_enum_variant(), substs)
}
AdtKind::Enum => {
if def.variants.is_empty() {
@ -744,11 +726,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
if !is_repr_nullable_ptr(cx, ty, def, substs) {
return FfiUnsafe {
ty,
reason: "enum has no representation hint",
reason: "enum has no representation hint".into(),
help: Some(
"consider adding a `#[repr(C)]`, \
`#[repr(transparent)]`, or integer `#[repr(...)]` \
attribute to this enum",
attribute to this enum"
.into(),
),
};
}
@ -757,7 +740,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
if def.is_variant_list_non_exhaustive() && !def.did.is_local() {
return FfiUnsafe {
ty,
reason: "this enum is non-exhaustive",
reason: "this enum is non-exhaustive".into(),
help: None,
};
}
@ -768,37 +751,17 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
if is_non_exhaustive && !variant.def_id.is_local() {
return FfiUnsafe {
ty,
reason: "this enum has non-exhaustive variants",
reason: "this enum has non-exhaustive variants".into(),
help: None,
};
}
for field in &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() && field_ty.is_zst(cx, field.did) {
continue;
}
let r = self.check_type_for_ffi(cache, field_ty);
match r {
FfiSafe => {}
FfiUnsafe { .. } => {
return r;
}
FfiPhantom(..) => {
return FfiUnsafe {
ty,
reason: "this enum contains a PhantomData field",
help: None,
};
}
}
match self.check_variant_for_ffi(cache, ty, def, variant, substs) {
FfiSafe => (),
r => return r,
}
}
FfiSafe
}
}
@ -806,13 +769,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
ty::Char => FfiUnsafe {
ty,
reason: "the `char` type has no C equivalent",
help: Some("consider using `u32` or `libc::wchar_t` instead"),
reason: "the `char` type has no C equivalent".into(),
help: Some("consider using `u32` or `libc::wchar_t` instead".into()),
},
ty::Int(ast::IntTy::I128) | ty::Uint(ast::UintTy::U128) => FfiUnsafe {
ty,
reason: "128-bit integers don't currently have a known stable ABI",
reason: "128-bit integers don't currently have a known stable ABI".into(),
help: None,
},
@ -821,24 +784,24 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
ty::Slice(_) => FfiUnsafe {
ty,
reason: "slices have no C equivalent",
help: Some("consider using a raw pointer instead"),
reason: "slices have no C equivalent".into(),
help: Some("consider using a raw pointer instead".into()),
},
ty::Dynamic(..) => {
FfiUnsafe { ty, reason: "trait objects have no C equivalent", help: None }
FfiUnsafe { ty, reason: "trait objects have no C equivalent".into(), help: None }
}
ty::Str => FfiUnsafe {
ty,
reason: "string slices have no C equivalent",
help: Some("consider using `*const u8` and a length instead"),
reason: "string slices have no C equivalent".into(),
help: Some("consider using `*const u8` and a length instead".into()),
},
ty::Tuple(..) => FfiUnsafe {
ty,
reason: "tuples have unspecified layout",
help: Some("consider using a struct instead"),
reason: "tuples have unspecified layout".into(),
help: Some("consider using a struct instead".into()),
},
ty::RawPtr(ty::TypeAndMut { ty, .. }) | ty::Ref(_, ty, _) => {
@ -852,10 +815,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
Abi::Rust | Abi::RustIntrinsic | Abi::PlatformIntrinsic | Abi::RustCall => {
return FfiUnsafe {
ty,
reason: "this function pointer has Rust-specific calling convention",
reason: "this function pointer has Rust-specific calling convention"
.into(),
help: Some(
"consider using an `extern fn(...) -> ...` \
function pointer instead",
function pointer instead"
.into(),
),
};
}
@ -886,6 +851,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
ty::Foreign(..) => FfiSafe,
// While opaque types are checked for earlier, if a projection in a struct field
// normalizes to an opaque type, then it will reach this branch.
ty::Opaque(..) => {
FfiUnsafe { ty, reason: "opaque types have no C equivalent".into(), help: None }
}
ty::Param(..)
| ty::Infer(..)
| ty::Bound(..)
@ -895,7 +866,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
| ty::GeneratorWitness(..)
| ty::Placeholder(..)
| ty::Projection(..)
| ty::Opaque(..)
| ty::FnDef(..) => bug!("unexpected type in foreign function: {:?}", ty),
}
}
@ -925,8 +895,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
}
fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool {
use rustc_middle::ty::TypeFoldable;
struct ProhibitOpaqueTypes<'tcx> {
ty: Option<Ty<'tcx>>,
};
@ -993,7 +961,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
// 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);
self.emit_ffi_unsafe_type_lint(ty, sp, &reason, help.as_deref());
}
}
}

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

@ -1807,6 +1807,19 @@ impl<'tcx> VariantDef {
pub fn is_field_list_non_exhaustive(&self) -> bool {
self.flags.intersects(VariantFlags::IS_FIELD_LIST_NON_EXHAUSTIVE)
}
/// `repr(transparent)` structs can have a single non-ZST field, this function returns that
/// field.
pub fn transparent_newtype_field(&self, tcx: TyCtxt<'tcx>) -> Option<&FieldDef> {
for field in &self.fields {
let field_ty = field.ty(tcx, InternalSubsts::identity_for_item(tcx, self.def_id));
if !field_ty.is_zst(tcx, self.def_id) {
return Some(field);
}
}
None
}
}
#[derive(Copy, Clone, Debug, PartialEq, Eq, RustcEncodable, RustcDecodable, HashStable)]
@ -2376,29 +2389,6 @@ 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 {

21
src/test/ui/lint/lint-ctypes-73249-1.rs Normal file
View File

@ -0,0 +1,21 @@
// check-pass
#![deny(improper_ctypes)]
pub trait Foo {
type Assoc: 'static;
}
impl Foo for () {
type Assoc = u32;
}
extern "C" {
pub fn lint_me(x: Bar<()>);
}
#[repr(transparent)]
pub struct Bar<T: Foo> {
value: &'static <T as Foo>::Assoc,
}
fn main() {}

29
src/test/ui/lint/lint-ctypes-73249-2.rs Normal file
View File

@ -0,0 +1,29 @@
#![feature(type_alias_impl_trait)]
#![deny(improper_ctypes)]
pub trait Baz { }
impl Baz for () { }
type Qux = impl Baz;
fn assign() -> Qux {}
pub trait Foo {
type Assoc: 'static;
}
impl Foo for () {
type Assoc = Qux;
}
#[repr(transparent)]
pub struct A<T: Foo> {
x: &'static <T as Foo>::Assoc,
}
extern "C" {
pub fn lint_me() -> A<()>; //~ ERROR: uses type `impl Baz`
}
fn main() {}

15
src/test/ui/lint/lint-ctypes-73249-2.stderr Normal file
View File

@ -0,0 +1,15 @@
error: `extern` block uses type `impl Baz`, which is not FFI-safe
--> $DIR/lint-ctypes-73249-2.rs:26:25
|
LL | pub fn lint_me() -> A<()>;
| ^^^^^ not FFI-safe
|
note: the lint level is defined here
--> $DIR/lint-ctypes-73249-2.rs:2:9
|
LL | #![deny(improper_ctypes)]
| ^^^^^^^^^^^^^^^
= note: opaque types have no C equivalent
error: aborting due to previous error

21
src/test/ui/lint/lint-ctypes-73249-3.rs Normal file
View File

@ -0,0 +1,21 @@
#![feature(type_alias_impl_trait)]
#![deny(improper_ctypes)]
pub trait Baz { }
impl Baz for u32 { }
type Qux = impl Baz;
fn assign() -> Qux { 3 }
#[repr(C)]
pub struct A {
x: Qux,
}
extern "C" {
pub fn lint_me() -> A; //~ ERROR: uses type `impl Baz`
}
fn main() {}

15
src/test/ui/lint/lint-ctypes-73249-3.stderr Normal file
View File

@ -0,0 +1,15 @@
error: `extern` block uses type `impl Baz`, which is not FFI-safe
--> $DIR/lint-ctypes-73249-3.rs:18:25
|
LL | pub fn lint_me() -> A;
| ^ not FFI-safe
|
note: the lint level is defined here
--> $DIR/lint-ctypes-73249-3.rs:2:9
|
LL | #![deny(improper_ctypes)]
| ^^^^^^^^^^^^^^^
= note: opaque types have no C equivalent
error: aborting due to previous error

24
src/test/ui/lint/lint-ctypes-73249-4.rs Normal file
View File

@ -0,0 +1,24 @@
// check-pass
#![deny(improper_ctypes)]
use std::marker::PhantomData;
trait Foo {
type Assoc;
}
impl Foo for () {
type Assoc = PhantomData<()>;
}
#[repr(transparent)]
struct Wow<T> where T: Foo<Assoc = PhantomData<T>> {
x: <T as Foo>::Assoc,
v: u32,
}
extern "C" {
fn test(v: Wow<()>);
}
fn main() {}

21
src/test/ui/lint/lint-ctypes-73249-5.rs Normal file
View File

@ -0,0 +1,21 @@
#![feature(type_alias_impl_trait)]
#![deny(improper_ctypes)]
pub trait Baz { }
impl Baz for u32 { }
type Qux = impl Baz;
fn assign() -> Qux { 3 }
#[repr(transparent)]
pub struct A {
x: Qux,
}
extern "C" {
pub fn lint_me() -> A; //~ ERROR: uses type `impl Baz`
}
fn main() {}

15
src/test/ui/lint/lint-ctypes-73249-5.stderr Normal file
View File

@ -0,0 +1,15 @@
error: `extern` block uses type `impl Baz`, which is not FFI-safe
--> $DIR/lint-ctypes-73249-5.rs:18:25
|
LL | pub fn lint_me() -> A;
| ^ not FFI-safe
|
note: the lint level is defined here
--> $DIR/lint-ctypes-73249-5.rs:2:9
|
LL | #![deny(improper_ctypes)]
| ^^^^^^^^^^^^^^^
= note: opaque types have no C equivalent
error: aborting due to previous error

21
src/test/ui/lint/lint-ctypes-73249.rs Normal file
View File

@ -0,0 +1,21 @@
// check-pass
#![deny(improper_ctypes)]
pub trait Foo {
type Assoc;
}
impl Foo for () {
type Assoc = u32;
}
extern "C" {
pub fn lint_me(x: Bar<()>);
}
#[repr(transparent)]
pub struct Bar<T: Foo> {
value: <T as Foo>::Assoc,
}
fn main() {}