From 558c8a8c3c7e7cdf83703a044f085e89d71680e8 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 26 Jun 2020 11:30:14 -0700 Subject: [PATCH 1/3] Handle stores to projections correctly in liveness analysis Previously, we were too conservative and `x.field = 4` was treated as a "use" of `x`. --- src/librustc_mir/dataflow/impls/liveness.rs | 27 +++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/liveness.rs b/src/librustc_mir/dataflow/impls/liveness.rs index d24faacd3779e..784b0bd9293e2 100644 --- a/src/librustc_mir/dataflow/impls/liveness.rs +++ b/src/librustc_mir/dataflow/impls/liveness.rs @@ -92,7 +92,27 @@ impl<'tcx, T> Visitor<'tcx> for TransferFunction<'_, T> where T: GenKill, { + fn visit_place(&mut self, place: &mir::Place<'tcx>, context: PlaceContext, location: Location) { + let mir::Place { projection, local } = *place; + + // We purposefully do not call `super_place` here to avoid calling `visit_local` for this + // place with one of the `Projection` variants of `PlaceContext`. + self.visit_projection(local, projection, context, location); + + match DefUse::for_place(context) { + // Treat derefs as a use of the base local. `*p = 4` is not a def of `p` but a use. + Some(_) if place.is_indirect() => self.0.gen(local), + + Some(DefUse::Def) if projection.is_empty() => self.0.kill(local), + Some(DefUse::Use) => self.0.gen(local), + _ => {} + } + } + fn visit_local(&mut self, &local: &Local, context: PlaceContext, _: Location) { + // Because we do not call `super_place` above, `visit_local` is only called for locals that + // do not appear as part of a `Place` in the MIR. This handles cases like the implicit use + // of the return place in a `Return` terminator or the index in an `Index` projection. match DefUse::for_place(context) { Some(DefUse::Def) => self.0.kill(local), Some(DefUse::Use) => self.0.gen(local), @@ -126,7 +146,6 @@ impl DefUse { | MutatingUseContext::AsmOutput | MutatingUseContext::Borrow | MutatingUseContext::Drop - | MutatingUseContext::Projection | MutatingUseContext::Retag, ) | PlaceContext::NonMutatingUse( @@ -134,11 +153,15 @@ impl DefUse { | NonMutatingUseContext::Copy | NonMutatingUseContext::Inspect | NonMutatingUseContext::Move - | NonMutatingUseContext::Projection | NonMutatingUseContext::ShallowBorrow | NonMutatingUseContext::SharedBorrow | NonMutatingUseContext::UniqueBorrow, ) => Some(DefUse::Use), + + PlaceContext::MutatingUse(MutatingUseContext::Projection) + | PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection) => { + unreachable!("A projection could be a def or a use and must be handled separately") + } } } } From 234019758bfff600e8972481158882d9e12280cf Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 26 Jun 2020 11:31:48 -0700 Subject: [PATCH 2/3] Add peek test for projections --- .../ui/mir-dataflow/liveness-projection.rs | 32 +++++++++++++++++++ .../mir-dataflow/liveness-projection.stderr | 16 ++++++++++ 2 files changed, 48 insertions(+) create mode 100644 src/test/ui/mir-dataflow/liveness-projection.rs create mode 100644 src/test/ui/mir-dataflow/liveness-projection.stderr diff --git a/src/test/ui/mir-dataflow/liveness-projection.rs b/src/test/ui/mir-dataflow/liveness-projection.rs new file mode 100644 index 0000000000000..ed571027dbd84 --- /dev/null +++ b/src/test/ui/mir-dataflow/liveness-projection.rs @@ -0,0 +1,32 @@ +#![feature(core_intrinsics, rustc_attrs)] + +use std::intrinsics::rustc_peek; + +#[rustc_mir(rustc_peek_liveness, stop_after_dataflow)] +fn foo() { + { + let mut x: (i32, i32) = (42, 0); + + // Assignment to a projection does not cause `x` to become live + unsafe { rustc_peek(x); } //~ ERROR bit not set + x.1 = 42; + + x = (0, 42); + + // ...but a read from a projection does. + unsafe { rustc_peek(x); } + println!("{}", x.1); + } + + { + let mut x = 42; + + // Derefs are treated like a read of a local even if they are on the RHS of an assignment. + let p = &mut x; + unsafe { rustc_peek(&p); } + *p = 24; + unsafe { rustc_peek(&p); } //~ ERROR bit not set + } +} + +fn main() {} diff --git a/src/test/ui/mir-dataflow/liveness-projection.stderr b/src/test/ui/mir-dataflow/liveness-projection.stderr new file mode 100644 index 0000000000000..f9480c880908a --- /dev/null +++ b/src/test/ui/mir-dataflow/liveness-projection.stderr @@ -0,0 +1,16 @@ +error: rustc_peek: bit not set + --> $DIR/liveness-projection.rs:11:18 + | +LL | unsafe { rustc_peek(x); } + | ^^^^^^^^^^^^^ + +error: rustc_peek: bit not set + --> $DIR/liveness-projection.rs:28:18 + | +LL | unsafe { rustc_peek(&p); } + | ^^^^^^^^^^^^^^ + +error: stop_after_dataflow ended compilation + +error: aborting due to 3 previous errors + From ffcfaa11055a2ec65d326003da41da24d7a7d66c Mon Sep 17 00:00:00 2001 From: ecstatic-morse Date: Sat, 27 Jun 2020 12:57:00 -0700 Subject: [PATCH 3/3] Fix comment. --- src/test/ui/mir-dataflow/liveness-projection.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/mir-dataflow/liveness-projection.rs b/src/test/ui/mir-dataflow/liveness-projection.rs index ed571027dbd84..486f31b635dca 100644 --- a/src/test/ui/mir-dataflow/liveness-projection.rs +++ b/src/test/ui/mir-dataflow/liveness-projection.rs @@ -21,7 +21,7 @@ fn foo() { { let mut x = 42; - // Derefs are treated like a read of a local even if they are on the RHS of an assignment. + // Derefs are treated like a read of a local even if they are on the LHS of an assignment. let p = &mut x; unsafe { rustc_peek(&p); } *p = 24;