111167.patch

   1 From 10b69dde3fd15334ea2382d2dc9e9a261de1afaf Mon Sep 17 00:00:00 2001
   2 From: Josh Stone <jistone@redhat.com>
   3 Date: Wed, 3 May 2023 15:52:31 -0700
   4 Subject: [PATCH] debuginfo: split method declaration and definition
   5
   6 When we're adding a method to a type DIE, we only want a DW_AT_declaration
   7 there, because LLVM LTO can't unify type definitions when a child DIE is a
   8 full subprogram definition. Now the subprogram definition gets added at the
   9 CU level with a specification link back to the abstract declaration.
  10 ---
  11  .../rustc_codegen_llvm/src/debuginfo/mod.rs   | 85 +++++++++++--------
  12  compiler/rustc_codegen_llvm/src/llvm/ffi.rs   | 15 ++++
  13  .../rustc_llvm/llvm-wrapper/RustWrapper.cpp   | 22 +++++
  14  .../issue-109934-lto-debuginfo/Makefile       | 12 +++
  15  .../issue-109934-lto-debuginfo/lib.rs         |  9 ++
  16  5 files changed, 109 insertions(+), 34 deletions(-)
  17  create mode 100644 tests/run-make/issue-109934-lto-debuginfo/Makefile
  18  create mode 100644 tests/run-make/issue-109934-lto-debuginfo/lib.rs
  19
  20 diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs
  21 index 2e9f89f419696..b138b0c0e70a1 100644
  22 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs
  23 +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs
  24 @@ -322,7 +322,7 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
  25          let tcx = self.tcx;
  26
  27          let def_id = instance.def_id();
  28 -        let containing_scope = get_containing_scope(self, instance);
  29 +        let (containing_scope, is_method) = get_containing_scope(self, instance);
  30          let span = tcx.def_span(def_id);
  31          let loc = self.lookup_debug_loc(span.lo());
  32          let file_metadata = file_metadata(self, &loc.file);
  33 @@ -378,8 +378,29 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
  34              }
  35          }
  36
  37 -        unsafe {
  38 -            return llvm::LLVMRustDIBuilderCreateFunction(
  39 +        // When we're adding a method to a type DIE, we only want a DW_AT_declaration there, because
  40 +        // LLVM LTO can't unify type definitions when a child DIE is a full subprogram definition.
  41 +        // When we use this `decl` below, the subprogram definition gets created at the CU level
  42 +        // with a DW_AT_specification pointing back to the type's declaration.
  43 +        let decl = is_method.then(|| unsafe {
  44 +            llvm::LLVMRustDIBuilderCreateMethod(
  45 +                DIB(self),
  46 +                containing_scope,
  47 +                name.as_ptr().cast(),
  48 +                name.len(),
  49 +                linkage_name.as_ptr().cast(),
  50 +                linkage_name.len(),
  51 +                file_metadata,
  52 +                loc.line,
  53 +                function_type_metadata,
  54 +                flags,
  55 +                spflags & !DISPFlags::SPFlagDefinition,
  56 +                template_parameters,
  57 +            )
  58 +        });
  59 +
  60 +        return unsafe {
  61 +            llvm::LLVMRustDIBuilderCreateFunction(
  62                  DIB(self),
  63                  containing_scope,
  64                  name.as_ptr().cast(),
  65 @@ -394,9 +415,9 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
  66                  spflags,
  67                  maybe_definition_llfn,
  68                  template_parameters,
  69 -                None,
  70 -            );
  71 -        }
  72 +                decl,
  73 +            )
  74 +        };
  75
  76          fn get_function_signature<'ll, 'tcx>(
  77              cx: &CodegenCx<'ll, 'tcx>,
  78 @@ -493,14 +514,16 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
  79              names
  80          }
  81
  82 +        /// Returns a scope, plus `true` if that's a type scope for "class" methods,
  83 +        /// otherwise `false` for plain namespace scopes.
  84          fn get_containing_scope<'ll, 'tcx>(
  85              cx: &CodegenCx<'ll, 'tcx>,
  86              instance: Instance<'tcx>,
  87 -        ) -> &'ll DIScope {
  88 +        ) -> (&'ll DIScope, bool) {
  89              // First, let's see if this is a method within an inherent impl. Because
  90              // if yes, we want to make the result subroutine DIE a child of the
  91              // subroutine's self-type.
  92 -            let self_type = cx.tcx.impl_of_method(instance.def_id()).and_then(|impl_def_id| {
  93 +            if let Some(impl_def_id) = cx.tcx.impl_of_method(instance.def_id()) {
  94                  // If the method does *not* belong to a trait, proceed
  95                  if cx.tcx.trait_id_of_impl(impl_def_id).is_none() {
  96                      let impl_self_ty = cx.tcx.subst_and_normalize_erasing_regions(
  97 @@ -511,39 +534,33 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
  98
  99                      // Only "class" methods are generally understood by LLVM,
 100                      // so avoid methods on other types (e.g., `<*mut T>::null`).
 101 -                    match impl_self_ty.kind() {
 102 -                        ty::Adt(def, ..) if !def.is_box() => {
 103 -                            // Again, only create type information if full debuginfo is enabled
 104 -                            if cx.sess().opts.debuginfo == DebugInfo::Full
 105 -                                && !impl_self_ty.needs_subst()
 106 -                            {
 107 -                                Some(type_di_node(cx, impl_self_ty))
 108 -                            } else {
 109 -                                Some(namespace::item_namespace(cx, def.did()))
 110 -                            }
 111 +                    if let ty::Adt(def, ..) = impl_self_ty.kind() && !def.is_box() {
 112 +                        // Again, only create type information if full debuginfo is enabled
 113 +                        if cx.sess().opts.debuginfo == DebugInfo::Full && !impl_self_ty.needs_subst()
 114 +                        {
 115 +                            return (type_di_node(cx, impl_self_ty), true);
 116 +                        } else {
 117 +                            return (namespace::item_namespace(cx, def.did()), false);
 118                          }
 119 -                        _ => None,
 120                      }
 121                  } else {
 122                      // For trait method impls we still use the "parallel namespace"
 123                      // strategy
 124 -                    None
 125                  }
 126 -            });
 127 +            }
 128
 129 -            self_type.unwrap_or_else(|| {
 130 -                namespace::item_namespace(
 131 -                    cx,
 132 -                    DefId {
 133 -                        krate: instance.def_id().krate,
 134 -                        index: cx
 135 -                            .tcx
 136 -                            .def_key(instance.def_id())
 137 -                            .parent
 138 -                            .expect("get_containing_scope: missing parent?"),
 139 -                    },
 140 -                )
 141 -            })
 142 +            let scope = namespace::item_namespace(
 143 +                cx,
 144 +                DefId {
 145 +                    krate: instance.def_id().krate,
 146 +                    index: cx
 147 +                        .tcx
 148 +                        .def_key(instance.def_id())
 149 +                        .parent
 150 +                        .expect("get_containing_scope: missing parent?"),
 151 +                },
 152 +            );
 153 +            (scope, false)
 154          }
 155      }
 156
 157 diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs
 158 index c95148013eb74..1f98d91c32054 100644
 159 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs
 160 +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs
 161 @@ -1987,6 +1987,21 @@ extern "C" {
 162          Decl: Option<&'a DIDescriptor>,
 163      ) -> &'a DISubprogram;
 164
 165 +    pub fn LLVMRustDIBuilderCreateMethod<'a>(
 166 +        Builder: &DIBuilder<'a>,
 167 +        Scope: &'a DIDescriptor,
 168 +        Name: *const c_char,
 169 +        NameLen: size_t,
 170 +        LinkageName: *const c_char,
 171 +        LinkageNameLen: size_t,
 172 +        File: &'a DIFile,
 173 +        LineNo: c_uint,
 174 +        Ty: &'a DIType,
 175 +        Flags: DIFlags,
 176 +        SPFlags: DISPFlags,
 177 +        TParam: &'a DIArray,
 178 +    ) -> &'a DISubprogram;
 179 +
 180      pub fn LLVMRustDIBuilderCreateBasicType<'a>(
 181          Builder: &DIBuilder<'a>,
 182          Name: *const c_char,
 183 diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
 184 index cadb6b1e23fe9..49acd71b3e106 100644
 185 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
 186 +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
 187 @@ -831,6 +831,28 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateFunction(
 188    return wrap(Sub);
 189  }
 190
 191 +extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateMethod(
 192 +    LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope,
 193 +    const char *Name, size_t NameLen,
 194 +    const char *LinkageName, size_t LinkageNameLen,
 195 +    LLVMMetadataRef File, unsigned LineNo,
 196 +    LLVMMetadataRef Ty, LLVMRustDIFlags Flags,
 197 +    LLVMRustDISPFlags SPFlags, LLVMMetadataRef TParam) {
 198 +  DITemplateParameterArray TParams =
 199 +      DITemplateParameterArray(unwrap<MDTuple>(TParam));
 200 +  DISubprogram::DISPFlags llvmSPFlags = fromRust(SPFlags);
 201 +  DINode::DIFlags llvmFlags = fromRust(Flags);
 202 +  DISubprogram *Sub = Builder->createMethod(
 203 +      unwrapDI<DIScope>(Scope),
 204 +      StringRef(Name, NameLen),
 205 +      StringRef(LinkageName, LinkageNameLen),
 206 +      unwrapDI<DIFile>(File), LineNo,
 207 +      unwrapDI<DISubroutineType>(Ty),
 208 +      0, 0, nullptr, // VTable params aren't used
 209 +      llvmFlags, llvmSPFlags, TParams);
 210 +  return wrap(Sub);
 211 +}
 212 +
 213  extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateBasicType(
 214      LLVMRustDIBuilderRef Builder, const char *Name, size_t NameLen,
 215      uint64_t SizeInBits, unsigned Encoding) {
 216 diff --git a/tests/run-make/issue-109934-lto-debuginfo/Makefile b/tests/run-make/issue-109934-lto-debuginfo/Makefile
 217 new file mode 100644
 218 index 0000000000000..3b7a99d3dbc62
 219 --- /dev/null
 220 +++ b/tests/run-make/issue-109934-lto-debuginfo/Makefile
 221 @@ -0,0 +1,12 @@
 222 +# ignore-cross-compile
 223 +include ../tools.mk
 224 +
 225 +# With the upgrade to LLVM 16, this was getting:
 226 +#
 227 +#   error: Cannot represent a difference across sections
 228 +#
 229 +# The error stemmed from DI function definitions under type scopes, fixed by
 230 +# only declaring in type scope and defining the subprogram elsewhere.
 231 +
 232 +all:
 233 +       $(RUSTC) lib.rs --test -C lto=fat -C debuginfo=2 -C incremental=$(TMPDIR)/inc-fat
 234 diff --git a/tests/run-make/issue-109934-lto-debuginfo/lib.rs b/tests/run-make/issue-109934-lto-debuginfo/lib.rs
 235 new file mode 100644
 236 index 0000000000000..c405928bd1824
 237 --- /dev/null
 238 +++ b/tests/run-make/issue-109934-lto-debuginfo/lib.rs
 239 @@ -0,0 +1,9 @@
 240 +extern crate alloc;
 241 +
 242 +#[cfg(test)]
 243 +mod tests {
 244 +    #[test]
 245 +    fn something_alloc() {
 246 +        assert_eq!(Vec::<u32>::new(), Vec::<u32>::new());
 247 +    }
 248 +}