Skip to content

Commit 9c15a66

Browse files
cramertjcramertj
committed
Ensure StorageDead is created even if variable initialization fails
1 parent fb97bb5 commit 9c15a66

File tree

8 files changed

+246
-44
lines changed

8 files changed

+246
-44
lines changed

‎src/librustc_mir/build/expr/as_rvalue.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
102102
});
103103
if let Some(scope) = scope {
104104
// schedule a shallow free of that memory, lest we unwind:
105-
this.schedule_drop(expr_span, scope, &Place::Local(result), value.ty);
105+
this.schedule_drop_storage_and_value(
106+
expr_span, scope, &Place::Local(result), value.ty,
107+
);
106108
}
107109

108110
// malloc some memory of suitable type (thus far, uninitialized):

‎src/librustc_mir/build/expr/as_temp.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
6262
// anything because no values with a destructor can be created in
6363
// a constant at this time, even if the type may need dropping.
6464
if let Some(temp_lifetime) = temp_lifetime {
65-
this.schedule_drop(expr_span, temp_lifetime, &Place::Local(temp), expr_ty);
65+
this.schedule_drop_storage_and_value(
66+
expr_span, temp_lifetime, &Place::Local(temp), expr_ty,
67+
);
6668
}
6769

6870
block.and(temp)

‎src/librustc_mir/build/matches/mod.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use build::{BlockAnd, BlockAndExtension, Builder};
1717
use build::{GuardFrame, GuardFrameLocal, LocalsForNode};
1818
use build::ForGuard::{self, OutsideGuard, RefWithinGuard, ValWithinGuard};
19+
use build::scope::{CachedBlock, DropKind};
1920
use rustc_data_structures::fx::FxHashMap;
2021
use rustc_data_structures::bitvec::BitVector;
2122
use rustc::ty::{self, Ty};
@@ -367,7 +368,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
367368
source_info,
368369
kind: StatementKind::StorageLive(local_id)
369370
});
370-
Place::Local(local_id)
371+
let place = Place::Local(local_id);
372+
let var_ty = self.local_decls[local_id].ty;
373+
let hir_id = self.hir.tcx().hir.node_to_hir_id(var);
374+
let region_scope = self.hir.region_scope_tree.var_scope(hir_id.local_id);
375+
self.schedule_drop(
376+
span, region_scope, &place, var_ty,
377+
DropKind::Storage,
378+
);
379+
place
371380
}
372381

373382
pub fn schedule_drop_for_binding(&mut self,
@@ -378,7 +387,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
378387
let var_ty = self.local_decls[local_id].ty;
379388
let hir_id = self.hir.tcx().hir.node_to_hir_id(var);
380389
let region_scope = self.hir.region_scope_tree.var_scope(hir_id.local_id);
381-
self.schedule_drop(span, region_scope, &Place::Local(local_id), var_ty);
390+
self.schedule_drop(
391+
span, region_scope, &Place::Local(local_id), var_ty,
392+
DropKind::Value {
393+
cached_block: CachedBlock::default(),
394+
},
395+
);
382396
}
383397

384398
pub fn visit_bindings<F>(&mut self, pattern: &Pattern<'tcx>, f: &mut F)

‎src/librustc_mir/build/mod.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111

1212
use build;
13+
use build::scope::{CachedBlock, DropKind};
1314
use hair::cx::Cx;
1415
use hair::{LintLevel, BindingMode, PatternKind};
1516
use rustc::hir;
@@ -735,9 +736,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
735736
}
736737

737738
// Make sure we drop (parts of) the argument even when not matched on.
738-
self.schedule_drop(pattern.as_ref().map_or(ast_body.span, |pat| pat.span),
739-
argument_scope, &place, ty);
740-
739+
self.schedule_drop(
740+
pattern.as_ref().map_or(ast_body.span, |pat| pat.span),
741+
argument_scope, &place, ty,
742+
DropKind::Value { cached_block: CachedBlock::default() },
743+
);
741744
}
742745

743746
// Enter the argument pattern bindings source scope, if it exists.

‎src/librustc_mir/build/scope.rs

Lines changed: 66 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,12 @@ struct DropData<'tcx> {
144144
/// place to drop
145145
location: Place<'tcx>,
146146

147-
/// Whether this is a full value Drop, or just a StorageDead.
148-
kind: DropKind
147+
/// Whether this is a value Drop or a StorageDead.
148+
kind: DropKind,
149149
}
150150

151151
#[derive(Debug, Default, Clone, Copy)]
152-
struct CachedBlock {
152+
pub(crate) struct CachedBlock {
153153
/// The cached block for the cleanups-on-diverge path. This block
154154
/// contains code to run the current drop and all the preceding
155155
/// drops (i.e. those having lower index in Drop’s Scope drop
@@ -166,7 +166,7 @@ struct CachedBlock {
166166
}
167167

168168
#[derive(Debug)]
169-
enum DropKind {
169+
pub(crate) enum DropKind {
170170
Value {
171171
cached_block: CachedBlock,
172172
},
@@ -622,25 +622,58 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
622622
abortblk
623623
}
624624

625+
pub fn schedule_drop_storage_and_value(
626+
&mut self,
627+
span: Span,
628+
region_scope: region::Scope,
629+
place: &Place<'tcx>,
630+
place_ty: Ty<'tcx>,
631+
) {
632+
self.schedule_drop(
633+
span, region_scope, place, place_ty,
634+
DropKind::Storage,
635+
);
636+
self.schedule_drop(
637+
span, region_scope, place, place_ty,
638+
DropKind::Value {
639+
cached_block: CachedBlock::default(),
640+
},
641+
);
642+
}
643+
625644
// Scheduling drops
626645
// ================
627646
/// Indicates that `place` should be dropped on exit from
628647
/// `region_scope`.
629-
pub fn schedule_drop(&mut self,
630-
span: Span,
631-
region_scope: region::Scope,
632-
place: &Place<'tcx>,
633-
place_ty: Ty<'tcx>) {
648+
///
649+
/// When called with `DropKind::Storage`, `place` should be a local
650+
/// with an index higher than the current `self.arg_count`.
651+
pub fn schedule_drop(
652+
&mut self,
653+
span: Span,
654+
region_scope: region::Scope,
655+
place: &Place<'tcx>,
656+
place_ty: Ty<'tcx>,
657+
drop_kind: DropKind,
658+
) {
634659
let needs_drop = self.hir.needs_drop(place_ty);
635-
let drop_kind = if needs_drop {
636-
DropKind::Value { cached_block: CachedBlock::default() }
637-
} else {
638-
// Only temps and vars need their storage dead.
639-
match *place {
640-
Place::Local(index) if index.index() > self.arg_count => DropKind::Storage,
641-
_ => return
660+
match drop_kind {
661+
DropKind::Value { .. } => if !needs_drop { return },
662+
DropKind::Storage => {
663+
match *place {
664+
Place::Local(index) => if index.index() <= self.arg_count {
665+
span_bug!(
666+
span, "`schedule_drop` called with index {} and arg_count {}",
667+
index.index(),
668+
self.arg_count,
669+
)
670+
},
671+
_ => span_bug!(
672+
span, "`schedule_drop` called with non-`Local` place {:?}", place
673+
),
674+
}
642675
}
643-
};
676+
}
644677

645678
for scope in self.scopes.iter_mut().rev() {
646679
let this_scope = scope.region_scope == region_scope;
@@ -895,24 +928,24 @@ fn build_scope_drops<'tcx>(cfg: &mut CFG<'tcx>,
895928
});
896929
block = next;
897930
}
898-
DropKind::Storage => {}
899-
}
900-
901-
// We do not need to emit StorageDead for generator drops
902-
if generator_drop {
903-
continue
904-
}
931+
DropKind::Storage => {
932+
// We do not need to emit StorageDead for generator drops
933+
if generator_drop {
934+
continue
935+
}
905936

906-
// Drop the storage for both value and storage drops.
907-
// Only temps and vars need their storage dead.
908-
match drop_data.location {
909-
Place::Local(index) if index.index() > arg_count => {
910-
cfg.push(block, Statement {
911-
source_info,
912-
kind: StatementKind::StorageDead(index)
913-
});
937+
// Drop the storage for both value and storage drops.
938+
// Only temps and vars need their storage dead.
939+
match drop_data.location {
940+
Place::Local(index) if index.index() > arg_count => {
941+
cfg.push(block, Statement {
942+
source_info,
943+
kind: StatementKind::StorageDead(index)
944+
});
945+
}
946+
_ => unreachable!(),
947+
}
914948
}
915-
_ => continue
916949
}
917950
}
918951
block.unit()

‎src/test/codegen/lifetime_start_end.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ pub fn test() {
3131
// CHECK: [[S__4:%[0-9]+]] = bitcast { i32, i32 }* %_4 to i8*
3232
// CHECK: call void @llvm.lifetime.start{{.*}}(i{{[0-9 ]+}}, i8* [[S__4]])
3333

34-
// CHECK: [[E_b:%[0-9]+]] = bitcast { i32, i32 }** %b to i8*
35-
// CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E_b]])
36-
3734
// CHECK: [[E__4:%[0-9]+]] = bitcast { i32, i32 }* %_4 to i8*
3835
// CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E__4]])
36+
37+
// CHECK: [[E_b:%[0-9]+]] = bitcast { i32, i32 }** %b to i8*
38+
// CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E_b]])
3939
}
4040

4141
let c = 1;

‎src/test/mir-opt/issue-49232.rs

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
// Copyright 2018 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+
// We must mark a variable whose initialization fails due to an
12+
// abort statement as StorageDead.
13+
14+
fn main() {
15+
loop {
16+
let beacon = {
17+
match true {
18+
false => 4,
19+
true => break,
20+
}
21+
};
22+
drop(&beacon);
23+
}
24+
}
25+
26+
// END RUST SOURCE
27+
// START rustc.main.mir_map.0.mir
28+
// fn main() -> (){
29+
// let mut _0: ();
30+
// scope 1 {
31+
// }
32+
// scope 2 {
33+
// let _2: i32;
34+
// }
35+
// let mut _1: ();
36+
// let mut _3: bool;
37+
// let mut _4: u8;
38+
// let mut _5: !;
39+
// let mut _6: ();
40+
// let mut _7: &i32;
41+
// bb0: {
42+
// goto -> bb1;
43+
// }
44+
// bb1: {
45+
// falseUnwind -> [real: bb3, cleanup: bb4];
46+
// }
47+
// bb2: {
48+
// goto -> bb29;
49+
// }
50+
// bb3: {
51+
// StorageLive(_2);
52+
// StorageLive(_3);
53+
// _3 = const true;
54+
// _4 = discriminant(_3);
55+
// switchInt(_3) -> [false: bb11, otherwise: bb10];
56+
// }
57+
// bb4: {
58+
// resume;
59+
// }
60+
// bb5: {
61+
// _2 = const 4i32;
62+
// goto -> bb14;
63+
// }
64+
// bb6: {
65+
// _0 = ();
66+
// goto -> bb15;
67+
// }
68+
// bb7: {
69+
// falseEdges -> [real: bb12, imaginary: bb8];
70+
// }
71+
// bb8: {
72+
// falseEdges -> [real: bb13, imaginary: bb9];
73+
// }
74+
// bb9: {
75+
// unreachable;
76+
// }
77+
// bb10: {
78+
// goto -> bb8;
79+
// }
80+
// bb11: {
81+
// goto -> bb7;
82+
// }
83+
// bb12: {
84+
// goto -> bb5;
85+
// }
86+
// bb13: {
87+
// goto -> bb6;
88+
// }
89+
// bb14: {
90+
// StorageDead(_3);
91+
// StorageLive(_7);
92+
// _7 = &_2;
93+
// _6 = const std::mem::drop(move _7) -> [return: bb28, unwind: bb4];
94+
// }
95+
// bb15: {
96+
// goto -> bb16;
97+
// }
98+
// bb16: {
99+
// goto -> bb17;
100+
// }
101+
// bb17: {
102+
// goto -> bb18;
103+
// }
104+
// bb18: {
105+
// goto -> bb19;
106+
// }
107+
// bb19: {
108+
// goto -> bb20;
109+
// }
110+
// bb20: {
111+
// StorageDead(_3);
112+
// goto -> bb21;
113+
// }
114+
// bb21: {
115+
// goto -> bb22;
116+
// }
117+
// bb22: {
118+
// StorageDead(_2);
119+
// goto -> bb23;
120+
// }
121+
// bb23: {
122+
// goto -> bb24;
123+
// }
124+
// bb24: {
125+
// goto -> bb25;
126+
// }
127+
// bb25: {
128+
// goto -> bb2;
129+
// }
130+
// bb26: {
131+
// _5 = ();
132+
// unreachable;
133+
// }
134+
// bb27: {
135+
// StorageDead(_5);
136+
// goto -> bb14;
137+
// }
138+
// bb28: {
139+
// StorageDead(_7);
140+
// _1 = ();
141+
// StorageDead(_2);
142+
// goto -> bb1;
143+
// }
144+
// bb29: {
145+
// return;
146+
// }
147+
// }
148+
// END rustc.main.mir_map.0.mir

‎src/test/mir-opt/storage_ranges.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ fn main() {
3131
// StorageDead(_5);
3232
// _3 = &_4;
3333
// _2 = ();
34-
// StorageDead(_3);
3534
// StorageDead(_4);
35+
// StorageDead(_3);
3636
// StorageLive(_6);
3737
// _6 = const 1i32;
3838
// _0 = ();

0 commit comments

Comments
 (0)