Skip to content

Commit 76a58cb

Browse files
committed
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]
1 parent 22fc74e commit 76a58cb

File tree

7 files changed

+260
-4
lines changed

7 files changed

+260
-4
lines changed

src/librustc/middle/infer/error_reporting.rs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
242242
}
243243
}
244244
SubSupConflict(var_origin, _, sub_r, _, sup_r) => {
245-
debug!("processing SubSupConflict");
245+
debug!("processing SubSupConflict sub: {:?} sup: {:?}", sub_r, sup_r);
246246
match free_regions_from_same_fn(self.tcx, sub_r, sup_r) {
247247
Some(ref same_frs) => {
248248
var_origins.push(var_origin);
@@ -709,6 +709,22 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
709709
sup,
710710
"");
711711
}
712+
infer::SafeDestructor(span) => {
713+
self.tcx.sess.span_err(
714+
span,
715+
"unsafe use of destructor: destructor might be called \
716+
while references are dead");
717+
note_and_explain_region(
718+
self.tcx,
719+
"superregion: ",
720+
sup,
721+
"");
722+
note_and_explain_region(
723+
self.tcx,
724+
"subregion: ",
725+
sub,
726+
"");
727+
}
712728
infer::BindingTypeIsNotValidAtDecl(span) => {
713729
self.tcx.sess.span_err(
714730
span,
@@ -1629,6 +1645,12 @@ impl<'a, 'tcx> ErrorReportingHelpers<'tcx> for InferCtxt<'a, 'tcx> {
16291645
&format!("...so that the declared lifetime parameter bounds \
16301646
are satisfied")[]);
16311647
}
1648+
infer::SafeDestructor(span) => {
1649+
self.tcx.sess.span_note(
1650+
span,
1651+
"...so that references are valid when the destructor \
1652+
runs")
1653+
}
16321654
}
16331655
}
16341656
}

src/librustc/middle/infer/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,9 @@ pub enum SubregionOrigin<'tcx> {
215215

216216
// An auto-borrow that does not enclose the expr where it occurs
217217
AutoBorrow(Span),
218+
219+
// Region constraint arriving from destructor safety
220+
SafeDestructor(Span),
218221
}
219222

220223
/// Times when we replace late-bound regions with variables:
@@ -1197,6 +1200,7 @@ impl<'tcx> SubregionOrigin<'tcx> {
11971200
CallReturn(a) => a,
11981201
AddrOf(a) => a,
11991202
AutoBorrow(a) => a,
1203+
SafeDestructor(a) => a,
12001204
}
12011205
}
12021206
}
@@ -1259,6 +1263,7 @@ impl<'tcx> Repr<'tcx> for SubregionOrigin<'tcx> {
12591263
CallReturn(a) => format!("CallReturn({})", a.repr(tcx)),
12601264
AddrOf(a) => format!("AddrOf({})", a.repr(tcx)),
12611265
AutoBorrow(a) => format!("AutoBorrow({})", a.repr(tcx)),
1266+
SafeDestructor(a) => format!("SafeDestructor({})", a.repr(tcx)),
12621267
}
12631268
}
12641269
}

src/librustc/middle/infer/region_inference/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,6 +1399,8 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> {
13991399
for upper_bound in &upper_bounds {
14001400
if !self.is_subregion_of(lower_bound.region,
14011401
upper_bound.region) {
1402+
debug!("pushing SubSupConflict sub: {:?} sup: {:?}",
1403+
lower_bound.region, upper_bound.region);
14021404
errors.push(SubSupConflict(
14031405
(*self.var_origins.borrow())[node_idx.index as uint].clone(),
14041406
lower_bound.origin.clone(),

src/librustc_typeck/check/dropck.rs

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright 2014-2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use check::regionck::{self, Rcx};
12+
13+
use middle::infer;
14+
use middle::region;
15+
use middle::ty::{self, Ty};
16+
use util::ppaux::{Repr};
17+
18+
use syntax::codemap::Span;
19+
20+
pub fn check_safety_of_destructor_if_necessary<'a, 'tcx>(rcx: &mut Rcx<'a, 'tcx>,
21+
typ: ty::Ty<'tcx>,
22+
span: Span,
23+
scope: region::CodeExtent) {
24+
debug!("check_safety_of_destructor_if_necessary typ: {} scope: {:?}",
25+
typ.repr(rcx.tcx()), scope);
26+
27+
// types that have been traversed so far by `traverse_type_if_unseen`
28+
let mut breadcrumbs: Vec<Ty<'tcx>> = Vec::new();
29+
30+
iterate_over_potentially_unsafe_regions_in_type(
31+
rcx,
32+
&mut breadcrumbs,
33+
typ,
34+
span,
35+
scope,
36+
0);
37+
}
38+
39+
fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>(
40+
rcx: &mut Rcx<'a, 'tcx>,
41+
breadcrumbs: &mut Vec<Ty<'tcx>>,
42+
ty_root: ty::Ty<'tcx>,
43+
span: Span,
44+
scope: region::CodeExtent,
45+
depth: uint)
46+
{
47+
let origin = |&:| infer::SubregionOrigin::SafeDestructor(span);
48+
let mut walker = ty_root.walk();
49+
while let Some(typ) = walker.next() {
50+
// Avoid recursing forever.
51+
if breadcrumbs.contains(&typ) {
52+
continue;
53+
}
54+
breadcrumbs.push(typ);
55+
56+
let has_dtor = match typ.sty {
57+
ty::ty_struct(struct_did, _) => ty::has_dtor(rcx.tcx(), struct_did),
58+
ty::ty_enum(enum_did, _) => ty::has_dtor(rcx.tcx(), enum_did),
59+
_ => false,
60+
};
61+
62+
debug!("iterate_over_potentially_unsafe_regions_in_type \
63+
{}typ: {} scope: {:?} has_dtor: {}",
64+
(0..depth).map(|_| ' ').collect::<String>(),
65+
typ.repr(rcx.tcx()), scope, has_dtor);
66+
67+
if has_dtor {
68+
// If `typ` has a destructor, then we must ensure that all
69+
// borrowed data reachable via `typ` must outlive the
70+
// parent of `scope`. (It does not suffice for it to
71+
// outlive `scope` because that could imply that the
72+
// borrowed data is torn down in between the end of
73+
// `scope` and when the destructor itself actually runs.
74+
75+
let parent_region =
76+
match rcx.tcx().region_maps.opt_encl_scope(scope) {
77+
Some(parent_scope) => ty::ReScope(parent_scope),
78+
None => rcx.tcx().sess.span_bug(
79+
span, format!("no enclosing scope found for scope: {:?}",
80+
scope).as_slice()),
81+
};
82+
83+
regionck::type_must_outlive(rcx, origin(), typ, parent_region);
84+
85+
} else {
86+
// Okay, `typ` itself is itself not reachable by a
87+
// destructor; but it may contain substructure that has a
88+
// destructor.
89+
90+
match typ.sty {
91+
ty::ty_struct(struct_did, substs) => {
92+
// Don't recurse; we extract type's substructure,
93+
// so do not process subparts of type expression.
94+
walker.skip_current_subtree();
95+
96+
let fields =
97+
ty::lookup_struct_fields(rcx.tcx(), struct_did);
98+
for field in fields.iter() {
99+
let field_type =
100+
ty::lookup_field_type(rcx.tcx(),
101+
struct_did,
102+
field.id,
103+
substs);
104+
iterate_over_potentially_unsafe_regions_in_type(
105+
rcx,
106+
breadcrumbs,
107+
field_type,
108+
span,
109+
scope,
110+
depth+1)
111+
}
112+
}
113+
114+
ty::ty_enum(enum_did, substs) => {
115+
// Don't recurse; we extract type's substructure,
116+
// so do not process subparts of type expression.
117+
walker.skip_current_subtree();
118+
119+
let all_variant_info =
120+
ty::substd_enum_variants(rcx.tcx(),
121+
enum_did,
122+
substs);
123+
for variant_info in all_variant_info.iter() {
124+
for argument_type in variant_info.args.iter() {
125+
iterate_over_potentially_unsafe_regions_in_type(
126+
rcx,
127+
breadcrumbs,
128+
*argument_type,
129+
span,
130+
scope,
131+
depth+1)
132+
}
133+
}
134+
}
135+
136+
ty::ty_rptr(..) | ty::ty_ptr(_) | ty::ty_bare_fn(..) => {
137+
// Don't recurse, since references, pointers,
138+
// boxes, and bare functions don't own instances
139+
// of the types appearing within them.
140+
walker.skip_current_subtree();
141+
}
142+
_ => {}
143+
};
144+
145+
// You might be tempted to pop breadcrumbs here after
146+
// processing type's internals above, but then you hit
147+
// exponential time blowup e.g. on
148+
// compile-fail/huge-struct.rs. Instead, we do not remove
149+
// anything from the breadcrumbs vector during any particular
150+
// traversal, and instead clear it after the whole traversal
151+
// is done.
152+
}
153+
}
154+
}

src/librustc_typeck/check/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ use syntax::ptr::P;
127127
use syntax::visit::{self, Visitor};
128128

129129
mod assoc;
130+
pub mod dropck;
130131
pub mod _match;
131132
pub mod vtable;
132133
pub mod writeback;

src/librustc_typeck/check/regionck.rs

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
//! contents.
8484
8585
use astconv::AstConv;
86+
use check::dropck;
8687
use check::FnCtxt;
8788
use check::regionmanip;
8889
use check::vtable;
@@ -171,6 +172,7 @@ pub struct Rcx<'a, 'tcx: 'a> {
171172

172173
// id of AST node being analyzed (the subject of the analysis).
173174
subject: SubjectNode,
175+
174176
}
175177

176178
/// Returns the validity region of `def` -- that is, how long is `def` valid?
@@ -198,7 +200,8 @@ impl<'a, 'tcx> Rcx<'a, 'tcx> {
198200
Rcx { fcx: fcx,
199201
repeating_scope: initial_repeating_scope,
200202
subject: subject,
201-
region_bound_pairs: Vec::new() }
203+
region_bound_pairs: Vec::new()
204+
}
202205
}
203206

204207
pub fn tcx(&self) -> &'a ty::ctxt<'tcx> {
@@ -469,6 +472,10 @@ fn constrain_bindings_in_pat(pat: &ast::Pat, rcx: &mut Rcx) {
469472
type_of_node_must_outlive(
470473
rcx, infer::BindingTypeIsNotValidAtDecl(span),
471474
id, var_region);
475+
476+
let var_scope = tcx.region_maps.var_scope(id);
477+
let typ = rcx.resolve_node_type(id);
478+
dropck::check_safety_of_destructor_if_necessary(rcx, typ, span, var_scope);
472479
})
473480
}
474481

@@ -517,6 +524,40 @@ fn visit_expr(rcx: &mut Rcx, expr: &ast::Expr) {
517524
*/
518525
_ => {}
519526
}
527+
528+
// If necessary, constrain destructors in the unadjusted form of this
529+
// expression.
530+
let cmt_result = {
531+
let mc = mc::MemCategorizationContext::new(rcx.fcx);
532+
mc.cat_expr_unadjusted(expr)
533+
};
534+
match cmt_result {
535+
Ok(head_cmt) => {
536+
check_safety_of_rvalue_destructor_if_necessary(rcx,
537+
head_cmt,
538+
expr.span);
539+
}
540+
Err(..) => {
541+
rcx.fcx.tcx().sess.span_note(expr.span,
542+
"cat_expr_unadjusted Errd during dtor check");
543+
}
544+
}
545+
}
546+
547+
// If necessary, constrain destructors in this expression. This will be
548+
// the adjusted form if there is an adjustment.
549+
let cmt_result = {
550+
let mc = mc::MemCategorizationContext::new(rcx.fcx);
551+
mc.cat_expr(expr)
552+
};
553+
match cmt_result {
554+
Ok(head_cmt) => {
555+
check_safety_of_rvalue_destructor_if_necessary(rcx, head_cmt, expr.span);
556+
}
557+
Err(..) => {
558+
rcx.fcx.tcx().sess.span_note(expr.span,
559+
"cat_expr Errd during dtor check");
560+
}
520561
}
521562

522563
match expr.node {
@@ -995,6 +1036,33 @@ pub fn mk_subregion_due_to_dereference(rcx: &mut Rcx,
9951036
minimum_lifetime, maximum_lifetime)
9961037
}
9971038

1039+
fn check_safety_of_rvalue_destructor_if_necessary<'a, 'tcx>(rcx: &mut Rcx<'a, 'tcx>,
1040+
cmt: mc::cmt<'tcx>,
1041+
span: Span) {
1042+
match cmt.cat {
1043+
mc::cat_rvalue(region) => {
1044+
match region {
1045+
ty::ReScope(rvalue_scope) => {
1046+
let typ = rcx.resolve_type(cmt.ty);
1047+
dropck::check_safety_of_destructor_if_necessary(rcx,
1048+
typ,
1049+
span,
1050+
rvalue_scope);
1051+
}
1052+
ty::ReStatic => {}
1053+
region => {
1054+
rcx.tcx()
1055+
.sess
1056+
.span_bug(span,
1057+
format!("unexpected rvalue region in rvalue \
1058+
destructor safety checking: `{}`",
1059+
region.repr(rcx.tcx())).as_slice());
1060+
}
1061+
}
1062+
}
1063+
_ => {}
1064+
}
1065+
}
9981066

9991067
/// Invoked on any index expression that occurs. Checks that if this is a slice being indexed, the
10001068
/// lifetime of the pointer includes the deref expr.
@@ -1404,7 +1472,7 @@ fn link_reborrowed_region<'a, 'tcx>(rcx: &Rcx<'a, 'tcx>,
14041472
}
14051473

14061474
/// Ensures that all borrowed data reachable via `ty` outlives `region`.
1407-
fn type_must_outlive<'a, 'tcx>(rcx: &mut Rcx<'a, 'tcx>,
1475+
pub fn type_must_outlive<'a, 'tcx>(rcx: &mut Rcx<'a, 'tcx>,
14081476
origin: infer::SubregionOrigin<'tcx>,
14091477
ty: Ty<'tcx>,
14101478
region: ty::Region)

src/librustc_typeck/check/wf.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,9 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> {
147147
item.span,
148148
item.id,
149149
Some(&mut this.cache));
150-
for variant in &variants {
150+
debug!("check_type_defn at bounds_checker.scope: {:?}", bounds_checker.scope);
151+
152+
for variant in &variants {
151153
for field in &variant.fields {
152154
// Regions are checked below.
153155
bounds_checker.check_traits_in_ty(field.ty);
@@ -182,6 +184,7 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> {
182184
item.span,
183185
item.id,
184186
Some(&mut this.cache));
187+
debug!("check_item_type at bounds_checker.scope: {:?}", bounds_checker.scope);
185188

186189
let type_scheme = ty::lookup_item_type(fcx.tcx(), local_def(item.id));
187190
let item_ty = fcx.instantiate_type_scheme(item.span,
@@ -200,6 +203,7 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> {
200203
item.span,
201204
item.id,
202205
Some(&mut this.cache));
206+
debug!("check_impl at bounds_checker.scope: {:?}", bounds_checker.scope);
203207

204208
// Find the impl self type as seen from the "inside" --
205209
// that is, with all type parameters converted from bound

0 commit comments

Comments
 (0)