Skip to content

Commit

Permalink
Auto merge of #133474 - RalfJung:gvn-miscompile, r=compiler-errors
Browse files Browse the repository at this point in the history
Do not unify dereferences of shared borrows in GVN

Repost of #132461, the last commit applies my suggestions.

Fixes #130853
  • Loading branch information
bors committed Nov 27, 2024
2 parents c322cd5 + 906f66f commit 6b6a867
Show file tree
Hide file tree
Showing 33 changed files with 614 additions and 482 deletions.
4 changes: 3 additions & 1 deletion compiler/rustc_mir_transform/src/gvn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,9 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
let proj = match proj {
ProjectionElem::Deref => {
let ty = place.ty(self.local_decls, self.tcx).ty;
if let Some(Mutability::Not) = ty.ref_mutability()
// unsound: https://github.com/rust-lang/rust/issues/130853
if self.tcx.sess.opts.unstable_opts.unsound_mir_opts
&& let Some(Mutability::Not) = ty.ref_mutability()
&& let Some(pointee_ty) = ty.builtin_deref(true)
&& pointee_ty.is_freeze(self.tcx, self.typing_env())
{
Expand Down
18 changes: 10 additions & 8 deletions tests/coverage/closure.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -140,17 +140,19 @@ Number of file 0 mappings: 6
- Code(Counter(0)) at (prev + 2, 9) to (start + 0, 10)
Highest counter ID seen: c1

Function name: closure::main::{closure#18} (unused)
Raw bytes (24): 0x[01, 01, 00, 04, 00, 19, 0d, 02, 1c, 00, 02, 1d, 02, 12, 00, 02, 11, 00, 12, 00, 01, 11, 01, 0e]
Function name: closure::main::{closure#18}
Raw bytes (26): 0x[01, 01, 01, 01, 05, 04, 01, 19, 0d, 02, 1c, 05, 02, 1d, 02, 12, 02, 02, 11, 00, 12, 01, 01, 11, 01, 0e]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of expressions: 1
- expression 0 operands: lhs = Counter(0), rhs = Counter(1)
Number of file 0 mappings: 4
- Code(Zero) at (prev + 25, 13) to (start + 2, 28)
- Code(Zero) at (prev + 2, 29) to (start + 2, 18)
- Code(Zero) at (prev + 2, 17) to (start + 0, 18)
- Code(Zero) at (prev + 1, 17) to (start + 1, 14)
Highest counter ID seen: (none)
- Code(Counter(0)) at (prev + 25, 13) to (start + 2, 28)
- Code(Counter(1)) at (prev + 2, 29) to (start + 2, 18)
- Code(Expression(0, Sub)) at (prev + 2, 17) to (start + 0, 18)
= (c0 - c1)
- Code(Counter(0)) at (prev + 1, 17) to (start + 1, 14)
Highest counter ID seen: c1

Function name: closure::main::{closure#19}
Raw bytes (26): 0x[01, 01, 01, 01, 05, 04, 01, 43, 0d, 02, 1c, 05, 02, 1d, 02, 12, 02, 02, 11, 00, 12, 01, 01, 11, 01, 0e]
Expand Down
205 changes: 110 additions & 95 deletions tests/coverage/issue-84561.cov-map

Large diffs are not rendered by default.

14 changes: 5 additions & 9 deletions tests/mir-opt/const_prop/read_immutable_static.main.GVN.diff
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,19 @@

bb0: {
StorageLive(_1);
- StorageLive(_2);
StorageLive(_2);
- StorageLive(_3);
+ nop;
+ nop;
_3 = const {ALLOC0: &u8};
- _2 = copy (*_3);
+ _2 = const 2_u8;
_2 = copy (*_3);
StorageLive(_4);
StorageLive(_5);
_5 = const {ALLOC0: &u8};
- _4 = copy (*_5);
- _1 = Add(move _2, move _4);
+ _4 = const 2_u8;
+ _1 = const 4_u8;
+ _4 = copy (*_3);
_1 = Add(move _2, move _4);
StorageDead(_4);
- StorageDead(_2);
+ nop;
StorageDead(_2);
StorageDead(_5);
- StorageDead(_3);
+ nop;
Expand Down
3 changes: 2 additions & 1 deletion tests/mir-opt/const_prop/read_immutable_static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ static FOO: u8 = 2;
fn main() {
// CHECK-LABEL: fn main(
// CHECK: debug x => [[x:_.*]];
// CHECK: [[x]] = const 4_u8;
// Disabled due to <https://github.com/rust-lang/rust/issues/130853>
// COM: CHECK: [[x]] = const 4_u8;
let x = FOO + FOO;
}
3 changes: 1 addition & 2 deletions tests/mir-opt/const_prop/ref_deref.main.GVN.diff
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@
StorageLive(_2);
_4 = const main::promoted[0];
_2 = &(*_4);
- _1 = copy (*_2);
+ _1 = const 4_i32;
_1 = copy (*_2);
StorageDead(_2);
_0 = const ();
StorageDead(_1);
Expand Down
3 changes: 1 addition & 2 deletions tests/mir-opt/const_prop/ref_deref_project.main.GVN.diff
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@
StorageLive(_2);
_4 = const main::promoted[0];
_2 = &((*_4).1: i32);
- _1 = copy (*_2);
+ _1 = const 5_i32;
_1 = copy (*_2);
StorageDead(_2);
_0 = const ();
StorageDead(_1);
Expand Down
3 changes: 2 additions & 1 deletion tests/mir-opt/const_prop/ref_deref_project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
fn main() {
// CHECK-LABEL: fn main(
// CHECK: debug a => [[a:_.*]];
// CHECK: [[a]] = const 5_i32;
// Disabled due to <https://github.com/rust-lang/rust/issues/130853>
// COM: CHECK: [[a]] = const 5_i32;
let a = *(&(4, 5).1);
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,16 @@
StorageDead(_3);
StorageLive(_6);
_6 = const 1_usize;
- _7 = Len((*_2));
_7 = Len((*_2));
- _8 = Lt(copy _6, copy _7);
- assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, copy _6) -> [success: bb1, unwind unreachable];
+ _7 = const 3_usize;
+ _8 = const true;
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 1_usize) -> [success: bb1, unwind unreachable];
+ _8 = Lt(const 1_usize, copy _7);
+ assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 1_usize) -> [success: bb1, unwind unreachable];
}

bb1: {
- _1 = copy (*_2)[_6];
+ _1 = const 2_u32;
+ _1 = copy (*_2)[1 of 2];
StorageDead(_6);
StorageDead(_4);
StorageDead(_2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,16 @@
StorageDead(_3);
StorageLive(_6);
_6 = const 1_usize;
- _7 = Len((*_2));
_7 = Len((*_2));
- _8 = Lt(copy _6, copy _7);
- assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, copy _6) -> [success: bb1, unwind continue];
+ _7 = const 3_usize;
+ _8 = const true;
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 1_usize) -> [success: bb1, unwind continue];
+ _8 = Lt(const 1_usize, copy _7);
+ assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 1_usize) -> [success: bb1, unwind continue];
}

bb1: {
- _1 = copy (*_2)[_6];
+ _1 = const 2_u32;
+ _1 = copy (*_2)[1 of 2];
StorageDead(_6);
StorageDead(_4);
StorageDead(_2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,16 @@
StorageDead(_3);
StorageLive(_6);
_6 = const 1_usize;
- _7 = Len((*_2));
_7 = Len((*_2));
- _8 = Lt(copy _6, copy _7);
- assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, copy _6) -> [success: bb1, unwind unreachable];
+ _7 = const 3_usize;
+ _8 = const true;
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 1_usize) -> [success: bb1, unwind unreachable];
+ _8 = Lt(const 1_usize, copy _7);
+ assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 1_usize) -> [success: bb1, unwind unreachable];
}

bb1: {
- _1 = copy (*_2)[_6];
+ _1 = const 2_u32;
+ _1 = copy (*_2)[1 of 2];
StorageDead(_6);
StorageDead(_4);
StorageDead(_2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,16 @@
StorageDead(_3);
StorageLive(_6);
_6 = const 1_usize;
- _7 = Len((*_2));
_7 = Len((*_2));
- _8 = Lt(copy _6, copy _7);
- assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, copy _6) -> [success: bb1, unwind continue];
+ _7 = const 3_usize;
+ _8 = const true;
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 1_usize) -> [success: bb1, unwind continue];
+ _8 = Lt(const 1_usize, copy _7);
+ assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 1_usize) -> [success: bb1, unwind continue];
}

bb1: {
- _1 = copy (*_2)[_6];
+ _1 = const 2_u32;
+ _1 = copy (*_2)[1 of 2];
StorageDead(_6);
StorageDead(_4);
StorageDead(_2);
Expand Down
5 changes: 3 additions & 2 deletions tests/mir-opt/const_prop/slice_len.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ fn main() {
// CHECK-LABEL: fn main(
// CHECK: debug a => [[a:_.*]];
// CHECK: [[slice:_.*]] = copy {{.*}} as &[u32] (PointerCoercion(Unsize, AsCast));
// CHECK: assert(const true,
// CHECK: [[a]] = const 2_u32;
// Disabled due to <https://github.com/rust-lang/rust/issues/130853>
// COM: CHECK: assert(const true,
// COM: CHECK: [[a]] = const 2_u32;
let a = (&[1u32, 2, 3] as &[u32])[1];
}
3 changes: 1 addition & 2 deletions tests/mir-opt/gvn.borrowed.GVN.panic-abort.diff
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
}

bb2: {
- _0 = opaque::<T>(copy (*_3)) -> [return: bb3, unwind unreachable];
+ _0 = opaque::<T>(copy _1) -> [return: bb3, unwind unreachable];
_0 = opaque::<T>(copy (*_3)) -> [return: bb3, unwind unreachable];
}

bb3: {
Expand Down
3 changes: 1 addition & 2 deletions tests/mir-opt/gvn.borrowed.GVN.panic-unwind.diff
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
}

bb2: {
- _0 = opaque::<T>(copy (*_3)) -> [return: bb3, unwind continue];
+ _0 = opaque::<T>(copy _1) -> [return: bb3, unwind continue];
_0 = opaque::<T>(copy (*_3)) -> [return: bb3, unwind continue];
}

bb3: {
Expand Down
30 changes: 10 additions & 20 deletions tests/mir-opt/gvn.dereferences.GVN.panic-abort.diff
Original file line number Diff line number Diff line change
Expand Up @@ -107,23 +107,18 @@
StorageLive(_18);
_18 = &(*_1);
StorageLive(_19);
- StorageLive(_20);
+ nop;
StorageLive(_20);
_20 = copy (*_18);
- _19 = opaque::<u32>(move _20) -> [return: bb7, unwind unreachable];
+ _19 = opaque::<u32>(copy _20) -> [return: bb7, unwind unreachable];
_19 = opaque::<u32>(move _20) -> [return: bb7, unwind unreachable];
}

bb7: {
- StorageDead(_20);
+ nop;
StorageDead(_20);
StorageDead(_19);
StorageLive(_21);
StorageLive(_22);
- _22 = copy (*_18);
- _21 = opaque::<u32>(move _22) -> [return: bb8, unwind unreachable];
+ _22 = copy _20;
+ _21 = opaque::<u32>(copy _20) -> [return: bb8, unwind unreachable];
_22 = copy (*_18);
_21 = opaque::<u32>(move _22) -> [return: bb8, unwind unreachable];
}

bb8: {
Expand Down Expand Up @@ -157,23 +152,18 @@
StorageDead(_28);
StorageDead(_27);
StorageLive(_29);
- StorageLive(_30);
+ nop;
StorageLive(_30);
_30 = copy ((*_3).0: u32);
- _29 = opaque::<u32>(move _30) -> [return: bb12, unwind unreachable];
+ _29 = opaque::<u32>(copy _30) -> [return: bb12, unwind unreachable];
_29 = opaque::<u32>(move _30) -> [return: bb12, unwind unreachable];
}

bb12: {
- StorageDead(_30);
+ nop;
StorageDead(_30);
StorageDead(_29);
StorageLive(_31);
StorageLive(_32);
- _32 = copy ((*_3).0: u32);
- _31 = opaque::<u32>(move _32) -> [return: bb13, unwind unreachable];
+ _32 = copy _30;
+ _31 = opaque::<u32>(copy _30) -> [return: bb13, unwind unreachable];
_32 = copy ((*_3).0: u32);
_31 = opaque::<u32>(move _32) -> [return: bb13, unwind unreachable];
}

bb13: {
Expand Down
30 changes: 10 additions & 20 deletions tests/mir-opt/gvn.dereferences.GVN.panic-unwind.diff
Original file line number Diff line number Diff line change
Expand Up @@ -107,23 +107,18 @@
StorageLive(_18);
_18 = &(*_1);
StorageLive(_19);
- StorageLive(_20);
+ nop;
StorageLive(_20);
_20 = copy (*_18);
- _19 = opaque::<u32>(move _20) -> [return: bb7, unwind continue];
+ _19 = opaque::<u32>(copy _20) -> [return: bb7, unwind continue];
_19 = opaque::<u32>(move _20) -> [return: bb7, unwind continue];
}

bb7: {
- StorageDead(_20);
+ nop;
StorageDead(_20);
StorageDead(_19);
StorageLive(_21);
StorageLive(_22);
- _22 = copy (*_18);
- _21 = opaque::<u32>(move _22) -> [return: bb8, unwind continue];
+ _22 = copy _20;
+ _21 = opaque::<u32>(copy _20) -> [return: bb8, unwind continue];
_22 = copy (*_18);
_21 = opaque::<u32>(move _22) -> [return: bb8, unwind continue];
}

bb8: {
Expand Down Expand Up @@ -157,23 +152,18 @@
StorageDead(_28);
StorageDead(_27);
StorageLive(_29);
- StorageLive(_30);
+ nop;
StorageLive(_30);
_30 = copy ((*_3).0: u32);
- _29 = opaque::<u32>(move _30) -> [return: bb12, unwind continue];
+ _29 = opaque::<u32>(copy _30) -> [return: bb12, unwind continue];
_29 = opaque::<u32>(move _30) -> [return: bb12, unwind continue];
}

bb12: {
- StorageDead(_30);
+ nop;
StorageDead(_30);
StorageDead(_29);
StorageLive(_31);
StorageLive(_32);
- _32 = copy ((*_3).0: u32);
- _31 = opaque::<u32>(move _32) -> [return: bb13, unwind continue];
+ _32 = copy _30;
+ _31 = opaque::<u32>(copy _30) -> [return: bb13, unwind continue];
_32 = copy ((*_3).0: u32);
_31 = opaque::<u32>(move _32) -> [return: bb13, unwind continue];
}

bb13: {
Expand Down
18 changes: 9 additions & 9 deletions tests/mir-opt/gvn.fn_pointers.GVN.panic-abort.diff
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@
let mut _3: fn(u8) -> u8;
let _5: ();
let mut _6: fn(u8) -> u8;
let mut _9: {closure@$DIR/gvn.rs:614:19: 614:21};
let mut _9: {closure@$DIR/gvn.rs:615:19: 615:21};
let _10: ();
let mut _11: fn();
let mut _13: {closure@$DIR/gvn.rs:614:19: 614:21};
let mut _13: {closure@$DIR/gvn.rs:615:19: 615:21};
let _14: ();
let mut _15: fn();
scope 1 {
debug f => _1;
let _4: fn(u8) -> u8;
scope 2 {
debug g => _4;
let _7: {closure@$DIR/gvn.rs:614:19: 614:21};
let _7: {closure@$DIR/gvn.rs:615:19: 615:21};
scope 3 {
debug closure => _7;
let _8: fn();
Expand Down Expand Up @@ -62,16 +62,16 @@
StorageDead(_6);
StorageDead(_5);
- StorageLive(_7);
- _7 = {closure@$DIR/gvn.rs:614:19: 614:21};
- _7 = {closure@$DIR/gvn.rs:615:19: 615:21};
- StorageLive(_8);
+ nop;
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
+ _7 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
+ nop;
StorageLive(_9);
- _9 = copy _7;
- _8 = move _9 as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
+ _9 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
+ _8 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
StorageDead(_9);
StorageLive(_10);
StorageLive(_11);
Expand All @@ -88,8 +88,8 @@
StorageLive(_13);
- _13 = copy _7;
- _12 = move _13 as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21};
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:614:19: 614:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
+ _13 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21};
+ _12 = const ZeroSized: {closure@$DIR/gvn.rs:615:19: 615:21} as fn() (PointerCoercion(ClosureFnPointer(Safe), AsCast));
StorageDead(_13);
StorageLive(_14);
StorageLive(_15);
Expand Down
Loading

0 comments on commit 6b6a867

Please sign in to comment.