Skip to content

Commit 91fe22d

Browse files
borsbors
committed
Auto merge of #157346 - dianqk:gvn-nest-ref, r=cjgillot
 GVN: Don't assume nested shared references are read-only. Fixes #155884. #150485 only checks the type, whether it is a reference, which is not enough. The PR extends to other types. cc @RalfJung @saethlin r? @cjgillot
2 parents d29dae8 + 0abc695 commit 91fe22d

15 files changed

Lines changed: 796 additions & 15 deletions

‎compiler/rustc_mir_transform/src/gvn.rs‎

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ use rustc_middle::mir::interpret::{AllocRange, GlobalAlloc};
116116
use rustc_middle::mir::visit::*;
117117
use rustc_middle::mir::*;
118118
use rustc_middle::ty::layout::HasTypingEnv;
119-
use rustc_middle::ty::{self, Ty, TyCtxt, Unnormalized};
119+
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt, Unnormalized};
120120
use rustc_mir_dataflow::{Analysis, ResultsCursor};
121121
use rustc_span::DUMMY_SP;
122122
use smallvec::SmallVec;
@@ -834,16 +834,20 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
834834
{
835835
return Some((projection_ty, value));
836836
}
837-
// DO NOT reason the pointer value.
838-
// We cannot unify two pointers that dereference same local, because they may
839-
// have different lifetimes.
837+
// We cannot unify two references produced by dereferencing the same nested reference,
838+
// because they may have different lifetimes.
840839
// ```
841840
// let b: &T = *a;
842841
// ... `a` is allowed to be modified. `c` and `b` have different borrowing lifetime.
843842
// Unifying them will extend the lifetime of `b`.
844843
// let c: &T = *a;
845844
// ```
846-
if projection_ty.ty.is_ref() {
845+
// Furthermore, unifying them can also violate Stacked Borrows or Tree Borrows.
846+
// We can only unify all `*b` and `*c` separately
847+
// because nested shared references are not read-only.
848+
// For more, see <https://github.com/rust-lang/rust/issues/155884> and
849+
// <https://github.com/rust-lang/rust/issues/130853>.
850+
if self.ty_may_have_ref(projection_ty.ty) {
847851
return None;
848852
}
849853

@@ -1688,6 +1692,59 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
16881692
}
16891693
}
16901694

1695+
fn ty_may_have_ref(&self, ty: Ty<'tcx>) -> bool {
1696+
fn ty_may_have_ref_inner<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, depth: usize) -> bool {
1697+
if !tcx.recursion_limit().value_within_limit(depth) {
1698+
return true;
1699+
}
1700+
let depth = depth + 1;
1701+
match ty.kind() {
1702+
ty::Int(_)
1703+
| ty::Uint(_)
1704+
| ty::Float(_)
1705+
| ty::Bool
1706+
| ty::Char
1707+
| ty::Str
1708+
| ty::Never
1709+
| ty::FnDef(..)
1710+
| ty::Error(_)
1711+
| ty::FnPtr(..) => false,
1712+
ty::Tuple(fields) => {
1713+
fields.iter().any(|field| ty_may_have_ref_inner(tcx, field, depth))
1714+
}
1715+
ty::Pat(ty, _) | ty::Slice(ty) | ty::Array(ty, _) => {
1716+
ty_may_have_ref_inner(tcx, *ty, depth)
1717+
}
1718+
ty::Adt(adt_def, args) => {
1719+
adt_def.has_param()
1720+
|| adt_def.has_aliases()
1721+
|| adt_def.all_fields().any(|field| {
1722+
ty_may_have_ref_inner(
1723+
tcx,
1724+
field.ty(tcx, args).skip_normalization(),
1725+
depth,
1726+
)
1727+
})
1728+
}
1729+
ty::Ref(..)
1730+
| ty::RawPtr(_, _)
1731+
| ty::Bound(..)
1732+
| ty::Closure(..)
1733+
| ty::CoroutineClosure(..)
1734+
| ty::Dynamic(..)
1735+
| ty::Foreign(_)
1736+
| ty::Coroutine(..)
1737+
| ty::CoroutineWitness(..)
1738+
| ty::UnsafeBinder(_)
1739+
| ty::Infer(_)
1740+
| ty::Alias(..)
1741+
| ty::Param(_)
1742+
| ty::Placeholder(_) => true,
1743+
}
1744+
}
1745+
ty_may_have_ref_inner(self.tcx, ty, 0)
1746+
}
1747+
16911748
/// Returns `false` if we're confident that the middle type doesn't have an
16921749
/// interesting niche so we can skip that step when transmuting.
16931750
///

‎tests/mir-opt/const_prop/indirect_ref.indirect_adt.GVN.diff‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
- // MIR for `indirect_adt` before GVN
2+
+ // MIR for `indirect_adt` after GVN
3+
4+
fn indirect_adt(_1: &Adt<'_>, _2: for<'a> fn(Adt<'a>), _3: fn()) -> () {
5+
debug x => _1;
6+
debug out => _2;
7+
debug clobber => _3;
8+
let mut _0: ();
9+
let _4: Adt<'_>;
10+
let _5: ();
11+
let mut _6: for<'a> fn(Adt<'a>);
12+
let mut _7: Adt<'_>;
13+
let _8: ();
14+
let mut _9: fn();
15+
let _11: ();
16+
let mut _12: for<'a> fn(Adt<'a>);
17+
let mut _13: Adt<'_>;
18+
scope 1 {
19+
debug y1 => _4;
20+
let _10: Adt<'_>;
21+
scope 2 {
22+
debug y2 => _10;
23+
}
24+
}
25+
26+
bb0: {
27+
StorageLive(_4);
28+
_4 = copy (*_1);
29+
StorageLive(_5);
30+
StorageLive(_6);
31+
_6 = copy _2;
32+
StorageLive(_7);
33+
_7 = copy _4;
34+
- _5 = move _6(move _7) -> [return: bb1, unwind unreachable];
35+
+ _5 = copy _2(copy _4) -> [return: bb1, unwind unreachable];
36+
}
37+
38+
bb1: {
39+
StorageDead(_7);
40+
StorageDead(_6);
41+
StorageDead(_5);
42+
StorageLive(_8);
43+
StorageLive(_9);
44+
_9 = copy _3;
45+
- _8 = move _9() -> [return: bb2, unwind unreachable];
46+
+ _8 = copy _3() -> [return: bb2, unwind unreachable];
47+
}
48+
49+
bb2: {
50+
StorageDead(_9);
51+
StorageDead(_8);
52+
StorageLive(_10);
53+
_10 = copy (*_1);
54+
StorageLive(_11);
55+
StorageLive(_12);
56+
_12 = copy _2;
57+
StorageLive(_13);
58+
_13 = copy _10;
59+
- _11 = move _12(move _13) -> [return: bb3, unwind unreachable];
60+
+ _11 = copy _2(copy _10) -> [return: bb3, unwind unreachable];
61+
}
62+
63+
bb3: {
64+
StorageDead(_13);
65+
StorageDead(_12);
66+
StorageDead(_11);
67+
_0 = const ();
68+
StorageDead(_10);
69+
StorageDead(_4);
70+
return;
71+
}
72+
}
73+

‎tests/mir-opt/const_prop/indirect_ref.indirect_deref_1.GVN.diff‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
- // MIR for `indirect_deref_1` before GVN
2+
+ // MIR for `indirect_deref_1` after GVN
3+
4+
fn indirect_deref_1(_1: &(&i32,), _2: fn(i32), _3: fn()) -> () {
5+
debug x => _1;
6+
debug out => _2;
7+
debug clobber => _3;
8+
let mut _0: ();
9+
let _4: (&i32,);
10+
let _5: ();
11+
let mut _6: fn(i32);
12+
let mut _7: i32;
13+
let _8: ();
14+
let mut _9: fn();
15+
let _11: ();
16+
let mut _12: fn(i32);
17+
let mut _13: i32;
18+
let mut _14: &i32;
19+
let mut _15: &i32;
20+
scope 1 {
21+
debug y1 => _4;
22+
let _10: (&i32,);
23+
scope 2 {
24+
debug y2 => _10;
25+
}
26+
}
27+
28+
bb0: {
29+
StorageLive(_4);
30+
_4 = copy (*_1);
31+
StorageLive(_5);
32+
StorageLive(_6);
33+
_6 = copy _2;
34+
StorageLive(_7);
35+
_14 = no_retag copy (_4.0: &i32);
36+
_7 = copy (*_14);
37+
- _5 = move _6(move _7) -> [return: bb1, unwind unreachable];
38+
+ _5 = copy _2(move _7) -> [return: bb1, unwind unreachable];
39+
}
40+
41+
bb1: {
42+
StorageDead(_7);
43+
StorageDead(_6);
44+
StorageDead(_5);
45+
StorageLive(_8);
46+
StorageLive(_9);
47+
_9 = copy _3;
48+
- _8 = move _9() -> [return: bb2, unwind unreachable];
49+
+ _8 = copy _3() -> [return: bb2, unwind unreachable];
50+
}
51+
52+
bb2: {
53+
StorageDead(_9);
54+
StorageDead(_8);
55+
StorageLive(_10);
56+
_10 = copy (*_1);
57+
StorageLive(_11);
58+
StorageLive(_12);
59+
_12 = copy _2;
60+
StorageLive(_13);
61+
_15 = no_retag copy (_10.0: &i32);
62+
_13 = copy (*_15);
63+
- _11 = move _12(move _13) -> [return: bb3, unwind unreachable];
64+
+ _11 = copy _2(move _13) -> [return: bb3, unwind unreachable];
65+
}
66+
67+
bb3: {
68+
StorageDead(_13);
69+
StorageDead(_12);
70+
StorageDead(_11);
71+
_0 = const ();
72+
StorageDead(_10);
73+
StorageDead(_4);
74+
return;
75+
}
76+
}
77+

‎tests/mir-opt/const_prop/indirect_ref.indirect_deref_2.GVN.diff‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
- // MIR for `indirect_deref_2` before GVN
2+
+ // MIR for `indirect_deref_2` after GVN
3+
4+
fn indirect_deref_2(_1: &(&i32,), _2: fn(i32), _3: fn()) -> () {
5+
debug x => _1;
6+
debug out => _2;
7+
debug clobber => _3;
8+
let mut _0: ();
9+
let _4: ();
10+
let mut _5: fn(i32);
11+
let mut _6: i32;
12+
let _7: ();
13+
let mut _8: fn();
14+
let _9: ();
15+
let mut _10: fn(i32);
16+
let mut _11: i32;
17+
let mut _12: &i32;
18+
let mut _13: &i32;
19+
20+
bb0: {
21+
StorageLive(_4);
22+
StorageLive(_5);
23+
_5 = copy _2;
24+
StorageLive(_6);
25+
_12 = no_retag copy ((*_1).0: &i32);
26+
_6 = copy (*_12);
27+
- _4 = move _5(move _6) -> [return: bb1, unwind unreachable];
28+
+ _4 = copy _2(move _6) -> [return: bb1, unwind unreachable];
29+
}
30+
31+
bb1: {
32+
StorageDead(_6);
33+
StorageDead(_5);
34+
StorageDead(_4);
35+
StorageLive(_7);
36+
StorageLive(_8);
37+
_8 = copy _3;
38+
- _7 = move _8() -> [return: bb2, unwind unreachable];
39+
+ _7 = copy _3() -> [return: bb2, unwind unreachable];
40+
}
41+
42+
bb2: {
43+
StorageDead(_8);
44+
StorageDead(_7);
45+
StorageLive(_9);
46+
StorageLive(_10);
47+
_10 = copy _2;
48+
StorageLive(_11);
49+
_13 = no_retag copy ((*_1).0: &i32);
50+
_11 = copy (*_13);
51+
- _9 = move _10(move _11) -> [return: bb3, unwind unreachable];
52+
+ _9 = copy _2(move _11) -> [return: bb3, unwind unreachable];
53+
}
54+
55+
bb3: {
56+
StorageDead(_11);
57+
StorageDead(_10);
58+
StorageDead(_9);
59+
_0 = const ();
60+
return;
61+
}
62+
}
63+

‎tests/mir-opt/const_prop/indirect_ref.indirect_ref_1.GVN.diff‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
- // MIR for `indirect_ref_1` before GVN
2+
+ // MIR for `indirect_ref_1` after GVN
3+
4+
fn indirect_ref_1(_1: &(&i32,), _2: for<'a> fn(&'a i32), _3: fn()) -> () {
5+
debug x => _1;
6+
debug out => _2;
7+
debug clobber => _3;
8+
let mut _0: ();
9+
let _4: ();
10+
let mut _5: for<'a> fn(&'a i32);
11+
let mut _6: &i32;
12+
let _7: ();
13+
let mut _8: fn();
14+
let _9: ();
15+
let mut _10: for<'a> fn(&'a i32);
16+
let mut _11: &i32;
17+
let mut _12: &i32;
18+
let mut _13: &i32;
19+
20+
bb0: {
21+
StorageLive(_4);
22+
StorageLive(_5);
23+
_5 = copy _2;
24+
StorageLive(_6);
25+
_12 = no_retag copy ((*_1).0: &i32);
26+
_6 = &(*_12);
27+
- _4 = move _5(move _6) -> [return: bb1, unwind unreachable];
28+
+ _4 = copy _2(move _6) -> [return: bb1, unwind unreachable];
29+
}
30+
31+
bb1: {
32+
StorageDead(_6);
33+
StorageDead(_5);
34+
StorageDead(_4);
35+
StorageLive(_7);
36+
StorageLive(_8);
37+
_8 = copy _3;
38+
- _7 = move _8() -> [return: bb2, unwind unreachable];
39+
+ _7 = copy _3() -> [return: bb2, unwind unreachable];
40+
}
41+
42+
bb2: {
43+
StorageDead(_8);
44+
StorageDead(_7);
45+
StorageLive(_9);
46+
StorageLive(_10);
47+
_10 = copy _2;
48+
StorageLive(_11);
49+
_13 = no_retag copy ((*_1).0: &i32);
50+
_11 = &(*_13);
51+
- _9 = move _10(move _11) -> [return: bb3, unwind unreachable];
52+
+ _9 = copy _2(move _11) -> [return: bb3, unwind unreachable];
53+
}
54+
55+
bb3: {
56+
StorageDead(_11);
57+
StorageDead(_10);
58+
StorageDead(_9);
59+
_0 = const ();
60+
return;
61+
}
62+
}
63+

0 commit comments

Comments
 (0)