From d15ca96eda708de1c4d2c02bb5014d8e4fe7ca18 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 6 Feb 2016 04:28:20 +0100 Subject: [PATCH 1/2] Split dummy in region inference graph into distinct source and sink nodes. Why do this: The RegionGraph representation previously conflated all of the non-variable regions (i.e. the concrete regions such as lifetime parameters to the current function) into a single dummy node. A single dummy node leads DFS on a graph `'a -> '_#1 -> '_#0 -> 'b` to claim that `'_#1` is reachable from `'_#0` (due to `'a` and `'b` being conflated in the graph representation), which is incorrect (and can lead to soundness bugs later on in compilation, see #30438). Splitting the dummy node ensures that DFS will never introduce new ancestor relationships between nodes for variable regions in the graph. --- src/librustc/middle/infer/region_inference/mod.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/librustc/middle/infer/region_inference/mod.rs b/src/librustc/middle/infer/region_inference/mod.rs index 2c2b69ff85b4e..bfc770d53aaba 100644 --- a/src/librustc/middle/infer/region_inference/mod.rs +++ b/src/librustc/middle/infer/region_inference/mod.rs @@ -1105,7 +1105,14 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> { for _ in 0..num_vars { graph.add_node(()); } - let dummy_idx = graph.add_node(()); + + // Issue #30438: two distinct dummy nodes, one for incoming + // edges (dummy_source) and another for outgoing edges + // (dummy_sink). In `dummy -> a -> b -> dummy`, using one + // dummy node leads one to think (erroneously) there exists a + // path from `b` to `a`. Two dummy nodes sidesteps the issue. + let dummy_source = graph.add_node(()); + let dummy_sink = graph.add_node(()); for (constraint, _) in constraints.iter() { match *constraint { @@ -1115,10 +1122,10 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> { *constraint); } ConstrainRegSubVar(_, b_id) => { - graph.add_edge(dummy_idx, NodeIndex(b_id.index as usize), *constraint); + graph.add_edge(dummy_source, NodeIndex(b_id.index as usize), *constraint); } ConstrainVarSubReg(a_id, _) => { - graph.add_edge(NodeIndex(a_id.index as usize), dummy_idx, *constraint); + graph.add_edge(NodeIndex(a_id.index as usize), dummy_sink, *constraint); } } } From c136be69e2e4920cd538d4c65d72064cf66353f2 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 6 Feb 2016 04:37:21 +0100 Subject: [PATCH 2/2] regression tests for issue #30438. Fix #30438. --- src/test/compile-fail/issue-30438-a.rs | 33 +++++++++++++++++++++++++ src/test/compile-fail/issue-30438-b.rs | 34 ++++++++++++++++++++++++++ src/test/compile-fail/issue-30438-c.rs | 30 +++++++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 src/test/compile-fail/issue-30438-a.rs create mode 100644 src/test/compile-fail/issue-30438-b.rs create mode 100644 src/test/compile-fail/issue-30438-c.rs diff --git a/src/test/compile-fail/issue-30438-a.rs b/src/test/compile-fail/issue-30438-a.rs new file mode 100644 index 0000000000000..441815de81d30 --- /dev/null +++ b/src/test/compile-fail/issue-30438-a.rs @@ -0,0 +1,33 @@ +// Copyright 2016 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. + +// Original regression test for Issue #30438. + +use std::ops::Index; + +struct Test<'a> { + s: &'a String +} + +impl <'a> Index for Test<'a> { + type Output = Test<'a>; + fn index(&self, _: usize) -> &Self::Output { + return &Test { s: &self.s}; + //~^ ERROR: borrowed value does not live long enough + } +} + +fn main() { + let s = "Hello World".to_string(); + let test = Test{s: &s}; + let r = &test[0]; + println!("{}", test.s); // OK since test is valid + println!("{}", r.s); // Segfault since value pointed by r has already been dropped +} diff --git a/src/test/compile-fail/issue-30438-b.rs b/src/test/compile-fail/issue-30438-b.rs new file mode 100644 index 0000000000000..981b196c4aeb2 --- /dev/null +++ b/src/test/compile-fail/issue-30438-b.rs @@ -0,0 +1,34 @@ +// Copyright 2016 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. + +// Modified regression test for Issue #30438 that exposed an +// independent issue (see discussion on ticket). + +use std::ops::Index; + +struct Test<'a> { + s: &'a String +} + +impl <'a> Index for Test<'a> { + type Output = Test<'a>; + fn index(&self, _: usize) -> &Self::Output { + &Test { s: &self.s} + //~^ ERROR: borrowed value does not live long enough + } +} + +fn main() { + let s = "Hello World".to_string(); + let test = Test{s: &s}; + let r = &test[0]; + println!("{}", test.s); // OK since test is valid + println!("{}", r.s); // Segfault since value pointed by r has already been dropped +} diff --git a/src/test/compile-fail/issue-30438-c.rs b/src/test/compile-fail/issue-30438-c.rs new file mode 100644 index 0000000000000..06d391af559cc --- /dev/null +++ b/src/test/compile-fail/issue-30438-c.rs @@ -0,0 +1,30 @@ +// Copyright 2016 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. + +// Simplfied regression test for #30438, inspired by arielb1. + +trait Trait { type Out; } + +struct Test<'a> { s: &'a str } + +fn silly<'y, 'z>(_s: &'y Test<'z>) -> &'y as Trait>::Out where 'z: 'static { + let x = Test { s: "this cannot last" }; + &x + //~^ ERROR: `x` does not live long enough +} + +impl<'b> Trait for Test<'b> { type Out = Test<'b>; } + +fn main() { + let orig = Test { s: "Hello World" }; + let r = silly(&orig); + println!("{}", orig.s); // OK since `orig` is valid + println!("{}", r.s); // Segfault (method does not return a sane value) +}