Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1098,6 +1098,10 @@ macro_rules! super_body {
}
}

for var_debug_info in &$($mutability)? $body.var_debug_info {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if relying on the order of visiting is reasonable.

$self.visit_var_debug_info(var_debug_info);
}

for (bb, data) in basic_blocks_iter!($body, $($mutability, $invalidate)?) {
$self.visit_basic_block_data(bb, data);
}
Expand Down Expand Up @@ -1127,10 +1131,6 @@ macro_rules! super_body {
);
}

for var_debug_info in &$($mutability)? $body.var_debug_info {
$self.visit_var_debug_info(var_debug_info);
}

$self.visit_span($(& $mutability)? $body.span);

if let Some(required_consts) = &$($mutability)? $body.required_consts {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_mir_transform/src/dest_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,6 @@ impl<'tcx> MutVisitor<'tcx> for Merger<'tcx> {
if self.merged_locals.contains(*local) =>
{
statement.make_nop(true);
return;
}
_ => (),
};
Expand Down
45 changes: 36 additions & 9 deletions compiler/rustc_mir_transform/src/ref_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ fn compute_replacement<'tcx>(
return Replacer {
tcx,
targets: finder.targets,
remap_var_debug_infos: IndexVec::from_elem(None, body.local_decls()),
storage_to_remove,
allowed_replacements,
any_replacement: false,
Expand Down Expand Up @@ -381,6 +382,7 @@ fn fully_replaceable_locals(ssa: &SsaLocals) -> DenseBitSet<Local> {
struct Replacer<'tcx> {
tcx: TyCtxt<'tcx>,
targets: IndexVec<Local, Value<'tcx>>,
remap_var_debug_infos: IndexVec<Local, Option<Local>>,
storage_to_remove: DenseBitSet<Local>,
allowed_replacements: FxHashSet<(Local, Location)>,
any_replacement: bool,
Expand All @@ -392,21 +394,45 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'tcx> {
}

fn visit_var_debug_info(&mut self, debuginfo: &mut VarDebugInfo<'tcx>) {
// If the debuginfo is a pointer to another place
// and it's a reborrow: see through it
while let VarDebugInfoContents::Place(ref mut place) = debuginfo.value
if let VarDebugInfoContents::Place(ref mut place) = debuginfo.value
&& place.projection.is_empty()
&& let Value::Pointer(target, _) = self.targets[place.local]
&& let &[PlaceElem::Deref] = &target.projection[..]
{
*place = Place::from(target.local);
self.any_replacement = true;
let mut new_local = place.local;

// If the debuginfo is a pointer to another place
// and it's a reborrow: see through it
while let Value::Pointer(target, _) = self.targets[new_local]
&& let &[PlaceElem::Deref] = &target.projection[..]
{
new_local = target.local;
}
if place.local != new_local {
self.remap_var_debug_infos[place.local] = Some(new_local);
place.local = new_local;

self.any_replacement = true;
}
}

// Simplify eventual projections left inside `debuginfo`.
self.super_var_debug_info(debuginfo);
}

fn visit_statement_debuginfo(
&mut self,
stmt_debuginfo: &mut StmtDebugInfo<'tcx>,
location: Location,
) {
let local = match stmt_debuginfo {
StmtDebugInfo::AssignRef(local, _) | StmtDebugInfo::InvalidAssign(local) => local,
};
if let Some(target) = self.remap_var_debug_infos[*local] {
*local = target;
self.any_replacement = true;
}
self.super_statement_debuginfo(stmt_debuginfo, location);
}

fn visit_place(&mut self, place: &mut Place<'tcx>, ctxt: PlaceContext, loc: Location) {
loop {
let Some((&PlaceElem::Deref, rest)) = place.projection.split_first() else { return };
Expand Down Expand Up @@ -437,8 +463,9 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'tcx> {
{
stmt.make_nop(true);
}
// Do not remove assignments as they may still be useful for debuginfo.
_ => self.super_statement(stmt, loc),
_ => {}
}
// Do not remove assignments as they may still be useful for debuginfo.
self.super_statement(stmt, loc);
}
}
33 changes: 33 additions & 0 deletions compiler/rustc_mir_transform/src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use rustc_middle::ty::{
self, CoroutineArgsExt, InstanceKind, ScalarInt, Ty, TyCtxt, TypeVisitableExt, Upcast, Variance,
};
use rustc_middle::{bug, span_bug};
use rustc_mir_dataflow::debuginfo::debuginfo_locals;
use rustc_trait_selection::traits::ObligationCtxt;

use crate::util::{self, is_within_packed};
Expand Down Expand Up @@ -80,6 +81,11 @@ impl<'tcx> crate::MirPass<'tcx> for Validator {
cfg_checker.fail(location, msg);
}

// Ensure that debuginfo records are not emitted for locals that are not in debuginfo.
for (location, msg) in validate_debuginfos(body) {
cfg_checker.fail(location, msg);
}

if let MirPhase::Runtime(_) = body.phase
&& let ty::InstanceKind::Item(_) = body.source.instance
&& body.has_free_regions()
Expand Down Expand Up @@ -1595,3 +1601,30 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
self.super_terminator(terminator, location);
}
}

pub(super) fn validate_debuginfos<'tcx>(body: &Body<'tcx>) -> Vec<(Location, String)> {
let mut debuginfo_checker =
DebuginfoChecker { debuginfo_locals: debuginfo_locals(body), failures: Vec::new() };
debuginfo_checker.visit_body(body);
debuginfo_checker.failures
}

struct DebuginfoChecker {
debuginfo_locals: DenseBitSet<Local>,
failures: Vec<(Location, String)>,
}

impl<'tcx> Visitor<'tcx> for DebuginfoChecker {
fn visit_statement_debuginfo(
&mut self,
stmt_debuginfo: &StmtDebugInfo<'tcx>,
location: Location,
) {
let local = match stmt_debuginfo {
StmtDebugInfo::AssignRef(local, _) | StmtDebugInfo::InvalidAssign(local) => *local,
};
if !self.debuginfo_locals.contains(local) {
self.failures.push((location, format!("{local:?} is not in debuginfo")));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
- // MIR for `remap_debuginfo_locals` before DestinationPropagation
+ // MIR for `remap_debuginfo_locals` after DestinationPropagation

fn remap_debuginfo_locals(_1: bool, _2: &bool) -> &bool {
- debug c => _3;
+ debug c => _2;
let mut _0: &bool;
let mut _3: &bool;
let mut _4: bool;

bb0: {
- // DBG: _3 = &_1;
- StorageLive(_4);
- _4 = copy _1;
- _3 = copy _2;
- switchInt(copy _4) -> [1: bb1, otherwise: bb2];
+ // DBG: _2 = &_1;
+ nop;
+ nop;
+ nop;
+ switchInt(copy _1) -> [1: bb1, otherwise: bb2];
}

bb1: {
goto -> bb2;
}

bb2: {
- StorageDead(_4);
- _0 = copy _3;
+ nop;
+ _0 = copy _2;
return;
}
}

39 changes: 39 additions & 0 deletions tests/mir-opt/debuginfo/dest_prop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
//@ test-mir-pass: DestinationPropagation
//@ compile-flags: -g -Zmir-enable-passes=+DeadStoreElimination-initial

#![feature(core_intrinsics, custom_mir)]
#![crate_type = "lib"]

use std::intrinsics::mir::*;

// EMIT_MIR dest_prop.remap_debuginfo_locals.DestinationPropagation.diff
#[custom_mir(dialect = "runtime", phase = "post-cleanup")]
pub fn remap_debuginfo_locals(a: bool, b: &bool) -> &bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some filecheck annotations?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, added.

// CHECK-LABEL: fn remap_debuginfo_locals(
// CHECK: debug c => [[c:_.*]];
// CHECK: bb0:
// CHECK-NEXT: DBG: [[c]] = &_1;
mir! {
let _3: &bool;
let _4: bool;
debug c => _3;
{
_3 = &a;
StorageLive(_4);
_4 = a;
_3 = b;
match _4 {
true => bb1,
_ => bb2,
}
}
bb1 = {
Goto(bb2)
}
bb2 = {
StorageDead(_4);
RET = _3;
Return()
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
- // MIR for `remap_debuginfo_locals` before ReferencePropagation
+ // MIR for `remap_debuginfo_locals` after ReferencePropagation

fn remap_debuginfo_locals() -> () {
let mut _0: ();
let _1: &usize;
let mut _2: *const usize;
let _3: &usize;
let _4: usize;
let mut _5: &usize;
scope 1 (inlined foo) {
- debug a => _1;
+ debug a => _5;
}

bb0: {
- StorageLive(_1);
- StorageLive(_2);
- StorageLive(_3);
_5 = const remap_debuginfo_locals::promoted[0];
- _3 = &(*_5);
- _2 = &raw const (*_3);
- // DBG: _1 = &(*_2);
- _1 = &(*_2);
- StorageDead(_2);
- StorageDead(_3);
- StorageDead(_1);
+ // DBG: _5 = &(*_5);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep this statement, or drop it as a tautology?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not sure. It looks like LLVM doesn't care about what the value is: https://rust.godbolt.org/z/vc3W5fKr4.

_0 = const ();
return;
}
}

30 changes: 30 additions & 0 deletions tests/mir-opt/debuginfo/ref_prop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//@ test-mir-pass: ReferencePropagation
//@ compile-flags: -g -Zub_checks=false -Zinline-mir -Zmir-enable-passes=+DeadStoreElimination-initial

#![feature(core_intrinsics, custom_mir)]
#![crate_type = "lib"]

use std::intrinsics::mir::*;

// EMIT_MIR ref_prop.remap_debuginfo_locals.ReferencePropagation.diff
pub fn remap_debuginfo_locals() {
// CHECK-LABEL: fn remap_debuginfo_locals()
// CHECK: debug a => [[a:_.*]];
// CHECK: bb0:
// CHECK-NEXT: [[a]] = const
// CHECK-NEXT: DBG: [[a]] = &(*[[a]]);
foo(&0);
}

#[custom_mir(dialect = "runtime", phase = "post-cleanup")]
#[inline]
fn foo(x: *const usize) -> &'static usize {
mir! {
debug a => RET;
{
RET = &*x;
RET = &*x;
Return()
}
}
}
18 changes: 18 additions & 0 deletions tests/ui/debuginfo/dest_prop_debuginfo-147485.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//@ build-pass
//@ compile-flags: -g -O

// Regression test for #147485.

#![crate_type = "lib"]

pub fn foo(a: bool, b: bool) -> bool {
let mut c = &a;
if false {
return *c;
}
let d = b && a;
if d {
c = &b;
}
b
}
16 changes: 16 additions & 0 deletions tests/ui/debuginfo/ref_prop_debuginfo-147485.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//@ build-pass
//@ compile-flags: -g -O

// Regression test for #147485.

#![crate_type = "lib"]

pub fn f(x: *const usize) -> &'static usize {
let mut a = unsafe { &*x };
a = unsafe { &*x };
a
}

pub fn g() {
f(&0);
}
Loading