Skip to content

Commit a7976cf

Browse files
authored
Unrolled build for #146566
Rollup merge of #146566 - cjgillot:mir-overlap-lint, r=saethlin Lint more overlapping assignments in MIR. In an effort to make bugs like #146383 more easily discovered, this PR extends the "overlapping assignment" MIR lint. I had to whitelist some rvalues, as they are actually allowed to alias, like `a = a + 1`.
2 parents 9311767 + 912785d commit a7976cf

File tree

4 files changed

+53
-28
lines changed

4 files changed

+53
-28
lines changed

compiler/rustc_middle/src/mir/visit.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,3 +1475,20 @@ impl PlaceContext {
14751475
}
14761476
}
14771477
}
1478+
1479+
/// Small utility to visit places and locals without manually implementing a full visitor.
1480+
pub struct VisitPlacesWith<F>(pub F);
1481+
1482+
impl<'tcx, F> Visitor<'tcx> for VisitPlacesWith<F>
1483+
where
1484+
F: FnMut(Place<'tcx>, PlaceContext),
1485+
{
1486+
fn visit_local(&mut self, local: Local, ctxt: PlaceContext, _: Location) {
1487+
(self.0)(local.into(), ctxt);
1488+
}
1489+
1490+
fn visit_place(&mut self, place: &Place<'tcx>, ctxt: PlaceContext, location: Location) {
1491+
(self.0)(*place, ctxt);
1492+
self.visit_projection(place.as_ref(), ctxt, location);
1493+
}
1494+
}

compiler/rustc_mir_transform/src/dest_prop.rs

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ use rustc_data_structures::union_find::UnionFind;
141141
use rustc_index::bit_set::DenseBitSet;
142142
use rustc_index::interval::SparseIntervalMatrix;
143143
use rustc_index::{IndexVec, newtype_index};
144-
use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor};
144+
use rustc_middle::mir::visit::{MutVisitor, PlaceContext, VisitPlacesWith, Visitor};
145145
use rustc_middle::mir::*;
146146
use rustc_middle::ty::TyCtxt;
147147
use rustc_mir_dataflow::impls::{DefUse, MaybeLiveLocals};
@@ -503,22 +503,6 @@ impl TwoStepIndex {
503503
}
504504
}
505505

506-
struct VisitPlacesWith<F>(F);
507-
508-
impl<'tcx, F> Visitor<'tcx> for VisitPlacesWith<F>
509-
where
510-
F: FnMut(Place<'tcx>, PlaceContext),
511-
{
512-
fn visit_local(&mut self, local: Local, ctxt: PlaceContext, _: Location) {
513-
(self.0)(local.into(), ctxt);
514-
}
515-
516-
fn visit_place(&mut self, place: &Place<'tcx>, ctxt: PlaceContext, location: Location) {
517-
(self.0)(*place, ctxt);
518-
self.visit_projection(place.as_ref(), ctxt, location);
519-
}
520-
}
521-
522506
/// Add points depending on the result of the given dataflow analysis.
523507
fn save_as_intervals<'tcx>(
524508
elements: &DenseLocationMap,

compiler/rustc_mir_transform/src/lint.rs

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::borrow::Cow;
66

77
use rustc_data_structures::fx::FxHashSet;
88
use rustc_index::bit_set::DenseBitSet;
9-
use rustc_middle::mir::visit::{PlaceContext, Visitor};
9+
use rustc_middle::mir::visit::{PlaceContext, VisitPlacesWith, Visitor};
1010
use rustc_middle::mir::*;
1111
use rustc_middle::ty::TyCtxt;
1212
use rustc_mir_dataflow::impls::{MaybeStorageDead, MaybeStorageLive, always_storage_live_locals};
@@ -79,15 +79,39 @@ impl<'a, 'tcx> Visitor<'tcx> for Lint<'a, 'tcx> {
7979
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
8080
match &statement.kind {
8181
StatementKind::Assign(box (dest, rvalue)) => {
82-
if let Rvalue::Use(Operand::Copy(src) | Operand::Move(src)) = rvalue {
83-
// The sides of an assignment must not alias. Currently this just checks whether
84-
// the places are identical.
85-
if dest == src {
86-
self.fail(
87-
location,
88-
"encountered `Assign` statement with overlapping memory",
89-
);
90-
}
82+
let forbid_aliasing = match rvalue {
83+
Rvalue::Use(..)
84+
| Rvalue::CopyForDeref(..)
85+
| Rvalue::Repeat(..)
86+
| Rvalue::Aggregate(..)
87+
| Rvalue::Cast(..)
88+
| Rvalue::ShallowInitBox(..)
89+
| Rvalue::WrapUnsafeBinder(..) => true,
90+
Rvalue::ThreadLocalRef(..)
91+
| Rvalue::NullaryOp(..)
92+
| Rvalue::UnaryOp(..)
93+
| Rvalue::BinaryOp(..)
94+
| Rvalue::Ref(..)
95+
| Rvalue::RawPtr(..)
96+
| Rvalue::Discriminant(..) => false,
97+
};
98+
// The sides of an assignment must not alias.
99+
if forbid_aliasing {
100+
VisitPlacesWith(|src: Place<'tcx>, _| {
101+
if *dest == src
102+
|| (dest.local == src.local
103+
&& !dest.is_indirect()
104+
&& !src.is_indirect())
105+
{
106+
self.fail(
107+
location,
108+
format!(
109+
"encountered `{statement:?}` statement with overlapping memory"
110+
),
111+
);
112+
}
113+
})
114+
.visit_rvalue(rvalue, location);
91115
}
92116
}
93117
StatementKind::StorageLive(local) => {

tests/ui/mir/lint/assignment-overlap.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ pub fn main() {
1313
let a: [u8; 1024];
1414
{
1515
a = a; //~ ERROR broken MIR
16-
//~^ ERROR encountered `Assign` statement with overlapping memory
16+
//~^ ERROR encountered `_1 = copy _1` statement with overlapping memory
1717
Return()
1818
}
1919
}

0 commit comments

Comments
 (0)