Skip to content

Commit

Permalink
Auto merge of rust-lang#114904 - cjgillot:no-ref-debuginfo, r=wesleyw…
Browse files Browse the repository at this point in the history
…iser

Remove references in VarDebugInfo

The codegen implementation is broken, and attempted to read uninitialized memory.

Fixes rust-lang#114488
  • Loading branch information
bors committed Aug 17, 2023
2 parents 0768872 + 3798bca commit ccc3ac0
Show file tree
Hide file tree
Showing 38 changed files with 787 additions and 626 deletions.
54 changes: 12 additions & 42 deletions compiler/rustc_codegen_ssa/src/mir/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ pub struct PerLocalVarDebugInfo<'tcx, D> {

/// `.place.projection` from `mir::VarDebugInfo`.
pub projection: &'tcx ty::List<mir::PlaceElem<'tcx>>,

/// `references` from `mir::VarDebugInfo`.
pub references: u8,
}

#[derive(Clone, Copy, Debug)]
Expand Down Expand Up @@ -323,7 +320,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
dbg_var,
fragment: None,
projection: ty::List::empty(),
references: 0,
})
}
} else {
Expand Down Expand Up @@ -399,15 +395,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
&self,
bx: &mut Bx,
local: mir::Local,
mut base: PlaceRef<'tcx, Bx::Value>,
base: PlaceRef<'tcx, Bx::Value>,
var: PerLocalVarDebugInfo<'tcx, Bx::DIVariable>,
) {
let Some(dbg_var) = var.dbg_var else { return };
let Some(dbg_loc) = self.dbg_loc(var.source_info) else { return };

let DebugInfoOffset { mut direct_offset, indirect_offsets, result: _ } =
let DebugInfoOffset { direct_offset, indirect_offsets, result: _ } =
calculate_debuginfo_offset(bx, local, &var, base.layout);
let mut indirect_offsets = &indirect_offsets[..];

// When targeting MSVC, create extra allocas for arguments instead of pointing multiple
// dbg_var_addr() calls into the same alloca with offsets. MSVC uses CodeView records
Expand All @@ -421,45 +416,29 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
// LLVM can handle simple things but anything more complex than just a direct
// offset or one indirect offset of 0 is too complex for it to generate CV records
// correctly.
&& (direct_offset != Size::ZERO || !matches!(indirect_offsets, [Size::ZERO] | []));
&& (direct_offset != Size::ZERO || !matches!(&indirect_offsets[..], [Size::ZERO] | []));

if should_create_individual_allocas {
let DebugInfoOffset { direct_offset: _, indirect_offsets: _, result: place } =
calculate_debuginfo_offset(bx, local, &var, base);

let create_alloca = |bx: &mut Bx, place: PlaceRef<'tcx, Bx::Value>, refcount| {
// Create a variable which will be a pointer to the actual value
let ptr_ty = Ty::new_ptr(
bx.tcx(),
ty::TypeAndMut { mutbl: mir::Mutability::Mut, ty: place.layout.ty },
);
let ptr_layout = bx.layout_of(ptr_ty);
let alloca = PlaceRef::alloca(bx, ptr_layout);
bx.set_var_name(alloca.llval, &format!("{}.ref{}.dbg.spill", var.name, refcount));
bx.set_var_name(alloca.llval, &(var.name.to_string() + ".dbg.spill"));

// Write the pointer to the variable
bx.store(place.llval, alloca.llval, alloca.align);

// Point the debug info to `*alloca` for the current variable
alloca
};

if var.references > 0 {
base = calculate_debuginfo_offset(bx, local, &var, base).result;

// Point the debug info to `&...&base == alloca` for the current variable
for refcount in 0..var.references {
base = create_alloca(bx, base, refcount);
}

direct_offset = Size::ZERO;
indirect_offsets = &[];
} else if should_create_individual_allocas {
let place = calculate_debuginfo_offset(bx, local, &var, base).result;

// Point the debug info to `*alloca` for the current variable
base = create_alloca(bx, place, 0);
direct_offset = Size::ZERO;
indirect_offsets = &[Size::ZERO];
bx.dbg_var_addr(dbg_var, dbg_loc, alloca.llval, Size::ZERO, &[Size::ZERO], None);
} else {
bx.dbg_var_addr(dbg_var, dbg_loc, base.llval, direct_offset, &indirect_offsets, None);
}

bx.dbg_var_addr(dbg_var, dbg_loc, base.llval, direct_offset, indirect_offsets, None);
}

pub fn debug_introduce_locals(&self, bx: &mut Bx) {
Expand Down Expand Up @@ -492,7 +471,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
};

let dbg_var = dbg_scope_and_span.map(|(dbg_scope, _, span)| {
let (mut var_ty, var_kind) = match var.value {
let (var_ty, var_kind) = match var.value {
mir::VarDebugInfoContents::Place(place) => {
let var_ty = self.monomorphized_place_ty(place.as_ref());
let var_kind = if let Some(arg_index) = var.argument_index
Expand Down Expand Up @@ -529,13 +508,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}
};

for _ in 0..var.references {
var_ty = Ty::new_ptr(
bx.tcx(),
ty::TypeAndMut { mutbl: mir::Mutability::Mut, ty: var_ty },
);
}

self.cx.create_dbg_var(var.name, var_ty, dbg_scope, var_kind, span)
});

Expand All @@ -547,7 +519,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
dbg_var,
fragment: None,
projection: place.projection,
references: var.references,
});
}
mir::VarDebugInfoContents::Const(c) => {
Expand Down Expand Up @@ -601,7 +572,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
Some(fragment_start..fragment_start + fragment_layout.size)
},
projection: place.projection,
references: var.references,
});
}
}
Expand Down
6 changes: 0 additions & 6 deletions compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,12 +701,6 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
VarDebugInfoContents::Const(_) => {}
VarDebugInfoContents::Place(place) => {
check_place(self, place);
if debuginfo.references != 0 && place.projection.last() == Some(&PlaceElem::Deref) {
self.fail(
START_BLOCK.start_location(),
format!("debuginfo {debuginfo:?}, has both ref and deref"),
);
}
}
VarDebugInfoContents::Composite { ty, ref fragments } => {
for f in fragments {
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1109,10 +1109,6 @@ pub struct VarDebugInfo<'tcx> {
/// originated from (starting from 1). Note, if MIR inlining is enabled, then this is the
/// argument number in the original function before it was inlined.
pub argument_index: Option<u16>,

/// The data represents `name` dereferenced `references` times,
/// and not the direct value.
pub references: u8,
}

///////////////////////////////////////////////////////////////////////////
Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,13 +555,8 @@ fn write_scope_tree(
}

let indented_debug_info = format!(
"{0:1$}debug {2} => {3:&<4$}{5:?};",
INDENT,
indent,
var_debug_info.name,
"",
var_debug_info.references as usize,
var_debug_info.value,
"{0:1$}debug {2} => {3:?};",
INDENT, indent, var_debug_info.name, var_debug_info.value,
);

if tcx.sess.opts.unstable_opts.mir_include_spans {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,6 @@ macro_rules! make_mir_visitor {
source_info,
value,
argument_index: _,
references: _,
} = var_debug_info;

self.visit_source_info(source_info);
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,6 @@ CloneLiftImpls! {
(),
bool,
usize,
u8,
u16,
u32,
u64,
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2242,7 +2242,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.var_debug_info.push(VarDebugInfo {
name,
source_info: debug_source_info,
references: 0,
value: VarDebugInfoContents::Place(for_arm_body.into()),
argument_index: None,
});
Expand All @@ -2262,7 +2261,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.var_debug_info.push(VarDebugInfo {
name,
source_info: debug_source_info,
references: 0,
value: VarDebugInfoContents::Place(ref_for_guard.into()),
argument_index: None,
});
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_mir_build/src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
};
self.var_debug_info.push(VarDebugInfo {
name,
references: 0,
source_info: SourceInfo::outermost(captured_place.var_ident.span),
value: VarDebugInfoContents::Place(use_place),
argument_index: None,
Expand Down Expand Up @@ -851,7 +850,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.var_debug_info.push(VarDebugInfo {
name,
source_info,
references: 0,
value: VarDebugInfoContents::Place(arg_local.into()),
argument_index: Some(argument_index as u16 + 1),
});
Expand Down
8 changes: 0 additions & 8 deletions compiler/rustc_mir_transform/src/ref_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ fn compute_replacement<'tcx>(
targets,
storage_to_remove,
allowed_replacements,
fully_replacable_locals,
any_replacement: false,
};

Expand Down Expand Up @@ -346,7 +345,6 @@ struct Replacer<'tcx> {
storage_to_remove: BitSet<Local>,
allowed_replacements: FxHashSet<(Local, Location)>,
any_replacement: bool,
fully_replacable_locals: BitSet<Local>,
}

impl<'tcx> MutVisitor<'tcx> for Replacer<'tcx> {
Expand All @@ -366,12 +364,6 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'tcx> {
if let Some((&PlaceElem::Deref, rest)) = target.projection.split_last() {
*place = Place::from(target.local).project_deeper(rest, self.tcx);
self.any_replacement = true;
} else if self.fully_replacable_locals.contains(place.local)
&& let Some(references) = debuginfo.references.checked_add(1)
{
debuginfo.references = references;
*place = target;
self.any_replacement = true;
} else {
break
}
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_type_ir/src/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ TrivialTypeTraversalImpls! {
(),
bool,
usize,
u8,
u16,
u32,
u64,
Expand Down
32 changes: 16 additions & 16 deletions tests/codegen/slice-ref-equality.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,48 +44,48 @@ pub fn is_zero_array(data: &[u8; 4]) -> bool {
// equality for non-byte types also just emit a `bcmp`, not a loop.

// CHECK-LABEL: @eq_slice_of_nested_u8(
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %x.1
// CHECK-SAME: [[USIZE]] noundef %y.1
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
// CHECK-SAME: [[USIZE]] noundef %3
#[no_mangle]
fn eq_slice_of_nested_u8(x: &[[u8; 3]], y: &[[u8; 3]]) -> bool {
// CHECK: icmp eq [[USIZE]] %x.1, %y.1
// CHECK: %[[BYTES:.+]] = mul nsw [[USIZE]] %x.1, 3
// CHECK: icmp eq [[USIZE]] %1, %3
// CHECK: %[[BYTES:.+]] = mul nsw [[USIZE]] %1, 3
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}(ptr
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
x == y
}

// CHECK-LABEL: @eq_slice_of_i32(
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %x.1
// CHECK-SAME: [[USIZE]] noundef %y.1
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
// CHECK-SAME: [[USIZE]] noundef %3
#[no_mangle]
fn eq_slice_of_i32(x: &[i32], y: &[i32]) -> bool {
// CHECK: icmp eq [[USIZE]] %x.1, %y.1
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %x.1, 2
// CHECK: icmp eq [[USIZE]] %1, %3
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %1, 2
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}(ptr
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
x == y
}

// CHECK-LABEL: @eq_slice_of_nonzero(
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %x.1
// CHECK-SAME: [[USIZE]] noundef %y.1
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
// CHECK-SAME: [[USIZE]] noundef %3
#[no_mangle]
fn eq_slice_of_nonzero(x: &[NonZeroU32], y: &[NonZeroU32]) -> bool {
// CHECK: icmp eq [[USIZE]] %x.1, %y.1
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %x.1, 2
// CHECK: icmp eq [[USIZE]] %1, %3
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %1, 2
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}(ptr
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
x == y
}

// CHECK-LABEL: @eq_slice_of_option_of_nonzero(
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %x.1
// CHECK-SAME: [[USIZE]] noundef %y.1
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
// CHECK-SAME: [[USIZE]] noundef %3
#[no_mangle]
fn eq_slice_of_option_of_nonzero(x: &[Option<NonZeroI16>], y: &[Option<NonZeroI16>]) -> bool {
// CHECK: icmp eq [[USIZE]] %x.1, %y.1
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %x.1, 1
// CHECK: icmp eq [[USIZE]] %1, %3
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %1, 1
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}(ptr
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
x == y
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
let mut _2: std::option::Option<T>;
+ scope 1 (inlined #[track_caller] Option::<T>::unwrap_unchecked) {
+ debug self => _2;
+ let mut _3: isize;
+ let mut _3: &std::option::Option<T>;
+ let mut _4: isize;
+ scope 2 {
+ debug val => _0;
+ }
Expand All @@ -20,16 +21,17 @@
+ }
+ }
+ scope 4 (inlined Option::<T>::is_some) {
+ debug self => &_2;
+ debug self => _3;
+ }
+ }

bb0: {
StorageLive(_2);
_2 = move _1;
- _0 = Option::<T>::unwrap_unchecked(move _2) -> [return: bb1, unwind unreachable];
+ _3 = discriminant(_2);
+ switchInt(move _3) -> [1: bb2, otherwise: bb1];
+ StorageLive(_3);
+ _4 = discriminant(_2);
+ switchInt(move _4) -> [1: bb2, otherwise: bb1];
}

bb1: {
Expand All @@ -38,6 +40,7 @@
+
+ bb2: {
+ _0 = move ((_2 as Some).0: T);
+ StorageDead(_3);
StorageDead(_2);
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
let mut _2: std::option::Option<T>;
+ scope 1 (inlined #[track_caller] Option::<T>::unwrap_unchecked) {
+ debug self => _2;
+ let mut _3: isize;
+ let mut _3: &std::option::Option<T>;
+ let mut _4: isize;
+ scope 2 {
+ debug val => _0;
+ }
Expand All @@ -20,16 +21,17 @@
+ }
+ }
+ scope 4 (inlined Option::<T>::is_some) {
+ debug self => &_2;
+ debug self => _3;
+ }
+ }

bb0: {
StorageLive(_2);
_2 = move _1;
- _0 = Option::<T>::unwrap_unchecked(move _2) -> [return: bb1, unwind: bb2];
+ _3 = discriminant(_2);
+ switchInt(move _3) -> [1: bb2, otherwise: bb1];
+ StorageLive(_3);
+ _4 = discriminant(_2);
+ switchInt(move _4) -> [1: bb2, otherwise: bb1];
}

bb1: {
Expand All @@ -42,6 +44,7 @@
- resume;
+ bb2: {
+ _0 = move ((_2 as Some).0: T);
+ StorageDead(_3);
+ StorageDead(_2);
+ return;
}
Expand Down
Loading

0 comments on commit ccc3ac0

Please sign in to comment.