Skip to content

Commit

Permalink
Rework MIR inlining debuginfo so function parameters show up in debug…
Browse files Browse the repository at this point in the history
…gers.

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 (8b38707 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.
  • Loading branch information
khuey committed Aug 13, 2024
1 parent 97e7252 commit 1c5e3c9
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 20 deletions.
22 changes: 10 additions & 12 deletions compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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)| {
Expand Down
15 changes: 14 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/debuginfo.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::hash_map::Entry;
use std::ops::Range;

use rustc_data_structures::fx::FxHashMap;
Expand Down Expand Up @@ -447,6 +448,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}

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)
Expand Down Expand Up @@ -491,7 +493,18 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
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 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<IndexVec<mir::Local, Vec<PerLocalVarDebugInfo<'tcx, Bx::DIVariable>>>>,

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_ssa/src/traits/backend.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::any::Any;
use std::hash::Hash;

use rustc_ast::expand::allocator::AllocatorKind;
use rustc_data_structures::fx::FxIndexMap;
Expand Down Expand Up @@ -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;
}
Expand Down
9 changes: 4 additions & 5 deletions tests/codegen/debuginfo-inline-callsite-location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<i32>"
// 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<i32>"
// 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"]

Expand Down
1 change: 1 addition & 0 deletions tests/codegen/inline-function-args-debug-info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1c5e3c9

Please sign in to comment.