Fix invalid DWARF for enums when using thinlto

We were setting the same identifier for both the DW_TAG_structure_type
and the DW_TAG_variant_part. This becomes a problem when using thinlto
becauses it uses the identifier as a key for a map of types that is used
to delete duplicates based on the ODR, so one of them is deleted as a
duplicate, resulting in invalid DWARF.

The DW_TAG_variant_part isn't a standalone type, so it doesn't need
an identifier. Fix by omitting its identifier.

Author: philipc

src/librustc_codegen_llvm/debuginfo/metadata.rs

@@ -266,6 +266,7 @@ impl RecursiveTypeDescription<'ll, 'tcx> {
                 // ... and attach them to the stub to complete it.
                 set_members_of_composite_type(cx,
                                               unfinished_type,
+                                              metadata_stub,
                                               member_holding_stub,
                                               member_descriptions);
                 return MetadataCreationResult::new(metadata_stub, true);
@@ -1215,6 +1216,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                 set_members_of_composite_type(cx,
                                               self.enum_type,
                                               variant_type_metadata,
+                                              variant_type_metadata,
                                               member_descriptions);
                 vec![
                     MemberDescription {
@@ -1256,6 +1258,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                     set_members_of_composite_type(cx,
                                                   self.enum_type,
                                                   variant_type_metadata,
+                                                  variant_type_metadata,
                                                   member_descriptions);
                     MemberDescription {
                         name: if fallback {
@@ -1298,6 +1301,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                     set_members_of_composite_type(cx,
                                                   self.enum_type,
                                                   variant_type_metadata,
+                                                  variant_type_metadata,
                                                   variant_member_descriptions);
 
                     // Encode the information about the null variant in the union
@@ -1358,6 +1362,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
                         set_members_of_composite_type(cx,
                                                       self.enum_type,
                                                       variant_type_metadata,
+                                                      variant_type_metadata,
                                                       member_descriptions);
 
                         let niche_value = if i == dataful_variant {
@@ -1698,8 +1703,7 @@ fn prepare_enum_metadata(
             layout.align.abi.bits() as u32,
             DIFlags::FlagZero,
             discriminator_metadata,
-            empty_array,
-            unique_type_id_str.as_ptr())
+            empty_array)
     };
 
     // The variant part must be wrapped in a struct according to DWARF.
@@ -1770,14 +1774,16 @@ fn composite_type_metadata(
     set_members_of_composite_type(cx,
                                   composite_type,
                                   composite_type_metadata,
+                                  composite_type_metadata,
                                   member_descriptions);
 
     composite_type_metadata
 }
 
 fn set_members_of_composite_type(cx: &CodegenCx<'ll, 'tcx>,
                                  composite_type: Ty<'tcx>,
-                                 composite_type_metadata: &'ll DICompositeType,
+                                 metadata_stub: &'ll DICompositeType,
+                                 member_holding_stub: &'ll DICompositeType,
                                  member_descriptions: Vec<MemberDescription<'ll>>) {
     // In some rare cases LLVM metadata uniquing would lead to an existing type
     // description being used instead of a new one created in
@@ -1788,11 +1794,11 @@ fn set_members_of_composite_type(cx: &CodegenCx<'ll, 'tcx>,
     {
         let mut composite_types_completed =
             debug_context(cx).composite_types_completed.borrow_mut();
-        if composite_types_completed.contains(&composite_type_metadata) {
+        if composite_types_completed.contains(&metadata_stub) {
             bug!("debuginfo::set_members_of_composite_type() - \
                   Already completed forward declaration re-encountered.");
         } else {
-            composite_types_completed.insert(composite_type_metadata);
+            composite_types_completed.insert(metadata_stub);
         }
     }
 
@@ -1803,7 +1809,7 @@ fn set_members_of_composite_type(cx: &CodegenCx<'ll, 'tcx>,
             unsafe {
                 Some(llvm::LLVMRustDIBuilderCreateVariantMemberType(
                     DIB(cx),
-                    composite_type_metadata,
+                    member_holding_stub,
                     member_name.as_ptr(),
                     unknown_file_metadata(cx),
                     UNKNOWN_LINE_NUMBER,
@@ -1824,7 +1830,7 @@ fn set_members_of_composite_type(cx: &CodegenCx<'ll, 'tcx>,
     unsafe {
         let type_array = create_DIArray(DIB(cx), &member_metadata[..]);
         llvm::LLVMRustDICompositeTypeReplaceArrays(
-            DIB(cx), composite_type_metadata, Some(type_array), type_params);
+            DIB(cx), member_holding_stub, Some(type_array), type_params);
     }
 }
 

src/librustc_codegen_llvm/llvm/ffi.rs

@@ -1587,8 +1587,7 @@ extern "C" {
                                               AlignInBits: u32,
                                               Flags: DIFlags,
                                               Discriminator: Option<&'a DIDerivedType>,
-                                              Elements: &'a DIArray,
-                                              UniqueId: *const c_char)
+                                              Elements: &'a DIArray)
                                               -> &'a DIDerivedType;
 
     pub fn LLVMSetUnnamedAddr(GlobalVar: &Value, UnnamedAddr: Bool);

src/rustllvm/RustWrapper.cpp

@@ -723,12 +723,12 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateVariantPart(
     LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope, const char *Name,
     LLVMMetadataRef File, unsigned LineNumber, uint64_t SizeInBits,
     uint32_t AlignInBits, LLVMRustDIFlags Flags, LLVMMetadataRef Discriminator,
-    LLVMMetadataRef Elements, const char *UniqueId) {
+    LLVMMetadataRef Elements) {
 #if LLVM_VERSION_GE(7, 0)
   return wrap(Builder->createVariantPart(
       unwrapDI<DIDescriptor>(Scope), Name, unwrapDI<DIFile>(File), LineNumber,
       SizeInBits, AlignInBits, fromRust(Flags), unwrapDI<DIDerivedType>(Discriminator),
-      DINodeArray(unwrapDI<MDTuple>(Elements)), UniqueId));
+      DINodeArray(unwrapDI<MDTuple>(Elements))));
 #else
   abort();
 #endif

src/test/debuginfo/enum-thinlto.rs

@@ -0,0 +1,48 @@
+// ignore-tidy-linelength
+
+// Require LLVM with DW_TAG_variant_part and a gdb that can read it.
+// min-system-llvm-version: 7.0
+// min-gdb-version: 8.2
+
+// compile-flags:-g -Z thinlto
+
+// === GDB TESTS ===================================================================================
+
+// gdb-command:run
+
+// gdb-command:print *abc
+// gdbr-check:$1 = enum_thinlto::ABC::TheA{x: 0, y: 8970181431921507452}
+
+// === LLDB TESTS ==================================================================================
+
+// lldb-command:run
+
+// lldb-command:print *abc
+// lldbg-check:(enum_thinlto::ABC) $0 = ABC { }
+
+#![allow(unused_variables)]
+#![feature(omit_gdb_pretty_printer_section)]
+#![omit_gdb_pretty_printer_section]
+
+// The first element is to ensure proper alignment, irrespective of the machines word size. Since
+// the size of the discriminant value is machine dependent, this has be taken into account when
+// datatype layout should be predictable as in this case.
+#[derive(Debug)]
+enum ABC {
+    TheA { x: i64, y: i64 },
+    TheB (i64, i32, i32),
+}
+
+fn main() {
+    let abc = ABC::TheA { x: 0, y: 0x7c7c_7c7c_7c7c_7c7c };
+
+    f(&abc);
+}
+
+fn f(abc: &ABC) {
+    zzz(); // #break
+
+    println!("{:?}", abc);
+}
+
+fn zzz() {()}