Auto merge of rust-lang#114643 - dpaoliello:inlinedebuginfo, r=wesleywiser

Use the same DISubprogram for each instance of the same inlined function within a caller

# Issue Details:
The call to `panic` within a function like `Option::unwrap` is translated to LLVM as a `tail call` (as it will never return), when multiple calls to the same function like this is inlined LLVM will notice the common `tail call` block (i.e., loading the same panic string + location info and then calling `panic`) and merge them together.

When merging these instructions together, LLVM will also attempt to merge the debug locations as well, but this fails (i.e., debug info is dropped) as Rust emits a new `DISubprogram` at each inline site thus LLVM doesn't recognize that these are actually the same function and so thinks that there isn't a common debug location.

As an example of this when building for x86_64 Windows (note the lack of `.cv_loc` before the call to `panic`, thus it will be attributed to the same line at the `addq` instruction):

```
	.cv_loc	0 1 23 0                        # src\lib.rs:23:0
	addq	$40, %rsp
	retq
	leaq	.Lalloc_f570dea0a53168780ce9a91e67646421(%rip), %rcx
	leaq	.Lalloc_629ace53b7e5b76aaa810d549cc84ea3(%rip), %r8
	movl	$43, %edx
	callq	_ZN4core9panicking5panic17h12e60b9063f6dee8E
	int3
```

# Fix Details:
Cache the `DISubprogram` emitted for each inlined function instance within a caller so that this can be reused if that instance is encountered again, this also requires caching the `DILexicalBlock` and `DIVariable` objects to avoid creating duplicates.

After this change the above assembly now looks like:

```
	.cv_loc	0 1 23 0                        # src\lib.rs:23:0
	addq	$40, %rsp
	retq
	.cv_inline_site_id 5 within 0 inlined_at 1 0 0
	.cv_inline_site_id 6 within 5 inlined_at 1 12 0
	.cv_loc	6 2 935 0                       # library\core\src\option.rs:935:0
	leaq	.Lalloc_5f55955de67e57c79064b537689facea(%rip), %rcx
	leaq	.Lalloc_e741d4de8cb5801e1fd7a6c6795c1559(%rip), %r8
	movl	$43, %edx
	callq	_ZN4core9panicking5panic17hde1558f32d5b1c04E
	int3
```

Author: bors

compiler/rustc_codegen_gcc/src/debuginfo.rs

@@ -55,7 +55,7 @@ impl<'gcc, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
         _fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
         _llfn: RValue<'gcc>,
         _mir: &mir::Body<'tcx>,
-    ) -> Option<FunctionDebugContext<Self::DIScope, Self::DILocation>> {
+    ) -> Option<FunctionDebugContext<'tcx, Self::DIScope, Self::DILocation>> {
         // TODO(antoyo)
         None
     }

compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs

@@ -20,7 +20,7 @@ pub fn compute_mir_scopes<'ll, 'tcx>(
     cx: &CodegenCx<'ll, 'tcx>,
     instance: Instance<'tcx>,
     mir: &Body<'tcx>,
-    debug_context: &mut FunctionDebugContext<&'ll DIScope, &'ll DILocation>,
+    debug_context: &mut FunctionDebugContext<'tcx, &'ll DIScope, &'ll DILocation>,
 ) {
     // Find all scopes with variables defined in them.
     let variables = if cx.sess().opts.debuginfo == DebugInfo::Full {
@@ -51,7 +51,7 @@ fn make_mir_scope<'ll, 'tcx>(
     instance: Instance<'tcx>,
     mir: &Body<'tcx>,
     variables: &Option<BitSet<SourceScope>>,
-    debug_context: &mut FunctionDebugContext<&'ll DIScope, &'ll DILocation>,
+    debug_context: &mut FunctionDebugContext<'tcx, &'ll DIScope, &'ll DILocation>,
     instantiated: &mut BitSet<SourceScope>,
     scope: SourceScope,
 ) {
@@ -84,7 +84,6 @@ 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 dbg_scope = match scope_data.inlined {
         Some((callee, _)) => {
@@ -95,18 +94,26 @@ fn make_mir_scope<'ll, 'tcx>(
                 ty::ParamEnv::reveal_all(),
                 ty::EarlyBinder::bind(callee),
             );
-            let callee_fn_abi = cx.fn_abi_of_instance(callee, ty::List::empty());
-            cx.dbg_scope_fn(callee, callee_fn_abi, None)
+            debug_context.inlined_function_scopes.entry(callee).or_insert_with(|| {
+                let callee_fn_abi = cx.fn_abi_of_instance(callee, ty::List::empty());
+                cx.dbg_scope_fn(callee, callee_fn_abi, None)
+            })
+        }
+        None => {
+            let file_metadata = file_metadata(cx, &loc.file);
+            debug_context
+                .lexical_blocks
+                .entry((parent_scope.dbg_scope, loc.line, loc.col, file_metadata))
+                .or_insert_with(|| unsafe {
+                    llvm::LLVMRustDIBuilderCreateLexicalBlock(
+                        DIB(cx),
+                        parent_scope.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)| {

compiler/rustc_codegen_llvm/src/debuginfo/mod.rs

@@ -5,7 +5,7 @@ use rustc_codegen_ssa::mir::debuginfo::VariableKind::*;
 use self::metadata::{file_metadata, type_di_node};
 use self::metadata::{UNKNOWN_COLUMN_NUMBER, UNKNOWN_LINE_NUMBER};
 use self::namespace::mangled_name_of_instance;
-use self::utils::{create_DIArray, is_node_local_to_unit, DIB};
+use self::utils::{create_DIArray, debug_context, is_node_local_to_unit, DIB};
 
 use crate::abi::FnAbi;
 use crate::builder::Builder;
@@ -67,6 +67,8 @@ pub struct CodegenUnitDebugContext<'ll, 'tcx> {
     type_map: metadata::TypeMap<'ll, 'tcx>,
     namespace_map: RefCell<DefIdMap<&'ll DIScope>>,
     recursion_marker_type: OnceCell<&'ll DIType>,
+    /// Maps a variable (name, scope, kind (argument or local), span) to its debug information.
+    variables: RefCell<FxHashMap<(Symbol, &'ll DIScope, VariableKind, Span), &'ll DIVariable>>,
 }
 
 impl Drop for CodegenUnitDebugContext<'_, '_> {
@@ -91,6 +93,7 @@ impl<'ll, 'tcx> CodegenUnitDebugContext<'ll, 'tcx> {
             type_map: Default::default(),
             namespace_map: RefCell::new(Default::default()),
             recursion_marker_type: OnceCell::new(),
+            variables: RefCell::new(Default::default()),
         }
     }
 
@@ -292,7 +295,7 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
         fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
         llfn: &'ll Value,
         mir: &mir::Body<'tcx>,
-    ) -> Option<FunctionDebugContext<&'ll DIScope, &'ll DILocation>> {
+    ) -> Option<FunctionDebugContext<'tcx, &'ll DIScope, &'ll DILocation>> {
         if self.sess().opts.debuginfo == DebugInfo::None {
             return None;
         }
@@ -304,8 +307,11 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
             file_start_pos: BytePos(0),
             file_end_pos: BytePos(0),
         };
-        let mut fn_debug_context =
-            FunctionDebugContext { scopes: IndexVec::from_elem(empty_scope, &mir.source_scopes) };
+        let mut fn_debug_context = FunctionDebugContext {
+            scopes: IndexVec::from_elem(empty_scope, &mir.source_scopes),
+            inlined_function_scopes: Default::default(),
+            lexical_blocks: Default::default(),
+        };
 
         // Fill in all the scopes, with the information from the MIR body.
         compute_mir_scopes(self, instance, mir, &mut fn_debug_context);
@@ -606,33 +612,39 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
         variable_kind: VariableKind,
         span: Span,
     ) -> &'ll DIVariable {
-        let loc = self.lookup_debug_loc(span.lo());
-        let file_metadata = file_metadata(self, &loc.file);
-
-        let type_metadata = type_di_node(self, variable_type);
-
-        let (argument_index, dwarf_tag) = match variable_kind {
-            ArgumentVariable(index) => (index as c_uint, DW_TAG_arg_variable),
-            LocalVariable => (0, DW_TAG_auto_variable),
-        };
-        let align = self.align_of(variable_type);
-
-        let name = variable_name.as_str();
-        unsafe {
-            llvm::LLVMRustDIBuilderCreateVariable(
-                DIB(self),
-                dwarf_tag,
-                scope_metadata,
-                name.as_ptr().cast(),
-                name.len(),
-                file_metadata,
-                loc.line,
-                type_metadata,
-                true,
-                DIFlags::FlagZero,
-                argument_index,
-                align.bytes() as u32,
-            )
-        }
+        debug_context(self)
+            .variables
+            .borrow_mut()
+            .entry((variable_name, scope_metadata, variable_kind, span))
+            .or_insert_with(|| {
+                let loc = self.lookup_debug_loc(span.lo());
+                let file_metadata = file_metadata(self, &loc.file);
+
+                let type_metadata = type_di_node(self, variable_type);
+
+                let (argument_index, dwarf_tag) = match variable_kind {
+                    ArgumentVariable(index) => (index as c_uint, DW_TAG_arg_variable),
+                    LocalVariable => (0, DW_TAG_auto_variable),
+                };
+                let align = self.align_of(variable_type);
+
+                let name = variable_name.as_str();
+                unsafe {
+                    llvm::LLVMRustDIBuilderCreateVariable(
+                        DIB(self),
+                        dwarf_tag,
+                        scope_metadata,
+                        name.as_ptr().cast(),
+                        name.len(),
+                        file_metadata,
+                        loc.line,
+                        type_metadata,
+                        true,
+                        DIFlags::FlagZero,
+                        argument_index,
+                        align.bytes() as u32,
+                    )
+                }
+            })
     }
 }

compiler/rustc_codegen_ssa/src/mir/debuginfo.rs

@@ -1,10 +1,12 @@
 use crate::traits::*;
+use rustc_data_structures::fx::FxHashMap;
 use rustc_index::IndexVec;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc_middle::mir;
 use rustc_middle::ty;
 use rustc_middle::ty::layout::TyAndLayout;
 use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf};
+use rustc_middle::ty::Instance;
 use rustc_middle::ty::Ty;
 use rustc_session::config::DebugInfo;
 use rustc_span::symbol::{kw, Symbol};
@@ -17,11 +19,19 @@ use super::{FunctionCx, LocalRef};
 
 use std::ops::Range;
 
-pub struct FunctionDebugContext<S, L> {
+pub struct FunctionDebugContext<'tcx, S, L> {
+    /// Maps from source code to the corresponding debug info scope.
     pub scopes: IndexVec<mir::SourceScope, DebugScope<S, L>>,
+
+    /// Maps from a given inlined function to its debug info declaration.
+    pub inlined_function_scopes: FxHashMap<Instance<'tcx>, S>,
+
+    /// Maps from a lexical block (parent scope, line, column, file) to its debug info declaration.
+    /// This is particularily useful if the parent scope is an inlined function.
+    pub lexical_blocks: FxHashMap<(S, u32, u32, S), S>,
 }
 
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Eq, PartialEq, Hash)]
 pub enum VariableKind {
     ArgumentVariable(usize /*index*/),
     LocalVariable,

compiler/rustc_codegen_ssa/src/mir/mod.rs

@@ -45,7 +45,7 @@ pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> {
 
     mir: &'tcx mir::Body<'tcx>,
 
-    debug_context: Option<FunctionDebugContext<Bx::DIScope, Bx::DILocation>>,
+    debug_context: Option<FunctionDebugContext<'tcx, Bx::DIScope, Bx::DILocation>>,
 
     llfn: Bx::Function,
 

compiler/rustc_codegen_ssa/src/traits/debuginfo.rs

@@ -26,7 +26,7 @@ pub trait DebugInfoMethods<'tcx>: BackendTypes {
         fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
         llfn: Self::Function,
         mir: &mir::Body<'tcx>,
-    ) -> Option<FunctionDebugContext<Self::DIScope, Self::DILocation>>;
+    ) -> Option<FunctionDebugContext<'tcx, Self::DIScope, Self::DILocation>>;
 
     // FIXME(eddyb) find a common convention for all of the debuginfo-related
     // names (choose between `dbg`, `debug`, `debuginfo`, `debug_info` etc.).

tests/codegen/debuginfo-inline-callsite-location.rs

@@ -0,0 +1,26 @@
+// compile-flags: -g -O
+
+// Check that each inline call site for the same function uses the same "sub-program" so that LLVM
+// can correctly merge the debug info if it merges the inlined code (e.g., for merging of tail
+// calls to panic.
+
+// CHECK:       tail call void @_ZN4core9panicking5panic17h{{([0-9a-z]{16})}}E
+// CHECK-SAME:  !dbg ![[#first_dbg:]]
+// CHECK:       tail call void @_ZN4core9panicking5panic17h{{([0-9a-z]{16})}}E
+// CHECK-SAME:  !dbg ![[#second_dbg:]]
+
+// CHECK:       ![[#func_dbg:]] = distinct !DISubprogram(name: "unwrap<i32>"
+// CHECK:       ![[#first_dbg]] = !DILocation(line: [[#]]
+// CHECK-SAME:  scope: ![[#func_dbg]], inlinedAt: ![[#]])
+// CHECK:       ![[#second_dbg]] = !DILocation(line: [[#]]
+// CHECK-SAME:  scope: ![[#func_dbg]], inlinedAt: ![[#]])
+
+#![crate_type = "lib"]
+
+#[no_mangle]
+extern "C" fn add_numbers(x: &Option<i32>, y: &Option<i32>) -> i32 {
+    let x1 = x.unwrap();
+    let y1 = y.unwrap();
+
+    x1 + y1
+}