From e02b6d17486ecef8541d03bb6a38c52d1a35b339 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 27 Oct 2014 12:55:16 +0100 Subject: [PATCH] destructor checker (dropck). Largely adapted from pcwalton's original branch, with following notable modifications: Use `regionck::type_must_outlive` to generate `SafeDestructor` constraints. (this plugged some soundness holes in the analysis). Avoid exponential time blowup on compile-fail/huge-struct.rs by keeping the breadcrumbs until end of traversal. Avoid premature return from regionck::visit_expr. Factored drop-checking code out into dropck module. Added `SafeDestructor` to enum `SubregionOrigin` (for error reporting). ---- Since this imposes restrictions on the lifetimes used in types with destructors, this is a (wait for it) [breaking-change] --- src/librustc/middle/infer/error_reporting.rs | 25 ++- src/librustc/middle/infer/mod.rs | 5 + .../middle/infer/region_inference/mod.rs | 2 + src/librustc_typeck/check/dropck.rs | 154 ++++++++++++++++++ src/librustc_typeck/check/mod.rs | 1 + src/librustc_typeck/check/regionck.rs | 72 +++++++- src/librustc_typeck/check/wf.rs | 6 +- 7 files changed, 261 insertions(+), 4 deletions(-) create mode 100644 src/librustc_typeck/check/dropck.rs diff --git a/src/librustc/middle/infer/error_reporting.rs b/src/librustc/middle/infer/error_reporting.rs index 6cacf46b2fe..5d7a56ef0e6 100644 --- a/src/librustc/middle/infer/error_reporting.rs +++ b/src/librustc/middle/infer/error_reporting.rs @@ -242,7 +242,7 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> { } } SubSupConflict(var_origin, _, sub_r, _, sup_r) => { - debug!("processing SubSupConflict"); + debug!("processing SubSupConflict sub: {:?} sup: {:?}", sub_r, sup_r); match free_regions_from_same_fn(self.tcx, sub_r, sup_r) { Some(ref same_frs) => { var_origins.push(var_origin); @@ -709,6 +709,23 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> { sup, ""); } + infer::SafeDestructor(span) => { + self.tcx.sess.span_err( + span, + "unsafe use of destructor: destructor might be called \ + while references are dead"); + // FIXME (22171): terms "super/subregion" are suboptimal + note_and_explain_region( + self.tcx, + "superregion: ", + sup, + ""); + note_and_explain_region( + self.tcx, + "subregion: ", + sub, + ""); + } infer::BindingTypeIsNotValidAtDecl(span) => { self.tcx.sess.span_err( span, @@ -1629,6 +1646,12 @@ impl<'a, 'tcx> ErrorReportingHelpers<'tcx> for InferCtxt<'a, 'tcx> { &format!("...so that the declared lifetime parameter bounds \ are satisfied")[]); } + infer::SafeDestructor(span) => { + self.tcx.sess.span_note( + span, + "...so that references are valid when the destructor \ + runs") + } } } } diff --git a/src/librustc/middle/infer/mod.rs b/src/librustc/middle/infer/mod.rs index f8dae3e92da..41310f05588 100644 --- a/src/librustc/middle/infer/mod.rs +++ b/src/librustc/middle/infer/mod.rs @@ -215,6 +215,9 @@ pub enum SubregionOrigin<'tcx> { // An auto-borrow that does not enclose the expr where it occurs AutoBorrow(Span), + + // Region constraint arriving from destructor safety + SafeDestructor(Span), } /// Times when we replace late-bound regions with variables: @@ -1197,6 +1200,7 @@ impl<'tcx> SubregionOrigin<'tcx> { CallReturn(a) => a, AddrOf(a) => a, AutoBorrow(a) => a, + SafeDestructor(a) => a, } } } @@ -1259,6 +1263,7 @@ impl<'tcx> Repr<'tcx> for SubregionOrigin<'tcx> { CallReturn(a) => format!("CallReturn({})", a.repr(tcx)), AddrOf(a) => format!("AddrOf({})", a.repr(tcx)), AutoBorrow(a) => format!("AutoBorrow({})", a.repr(tcx)), + SafeDestructor(a) => format!("SafeDestructor({})", a.repr(tcx)), } } } diff --git a/src/librustc/middle/infer/region_inference/mod.rs b/src/librustc/middle/infer/region_inference/mod.rs index 6b6887ba8e2..3dba8045d60 100644 --- a/src/librustc/middle/infer/region_inference/mod.rs +++ b/src/librustc/middle/infer/region_inference/mod.rs @@ -1399,6 +1399,8 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> { for upper_bound in &upper_bounds { if !self.is_subregion_of(lower_bound.region, upper_bound.region) { + debug!("pushing SubSupConflict sub: {:?} sup: {:?}", + lower_bound.region, upper_bound.region); errors.push(SubSupConflict( (*self.var_origins.borrow())[node_idx.index as uint].clone(), lower_bound.origin.clone(), diff --git a/src/librustc_typeck/check/dropck.rs b/src/librustc_typeck/check/dropck.rs new file mode 100644 index 00000000000..a1e0f91984b --- /dev/null +++ b/src/librustc_typeck/check/dropck.rs @@ -0,0 +1,154 @@ +// Copyright 2014-2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use check::regionck::{self, Rcx}; + +use middle::infer; +use middle::region; +use middle::ty::{self, Ty}; +use util::ppaux::{Repr}; + +use syntax::codemap::Span; + +pub fn check_safety_of_destructor_if_necessary<'a, 'tcx>(rcx: &mut Rcx<'a, 'tcx>, + typ: ty::Ty<'tcx>, + span: Span, + scope: region::CodeExtent) { + debug!("check_safety_of_destructor_if_necessary typ: {} scope: {:?}", + typ.repr(rcx.tcx()), scope); + + // types that have been traversed so far by `traverse_type_if_unseen` + let mut breadcrumbs: Vec> = Vec::new(); + + iterate_over_potentially_unsafe_regions_in_type( + rcx, + &mut breadcrumbs, + typ, + span, + scope, + 0); +} + +fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>( + rcx: &mut Rcx<'a, 'tcx>, + breadcrumbs: &mut Vec>, + ty_root: ty::Ty<'tcx>, + span: Span, + scope: region::CodeExtent, + depth: uint) +{ + let origin = |&:| infer::SubregionOrigin::SafeDestructor(span); + let mut walker = ty_root.walk(); + while let Some(typ) = walker.next() { + // Avoid recursing forever. + if breadcrumbs.contains(&typ) { + continue; + } + breadcrumbs.push(typ); + + let has_dtor = match typ.sty { + ty::ty_struct(struct_did, _) => ty::has_dtor(rcx.tcx(), struct_did), + ty::ty_enum(enum_did, _) => ty::has_dtor(rcx.tcx(), enum_did), + _ => false, + }; + + debug!("iterate_over_potentially_unsafe_regions_in_type \ + {}typ: {} scope: {:?} has_dtor: {}", + (0..depth).map(|_| ' ').collect::(), + typ.repr(rcx.tcx()), scope, has_dtor); + + if has_dtor { + // If `typ` has a destructor, then we must ensure that all + // borrowed data reachable via `typ` must outlive the + // parent of `scope`. (It does not suffice for it to + // outlive `scope` because that could imply that the + // borrowed data is torn down in between the end of + // `scope` and when the destructor itself actually runs. + + let parent_region = + match rcx.tcx().region_maps.opt_encl_scope(scope) { + Some(parent_scope) => ty::ReScope(parent_scope), + None => rcx.tcx().sess.span_bug( + span, format!("no enclosing scope found for scope: {:?}", + scope).as_slice()), + }; + + regionck::type_must_outlive(rcx, origin(), typ, parent_region); + + } else { + // Okay, `typ` itself is itself not reachable by a + // destructor; but it may contain substructure that has a + // destructor. + + match typ.sty { + ty::ty_struct(struct_did, substs) => { + // Don't recurse; we extract type's substructure, + // so do not process subparts of type expression. + walker.skip_current_subtree(); + + let fields = + ty::lookup_struct_fields(rcx.tcx(), struct_did); + for field in fields.iter() { + let field_type = + ty::lookup_field_type(rcx.tcx(), + struct_did, + field.id, + substs); + iterate_over_potentially_unsafe_regions_in_type( + rcx, + breadcrumbs, + field_type, + span, + scope, + depth+1) + } + } + + ty::ty_enum(enum_did, substs) => { + // Don't recurse; we extract type's substructure, + // so do not process subparts of type expression. + walker.skip_current_subtree(); + + let all_variant_info = + ty::substd_enum_variants(rcx.tcx(), + enum_did, + substs); + for variant_info in all_variant_info.iter() { + for argument_type in variant_info.args.iter() { + iterate_over_potentially_unsafe_regions_in_type( + rcx, + breadcrumbs, + *argument_type, + span, + scope, + depth+1) + } + } + } + + ty::ty_rptr(..) | ty::ty_ptr(_) | ty::ty_bare_fn(..) => { + // Don't recurse, since references, pointers, + // boxes, and bare functions don't own instances + // of the types appearing within them. + walker.skip_current_subtree(); + } + _ => {} + }; + + // You might be tempted to pop breadcrumbs here after + // processing type's internals above, but then you hit + // exponential time blowup e.g. on + // compile-fail/huge-struct.rs. Instead, we do not remove + // anything from the breadcrumbs vector during any particular + // traversal, and instead clear it after the whole traversal + // is done. + } + } +} diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index a081aabe559..d90ed7eda59 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -127,6 +127,7 @@ use syntax::ptr::P; use syntax::visit::{self, Visitor}; mod assoc; +pub mod dropck; pub mod _match; pub mod vtable; pub mod writeback; diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index 9df0403794d..80f6e3800f7 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -83,6 +83,7 @@ //! contents. use astconv::AstConv; +use check::dropck; use check::FnCtxt; use check::regionmanip; use check::vtable; @@ -171,6 +172,7 @@ pub struct Rcx<'a, 'tcx: 'a> { // id of AST node being analyzed (the subject of the analysis). subject: SubjectNode, + } /// Returns the validity region of `def` -- that is, how long is `def` valid? @@ -198,7 +200,8 @@ impl<'a, 'tcx> Rcx<'a, 'tcx> { Rcx { fcx: fcx, repeating_scope: initial_repeating_scope, subject: subject, - region_bound_pairs: Vec::new() } + region_bound_pairs: Vec::new() + } } pub fn tcx(&self) -> &'a ty::ctxt<'tcx> { @@ -469,6 +472,10 @@ fn constrain_bindings_in_pat(pat: &ast::Pat, rcx: &mut Rcx) { type_of_node_must_outlive( rcx, infer::BindingTypeIsNotValidAtDecl(span), id, var_region); + + let var_scope = tcx.region_maps.var_scope(id); + let typ = rcx.resolve_node_type(id); + dropck::check_safety_of_destructor_if_necessary(rcx, typ, span, var_scope); }) } @@ -517,6 +524,40 @@ fn visit_expr(rcx: &mut Rcx, expr: &ast::Expr) { */ _ => {} } + + // If necessary, constrain destructors in the unadjusted form of this + // expression. + let cmt_result = { + let mc = mc::MemCategorizationContext::new(rcx.fcx); + mc.cat_expr_unadjusted(expr) + }; + match cmt_result { + Ok(head_cmt) => { + check_safety_of_rvalue_destructor_if_necessary(rcx, + head_cmt, + expr.span); + } + Err(..) => { + rcx.fcx.tcx().sess.span_note(expr.span, + "cat_expr_unadjusted Errd during dtor check"); + } + } + } + + // If necessary, constrain destructors in this expression. This will be + // the adjusted form if there is an adjustment. + let cmt_result = { + let mc = mc::MemCategorizationContext::new(rcx.fcx); + mc.cat_expr(expr) + }; + match cmt_result { + Ok(head_cmt) => { + check_safety_of_rvalue_destructor_if_necessary(rcx, head_cmt, expr.span); + } + Err(..) => { + rcx.fcx.tcx().sess.span_note(expr.span, + "cat_expr Errd during dtor check"); + } } match expr.node { @@ -995,6 +1036,33 @@ pub fn mk_subregion_due_to_dereference(rcx: &mut Rcx, minimum_lifetime, maximum_lifetime) } +fn check_safety_of_rvalue_destructor_if_necessary<'a, 'tcx>(rcx: &mut Rcx<'a, 'tcx>, + cmt: mc::cmt<'tcx>, + span: Span) { + match cmt.cat { + mc::cat_rvalue(region) => { + match region { + ty::ReScope(rvalue_scope) => { + let typ = rcx.resolve_type(cmt.ty); + dropck::check_safety_of_destructor_if_necessary(rcx, + typ, + span, + rvalue_scope); + } + ty::ReStatic => {} + region => { + rcx.tcx() + .sess + .span_bug(span, + format!("unexpected rvalue region in rvalue \ + destructor safety checking: `{}`", + region.repr(rcx.tcx())).as_slice()); + } + } + } + _ => {} + } +} /// Invoked on any index expression that occurs. Checks that if this is a slice being indexed, the /// lifetime of the pointer includes the deref expr. @@ -1404,7 +1472,7 @@ fn link_reborrowed_region<'a, 'tcx>(rcx: &Rcx<'a, 'tcx>, } /// Ensures that all borrowed data reachable via `ty` outlives `region`. -fn type_must_outlive<'a, 'tcx>(rcx: &mut Rcx<'a, 'tcx>, +pub fn type_must_outlive<'a, 'tcx>(rcx: &mut Rcx<'a, 'tcx>, origin: infer::SubregionOrigin<'tcx>, ty: Ty<'tcx>, region: ty::Region) diff --git a/src/librustc_typeck/check/wf.rs b/src/librustc_typeck/check/wf.rs index 5122f9e2d38..923614f9e8a 100644 --- a/src/librustc_typeck/check/wf.rs +++ b/src/librustc_typeck/check/wf.rs @@ -147,7 +147,9 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> { item.span, item.id, Some(&mut this.cache)); - for variant in &variants { + debug!("check_type_defn at bounds_checker.scope: {:?}", bounds_checker.scope); + + for variant in &variants { for field in &variant.fields { // Regions are checked below. bounds_checker.check_traits_in_ty(field.ty); @@ -182,6 +184,7 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> { item.span, item.id, Some(&mut this.cache)); + debug!("check_item_type at bounds_checker.scope: {:?}", bounds_checker.scope); let type_scheme = ty::lookup_item_type(fcx.tcx(), local_def(item.id)); let item_ty = fcx.instantiate_type_scheme(item.span, @@ -200,6 +203,7 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> { item.span, item.id, Some(&mut this.cache)); + debug!("check_impl at bounds_checker.scope: {:?}", bounds_checker.scope); // Find the impl self type as seen from the "inside" -- // that is, with all type parameters converted from bound