Auto merge of rust-lang#132613 - khuey:master, r=<try>

Add discriminators to DILocations when multiple functions are inlined into a single point.

LLVM does not expect to ever see multiple dbg_declares for the same variable at the same location with different values. proc-macros make it possible for arbitrary code, including multiple calls that get inlined, to happen at any given location in the source code. Add discriminators when that happens so these locations are different to LLVM.

This may interfere with the AddDiscriminators pass in LLVM, which is added by the unstable flag -Zdebug-info-for-profiling.

Fixes rust-lang#131944

Author: bors

compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs

@@ -1,11 +1,15 @@
+use std::collections::hash_map::Entry;
+
 use rustc_codegen_ssa::mir::debuginfo::{DebugScope, FunctionDebugContext};
 use rustc_codegen_ssa::traits::*;
+use rustc_data_structures::fx::FxHashMap;
 use rustc_index::Idx;
 use rustc_index::bit_set::BitSet;
 use rustc_middle::mir::{Body, SourceScope};
 use rustc_middle::ty::layout::FnAbiOf;
 use rustc_middle::ty::{self, Instance};
 use rustc_session::config::DebugInfo;
+use rustc_span::BytePos;
 
 use super::metadata::file_metadata;
 use super::utils::DIB;
@@ -37,10 +41,20 @@ pub(crate) fn compute_mir_scopes<'ll, 'tcx>(
         None
     };
     let mut instantiated = BitSet::new_empty(mir.source_scopes.len());
+    let mut discriminators = FxHashMap::default();
     // Instantiate all scopes.
     for idx in 0..mir.source_scopes.len() {
         let scope = SourceScope::new(idx);
-        make_mir_scope(cx, instance, mir, &variables, debug_context, &mut instantiated, scope);
+        make_mir_scope(
+            cx,
+            instance,
+            mir,
+            &variables,
+            debug_context,
+            &mut instantiated,
+            &mut discriminators,
+            scope,
+        );
     }
     assert!(instantiated.count() == mir.source_scopes.len());
 }
@@ -52,6 +66,7 @@ fn make_mir_scope<'ll, 'tcx>(
     variables: &Option<BitSet<SourceScope>>,
     debug_context: &mut FunctionDebugContext<'tcx, &'ll DIScope, &'ll DILocation>,
     instantiated: &mut BitSet<SourceScope>,
+    discriminators: &mut FxHashMap<BytePos, u32>,
     scope: SourceScope,
 ) {
     if instantiated.contains(scope) {
@@ -60,7 +75,16 @@ fn make_mir_scope<'ll, 'tcx>(
 
     let scope_data = &mir.source_scopes[scope];
     let parent_scope = if let Some(parent) = scope_data.parent_scope {
-        make_mir_scope(cx, instance, mir, variables, debug_context, instantiated, parent);
+        make_mir_scope(
+            cx,
+            instance,
+            mir,
+            variables,
+            debug_context,
+            instantiated,
+            discriminators,
+            parent,
+        );
         debug_context.scopes[parent]
     } else {
         // The root is the function itself.
@@ -117,7 +141,37 @@ fn make_mir_scope<'ll, 'tcx>(
         // FIXME(eddyb) this doesn't account for the macro-related
         // `Span` fixups that `rustc_codegen_ssa::mir::debuginfo` does.
         let callsite_scope = parent_scope.adjust_dbg_scope_for_span(cx, callsite_span);
-        cx.dbg_loc(callsite_scope, parent_scope.inlined_at, callsite_span)
+        let loc = cx.dbg_loc(callsite_scope, parent_scope.inlined_at, callsite_span);
+
+        // NB: In order to produce proper debug info for variables (particularly
+        // arguments) in multiply-inline functions, LLVM expects to see a single
+        // DILocalVariable with multiple different DILocations in the IR. While
+        // the source information for each DILocation would be identical, their
+        // inlinedAt attributes will be unique to the particular callsite.
+        //
+        // We generate DILocations here based on the callsite's location in the
+        // source code. A single location in the source code usually can't
+        // produce multiple distinct calls so this mostly works, until
+        // proc-macros get involved. A proc-macro can generate multiple calls
+        // at the same span, which breaks the assumption that we're going to
+        // produce a unique DILocation for every scope we process here. We
+        // have to explicitly add discriminators if we see inlines into the
+        // same source code location.
+        //
+        // Note further that we can't key this hashtable on the span itself,
+        // because these spans could have distinct SyntaxContexts. We have
+        // to key on exactly what we're giving to LLVM.
+        match discriminators.entry(callsite_span.lo()) {
+            Entry::Occupied(mut o) => {
+                *o.get_mut() += 1;
+                unsafe { llvm::LLVMRustDILocationCloneWithBaseDiscriminator(loc, *o.get()) }
+                    .expect("Failed to encode discriminator in DILocation")
+            }
+            Entry::Vacant(v) => {
+                v.insert(0);
+                loc
+            }
+        }
     });
 
     debug_context.scopes[scope] = DebugScope {

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

@@ -2179,6 +2179,10 @@ unsafe extern "C" {
         Scope: &'a DIScope,
         InlinedAt: Option<&'a DILocation>,
     ) -> &'a DILocation;
+    pub fn LLVMRustDILocationCloneWithBaseDiscriminator<'a>(
+        Location: &'a DILocation,
+        BD: c_uint,
+    ) -> Option<&'a DILocation>;
     pub fn LLVMRustDIBuilderCreateOpDeref() -> u64;
     pub fn LLVMRustDIBuilderCreateOpPlusUconst() -> u64;
     pub fn LLVMRustDIBuilderCreateOpLLVMFragment() -> u64;

compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp

@@ -37,7 +37,6 @@
 #include "llvm/Transforms/IPO/ThinLTOBitcodeWriter.h"
 #include "llvm/Transforms/Instrumentation/AddressSanitizer.h"
 #include "llvm/Transforms/Instrumentation/DataFlowSanitizer.h"
-#include "llvm/Transforms/Utils/AddDiscriminators.h"
 #include "llvm/Transforms/Utils/FunctionImportUtils.h"
 #if LLVM_VERSION_GE(19, 0)
 #include "llvm/Support/PGOOptions.h"

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

@@ -1261,6 +1261,14 @@ LLVMRustDIBuilderCreateDebugLocation(unsigned Line, unsigned Column,
   return wrap(Loc);
 }
 
+extern "C" LLVMMetadataRef
+LLVMRustDILocationCloneWithBaseDiscriminator(LLVMMetadataRef Location,
+                                             unsigned BD) {
+  DILocation *Loc = unwrapDIPtr<DILocation>(Location);
+  auto NewLoc = Loc->cloneWithBaseDiscriminator(BD);
+  return wrap(NewLoc.has_value() ? NewLoc.value() : nullptr);
+}
+
 extern "C" uint64_t LLVMRustDIBuilderCreateOpDeref() {
   return dwarf::DW_OP_deref;
 }

tests/codegen/debuginfo-proc-macro/auxiliary/macro_def.rs

@@ -0,0 +1,10 @@
+//@ no-prefer-dynamic
+#![crate_type = "proc-macro"]
+
+extern crate proc_macro;
+use proc_macro::*;
+
+#[proc_macro]
+pub fn square_twice(_item: TokenStream) -> TokenStream {
+    "(square(env::vars().count() as i32), square(env::vars().count() as i32))".parse().unwrap()
+}

tests/codegen/debuginfo-proc-macro/mir_inlined_twice_var_locs.rs

@@ -0,0 +1,51 @@
+//@ min-llvm-version: 19
+//@ compile-flags: -Cdebuginfo=2 -Copt-level=0 -Zmir-enable-passes=+Inline
+
+//@ aux-build:macro_def.rs
+
+// Find the variable.
+// CHECK-DAG: ![[#var_dbg:]] = !DILocalVariable(name: "n",{{( arg: 1,)?}} scope: ![[#var_scope:]]
+
+// Find both dbg_declares. These will proceed the variable metadata, of course, so we're looking
+// backwards.
+// CHECK-DAG: dbg_declare(ptr %n.dbg.spill3, ![[#var_dbg]], !DIExpression(), ![[#var_loc2:]])
+// CHECK-DAG: dbg_declare(ptr %n.dbg.spill, ![[#var_dbg]], !DIExpression(), ![[#var_loc1:]])
+
+// Find the first location definition, looking forwards again.
+// CHECK: ![[#var_loc1]] = !DILocation
+// CHECK-SAME: scope: ![[#var_scope:]], inlinedAt: ![[#var_inlinedAt1:]]
+
+// Find the first location's inlinedAt
+// NB: If we fail here it's *probably* because we failed to produce two
+// different locations and ended up reusing an earlier one.
+// CHECK: ![[#var_inlinedAt1]] = !DILocation
+// CHECK-SAME: scope: ![[var_inlinedAt1_scope:]]
+
+// Find the second location definition, still looking forwards.
+// NB: If we failed to produce two different locations, the test will
+// definitely fail by this point (if it hasn't already) because we won't
+// be able to find the same line again.
+// CHECK: ![[#var_loc2]] = !DILocation
+// CHECK-SAME: scope: ![[#var_scope]], inlinedAt: ![[#var_inlinedAt2:]]
+
+// Find the second location's inlinedAt.
+// CHECK: ![[#var_inlinedAt2]] = !DILocation
+// CHECK-SAME: scope: ![[#var_inlinedAt2_scope:]]
+
+// Finally, check that a discriminator was emitted for the second scope.
+// FIXMEkhuey ideally we would check that *either* scope has a discriminator
+// but I don't know that it's possible to check that with FileCheck.
+// CHECK: ![[#var_inlinedAt2_scope]] = !DILexicalBlockFile
+// CHECK-SAME: discriminator: [[#]]
+extern crate macro_def;
+
+use std::env;
+
+fn square(n: i32) -> i32 {
+    n * n
+}
+
+fn main() {
+    let (z1, z2) = macro_def::square_twice!();
+    println!("{z1} == {z2}");
+}