Skip to content

Commit

Permalink
lint/ctypes: Don't warn on non-unsized structs with PhantomData.
Browse files Browse the repository at this point in the history
  • Loading branch information
emilio committed Feb 3, 2017
1 parent aed6410 commit e866d07
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 7 deletions.
46 changes: 39 additions & 7 deletions src/librustc_lint/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ struct ImproperCTypesVisitor<'a, 'tcx: 'a> {

enum FfiResult {
FfiSafe,
FfiPhantom,
FfiUnsafe(&'static str),
FfiBadStruct(DefId, &'static str),
FfiBadUnion(DefId, &'static str),
Expand Down Expand Up @@ -385,8 +386,11 @@ fn is_repr_nullable_ptr<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
/// Check 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 {
fn check_type_for_ffi(&self,
cache: &mut FxHashSet<Ty<'tcx>>,
ty: Ty<'tcx>) -> FfiResult {
use self::FfiResult::*;

let cx = self.cx.tcx;

// Protect against infinite recursion, for example
Expand All @@ -399,6 +403,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {

match ty.sty {
ty::TyAdt(def, substs) => {
if def.is_phantom_data() {
return FfiPhantom;
}
match def.adt_kind() {
AdtKind::Struct => {
if !cx.lookup_repr_hints(def.did).contains(&attr::ReprExtern) {
Expand All @@ -407,18 +414,22 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
consider adding a #[repr(C)] attribute to the type");
}

// We can't completely trust repr(C) markings; make sure the
// fields are actually safe.
if def.struct_variant().fields.is_empty() {
return FfiUnsafe("found zero-size struct in foreign module, consider \
adding a member to this struct");
}

// We can't completely trust repr(C) markings; make sure the
// fields are actually safe.
let mut all_phantom = true;
for field in &def.struct_variant().fields {
let field_ty = cx.normalize_associated_type(&field.ty(cx, substs));
let r = self.check_type_for_ffi(cache, field_ty);
match r {
FfiSafe => {}
FfiSafe => {
all_phantom = false;
}
FfiPhantom => {}
FfiBadStruct(..) | FfiBadUnion(..) | FfiBadEnum(..) => {
return r;
}
Expand All @@ -427,7 +438,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
}
}
}
FfiSafe

if all_phantom { FfiPhantom } else { FfiSafe }
}
AdtKind::Union => {
if !cx.lookup_repr_hints(def.did).contains(&attr::ReprExtern) {
Expand All @@ -436,11 +448,20 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
consider adding a #[repr(C)] attribute to the type");
}

if def.struct_variant().fields.is_empty() {
return FfiUnsafe("found zero-size union in foreign module, consider \
adding a member to this union");
}

let mut all_phantom = true;
for field in &def.struct_variant().fields {
let field_ty = cx.normalize_associated_type(&field.ty(cx, substs));
let r = self.check_type_for_ffi(cache, field_ty);
match r {
FfiSafe => {}
FfiSafe => {
all_phantom = false;
}
FfiPhantom => {}
FfiBadStruct(..) | FfiBadUnion(..) | FfiBadEnum(..) => {
return r;
}
Expand All @@ -449,7 +470,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
}
}
}
FfiSafe

if all_phantom { FfiPhantom } else { FfiSafe }
}
AdtKind::Enum => {
if def.variants.is_empty() {
Expand Down Expand Up @@ -500,6 +522,10 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
FfiBadStruct(..) | FfiBadUnion(..) | FfiBadEnum(..) => {
return r;
}
FfiPhantom => {
return FfiBadEnum(def.did,
"Found phantom data in enum variant");
}
FfiUnsafe(s) => {
return FfiBadEnum(def.did, s);
}
Expand Down Expand Up @@ -593,6 +619,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {

match self.check_type_for_ffi(&mut FxHashSet(), ty) {
FfiResult::FfiSafe => {}
FfiResult::FfiPhantom => {
self.cx.span_lint(IMPROPER_CTYPES,
sp,
&format!("found zero-sized type composed only \
of phantom-data in a foreign-function."));
}
FfiResult::FfiUnsafe(s) => {
self.cx.span_lint(IMPROPER_CTYPES, sp, s);
}
Expand Down
6 changes: 6 additions & 0 deletions src/test/compile-fail/lint-ctypes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ pub type RustBadRet = extern fn() -> Box<u32>;
pub type CVoidRet = ();
pub struct Foo;

#[repr(C)]
pub struct ZeroSizeWithPhantomData(::std::marker::PhantomData<i32>);

extern {
pub fn ptr_type1(size: *const Foo); //~ ERROR: found struct without
pub fn ptr_type2(size: *const Foo); //~ ERROR: found struct without
Expand All @@ -40,6 +43,9 @@ extern {
pub fn tuple_type(p: (i32, i32)); //~ ERROR found Rust tuple type
pub fn tuple_type2(p: I32Pair); //~ ERROR found Rust tuple type
pub fn zero_size(p: ZeroSize); //~ ERROR found zero-size struct
pub fn zero_size_phantom(p: ZeroSizeWithPhantomData); //~ ERROR found zero-sized type
pub fn zero_size_phantom_toplevel()
-> ::std::marker::PhantomData<bool>; //~ ERROR: found zero-sized type
pub fn fn_type(p: RustFn); //~ ERROR found function pointer with Rust
pub fn fn_type2(p: fn()); //~ ERROR found function pointer with Rust
pub fn fn_contained(p: RustBadRet); //~ ERROR: found struct without
Expand Down
34 changes: 34 additions & 0 deletions src/test/run-pass/issue-34798.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![forbid(improper_ctypes)]
#![allow(dead_code)]

#[repr(C)]
pub struct Foo {
size: u8,
__value: ::std::marker::PhantomData<i32>,
}

#[repr(C)]
pub struct ZeroSizeWithPhantomData<T>(::std::marker::PhantomData<T>);

#[repr(C)]
pub struct Bar {
size: u8,
baz: ZeroSizeWithPhantomData<i32>,
}

extern "C" {
pub fn bar(_: *mut Foo, _: *mut Bar);
}

fn main() {
}

0 comments on commit e866d07

Please sign in to comment.