Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make accesses to fields of packed structs unsafe #44884

Merged
merged 10 commits into from
Nov 27, 2017
224 changes: 112 additions & 112 deletions src/Cargo.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,7 @@ define_dep_nodes!( <'tcx>
[] BorrowCheck(DefId),
[] MirBorrowCheck(DefId),
[] UnsafetyCheckResult(DefId),
[] UnsafeDeriveOnReprPacked(DefId),

[] Reachability,
[] MirKeys,
Expand Down
21 changes: 20 additions & 1 deletion src/librustc/ich/impls_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,28 @@ impl_stable_hash_for!(struct mir::LocalDecl<'tcx> {
});
impl_stable_hash_for!(struct mir::UpvarDecl { debug_name, by_ref });
impl_stable_hash_for!(struct mir::BasicBlockData<'tcx> { statements, terminator, is_cleanup });
impl_stable_hash_for!(struct mir::UnsafetyViolation { source_info, description, lint_node_id });
impl_stable_hash_for!(struct mir::UnsafetyViolation { source_info, description, kind });
impl_stable_hash_for!(struct mir::UnsafetyCheckResult { violations, unsafe_blocks });

impl<'gcx> HashStable<StableHashingContext<'gcx>>
for mir::UnsafetyViolationKind {
#[inline]
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'gcx>,
hasher: &mut StableHasher<W>) {

mem::discriminant(self).hash_stable(hcx, hasher);

match *self {
mir::UnsafetyViolationKind::General => {}
mir::UnsafetyViolationKind::ExternStatic(lint_node_id) |
mir::UnsafetyViolationKind::BorrowPacked(lint_node_id) => {
lint_node_id.hash_stable(hcx, hasher);
}

}
}
}
impl<'gcx> HashStable<StableHashingContext<'gcx>>
for mir::Terminator<'gcx> {
#[inline]
Expand Down
7 changes: 7 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,12 @@ declare_lint! {
"safe access to extern statics was erroneously allowed"
}

declare_lint! {
pub SAFE_PACKED_BORROWS,
Warn,
"safe borrows of fields of packed structs were was erroneously allowed"
}

declare_lint! {
pub PATTERNS_IN_FNS_WITHOUT_BODY,
Warn,
Expand Down Expand Up @@ -247,6 +253,7 @@ impl LintPass for HardwiredLints {
RENAMED_AND_REMOVED_LINTS,
RESOLVE_TRAIT_ON_DEFAULTED_UNIT,
SAFE_EXTERN_STATICS,
SAFE_PACKED_BORROWS,
PATTERNS_IN_FNS_WITHOUT_BODY,
LEGACY_DIRECTORY_OWNERSHIP,
LEGACY_IMPORTS,
Expand Down
9 changes: 8 additions & 1 deletion src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1722,11 +1722,18 @@ impl Location {
}
}

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub enum UnsafetyViolationKind {
General,
ExternStatic(ast::NodeId),
BorrowPacked(ast::NodeId),
}

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub struct UnsafetyViolation {
pub source_info: SourceInfo,
pub description: &'static str,
pub lint_node_id: Option<ast::NodeId>,
pub kind: UnsafetyViolationKind,
}

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
Expand Down
125 changes: 79 additions & 46 deletions src/librustc/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,18 @@ use ty::subst::Subst;

use infer::{InferCtxt, InferOk};

#[derive(Copy, Clone)]
struct InferIsLocal(bool);
#[derive(Copy, Clone, Debug)]
enum InferIsLocal {
BrokenYes,
Yes,
No
}

#[derive(Debug, Copy, Clone)]
pub enum Conflict {
Upstream,
Downstream
}

pub struct OverlapResult<'tcx> {
pub impl_header: ty::ImplHeader<'tcx>,
Expand Down Expand Up @@ -126,32 +136,46 @@ fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
}

pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
trait_ref: ty::TraitRef<'tcx>) -> bool
trait_ref: ty::TraitRef<'tcx>,
broken: bool)
-> Option<Conflict>
{
debug!("trait_ref_is_knowable(trait_ref={:?})", trait_ref);

// if the orphan rules pass, that means that no ancestor crate can
// impl this, so it's up to us.
if orphan_check_trait_ref(tcx, trait_ref, InferIsLocal(false)).is_ok() {
debug!("trait_ref_is_knowable: orphan check passed");
return true;
debug!("trait_ref_is_knowable(trait_ref={:?}, broken={:?})", trait_ref, broken);
let mode = if broken {
InferIsLocal::BrokenYes
} else {
InferIsLocal::Yes
};
if orphan_check_trait_ref(tcx, trait_ref, mode).is_ok() {
// A downstream or cousin crate is allowed to implement some
// substitution of this trait-ref.
debug!("trait_ref_is_knowable: downstream crate might implement");
return Some(Conflict::Downstream);
}

// if the trait is not marked fundamental, then it's always possible that
// an ancestor crate will impl this in the future, if they haven't
// already
if !trait_ref_is_local_or_fundamental(tcx, trait_ref) {
debug!("trait_ref_is_knowable: trait is neither local nor fundamental");
return false;
if trait_ref_is_local_or_fundamental(tcx, trait_ref) {
// This is a local or fundamental trait, so future-compatibility
// is no concern. We know that downstream/cousin crates are not
// allowed to implement a substitution of this trait ref, which
// means impls could only come from dependencies of this crate,
// which we already know about.
return None;
}
// This is a remote non-fundamental trait, so if another crate
// can be the "final owner" of a substitution of this trait-ref,
// they are allowed to implement it future-compatibly.
//
// However, if we are a final owner, then nobody else can be,
// and if we are an intermediate owner, then we don't care
// about future-compatibility, which means that we're OK if
// we are an owner.
if orphan_check_trait_ref(tcx, trait_ref, InferIsLocal::No).is_ok() {
debug!("trait_ref_is_knowable: orphan check passed");
return None;
} else {
debug!("trait_ref_is_knowable: nonlocal, nonfundamental, unowned");
return Some(Conflict::Upstream);
}

// find out when some downstream (or cousin) crate could impl this
// trait-ref, presuming that all the parameters were instantiated
// with downstream types. If not, then it could only be
// implemented by an upstream crate, which means that the impl
// must be visible to us, and -- since the trait is fundamental
// -- we can test.
orphan_check_trait_ref(tcx, trait_ref, InferIsLocal(true)).is_err()
}

pub fn trait_ref_is_local_or_fundamental<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
Expand Down Expand Up @@ -189,16 +213,16 @@ pub fn orphan_check<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
return Ok(());
}

orphan_check_trait_ref(tcx, trait_ref, InferIsLocal(false))
orphan_check_trait_ref(tcx, trait_ref, InferIsLocal::No)
}

fn orphan_check_trait_ref<'tcx>(tcx: TyCtxt,
trait_ref: ty::TraitRef<'tcx>,
infer_is_local: InferIsLocal)
-> Result<(), OrphanCheckErr<'tcx>>
{
debug!("orphan_check_trait_ref(trait_ref={:?}, infer_is_local={})",
trait_ref, infer_is_local.0);
debug!("orphan_check_trait_ref(trait_ref={:?}, infer_is_local={:?})",
trait_ref, infer_is_local);

// First, create an ordered iterator over all the type parameters to the trait, with the self
// type appearing first.
Expand All @@ -212,7 +236,9 @@ fn orphan_check_trait_ref<'tcx>(tcx: TyCtxt,
// uncovered type parameters.
let uncovered_tys = uncovered_tys(tcx, input_ty, infer_is_local);
for uncovered_ty in uncovered_tys {
if let Some(param) = uncovered_ty.walk().find(|t| is_type_parameter(t)) {
if let Some(param) = uncovered_ty.walk()
.find(|t| is_possibly_remote_type(t, infer_is_local))
{
debug!("orphan_check_trait_ref: uncovered type `{:?}`", param);
return Err(OrphanCheckErr::UncoveredTy(param));
}
Expand All @@ -224,11 +250,11 @@ fn orphan_check_trait_ref<'tcx>(tcx: TyCtxt,

// Otherwise, enforce invariant that there are no type
// parameters reachable.
if !infer_is_local.0 {
if let Some(param) = input_ty.walk().find(|t| is_type_parameter(t)) {
debug!("orphan_check_trait_ref: uncovered type `{:?}`", param);
return Err(OrphanCheckErr::UncoveredTy(param));
}
if let Some(param) = input_ty.walk()
.find(|t| is_possibly_remote_type(t, infer_is_local))
{
debug!("orphan_check_trait_ref: uncovered type `{:?}`", param);
return Err(OrphanCheckErr::UncoveredTy(param));
}
}

Expand All @@ -250,7 +276,7 @@ fn uncovered_tys<'tcx>(tcx: TyCtxt, ty: Ty<'tcx>, infer_is_local: InferIsLocal)
}
}

fn is_type_parameter(ty: Ty) -> bool {
fn is_possibly_remote_type(ty: Ty, _infer_is_local: InferIsLocal) -> bool {
match ty.sty {
ty::TyProjection(..) | ty::TyParam(..) => true,
_ => false,
Expand All @@ -273,7 +299,15 @@ fn fundamental_ty(tcx: TyCtxt, ty: Ty) -> bool {
}
}

fn ty_is_local_constructor(ty: Ty, infer_is_local: InferIsLocal)-> bool {
fn def_id_is_local(def_id: DefId, infer_is_local: InferIsLocal) -> bool {
match infer_is_local {
InferIsLocal::Yes => false,
InferIsLocal::No |
InferIsLocal::BrokenYes => def_id.is_local()
}
}

fn ty_is_local_constructor(ty: Ty, infer_is_local: InferIsLocal) -> bool {
debug!("ty_is_local_constructor({:?})", ty);

match ty.sty {
Expand All @@ -296,20 +330,19 @@ fn ty_is_local_constructor(ty: Ty, infer_is_local: InferIsLocal)-> bool {
false
}

ty::TyInfer(..) => {
infer_is_local.0
}

ty::TyAdt(def, _) => {
def.did.is_local()
}
ty::TyInfer(..) => match infer_is_local {
InferIsLocal::No => false,
InferIsLocal::Yes |
InferIsLocal::BrokenYes => true
},

ty::TyForeign(did) => {
did.is_local()
}
ty::TyAdt(def, _) => def_id_is_local(def.did, infer_is_local),
ty::TyForeign(did) => def_id_is_local(did, infer_is_local),

ty::TyDynamic(ref tt, ..) => {
tt.principal().map_or(false, |p| p.def_id().is_local())
tt.principal().map_or(false, |p| {
def_id_is_local(p.def_id(), infer_is_local)
})
}

ty::TyError => {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
// terms of `Fn` etc, but we could probably make this more
// precise still.
let unbound_input_types = stack.fresh_trait_ref.input_types().any(|ty| ty.is_fresh());
if unbound_input_types && self.intercrate {
if unbound_input_types && self.intercrate && false {
debug!("evaluate_stack({:?}) --> unbound argument, intercrate --> ambiguous",
stack.fresh_trait_ref);
// Heuristics: show the diagnostics when there are no candidates in crate.
Expand Down Expand Up @@ -1221,7 +1221,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
// bound regions
let trait_ref = predicate.skip_binder().trait_ref;

coherence::trait_ref_is_knowable(self.tcx(), trait_ref)
coherence::trait_ref_is_knowable(self.tcx(), trait_ref, false).is_none()
}

/// Returns true if the global caches can be used.
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/ty/maps/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,9 @@ define_maps! { <'tcx>
/// The result of unsafety-checking this def-id.
[] fn unsafety_check_result: UnsafetyCheckResult(DefId) -> mir::UnsafetyCheckResult,

/// HACK: when evaluated, this reports a "unsafe derive on repr(packed)" error
[] fn unsafe_derive_on_repr_packed: UnsafeDeriveOnReprPacked(DefId) -> (),

/// The signature of functions and closures.
[] fn fn_sig: FnSignature(DefId) -> ty::PolyFnSig<'tcx>,

Expand Down
1 change: 1 addition & 0 deletions src/librustc/ty/maps/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>,
DepKind::BorrowCheck => { force!(borrowck, def_id!()); }
DepKind::MirBorrowCheck => { force!(mir_borrowck, def_id!()); }
DepKind::UnsafetyCheckResult => { force!(unsafety_check_result, def_id!()); }
DepKind::UnsafeDeriveOnReprPacked => { force!(unsafe_derive_on_repr_packed, def_id!()); }
DepKind::Reachability => { force!(reachable_set, LOCAL_CRATE); }
DepKind::MirKeys => { force!(mir_keys, LOCAL_CRATE); }
DepKind::CrateVariances => { force!(crate_variances, LOCAL_CRATE); }
Expand Down
5 changes: 5 additions & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,11 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
id: LintId::of(LATE_BOUND_LIFETIME_ARGUMENTS),
reference: "issue #42868 <https://github.com/rust-lang/rust/issues/42868>",
},
FutureIncompatibleInfo {
id: LintId::of(SAFE_PACKED_BORROWS),
reference: "issue #46043 <https://github.com/rust-lang/rust/issues/46043>",
},

]);

// Register renamed and removed lints
Expand Down
5 changes: 4 additions & 1 deletion src/librustc_mir/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ use syntax_pos::Span;
use std::fmt;
use std::iter;

use transform::{add_call_guards, no_landing_pads, simplify};
use transform::{add_moves_for_packed_drops, add_call_guards};
use transform::{no_landing_pads, simplify};
use util::elaborate_drops::{self, DropElaborator, DropStyle, DropFlagMode};
use util::patch::MirPatch;

Expand Down Expand Up @@ -114,6 +115,8 @@ fn make_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
}
};
debug!("make_shim({:?}) = untransformed {:?}", instance, result);
add_moves_for_packed_drops::add_moves_for_packed_drops(
tcx, &mut result, instance.def_id());
no_landing_pads::no_landing_pads(tcx, &mut result);
simplify::simplify_cfg(&mut result);
add_call_guards::CriticalCallEdges.add_call_guards(&mut result);
Expand Down
Loading