From 1c5e3c90cf2d6133210d801fc6b7ef0bcb115212 Mon Sep 17 00:00:00 2001 From: Kyle Huey Date: Mon, 12 Aug 2024 19:20:00 -0700 Subject: [PATCH] Rework MIR inlining debuginfo so function parameters show up in debuggers. Line numbers of multiply-inlined functions were fixed in #114643 by using a single DISubprogram. That, however, triggered assertions because parameters weren't deduplicated. The "solution" to that in #115417 was to insert a DILexicalScope below the DISubprogram and parent all of the parameters to that scope. That fixed the assertion, but debuggers (including gdb and lldb) don't recognize variables that are not parented to the subprogram itself as parameters, even if they are emitted with DW_TAG_formal_parameter. Consider the program: use std::env; fn square(n: i32) -> i32 { n * n } fn square_no_inline(n: i32) -> i32 { n * n } fn main() { let x = square(env::vars().count() as i32); let y = square_no_inline(env::vars().count() as i32); println!("{x} == {y}"); } When making a release build with debug=2 and rustc 1.82.0-nightly (8b3870784 2024-08-07) (gdb) r Starting program: /ephemeral/tmp/target/release/tmp [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Breakpoint 1, tmp::square () at src/main.rs:5 5 n * n (gdb) info args No arguments. (gdb) info locals n = 31 (gdb) c Continuing. Breakpoint 2, tmp::square_no_inline (n=31) at src/main.rs:10 10 n * n (gdb) info args n = 31 (gdb) info locals No locals. This issue is particularly annoying because it removes arguments from stack traces. The DWARF for the inlined function looks like this: < 2><0x00002132 GOFF=0x00002132> DW_TAG_subprogram DW_AT_linkage_name _ZN3tmp6square17hc507052ff3d2a488E DW_AT_name square DW_AT_decl_file 0x0000000f /ephemeral/tmp/src/main.rs DW_AT_decl_line 0x00000004 DW_AT_type 0x00001a56<.debug_info+0x00001a56> DW_AT_inline DW_INL_inlined < 3><0x00002142 GOFF=0x00002142> DW_TAG_lexical_block < 4><0x00002143 GOFF=0x00002143> DW_TAG_formal_parameter DW_AT_name n DW_AT_decl_file 0x0000000f /ephemeral/tmp/src/main.rs DW_AT_decl_line 0x00000004 DW_AT_type 0x00001a56<.debug_info+0x00001a56> < 4><0x0000214e GOFF=0x0000214e> DW_TAG_null < 3><0x0000214f GOFF=0x0000214f> DW_TAG_null That DW_TAG_lexical_block inhibits every debugger I've tested from recognizing 'n' as a parameter. This patch removes the additional lexical scope. Parameters can be easily deduplicated by a tuple of their scope and the argument index, at the trivial cost of taking a Hash + Eq bound on DIScope. --- .../src/debuginfo/create_scope_map.rs | 22 +++++++++---------- .../rustc_codegen_ssa/src/mir/debuginfo.rs | 15 ++++++++++++- compiler/rustc_codegen_ssa/src/mir/mod.rs | 2 +- .../rustc_codegen_ssa/src/traits/backend.rs | 3 ++- .../debuginfo-inline-callsite-location.rs | 9 ++++---- .../inline-function-args-debug-info.rs | 1 + 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs b/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs index efe616838bf..0a02c230cfc 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs @@ -88,7 +88,7 @@ fn make_mir_scope<'ll, 'tcx>( let loc = cx.lookup_debug_loc(scope_data.span.lo()); let file_metadata = file_metadata(cx, &loc.file); - let parent_dbg_scope = match scope_data.inlined { + let dbg_scope = match scope_data.inlined { Some((callee, _)) => { // FIXME(eddyb) this would be `self.monomorphize(&callee)` // if this is moved to `rustc_codegen_ssa::mir::debuginfo`. @@ -102,17 +102,15 @@ fn make_mir_scope<'ll, 'tcx>( cx.dbg_scope_fn(callee, callee_fn_abi, None) }) } - None => parent_scope.dbg_scope, - }; - - let dbg_scope = unsafe { - llvm::LLVMRustDIBuilderCreateLexicalBlock( - DIB(cx), - parent_dbg_scope, - file_metadata, - loc.line, - loc.col, - ) + None => unsafe { + llvm::LLVMRustDIBuilderCreateLexicalBlock( + DIB(cx), + parent_scope.dbg_scope, + file_metadata, + loc.line, + loc.col, + ) + }, }; let inlined_at = scope_data.inlined.map(|(_, callsite_span)| { diff --git a/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs b/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs index 0e495973a01..75692540c03 100644 --- a/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs +++ b/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs @@ -1,3 +1,4 @@ +use std::collections::hash_map::Entry; use std::ops::Range; use rustc_data_structures::fx::FxHashMap; @@ -447,6 +448,7 @@ pub fn compute_per_local_var_debug_info( } let mut per_local = IndexVec::from_elem(vec![], &self.mir.local_decls); + let mut params_seen: FxHashMap<_, Bx::DIVariable> = Default::default(); for var in &self.mir.var_debug_info { let dbg_scope_and_span = if full_debug_info { self.adjusted_span_and_dbg_scope(var.source_info) @@ -491,7 +493,18 @@ pub fn compute_per_local_var_debug_info( VariableKind::LocalVariable }; - self.cx.create_dbg_var(var.name, var_ty, dbg_scope, var_kind, span) + if let VariableKind::ArgumentVariable(arg_index) = var_kind { + match params_seen.entry((dbg_scope, arg_index)) { + Entry::Occupied(o) => o.get().clone(), + Entry::Vacant(v) => v + .insert( + self.cx.create_dbg_var(var.name, var_ty, dbg_scope, var_kind, span), + ) + .clone(), + } + } else { + self.cx.create_dbg_var(var.name, var_ty, dbg_scope, var_kind, span) + } }); let fragment = if let Some(ref fragment) = var.composite { diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 4ce07269cd2..de94d87bcea 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -106,7 +106,7 @@ pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> { locals: locals::Locals<'tcx, Bx::Value>, /// All `VarDebugInfo` from the MIR body, partitioned by `Local`. - /// This is `None` if no var`#[non_exhaustive]`iable debuginfo/names are needed. + /// This is `None` if no variable debuginfo/names are needed. per_local_var_debug_info: Option>>>, diff --git a/compiler/rustc_codegen_ssa/src/traits/backend.rs b/compiler/rustc_codegen_ssa/src/traits/backend.rs index 81e96413a9f..c5e2d55be83 100644 --- a/compiler/rustc_codegen_ssa/src/traits/backend.rs +++ b/compiler/rustc_codegen_ssa/src/traits/backend.rs @@ -1,4 +1,5 @@ use std::any::Any; +use std::hash::Hash; use rustc_ast::expand::allocator::AllocatorKind; use rustc_data_structures::fx::FxIndexMap; @@ -30,7 +31,7 @@ pub trait BackendTypes { // FIXME(eddyb) find a common convention for all of the debuginfo-related // names (choose between `Dbg`, `Debug`, `DebugInfo`, `DI` etc.). - type DIScope: Copy; + type DIScope: Copy + Hash + PartialEq + Eq; type DILocation: Copy; type DIVariable: Copy; } diff --git a/tests/codegen/debuginfo-inline-callsite-location.rs b/tests/codegen/debuginfo-inline-callsite-location.rs index aee07b4eb8c..c31788d82db 100644 --- a/tests/codegen/debuginfo-inline-callsite-location.rs +++ b/tests/codegen/debuginfo-inline-callsite-location.rs @@ -9,13 +9,12 @@ // CHECK: tail call void @{{[A-Za-z0-9_]+4core6option13unwrap_failed}} // CHECK-SAME: !dbg ![[#second_dbg:]] -// CHECK-DAG: ![[#func_dbg:]] = distinct !DISubprogram(name: "unwrap" -// CHECK-DAG: ![[#first_scope:]] = distinct !DILexicalBlock(scope: ![[#func_dbg]], -// CHECK: ![[#second_scope:]] = distinct !DILexicalBlock(scope: ![[#func_dbg]], +// CHECK-DAG: ![[#func_scope:]] = distinct !DISubprogram(name: "unwrap" +// CHECK-DAG: ![[#]] = !DILocalVariable(name: "self",{{( arg: 1,)?}} scope: ![[#func_scope]] // CHECK: ![[#first_dbg]] = !DILocation(line: [[#]] -// CHECK-SAME: scope: ![[#first_scope]], inlinedAt: ![[#]]) +// CHECK-SAME: scope: ![[#func_scope]], inlinedAt: ![[#]]) // CHECK: ![[#second_dbg]] = !DILocation(line: [[#]] -// CHECK-SAME: scope: ![[#second_scope]], inlinedAt: ![[#]]) +// CHECK-SAME: scope: ![[#func_scope]], inlinedAt: ![[#]]) #![crate_type = "lib"] diff --git a/tests/codegen/inline-function-args-debug-info.rs b/tests/codegen/inline-function-args-debug-info.rs index 7263374b22e..53a179160dc 100644 --- a/tests/codegen/inline-function-args-debug-info.rs +++ b/tests/codegen/inline-function-args-debug-info.rs @@ -15,6 +15,7 @@ pub fn outer_function(x: usize, y: usize) -> usize { fn inner_function(aaaa: usize, bbbb: usize) -> usize { // CHECK: !DILocalVariable(name: "aaaa", arg: 1 // CHECK-SAME: line: 15 + // CHECK-NOT: !DILexicalBlock( // CHECK: !DILocalVariable(name: "bbbb", arg: 2 // CHECK-SAME: line: 15 aaaa + bbbb