From 80bcf9c55349744e30bb17e78e005a656ed0667f Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Mon, 3 Feb 2020 19:42:39 +0200 Subject: [PATCH 1/3] rustc_codegen_ssa: don't treat inlined variables as debuginfo arguments. --- src/librustc_codegen_ssa/mir/debuginfo.rs | 5 +---- .../ui/mir/mir-inlining/var-debuginfo-issue-67586.rs | 11 +++++++++++ 2 files changed, 12 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/mir/mir-inlining/var-debuginfo-issue-67586.rs diff --git a/src/librustc_codegen_ssa/mir/debuginfo.rs b/src/librustc_codegen_ssa/mir/debuginfo.rs index f66496d10fb16..9cba77b72980e 100644 --- a/src/librustc_codegen_ssa/mir/debuginfo.rs +++ b/src/librustc_codegen_ssa/mir/debuginfo.rs @@ -307,11 +307,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let var_ty = self.monomorphized_place_ty(place.as_ref()); let var_kind = if self.mir.local_kind(place.local) == mir::LocalKind::Arg && place.projection.is_empty() + && var.source_info.scope == mir::OUTERMOST_SOURCE_SCOPE { - // FIXME(eddyb, #67586) take `var.source_info.scope` into - // account to avoid using `ArgumentVariable` more than once - // per argument local. - let arg_index = place.local.index() - 1; // FIXME(eddyb) shouldn't `ArgumentVariable` indices be diff --git a/src/test/ui/mir/mir-inlining/var-debuginfo-issue-67586.rs b/src/test/ui/mir/mir-inlining/var-debuginfo-issue-67586.rs new file mode 100644 index 0000000000000..23cc114880c63 --- /dev/null +++ b/src/test/ui/mir/mir-inlining/var-debuginfo-issue-67586.rs @@ -0,0 +1,11 @@ +// run-pass +// compile-flags: -Z mir-opt-level=2 -C opt-level=0 -C debuginfo=2 + +#[inline(never)] +pub fn foo(bar: usize) -> usize { + std::convert::identity(bar) +} + +fn main() { + foo(0); +} From 531dda6bbd4548eb2b9c9c19beb04f0480348a2d Mon Sep 17 00:00:00 2001 From: Valentin Lazureanu Date: Wed, 18 Dec 2019 10:56:59 +0000 Subject: [PATCH 2/3] Use llvm.dbg.value instead of llvm.dbg.declare when possible. --- src/librustc_codegen_llvm/debuginfo/mod.rs | 38 +++++++++----- src/librustc_codegen_llvm/llvm/ffi.rs | 10 ++++ src/librustc_codegen_ssa/mir/analyze.rs | 26 ++++++---- src/librustc_codegen_ssa/mir/debuginfo.rs | 52 ++++++++++++++------ src/librustc_codegen_ssa/traits/debuginfo.rs | 1 + src/rustllvm/RustWrapper.cpp | 11 +++++ src/test/debuginfo/borrowed-c-style-enum.rs | 2 +- 7 files changed, 103 insertions(+), 37 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/mod.rs b/src/librustc_codegen_llvm/debuginfo/mod.rs index c4a52a73e25d9..dc0ac6d1496fd 100644 --- a/src/librustc_codegen_llvm/debuginfo/mod.rs +++ b/src/librustc_codegen_llvm/debuginfo/mod.rs @@ -155,8 +155,9 @@ impl DebugInfoBuilderMethods for Builder<'a, 'll, 'tcx> { direct_offset: Size, indirect_offsets: &[Size], span: Span, + is_by_val: bool, ) { - assert!(!dbg_context.source_locations_enabled); + assert!(!dbg_context.source_locations_enabled || is_by_val); let cx = self.cx(); let loc = span_start(cx, span); @@ -186,20 +187,33 @@ impl DebugInfoBuilderMethods for Builder<'a, 'll, 'tcx> { ); unsafe { let debug_loc = llvm::LLVMGetCurrentDebugLocation(self.llbuilder); - // FIXME(eddyb) replace `llvm.dbg.declare` with `llvm.dbg.addr`. - let instr = llvm::LLVMRustDIBuilderInsertDeclareAtEnd( - DIB(cx), - variable_alloca, - dbg_var, - addr_ops.as_ptr(), - addr_ops.len() as c_uint, - debug_loc, - self.llbb(), - ); + let instr = if is_by_val { + llvm::LLVMRustDIBuilderInsertDbgValueAtEnd( + DIB(cx), + variable_alloca, + dbg_var, + addr_ops.as_ptr(), + addr_ops.len() as c_uint, + debug_loc, + self.llbb()) + } else { + // FIXME(eddyb) replace `llvm.dbg.declare` with `llvm.dbg.addr`. + llvm::LLVMRustDIBuilderInsertDeclareAtEnd( + DIB(cx), + variable_alloca, + dbg_var, + addr_ops.as_ptr(), + addr_ops.len() as c_uint, + debug_loc, + self.llbb(), + ) + }; llvm::LLVMSetInstDebugLocation(self.llbuilder, instr); } - source_loc::set_debug_location(self, UnknownLocation); + if !is_by_val { + source_loc::set_debug_location(self, UnknownLocation); + } } fn set_source_location( diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 3f37f86676c98..f0bb04e560a39 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -1730,6 +1730,16 @@ extern "C" { InsertAtEnd: &'a BasicBlock, ) -> &'a Value; + pub fn LLVMRustDIBuilderInsertDbgValueAtEnd( + Builder: &DIBuilder<'a>, + Val: &'a Value, + VarInfo: &'a DIVariable, + AddrOps: *const i64, + AddrOpsCount: c_uint, + DL: &'a Value, + InsertAtEnd: &'a BasicBlock, + ) -> &'a Value; + pub fn LLVMRustDIBuilderCreateEnumerator( Builder: &DIBuilder<'a>, Name: *const c_char, diff --git a/src/librustc_codegen_ssa/mir/analyze.rs b/src/librustc_codegen_ssa/mir/analyze.rs index d4b0ab0448a8b..5003ceb88b220 100644 --- a/src/librustc_codegen_ssa/mir/analyze.rs +++ b/src/librustc_codegen_ssa/mir/analyze.rs @@ -24,15 +24,6 @@ pub fn non_ssa_locals<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( analyzer.visit_body(mir); for (local, decl) in mir.local_decls.iter_enumerated() { - // FIXME(eddyb): We should figure out how to use llvm.dbg.value instead - // of putting everything in allocas just so we can use llvm.dbg.declare. - if fx.cx.sess().opts.debuginfo == DebugInfo::Full { - if fx.mir.local_kind(local) == mir::LocalKind::Arg { - analyzer.not_ssa(local); - continue; - } - } - let ty = fx.monomorphize(&decl.ty); debug!("local {:?} has type `{}`", local, ty); let layout = fx.cx.spanned_layout_of(ty, decl.source_info.span); @@ -41,6 +32,15 @@ pub fn non_ssa_locals<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( // in an Value without an alloca. } else if fx.cx.is_backend_scalar_pair(layout) { // We allow pairs and uses of any of their 2 fields. + + // FIXME(eddyb): We should figure out how to use llvm.dbg.value instead + // of putting everything in allocas just so we can use llvm.dbg.declare. + if fx.cx.sess().opts.debuginfo == DebugInfo::Full { + if fx.mir.local_kind(local) == mir::LocalKind::Arg { + analyzer.not_ssa(local); + continue; + } + } } else { // These sorts of types require an alloca. Note that // is_llvm_immediate() may *still* be true, particularly @@ -286,7 +286,13 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> // FIXME(eddyb): We should figure out how to use `llvm.dbg.value` instead // of putting everything in allocas just so we can use `llvm.dbg.declare`. if self.fx.cx.sess().opts.debuginfo == DebugInfo::Full { - self.not_ssa(local); + let decl_span = self.fx.mir.local_decls[local].source_info.span; + let ty = self.fx.mir.local_decls[local].ty; + let ty = self.fx.monomorphize(&ty); + let layout = self.fx.cx.spanned_layout_of(ty, decl_span); + if !self.fx.cx.is_backend_immediate(layout) { + self.not_ssa(local); + } } } diff --git a/src/librustc_codegen_ssa/mir/debuginfo.rs b/src/librustc_codegen_ssa/mir/debuginfo.rs index 9cba77b72980e..2ba2d3d50a25a 100644 --- a/src/librustc_codegen_ssa/mir/debuginfo.rs +++ b/src/librustc_codegen_ssa/mir/debuginfo.rs @@ -215,16 +215,19 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { None => return, }; - // FIXME(eddyb) add debuginfo for unsized places too. - let base = match local_ref { - LocalRef::Place(place) => place, - _ => return, + let base_layout = match local_ref { + LocalRef::Operand(None) => return, + LocalRef::Operand(Some(operand)) => operand.layout, + LocalRef::Place(place) => place.layout, + + // FIXME(eddyb) add debuginfo for unsized places too. + LocalRef::UnsizedPlace(_) => return, }; let vars = vars.iter().copied().chain(fallback_var); for var in vars { - let mut layout = base.layout; + let mut layout = base_layout; let mut direct_offset = Size::ZERO; // FIXME(eddyb) use smallvec here. let mut indirect_offsets = vec![]; @@ -263,15 +266,36 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let (scope, span) = self.debug_loc(var.source_info); if let Some(scope) = scope { if let Some(dbg_var) = var.dbg_var { - bx.dbg_var_addr( - debug_context, - dbg_var, - scope, - base.llval, - direct_offset, - &indirect_offsets, - span, - ); + match local_ref { + LocalRef::Place(place) => { + bx.dbg_var_addr( + debug_context, + dbg_var, + scope, + place.llval, + direct_offset, + &indirect_offsets, + span, + false, + ); + } + LocalRef::Operand(Some(operand)) => match operand.val { + OperandValue::Immediate(x) => { + bx.dbg_var_addr( + debug_context, + dbg_var, + scope, + x, + direct_offset, + &indirect_offsets, + span, + true, + ); + } + OperandValue::Ref(..) | OperandValue::Pair(..) => unreachable!(), + }, + LocalRef::UnsizedPlace(_) | LocalRef::Operand(None) => unreachable!(), + } } } } diff --git a/src/librustc_codegen_ssa/traits/debuginfo.rs b/src/librustc_codegen_ssa/traits/debuginfo.rs index 22a4e96b9e4bf..73606abaa213a 100644 --- a/src/librustc_codegen_ssa/traits/debuginfo.rs +++ b/src/librustc_codegen_ssa/traits/debuginfo.rs @@ -57,6 +57,7 @@ pub trait DebugInfoBuilderMethods: BackendTypes { // NB: each offset implies a deref (i.e. they're steps in a pointer chain). indirect_offsets: &[Size], span: Span, + is_by_value: bool, ); fn set_source_location( &mut self, diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index 46e467011b91a..60497b5877452 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -891,6 +891,17 @@ extern "C" LLVMValueRef LLVMRustDIBuilderInsertDeclareAtEnd( unwrap(InsertAtEnd))); } +extern "C" LLVMValueRef LLVMRustDIBuilderInsertDbgValueAtEnd( + LLVMRustDIBuilderRef Builder, LLVMValueRef V, LLVMMetadataRef VarInfo, + int64_t *AddrOps, unsigned AddrOpsCount, LLVMValueRef DL, + LLVMBasicBlockRef InsertAtEnd) { + return wrap(Builder->insertDbgValueIntrinsic( + unwrap(V), unwrap(VarInfo), + Builder->createExpression(llvm::ArrayRef(AddrOps, AddrOpsCount)), + DebugLoc(cast(unwrap(DL)->getMetadata())), + unwrap(InsertAtEnd))); +} + extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateEnumerator(LLVMRustDIBuilderRef Builder, const char *Name, uint64_t Val) { diff --git a/src/test/debuginfo/borrowed-c-style-enum.rs b/src/test/debuginfo/borrowed-c-style-enum.rs index f212ff3951e3e..7bb8f8d1c1a9b 100644 --- a/src/test/debuginfo/borrowed-c-style-enum.rs +++ b/src/test/debuginfo/borrowed-c-style-enum.rs @@ -40,7 +40,7 @@ enum ABC { TheA, TheB, TheC } -fn main() { +pub fn main() { let the_a = ABC::TheA; let the_a_ref: &ABC = &the_a; From 7f18cf72761f07efd1989afdc5f73beabe7d99a3 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Thu, 6 Feb 2020 15:09:01 +0200 Subject: [PATCH 3/3] [HACK] rustc_codegen_llvm: use inline asm to work around llvm.dbg.value issues. --- src/librustc_codegen_llvm/debuginfo/mod.rs | 63 ++++++++++++++++++++-- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/mod.rs b/src/librustc_codegen_llvm/debuginfo/mod.rs index dc0ac6d1496fd..975c91be43041 100644 --- a/src/librustc_codegen_llvm/debuginfo/mod.rs +++ b/src/librustc_codegen_llvm/debuginfo/mod.rs @@ -24,6 +24,7 @@ use crate::value::Value; use rustc::mir; use rustc::session::config::{self, DebugInfo}; use rustc::ty::{self, Instance, InstanceDef, ParamEnv, Ty}; +use rustc_codegen_ssa::common::TypeKind; use rustc_codegen_ssa::debuginfo::type_names; use rustc_codegen_ssa::mir::debuginfo::{DebugScope, FunctionDebugContext, VariableKind}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -188,14 +189,68 @@ impl DebugInfoBuilderMethods for Builder<'a, 'll, 'tcx> { unsafe { let debug_loc = llvm::LLVMGetCurrentDebugLocation(self.llbuilder); let instr = if is_by_val { - llvm::LLVMRustDIBuilderInsertDbgValueAtEnd( + let llval = variable_alloca; + + let instr = llvm::LLVMRustDIBuilderInsertDbgValueAtEnd( DIB(cx), - variable_alloca, + llval, dbg_var, addr_ops.as_ptr(), addr_ops.len() as c_uint, debug_loc, - self.llbb()) + self.llbb(), + ); + + // HACK(eddyb) keep the value we passed to `llvm.dbg.value` + // alive if we're not optimizing, otherwise LLVM will eagerly + // destroy it (and debuggers will show ``), + // if there are no other uses forcing the value to be computed. + // FIXME(#68817) implement the correct behavior in LLVM. + if self.sess().opts.optimize == config::OptLevel::No { + let llty = cx.val_ty(llval); + + // Avoid creating LLVM inline asm with unsupported types, + // usually ZST structs. + let asm_supported = match self.type_kind(llty) { + // Floats. + TypeKind::Half | + TypeKind::Float | + TypeKind::Double | + TypeKind::X86_FP80 | + TypeKind::FP128 | + TypeKind::PPC_FP128 | + // Integers. + TypeKind::Integer | + TypeKind::Pointer | + // Vectors. + TypeKind::Vector | + TypeKind::X86_MMX => true, + + TypeKind::Void | + TypeKind::Label | + TypeKind::Function | + TypeKind::Struct | + TypeKind::Array | + TypeKind::Metadata | + TypeKind::Token => false, + }; + + if asm_supported { + let asm = SmallCStr::new(""); + let constraints = SmallCStr::new("X"); + let asm = llvm::LLVMRustInlineAsm( + cx.type_func(&[llty], cx.type_void()), + asm.as_ptr(), + constraints.as_ptr(), + llvm::False, + llvm::False, + llvm::AsmDialect::Att, + ); + self.call(asm, &[llval], None); + } + } + + instr } else { // FIXME(eddyb) replace `llvm.dbg.declare` with `llvm.dbg.addr`. llvm::LLVMRustDIBuilderInsertDeclareAtEnd( @@ -583,7 +638,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { file_metadata, loc.line as c_uint, type_metadata, - self.sess().opts.optimize != config::OptLevel::No, + true, DIFlags::FlagZero, argument_index, align.bytes() as u32,