From c32ef5931b21442e162be3e35f620be13d41feb7 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Wed, 29 Nov 2017 14:42:25 -0700 Subject: [PATCH 1/5] Fix DWARF generation for enums The DWARF generated for Rust enums was always somewhat unusual. Rather than using DWARF constructs directly, it would emit magic field names like "RUST$ENCODED$ENUM$0$Name" and "RUST$ENUM$DISR". Since PR #45225, though, even this has not worked -- the ad hoc scheme was not updated to handle the wider variety of niche-filling layout optimizations now available. This patch changes the generated DWARF to use the standard tags meant for this purpose; namely, DW_TAG_variant and DW_TAG_variant_part. The patch to implement this went in to LLVM 7. In order to work with older versions of LLVM, and because LLVM doesn't do anything here for PDB, the existing code is kept as a fallback mode. Support for this DWARF is in the Rust lldb and in gdb 8.2. Closes #32920 Closes #32924 Closes #52762 Closes #53153 --- .../debuginfo/metadata.rs | 522 +++++++++++++----- src/librustc_codegen_llvm/llvm/ffi.rs | 29 +- src/llvm | 2 +- src/rustllvm/RustWrapper.cpp | 49 +- src/test/codegen/enum-debug.rs | 40 ++ 5 files changed, 494 insertions(+), 148 deletions(-) create mode 100644 src/test/codegen/enum-debug.rs diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index 706568b544661..9f0861dff40e9 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -31,9 +31,9 @@ use rustc::hir::def_id::{DefId, CrateNum, LOCAL_CRATE}; use rustc::ich::NodeIdHashingMode; use rustc_data_structures::fingerprint::Fingerprint; use rustc::ty::Instance; -use common::CodegenCx; +use common::{CodegenCx, C_u64}; use rustc::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt}; -use rustc::ty::layout::{self, Align, LayoutOf, PrimitiveExt, Size, TyLayout}; +use rustc::ty::layout::{self, Align, Integer, IntegerExt, LayoutOf, PrimitiveExt, Size, TyLayout}; use rustc::session::config; use rustc::util::nodemap::FxHashMap; use rustc_fs_util::path2cstr; @@ -213,6 +213,7 @@ enum RecursiveTypeDescription<'ll, 'tcx> { unfinished_type: Ty<'tcx>, unique_type_id: UniqueTypeId, metadata_stub: &'ll DICompositeType, + member_holding_stub: &'ll DICompositeType, member_description_factory: MemberDescriptionFactory<'ll, 'tcx>, }, FinalMetadata(&'ll DICompositeType) @@ -223,6 +224,7 @@ fn create_and_register_recursive_type_forward_declaration( unfinished_type: Ty<'tcx>, unique_type_id: UniqueTypeId, metadata_stub: &'ll DICompositeType, + member_holding_stub: &'ll DICompositeType, member_description_factory: MemberDescriptionFactory<'ll, 'tcx>, ) -> RecursiveTypeDescription<'ll, 'tcx> { @@ -235,6 +237,7 @@ fn create_and_register_recursive_type_forward_declaration( unfinished_type, unique_type_id, metadata_stub, + member_holding_stub, member_description_factory, } } @@ -250,6 +253,7 @@ impl RecursiveTypeDescription<'ll, 'tcx> { unfinished_type, unique_type_id, metadata_stub, + member_holding_stub, ref member_description_factory, } => { // Make sure that we have a forward declaration of the type in @@ -274,7 +278,7 @@ impl RecursiveTypeDescription<'ll, 'tcx> { // ... and attach them to the stub to complete it. set_members_of_composite_type(cx, - metadata_stub, + member_holding_stub, member_descriptions); return MetadataCreationResult::new(metadata_stub, true); } @@ -358,6 +362,7 @@ fn vec_slice_metadata( size: pointer_size, align: pointer_align, flags: DIFlags::FlagZero, + discriminant: None, }, MemberDescription { name: "length".to_string(), @@ -366,6 +371,7 @@ fn vec_slice_metadata( size: usize_size, align: usize_align, flags: DIFlags::FlagZero, + discriminant: None, }, ]; @@ -471,6 +477,7 @@ fn trait_pointer_metadata( size: data_ptr_field.size, align: data_ptr_field.align, flags: DIFlags::FlagArtificial, + discriminant: None, }, MemberDescription { name: "vtable".to_string(), @@ -479,6 +486,7 @@ fn trait_pointer_metadata( size: vtable_field.size, align: vtable_field.align, flags: DIFlags::FlagArtificial, + discriminant: None, }, ]; @@ -927,6 +935,7 @@ struct MemberDescription<'ll> { size: Size, align: Align, flags: DIFlags, + discriminant: Option, } // A factory for MemberDescriptions. It produces a list of member descriptions @@ -994,6 +1003,7 @@ impl<'tcx> StructMemberDescriptionFactory<'tcx> { size, align, flags: DIFlags::FlagZero, + discriminant: None, } }).collect() } @@ -1026,6 +1036,7 @@ fn prepare_struct_metadata( struct_type, unique_type_id, struct_metadata_stub, + struct_metadata_stub, StructMDF(StructMemberDescriptionFactory { ty: struct_type, variant, @@ -1058,6 +1069,7 @@ impl<'tcx> TupleMemberDescriptionFactory<'tcx> { size, align, flags: DIFlags::FlagZero, + discriminant: None, } }).collect() } @@ -1072,15 +1084,18 @@ fn prepare_tuple_metadata( ) -> RecursiveTypeDescription<'ll, 'tcx> { let tuple_name = compute_debuginfo_type_name(cx, tuple_type, false); + let struct_stub = create_struct_stub(cx, + tuple_type, + &tuple_name[..], + unique_type_id, + NO_SCOPE_METADATA); + create_and_register_recursive_type_forward_declaration( cx, tuple_type, unique_type_id, - create_struct_stub(cx, - tuple_type, - &tuple_name[..], - unique_type_id, - NO_SCOPE_METADATA), + struct_stub, + struct_stub, TupleMDF(TupleMemberDescriptionFactory { ty: tuple_type, component_types: component_types.to_vec(), @@ -1112,6 +1127,7 @@ impl<'tcx> UnionMemberDescriptionFactory<'tcx> { size, align, flags: DIFlags::FlagZero, + discriminant: None, } }).collect() } @@ -1143,6 +1159,7 @@ fn prepare_union_metadata( union_type, unique_type_id, union_metadata_stub, + union_metadata_stub, UnionMDF(UnionMemberDescriptionFactory { layout: cx.layout_of(union_type), variant, @@ -1155,6 +1172,20 @@ fn prepare_union_metadata( // Enums //=----------------------------------------------------------------------------- +// DWARF variant support is only available starting in LLVM 7. +// Although the earlier enum debug info output did not work properly +// in all situations, it is better for the time being to continue to +// sometimes emit the old style rather than emit something completely +// useless when rust is compiled against LLVM 6 or older. This +// function decides which representation will be emitted. +fn use_enum_fallback(cx: &CodegenCx) -> bool { + // On MSVC we have to use the fallback mode, because LLVM doesn't + // lower variant parts to PDB. + return cx.sess().target.target.options.is_like_msvc || unsafe { + llvm::LLVMRustVersionMajor() < 7 + }; +} + // Describes the members of an enum value: An enum is described as a union of // structs in DWARF. This MemberDescriptionFactory provides the description for // the members of this union; so for every variant of the given enum, this @@ -1172,6 +1203,15 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { fn create_member_descriptions(&self, cx: &CodegenCx<'ll, 'tcx>) -> Vec> { let adt = &self.enum_type.ty_adt_def().unwrap(); + + // This will always find the metadata in the type map. + let fallback = use_enum_fallback(cx); + let self_metadata = if fallback { + self.containing_scope + } else { + type_metadata(cx, self.enum_type, self.span) + }; + match self.layout.variants { layout::Variants::Single { .. } if adt.variants.is_empty() => vec![], layout::Variants::Single { index } => { @@ -1180,7 +1220,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { self.layout, &adt.variants[index], NoDiscriminant, - self.containing_scope, + self_metadata, self.span); let member_descriptions = @@ -1191,18 +1231,28 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { member_descriptions); vec![ MemberDescription { - name: String::new(), + name: if fallback { + String::new() + } else { + adt.variants[index].name.as_str().to_string() + }, type_metadata: variant_type_metadata, offset: Size::ZERO, size: self.layout.size, align: self.layout.align, - flags: DIFlags::FlagZero + flags: DIFlags::FlagZero, + discriminant: None, } ] } layout::Variants::Tagged { ref variants, .. } => { - let discriminant_info = RegularDiscriminant(self.discriminant_type_metadata - .expect("")); + let discriminant_info = if fallback { + RegularDiscriminant(self.discriminant_type_metadata + .expect("")) + } else { + // This doesn't matter in this case. + NoDiscriminant + }; (0..variants.len()).map(|i| { let variant = self.layout.for_variant(cx, i); let (variant_type_metadata, member_desc_factory) = @@ -1210,7 +1260,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { variant, &adt.variants[i], discriminant_info, - self.containing_scope, + self_metadata, self.span); let member_descriptions = member_desc_factory @@ -1220,75 +1270,124 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { variant_type_metadata, member_descriptions); MemberDescription { - name: String::new(), + name: if fallback { + String::new() + } else { + adt.variants[i].name.as_str().to_string() + }, type_metadata: variant_type_metadata, offset: Size::ZERO, - size: variant.size, - align: variant.align, - flags: DIFlags::FlagZero + size: self.layout.size, + align: self.layout.align, + flags: DIFlags::FlagZero, + discriminant: Some(self.layout.ty.ty_adt_def().unwrap() + .discriminant_for_variant(cx.tcx, i) + .val as u64), } }).collect() } - layout::Variants::NicheFilling { dataful_variant, ref niche_variants, .. } => { - let variant = self.layout.for_variant(cx, dataful_variant); - // Create a description of the non-null variant - let (variant_type_metadata, member_description_factory) = - describe_enum_variant(cx, - variant, - &adt.variants[dataful_variant], - OptimizedDiscriminant, - self.containing_scope, - self.span); + layout::Variants::NicheFilling { + ref niche_variants, + niche_start, + ref variants, + dataful_variant, + .. + } => { + if fallback { + let variant = self.layout.for_variant(cx, dataful_variant); + // Create a description of the non-null variant + let (variant_type_metadata, member_description_factory) = + describe_enum_variant(cx, + variant, + &adt.variants[dataful_variant], + OptimizedDiscriminant, + self.containing_scope, + self.span); - let variant_member_descriptions = - member_description_factory.create_member_descriptions(cx); + let variant_member_descriptions = + member_description_factory.create_member_descriptions(cx); - set_members_of_composite_type(cx, - variant_type_metadata, - variant_member_descriptions); - - // Encode the information about the null variant in the union - // member's name. - let mut name = String::from("RUST$ENCODED$ENUM$"); - // HACK(eddyb) the debuggers should just handle offset+size - // of discriminant instead of us having to recover its path. - // Right now it's not even going to work for `niche_start > 0`, - // and for multiple niche variants it only supports the first. - fn compute_field_path<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, - name: &mut String, - layout: TyLayout<'tcx>, - offset: Size, - size: Size) { - for i in 0..layout.fields.count() { - let field_offset = layout.fields.offset(i); - if field_offset > offset { - continue; - } - let inner_offset = offset - field_offset; - let field = layout.field(cx, i); - if inner_offset + size <= field.size { - write!(name, "{}$", i).unwrap(); - compute_field_path(cx, name, field, inner_offset, size); + set_members_of_composite_type(cx, + variant_type_metadata, + variant_member_descriptions); + + // Encode the information about the null variant in the union + // member's name. + let mut name = String::from("RUST$ENCODED$ENUM$"); + // Right now it's not even going to work for `niche_start > 0`, + // and for multiple niche variants it only supports the first. + fn compute_field_path<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>, + name: &mut String, + layout: TyLayout<'tcx>, + offset: Size, + size: Size) { + for i in 0..layout.fields.count() { + let field_offset = layout.fields.offset(i); + if field_offset > offset { + continue; + } + let inner_offset = offset - field_offset; + let field = layout.field(cx, i); + if inner_offset + size <= field.size { + write!(name, "{}$", i).unwrap(); + compute_field_path(cx, name, field, inner_offset, size); + } } } + compute_field_path(cx, &mut name, + self.layout, + self.layout.fields.offset(0), + self.layout.field(cx, 0).size); + name.push_str(&adt.variants[*niche_variants.start()].name.as_str()); + + // Create the (singleton) list of descriptions of union members. + vec![ + MemberDescription { + name, + type_metadata: variant_type_metadata, + offset: Size::ZERO, + size: variant.size, + align: variant.align, + flags: DIFlags::FlagZero, + discriminant: None, + } + ] + } else { + (0..variants.len()).map(|i| { + let variant = self.layout.for_variant(cx, i); + let (variant_type_metadata, member_desc_factory) = + describe_enum_variant(cx, + variant, + &adt.variants[i], + OptimizedDiscriminant, + self_metadata, + self.span); + + let member_descriptions = member_desc_factory + .create_member_descriptions(cx); + + set_members_of_composite_type(cx, + variant_type_metadata, + member_descriptions); + + let niche_value = if i == dataful_variant { + None + } else { + Some((i.wrapping_sub(*niche_variants.start()) as u128) + .wrapping_add(niche_start) as u64) + }; + + MemberDescription { + name: adt.variants[i].name.as_str().to_string(), + type_metadata: variant_type_metadata, + offset: Size::ZERO, + size: self.layout.size, + align: self.layout.align, + flags: DIFlags::FlagZero, + discriminant: niche_value, + } + }).collect() } - compute_field_path(cx, &mut name, - self.layout, - self.layout.fields.offset(0), - self.layout.field(cx, 0).size); - name.push_str(&adt.variants[*niche_variants.start()].name.as_str()); - - // Create the (singleton) list of descriptions of union members. - vec![ - MemberDescription { - name, - type_metadata: variant_type_metadata, - offset: Size::ZERO, - size: variant.size, - align: variant.align, - flags: DIFlags::FlagZero - } - ] } } } @@ -1310,14 +1409,19 @@ impl VariantMemberDescriptionFactory<'ll, 'tcx> { let (size, align) = cx.size_and_align_of(ty); MemberDescription { name: name.to_string(), - type_metadata: match self.discriminant_type_metadata { - Some(metadata) if i == 0 => metadata, - _ => type_metadata(cx, ty, self.span) + type_metadata: if use_enum_fallback(cx) { + match self.discriminant_type_metadata { + Some(metadata) if i == 0 => metadata, + _ => type_metadata(cx, ty, self.span) + } + } else { + type_metadata(cx, ty, self.span) }, offset: self.offsets[i], size, align, - flags: DIFlags::FlagZero + flags: DIFlags::FlagZero, + discriminant: None, } }).collect() } @@ -1330,10 +1434,10 @@ enum EnumDiscriminantInfo<'ll> { NoDiscriminant } -// Returns a tuple of (1) type_metadata_stub of the variant, (2) the llvm_type -// of the variant, and (3) a MemberDescriptionFactory for producing the -// descriptions of the fields of the variant. This is a rudimentary version of a -// full RecursiveTypeDescription. +// Returns a tuple of (1) type_metadata_stub of the variant, (2) a +// MemberDescriptionFactory for producing the descriptions of the +// fields of the variant. This is a rudimentary version of a full +// RecursiveTypeDescription. fn describe_enum_variant( cx: &CodegenCx<'ll, 'tcx>, layout: layout::TyLayout<'tcx>, @@ -1356,29 +1460,46 @@ fn describe_enum_variant( unique_type_id, Some(containing_scope)); - // If this is not a univariant enum, there is also the discriminant field. - let (discr_offset, discr_arg) = match discriminant_info { - RegularDiscriminant(_) => { - // We have the layout of an enum variant, we need the layout of the outer enum - let enum_layout = cx.layout_of(layout.ty); - (Some(enum_layout.fields.offset(0)), - Some(("RUST$ENUM$DISR".to_string(), enum_layout.field(cx, 0).ty))) - } - _ => (None, None), - }; - let offsets = discr_offset.into_iter().chain((0..layout.fields.count()).map(|i| { - layout.fields.offset(i) - })).collect(); - // Build an array of (field name, field type) pairs to be captured in the factory closure. - let args = discr_arg.into_iter().chain((0..layout.fields.count()).map(|i| { - let name = if variant.ctor_kind == CtorKind::Fn { - format!("__{}", i) - } else { - variant.fields[i].ident.to_string() + let (offsets, args) = if use_enum_fallback(cx) { + // If this is not a univariant enum, there is also the discriminant field. + let (discr_offset, discr_arg) = match discriminant_info { + RegularDiscriminant(_) => { + // We have the layout of an enum variant, we need the layout of the outer enum + let enum_layout = cx.layout_of(layout.ty); + (Some(enum_layout.fields.offset(0)), + Some(("RUST$ENUM$DISR".to_string(), enum_layout.field(cx, 0).ty))) + } + _ => (None, None), }; - (name, layout.field(cx, i).ty) - })).collect(); + ( + discr_offset.into_iter().chain((0..layout.fields.count()).map(|i| { + layout.fields.offset(i) + })).collect(), + discr_arg.into_iter().chain((0..layout.fields.count()).map(|i| { + let name = if variant.ctor_kind == CtorKind::Fn { + format!("__{}", i) + } else { + variant.fields[i].ident.to_string() + }; + (name, layout.field(cx, i).ty) + })).collect() + ) + } else { + ( + (0..layout.fields.count()).map(|i| { + layout.fields.offset(i) + }).collect(), + (0..layout.fields.count()).map(|i| { + let name = if variant.ctor_kind == CtorKind::Fn { + format!("__{}", i) + } else { + variant.fields[i].ident.to_string() + }; + (name, layout.field(cx, i).ty) + }).collect() + ) + }; let member_description_factory = VariantMDF(VariantMemberDescriptionFactory { @@ -1414,22 +1535,22 @@ fn prepare_enum_metadata( // let file_metadata = unknown_file_metadata(cx); - let def = enum_type.ty_adt_def().unwrap(); - let enumerators_metadata: Vec<_> = def.discriminants(cx.tcx) - .zip(&def.variants) - .map(|(discr, v)| { - let name = SmallCStr::new(&v.name.as_str()); - unsafe { - Some(llvm::LLVMRustDIBuilderCreateEnumerator( - DIB(cx), - name.as_ptr(), - // FIXME: what if enumeration has i128 discriminant? - discr.val as u64)) - } - }) - .collect(); - let discriminant_type_metadata = |discr: layout::Primitive| { + let def = enum_type.ty_adt_def().unwrap(); + let enumerators_metadata: Vec<_> = def.discriminants(cx.tcx) + .zip(&def.variants) + .map(|(discr, v)| { + let name = SmallCStr::new(&v.name.as_str()); + unsafe { + Some(llvm::LLVMRustDIBuilderCreateEnumerator( + DIB(cx), + name.as_ptr(), + // FIXME: what if enumeration has i128 discriminant? + discr.val as u64)) + } + }) + .collect(); + let disr_type_key = (enum_def_id, discr); let cached_discriminant_type_metadata = debug_context(cx).created_enum_disr_types .borrow() @@ -1454,7 +1575,7 @@ fn prepare_enum_metadata( discriminant_size.bits(), discriminant_align.abi_bits() as u32, create_DIArray(DIB(cx), &enumerators_metadata), - discriminant_base_type_metadata) + discriminant_base_type_metadata, true) }; debug_context(cx).created_enum_disr_types @@ -1468,16 +1589,9 @@ fn prepare_enum_metadata( let layout = cx.layout_of(enum_type); - let discriminant_type_metadata = match layout.variants { - layout::Variants::Single { .. } | - layout::Variants::NicheFilling { .. } => None, - layout::Variants::Tagged { ref tag, .. } => { - Some(discriminant_type_metadata(tag.value)) - } - }; - - match (&layout.abi, discriminant_type_metadata) { - (&layout::Abi::Scalar(_), Some(discr)) => return FinalMetadata(discr), + match (&layout.abi, &layout.variants) { + (&layout::Abi::Scalar(_), &layout::Variants::Tagged {ref tag, .. }) => + return FinalMetadata(discriminant_type_metadata(tag.value)), _ => {} } @@ -1487,30 +1601,146 @@ fn prepare_enum_metadata( let unique_type_id_str = SmallCStr::new( debug_context(cx).type_map.borrow().get_unique_type_id_as_string(unique_type_id) ); - let enum_metadata = unsafe { - llvm::LLVMRustDIBuilderCreateUnionType( - DIB(cx), - containing_scope, - enum_name.as_ptr(), - file_metadata, - UNKNOWN_LINE_NUMBER, - enum_type_size.bits(), - enum_type_align.abi_bits() as u32, - DIFlags::FlagZero, - None, - 0, // RuntimeLang - unique_type_id_str.as_ptr()) + + if use_enum_fallback(cx) { + let discriminant_type_metadata = match layout.variants { + layout::Variants::Single { .. } | + layout::Variants::NicheFilling { .. } => None, + layout::Variants::Tagged { ref tag, .. } => { + Some(discriminant_type_metadata(tag.value)) + } + }; + + let enum_metadata = unsafe { + llvm::LLVMRustDIBuilderCreateUnionType( + DIB(cx), + containing_scope, + enum_name.as_ptr(), + file_metadata, + UNKNOWN_LINE_NUMBER, + enum_type_size.bits(), + enum_type_align.abi_bits() as u32, + DIFlags::FlagZero, + None, + 0, // RuntimeLang + unique_type_id_str.as_ptr()) + }; + + return create_and_register_recursive_type_forward_declaration( + cx, + enum_type, + unique_type_id, + enum_metadata, + enum_metadata, + EnumMDF(EnumMemberDescriptionFactory { + enum_type, + layout, + discriminant_type_metadata, + containing_scope, + span, + }), + ); + } + + let discriminator_metadata = match &layout.variants { + // A single-variant enum has no discriminant. + &layout::Variants::Single { .. } => None, + + &layout::Variants::NicheFilling { ref niche, .. } => { + // Find the integer type of the correct size. + let discr_type = niche.value.to_ty(cx.tcx); + let (size, align) = cx.size_and_align_of(discr_type); + + let discr_type = (match size.bits() { + 8 => Integer::I8, + 16 => Integer::I16, + 32 => Integer::I32, + 64 => Integer::I64, + bits => bug!("prepare_enum_metadata: unknown niche bit size {}", bits), + }).to_ty(cx.tcx, false); + + let discr_metadata = basic_type_metadata(cx, discr_type); + unsafe { + Some(llvm::LLVMRustDIBuilderCreateMemberType( + DIB(cx), + containing_scope, + ptr::null_mut(), + file_metadata, + UNKNOWN_LINE_NUMBER, + size.bits(), + align.abi_bits() as u32, + layout.fields.offset(0).bits(), + DIFlags::FlagArtificial, + discr_metadata)) + } + }, + + &layout::Variants::Tagged { ref tag, .. } => { + let discr_type = tag.value.to_ty(cx.tcx); + let (size, align) = cx.size_and_align_of(discr_type); + + let discr_metadata = basic_type_metadata(cx, discr_type); + unsafe { + Some(llvm::LLVMRustDIBuilderCreateMemberType( + DIB(cx), + containing_scope, + ptr::null_mut(), + file_metadata, + UNKNOWN_LINE_NUMBER, + size.bits(), + align.abi_bits() as u32, + layout.fields.offset(0).bits(), + DIFlags::FlagArtificial, + discr_metadata)) + } + }, + }; + + let empty_array = create_DIArray(DIB(cx), &[]); + let variant_part = unsafe { + llvm::LLVMRustDIBuilderCreateVariantPart( + DIB(cx), + containing_scope, + ptr::null_mut(), + file_metadata, + UNKNOWN_LINE_NUMBER, + enum_type_size.bits(), + enum_type_align.abi_bits() as u32, + DIFlags::FlagZero, + discriminator_metadata, + empty_array, + unique_type_id_str.as_ptr()) + }; + + // The variant part must be wrapped in a struct according to DWARF. + let type_array = create_DIArray(DIB(cx), &[Some(variant_part)]); + let struct_wrapper = unsafe { + llvm::LLVMRustDIBuilderCreateStructType( + DIB(cx), + Some(containing_scope), + enum_name.as_ptr(), + file_metadata, + UNKNOWN_LINE_NUMBER, + enum_type_size.bits(), + enum_type_align.abi_bits() as u32, + DIFlags::FlagZero, + None, + type_array, + 0, + None, + unique_type_id_str.as_ptr()) }; return create_and_register_recursive_type_forward_declaration( cx, enum_type, unique_type_id, - enum_metadata, + struct_wrapper, + variant_part, EnumMDF(EnumMemberDescriptionFactory { enum_type, layout, - discriminant_type_metadata, + discriminant_type_metadata: None, containing_scope, span, }), @@ -1579,7 +1809,7 @@ fn set_members_of_composite_type(cx: &CodegenCx<'ll, '_>, .map(|member_description| { let member_name = CString::new(member_description.name).unwrap(); unsafe { - Some(llvm::LLVMRustDIBuilderCreateMemberType( + Some(llvm::LLVMRustDIBuilderCreateVariantMemberType( DIB(cx), composite_type_metadata, member_name.as_ptr(), @@ -1588,6 +1818,10 @@ fn set_members_of_composite_type(cx: &CodegenCx<'ll, '_>, member_description.size.bits(), member_description.align.abi_bits() as u32, member_description.offset.bits(), + match member_description.discriminant { + None => None, + Some(value) => Some(C_u64(cx, value)), + }, member_description.flags, member_description.type_metadata)) } diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index a5f4137c62b14..3373e81081589 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -1301,6 +1301,19 @@ extern "C" { Ty: &'a DIType) -> &'a DIDerivedType; + pub fn LLVMRustDIBuilderCreateVariantMemberType(Builder: &DIBuilder<'a>, + Scope: &'a DIScope, + Name: *const c_char, + File: &'a DIFile, + LineNumber: c_uint, + SizeInBits: u64, + AlignInBits: u32, + OffsetInBits: u64, + Discriminant: Option<&'a Value>, + Flags: DIFlags, + Ty: &'a DIType) + -> &'a DIType; + pub fn LLVMRustDIBuilderCreateLexicalBlock(Builder: &DIBuilder<'a>, Scope: &'a DIScope, File: &'a DIFile, @@ -1378,7 +1391,8 @@ extern "C" { SizeInBits: u64, AlignInBits: u32, Elements: &'a DIArray, - ClassType: &'a DIType) + ClassType: &'a DIType, + IsFixed: bool) -> &'a DIType; pub fn LLVMRustDIBuilderCreateUnionType(Builder: &DIBuilder<'a>, @@ -1394,6 +1408,19 @@ extern "C" { UniqueId: *const c_char) -> &'a DIType; + pub fn LLVMRustDIBuilderCreateVariantPart(Builder: &DIBuilder<'a>, + Scope: &'a DIScope, + Name: *const c_char, + File: &'a DIFile, + LineNo: c_uint, + SizeInBits: u64, + AlignInBits: u32, + Flags: DIFlags, + Discriminator: Option<&'a DIDerivedType>, + Elements: &'a DIArray, + UniqueId: *const c_char) + -> &'a DIDerivedType; + pub fn LLVMSetUnnamedAddr(GlobalVar: &Value, UnnamedAddr: Bool); pub fn LLVMRustDIBuilderCreateTemplateTypeParameter(Builder: &DIBuilder<'a>, diff --git a/src/llvm b/src/llvm index caddcd9b9dc94..7051ead40a5f8 160000 --- a/src/llvm +++ b/src/llvm @@ -1 +1 @@ -Subproject commit caddcd9b9dc9479a20908d93c3e47c49b021379e +Subproject commit 7051ead40a5f825878b59bf08d4e768be9e99a4a diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index 9b9c908ea5272..5aab7224f5c56 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -682,6 +682,21 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateStructType( unwrapDI(VTableHolder), UniqueId)); } +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) { +#if LLVM_VERSION_GE(7, 0) + return wrap(Builder->createVariantPart( + unwrapDI(Scope), Name, unwrapDI(File), LineNumber, + SizeInBits, AlignInBits, fromRust(Flags), unwrapDI(Discriminator), + DINodeArray(unwrapDI(Elements)), UniqueId)); +#else + abort(); +#endif +} + extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateMemberType( LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope, const char *Name, LLVMMetadataRef File, unsigned LineNo, uint64_t SizeInBits, @@ -693,6 +708,28 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateMemberType( fromRust(Flags), unwrapDI(Ty))); } +extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateVariantMemberType( + LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope, + const char *Name, LLVMMetadataRef File, unsigned LineNo, uint64_t SizeInBits, + uint32_t AlignInBits, uint64_t OffsetInBits, LLVMValueRef Discriminant, + LLVMRustDIFlags Flags, LLVMMetadataRef Ty) { +#if LLVM_VERSION_GE(7, 0) + llvm::ConstantInt* D = nullptr; + if (Discriminant) { + D = unwrap(Discriminant); + } + return wrap(Builder->createVariantMemberType(unwrapDI(Scope), Name, + unwrapDI(File), LineNo, + SizeInBits, AlignInBits, OffsetInBits, D, + fromRust(Flags), unwrapDI(Ty))); +#else + return wrap(Builder->createMemberType(unwrapDI(Scope), Name, + unwrapDI(File), LineNo, + SizeInBits, AlignInBits, OffsetInBits, + fromRust(Flags), unwrapDI(Ty))); +#endif +} + extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateLexicalBlock( LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope, LLVMMetadataRef File, unsigned Line, unsigned Col) { @@ -795,11 +832,19 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateEnumerationType( LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope, const char *Name, LLVMMetadataRef File, unsigned LineNumber, uint64_t SizeInBits, uint32_t AlignInBits, LLVMMetadataRef Elements, - LLVMMetadataRef ClassTy) { + LLVMMetadataRef ClassTy, bool IsFixed) { +#if LLVM_VERSION_GE(7, 0) return wrap(Builder->createEnumerationType( unwrapDI(Scope), Name, unwrapDI(File), LineNumber, SizeInBits, AlignInBits, DINodeArray(unwrapDI(Elements)), - unwrapDI(ClassTy))); + unwrapDI(ClassTy), "", IsFixed)); +#else + // Ignore IsFixed on older LLVM. + return wrap(Builder->createEnumerationType( + unwrapDI(Scope), Name, unwrapDI(File), LineNumber, + SizeInBits, AlignInBits, DINodeArray(unwrapDI(Elements)), + unwrapDI(ClassTy), "")); +#endif } extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateUnionType( diff --git a/src/test/codegen/enum-debug.rs b/src/test/codegen/enum-debug.rs new file mode 100644 index 0000000000000..2bcac922910c0 --- /dev/null +++ b/src/test/codegen/enum-debug.rs @@ -0,0 +1,40 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// This test depends on a patch that was committed to upstream LLVM +// before 7.0, then backported to the Rust LLVM fork. It tests that +// optimized enum debug info accurately reflects the enum layout. + +// ignore-tidy-linelength +// ignore-windows +// min-system-llvm-version 7.0 + +// compile-flags: -g -C no-prepopulate-passes + +// CHECK-LABEL: @main +// CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_variant_part,{{.*}}discriminator:{{.*}} +// CHECK: {{.*}}DIDerivedType{{.*}}tag: DW_TAG_member,{{.*}}name: "A",{{.*}}extraData:{{.*}} +// CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_structure_type,{{.*}}name: "A",{{.*}} +// CHECK: {{.*}}DIDerivedType{{.*}}tag: DW_TAG_member,{{.*}}name: "B",{{.*}}extraData:{{.*}} +// CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_structure_type,{{.*}}name: "B",{{.*}} +// CHECK: {{.*}}DIDerivedType{{.*}}tag: DW_TAG_member,{{.*}}name: "C",{{.*}}extraData:{{.*}} +// CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_structure_type,{{.*}}name: "C",{{.*}} +// CHECK: {{.*}}DIDerivedType{{.*}}tag: DW_TAG_member,{{.*}}name: "D",{{.*}} +// CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_structure_type,{{.*}}name: "D",{{.*}} + +#![allow(dead_code)] +#![allow(unused_variables)] +#![allow(unused_assignments)] + +enum E { A, B, C, D(bool) } + +pub fn main() { + let e = E::D(true); +} From f725771230c19e87b3578be474fe6f04dd1bc340 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 7 Sep 2018 09:29:40 -0600 Subject: [PATCH 2/5] Address review comments This fixes the issues pointed out in review. --- .../debuginfo/metadata.rs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index 9f0861dff40e9..14854d7aa1631 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -33,7 +33,8 @@ use rustc_data_structures::fingerprint::Fingerprint; use rustc::ty::Instance; use common::{CodegenCx, C_u64}; use rustc::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt}; -use rustc::ty::layout::{self, Align, Integer, IntegerExt, LayoutOf, PrimitiveExt, Size, TyLayout}; +use rustc::ty::layout::{self, Align, HasDataLayout, Integer, IntegerExt, LayoutOf, + PrimitiveExt, Size, TyLayout}; use rustc::session::config; use rustc::util::nodemap::FxHashMap; use rustc_fs_util::path2cstr; @@ -1648,16 +1649,15 @@ fn prepare_enum_metadata( &layout::Variants::NicheFilling { ref niche, .. } => { // Find the integer type of the correct size. - let discr_type = niche.value.to_ty(cx.tcx); - let (size, align) = cx.size_and_align_of(discr_type); - - let discr_type = (match size.bits() { - 8 => Integer::I8, - 16 => Integer::I16, - 32 => Integer::I32, - 64 => Integer::I64, - bits => bug!("prepare_enum_metadata: unknown niche bit size {}", bits), - }).to_ty(cx.tcx, false); + let size = niche.value.size(cx); + let align = niche.value.align(cx); + + let discr_type = match niche.value { + layout::Int(t, _) => t, + layout::Float(layout::FloatTy::F32) => Integer::I32, + layout::Float(layout::FloatTy::F64) => Integer::I64, + layout::Pointer => cx.data_layout().ptr_sized_integer(), + }.to_ty(cx.tcx, false); let discr_metadata = basic_type_metadata(cx, discr_type); unsafe { From d435460596aa5110e2d608e4de4144cb21b59264 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 7 Sep 2018 09:42:02 -0600 Subject: [PATCH 3/5] Tighten enum-debug test Update the new enum-debug to ensure that field "D" does not have a discrimnant. --- src/test/codegen/enum-debug.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/codegen/enum-debug.rs b/src/test/codegen/enum-debug.rs index 2bcac922910c0..6326ba93266c2 100644 --- a/src/test/codegen/enum-debug.rs +++ b/src/test/codegen/enum-debug.rs @@ -26,8 +26,10 @@ // CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_structure_type,{{.*}}name: "B",{{.*}} // CHECK: {{.*}}DIDerivedType{{.*}}tag: DW_TAG_member,{{.*}}name: "C",{{.*}}extraData:{{.*}} // CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_structure_type,{{.*}}name: "C",{{.*}} +// CHECK-NOT: {{.*}}DIDerivedType{{.*}}name: "D",{{.*}}extraData:{{.*}} // CHECK: {{.*}}DIDerivedType{{.*}}tag: DW_TAG_member,{{.*}}name: "D",{{.*}} // CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_structure_type,{{.*}}name: "D",{{.*}} +// CHECK: {{.*}}DIDerivedType{{.*}}tag: DW_TAG_member,{{.*}}flags: DIFlagArtificial{{.*}} #![allow(dead_code)] #![allow(unused_variables)] From c871bab6dc8f890931c7146353ae0b4d267214dc Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 7 Sep 2018 10:08:59 -0600 Subject: [PATCH 4/5] Add more enum debug info tests Rename the previous enum debug info test, and add more tests to cover c-like enums and tagged (ordinary) enums. --- src/test/codegen/enum-debug-clike.rs | 35 ++++++++++++++++ .../{enum-debug.rs => enum-debug-niche.rs} | 0 src/test/codegen/enum-debug-tagged.rs | 40 +++++++++++++++++++ 3 files changed, 75 insertions(+) create mode 100644 src/test/codegen/enum-debug-clike.rs rename src/test/codegen/{enum-debug.rs => enum-debug-niche.rs} (100%) create mode 100644 src/test/codegen/enum-debug-tagged.rs diff --git a/src/test/codegen/enum-debug-clike.rs b/src/test/codegen/enum-debug-clike.rs new file mode 100644 index 0000000000000..528e84b298c43 --- /dev/null +++ b/src/test/codegen/enum-debug-clike.rs @@ -0,0 +1,35 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// This test depends on a patch that was committed to upstream LLVM +// before 7.0, then backported to the Rust LLVM fork. It tests that +// debug info for "c-like" enums is properly emitted. + +// ignore-tidy-linelength +// ignore-windows +// min-system-llvm-version 7.0 + +// compile-flags: -g -C no-prepopulate-passes + +// CHECK-LABEL: @main +// CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_enumeration_type,{{.*}}name: "E",{{.*}}flags: DIFlagFixedEnum,{{.*}} +// CHECK: {{.*}}DIEnumerator{{.*}}name: "A",{{.*}}value: {{[0-9].*}} +// CHECK: {{.*}}DIEnumerator{{.*}}name: "B",{{.*}}value: {{[0-9].*}} +// CHECK: {{.*}}DIEnumerator{{.*}}name: "C",{{.*}}value: {{[0-9].*}} + +#![allow(dead_code)] +#![allow(unused_variables)] +#![allow(unused_assignments)] + +enum E { A, B, C } + +pub fn main() { + let e = E::C; +} diff --git a/src/test/codegen/enum-debug.rs b/src/test/codegen/enum-debug-niche.rs similarity index 100% rename from src/test/codegen/enum-debug.rs rename to src/test/codegen/enum-debug-niche.rs diff --git a/src/test/codegen/enum-debug-tagged.rs b/src/test/codegen/enum-debug-tagged.rs new file mode 100644 index 0000000000000..e862d29c940f0 --- /dev/null +++ b/src/test/codegen/enum-debug-tagged.rs @@ -0,0 +1,40 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// This test depends on a patch that was committed to upstream LLVM +// before 7.0, then backported to the Rust LLVM fork. It tests that +// debug info for tagged (ordinary) enums is properly emitted. + +// ignore-tidy-linelength +// ignore-windows +// min-system-llvm-version 7.0 + +// compile-flags: -g -C no-prepopulate-passes + +// CHECK-LABEL: @main +// CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_structure_type,{{.*}}name: "E",{{.*}} +// CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_variant_part,{{.*}}discriminator:{{.*}} +// CHECK: {{.*}}DIDerivedType{{.*}}tag: DW_TAG_member,{{.*}}name: "A",{{.*}}extraData:{{.*}} +// CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_structure_type,{{.*}}name: "A",{{.*}} +// CHECK: {{.*}}DIDerivedType{{.*}}tag: DW_TAG_member,{{.*}}name: "__0",{{.*}} +// CHECK: {{.*}}DIDerivedType{{.*}}tag: DW_TAG_member,{{.*}}name: "B",{{.*}}extraData:{{.*}} +// CHECK: {{.*}}DICompositeType{{.*}}tag: DW_TAG_structure_type,{{.*}}name: "B",{{.*}} +// CHECK: {{.*}}DIDerivedType{{.*}}tag: DW_TAG_member,{{.*}}name: "__0",{{.*}} +// CHECK: {{.*}}DIDerivedType{{.*}}tag: DW_TAG_member,{{.*}}flags: DIFlagArtificial{{.*}} + +#![allow(dead_code)] +#![allow(unused_variables)] +#![allow(unused_assignments)] + +enum E { A(u32), B(u32) } + +pub fn main() { + let e = E::A(23); +} From 22223cf73d01d846cc15f8998546c0b4eaa9f555 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 13 Sep 2018 06:51:17 -0600 Subject: [PATCH 5/5] Avoid possible integer overflow in nice value computation @eddyb pointed out in review that the niche value computation had a possible integer overflow problem, fixed here as he suggested. --- src/librustc_codegen_llvm/debuginfo/metadata.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index 14854d7aa1631..b0eecd26621ee 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -1374,8 +1374,11 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { let niche_value = if i == dataful_variant { None } else { - Some((i.wrapping_sub(*niche_variants.start()) as u128) - .wrapping_add(niche_start) as u64) + let niche = (i as u128) + .wrapping_sub(*niche_variants.start() as u128) + .wrapping_add(niche_start); + assert_eq!(niche as u64 as u128, niche); + Some(niche as u64) }; MemberDescription {