From 6eeedbcd70855b45cf41ef14bc91a15f0ee67c7c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 20 Nov 2018 15:08:26 +0100 Subject: [PATCH 1/3] generator fields are not necessarily initialized --- src/librustc_mir/interpret/visitor.rs | 33 ++++++++++++++++++--------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index f0a71242599bf..505195763d062 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -142,6 +142,7 @@ macro_rules! make_value_visitor { self.walk_value(v) } /// Visit the given value as a union. No automatic recursion can happen here. + /// Also called for the fields of a generator, which may or may not be initialized. #[inline(always)] fn visit_union(&mut self, _v: Self::V) -> EvalResult<'tcx> { @@ -291,17 +292,28 @@ macro_rules! make_value_visitor { // use that as an unambiguous signal for detecting primitives. Make sure // we did not miss any primitive. debug_assert!(fields > 0); - self.visit_union(v)?; + self.visit_union(v) }, layout::FieldPlacement::Arbitrary { ref offsets, .. } => { - // FIXME: We collect in a vec because otherwise there are lifetime errors: - // Projecting to a field needs (mutable!) access to `ecx`. - let fields: Vec> = - (0..offsets.len()).map(|i| { - v.project_field(self.ecx(), i as u64) - }) - .collect(); - self.visit_aggregate(v, fields.into_iter())?; + // Special handling needed for generators: All but the first field + // (which is the state) are actually implicitly `MaybeUninit`, i.e., + // they may or may not be initialized, so we cannot visit them. + match v.layout().ty.sty { + ty::Generator(..) => { + let field = v.project_field(self.ecx(), 0)?; + self.visit_aggregate(v, std::iter::once(Ok(field))) + } + _ => { + // FIXME: We collect in a vec because otherwise there are lifetime + // errors: Projecting to a field needs access to `ecx`. + let fields: Vec> = + (0..offsets.len()).map(|i| { + v.project_field(self.ecx(), i as u64) + }) + .collect(); + self.visit_aggregate(v, fields.into_iter()) + } + } }, layout::FieldPlacement::Array { .. } => { // Let's get an mplace first. @@ -317,10 +329,9 @@ macro_rules! make_value_visitor { .map(|f| f.and_then(|f| { Ok(Value::from_mem_place(f)) })); - self.visit_aggregate(v, iter)?; + self.visit_aggregate(v, iter) } } - Ok(()) } } } From 7f1077700c019b2dc8d651528ecad106c9267858 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 20 Nov 2018 15:55:23 +0100 Subject: [PATCH 2/3] fix comment --- src/librustc_mir/interpret/visitor.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 505195763d062..a645e06732269 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -142,7 +142,6 @@ macro_rules! make_value_visitor { self.walk_value(v) } /// Visit the given value as a union. No automatic recursion can happen here. - /// Also called for the fields of a generator, which may or may not be initialized. #[inline(always)] fn visit_union(&mut self, _v: Self::V) -> EvalResult<'tcx> { From 6befe6784f37d953884ef05982b41c3a11932170 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 20 Nov 2018 16:19:24 +0100 Subject: [PATCH 3/3] treat generator fields like unions --- src/librustc_mir/interpret/visitor.rs | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index a645e06732269..81e56f3115d26 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -158,7 +158,9 @@ macro_rules! make_value_visitor { ) -> EvalResult<'tcx> { self.walk_aggregate(v, fields) } - /// Called each time we recurse down to a field, passing in old and new value. + + /// Called each time we recurse down to a field of a "product-like" aggregate + /// (structs, tuples, arrays and the like, but not enums), passing in old and new value. /// This gives the visitor the chance to track the stack of nested fields that /// we are descending through. #[inline(always)] @@ -171,6 +173,19 @@ macro_rules! make_value_visitor { self.visit_value(new_val) } + /// Called for recursing into the field of a generator. These are not known to be + /// initialized, so we treat them like unions. + #[inline(always)] + fn visit_generator_field( + &mut self, + _old_val: Self::V, + _field: usize, + new_val: Self::V, + ) -> EvalResult<'tcx> { + self.visit_union(new_val) + } + + /// Called when recursing into an enum variant. #[inline(always)] fn visit_variant( &mut self, @@ -300,7 +315,12 @@ macro_rules! make_value_visitor { match v.layout().ty.sty { ty::Generator(..) => { let field = v.project_field(self.ecx(), 0)?; - self.visit_aggregate(v, std::iter::once(Ok(field))) + self.visit_aggregate(v, std::iter::once(Ok(field)))?; + for i in 1..offsets.len() { + let field = v.project_field(self.ecx(), i as u64)?; + self.visit_generator_field(v, i, field)?; + } + Ok(()) } _ => { // FIXME: We collect in a vec because otherwise there are lifetime