From f353cbf1a145603e1f69c2aaaef171dd60ca4c65 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Mon, 26 Apr 2021 14:39:57 -0400 Subject: [PATCH 01/10] Generate better debuginfo for directly tagged enums --- .../src/debuginfo/metadata.rs | 9 +++-- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 2 +- .../src/debuginfo/type_names.rs | 8 ++++ src/etc/natvis/intrinsic.natvis | 38 +++++++++++++++++++ src/etc/natvis/libcore.natvis | 17 --------- 5 files changed, 52 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 0db6659f8e256..7a44e887c8932 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -1574,7 +1574,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { MemberDescription { name: if fallback { - String::new() + format!("Variant{}", i.as_u32()) } else { variant_info.variant_name() }, @@ -1886,8 +1886,9 @@ fn describe_enum_variant( // We have the layout of an enum variant, we need the layout of the outer enum let enum_layout = cx.layout_of(layout.ty); let offset = enum_layout.fields.offset(tag_field.as_usize()); + let tag_name = if cx.tcx.sess.target.is_like_msvc { "variant$" } else { "RUST$ENUM$DISR" }; let args = - ("RUST$ENUM$DISR".to_owned(), enum_layout.field(cx, tag_field.as_usize()).ty); + (tag_name.to_owned(), enum_layout.field(cx, tag_field.as_usize()).ty); (Some(offset), Some(args)) } _ => (None, None), @@ -2062,7 +2063,7 @@ fn prepare_enum_metadata( unsafe { llvm::LLVMRustDIBuilderCreateUnionType( DIB(cx), - containing_scope, + None, enum_name.as_ptr().cast(), enum_name.len(), file_metadata, @@ -2437,7 +2438,7 @@ fn create_union_stub( llvm::LLVMRustDIBuilderCreateUnionType( DIB(cx), - containing_scope, + Some(containing_scope), union_type_name.as_ptr().cast(), union_type_name.len(), unknown_file_metadata(cx), diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 8b1dcea3fa262..54ef1a284689a 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -2038,7 +2038,7 @@ extern "C" { pub fn LLVMRustDIBuilderCreateUnionType( Builder: &DIBuilder<'a>, - Scope: &'a DIScope, + Scope: Option<&'a DIScope>, Name: *const c_char, NameLen: size_t, File: &'a DIFile, diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index d1bbf74307c6b..626c71abf63d9 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -45,8 +45,16 @@ pub fn push_debuginfo_type_name<'tcx>( ty::Float(float_ty) => output.push_str(float_ty.name_str()), ty::Foreign(def_id) => push_item_name(tcx, def_id, qualified, output), ty::Adt(def, substs) => { + if def.is_enum() && cpp_like_names { + output.push_str("_enum<"); + } + push_item_name(tcx, def.did, qualified, output); push_type_params(tcx, substs, output, visited); + + if def.is_enum() && cpp_like_names { + output.push('>'); + } } ty::Tuple(component_types) => { if cpp_like_names { diff --git a/src/etc/natvis/intrinsic.natvis b/src/etc/natvis/intrinsic.natvis index 030892a432b31..45e36f929b4d1 100644 --- a/src/etc/natvis/intrinsic.natvis +++ b/src/etc/natvis/intrinsic.natvis @@ -149,4 +149,42 @@ ... + + + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + {tag(),en} + + + Variant0 + Variant1 + Variant2 + Variant3 + Variant4 + Variant5 + Variant6 + Variant7 + Variant8 + Variant9 + Variant10 + Variant11 + Variant12 + Variant13 + Variant14 + Variant15 + + diff --git a/src/etc/natvis/libcore.natvis b/src/etc/natvis/libcore.natvis index 9c3c26f597838..17667770520ce 100644 --- a/src/etc/natvis/libcore.natvis +++ b/src/etc/natvis/libcore.natvis @@ -14,14 +14,6 @@ - - None - Some({__0}) - - __0 - - - None Some({($T1 *)this}) @@ -30,15 +22,6 @@ - - Ok({__0}) - Err({(*($T2*) &__0)}) - - __0 - (*($T2*) &__0) - - - {(void*) pointer} From b644f06326577e0926ddf9896fd702befc7bd43e Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Tue, 27 Apr 2021 10:42:06 -0400 Subject: [PATCH 02/10] Resolve EnumTagInfo FIXME --- .../src/debuginfo/metadata.rs | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 7a44e887c8932..0ab16473a54af 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -1507,7 +1507,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { cx, self.layout, variant_info, - NoTag, + None, self_metadata, self.span, ); @@ -1539,13 +1539,13 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { .. } => { let tag_info = if fallback { - RegularTag { + Some(DirectTag { tag_field: Field::from(tag_field), tag_type_metadata: self.tag_type_metadata.unwrap(), - } + }) } else { // This doesn't matter in this case. - NoTag + None }; variants .iter_enumerated() @@ -1606,7 +1606,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { cx, variant, variant_info_for(dataful_variant), - OptimizedTag, + Some(NicheTag), self.containing_scope, self.span, ); @@ -1681,7 +1681,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { cx, variant, variant_info, - OptimizedTag, + Some(NicheTag), self_metadata, self.span, ); @@ -1771,14 +1771,10 @@ impl VariantMemberDescriptionFactory<'ll, 'tcx> { } } -// FIXME: terminology here should be aligned with `abi::TagEncoding`. -// `OptimizedTag` is `TagEncoding::Niche`, `RegularTag` is `TagEncoding::Direct`. -// `NoTag` should be removed; users should use `Option` instead. #[derive(Copy, Clone)] enum EnumTagInfo<'ll> { - RegularTag { tag_field: Field, tag_type_metadata: &'ll DIType }, - OptimizedTag, - NoTag, + DirectTag { tag_field: Field, tag_type_metadata: &'ll DIType }, + NicheTag, } #[derive(Copy, Clone)] @@ -1859,7 +1855,7 @@ fn describe_enum_variant( cx: &CodegenCx<'ll, 'tcx>, layout: layout::TyAndLayout<'tcx>, variant: VariantInfo<'_, 'tcx>, - discriminant_info: EnumTagInfo<'ll>, + discriminant_info: Option>, containing_scope: &'ll DIScope, span: Span, ) -> (&'ll DICompositeType, MemberDescriptionFactory<'ll, 'tcx>) { @@ -1882,7 +1878,7 @@ fn describe_enum_variant( 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 { - RegularTag { tag_field, .. } => { + Some(DirectTag { tag_field, .. }) => { // We have the layout of an enum variant, we need the layout of the outer enum let enum_layout = cx.layout_of(layout.ty); let offset = enum_layout.fields.offset(tag_field.as_usize()); @@ -1919,7 +1915,7 @@ fn describe_enum_variant( offsets, args, tag_type_metadata: match discriminant_info { - RegularTag { tag_type_metadata, .. } => Some(tag_type_metadata), + Some(DirectTag { tag_type_metadata, .. }) => Some(tag_type_metadata), _ => None, }, span, From 2a025c1a765a593030fc6e80a45f4f1053f15aae Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Wed, 28 Apr 2021 16:32:54 -0400 Subject: [PATCH 03/10] Remove fallback for containing scopes This wasn't necessary for msvc and caused issues where different types with the same name such as different instantiations of `Option` would have colliding debuginfo. This confused the debugger which would pick one of the type definitions and use for all types with that name even though they had different layout. --- compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 0ab16473a54af..af0d95a354b24 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -1457,7 +1457,6 @@ struct EnumMemberDescriptionFactory<'ll, 'tcx> { enum_type: Ty<'tcx>, layout: TyAndLayout<'tcx>, tag_type_metadata: Option<&'ll DIType>, - containing_scope: &'ll DIScope, common_members: Vec>, span: Span, } @@ -1488,11 +1487,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { // 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) - }; + let self_metadata = type_metadata(cx, self.enum_type, self.span); match self.layout.variants { Variants::Single { index } => { @@ -1607,7 +1602,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { variant, variant_info_for(dataful_variant), Some(NicheTag), - self.containing_scope, + self_metadata, self.span, ); @@ -2085,7 +2080,6 @@ fn prepare_enum_metadata( enum_type, layout, tag_type_metadata: discriminant_type_metadata, - containing_scope, common_members: vec![], span, }), @@ -2238,7 +2232,6 @@ fn prepare_enum_metadata( enum_type, layout, tag_type_metadata: None, - containing_scope, common_members: outer_fields, span, }), From 141546c355f2de7501c1df75dd97229ed350eeb3 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 13 May 2021 16:39:19 -0400 Subject: [PATCH 04/10] Generate better debuginfo for niche-layout enums Previously, we would generate a single struct with the layout of the dataful variant plus an extra field whose name contained the value of the niche (this would only really work for things like `Option<&_>` where we can determine that the `None` case maps to `0` but for enums that have multiple tag only variants, this doesn't work). Now, we generate a union of two structs, one which is the layout of the dataful variant and one which just has a way of reading the discriminant. We also generate an enum which maps the discriminant value to the tag only variants. We also encode information about the range of values which correspond to the dataful variant in the type name and then use natvis to determine which union field we should display to the user. As a result of this change, all niche-layout enums render correctly in WinDbg and Visual Studio! --- .../src/debuginfo/metadata.rs | 199 +++++++++++------- .../src/debuginfo/type_names.rs | 59 +++++- src/etc/natvis/intrinsic.natvis | 14 ++ 3 files changed, 191 insertions(+), 81 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index af0d95a354b24..1f268bd187863 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -1594,77 +1594,144 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { ref variants, tag_field, } => { + let calculate_niche_value = |i: VariantIdx| { + if i == dataful_variant { + None + } else { + let value = (i.as_u32() as u128) + .wrapping_sub(niche_variants.start().as_u32() as u128) + .wrapping_add(niche_start); + let value = tag.value.size(cx).truncate(value); + // NOTE(eddyb) do *NOT* remove this assert, until + // we pass the full 128-bit value to LLVM, otherwise + // truncation will be silent and remain undetected. + assert_eq!(value as u64 as u128, value); + Some(value as u64) + } + }; + + // For MSVC, we will generate a union of two structs, one for the dataful variant and one that just points to + // the discriminant field. We also create an enum that contains tag values for the non-dataful variants and + // make the discriminant field that type. We then use natvis to render the enum type correctly in Windbg/VS. 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( + let unique_type_id = debug_context(cx) + .type_map + .borrow_mut() + .get_unique_type_id_of_enum_variant(cx, self.enum_type, "discriminant$"); + + let variant_metadata = create_struct_stub( + cx, + self.layout.ty, + &"discriminant$", + unique_type_id, + Some(self_metadata), + DIFlags::FlagArtificial, + ); + + let dataful_variant_layout = self.layout.for_variant(cx, dataful_variant); + + let mut discr_enum_ty = tag.value.to_ty(cx.tcx); + // If the niche is the NULL value of a reference, then `discr_enum_ty` will be a RawPtr. + // CodeView doesn't know what to do with enums whose base type is a pointer so we fix this up + // to just be `usize`. + if let ty::RawPtr(_) = discr_enum_ty.kind() { + discr_enum_ty = cx.tcx.types.usize; + } + + let tags: Vec<_> = variants + .iter_enumerated() + .filter_map(|(variant_idx, _)| { + calculate_niche_value(variant_idx).map(|tag| { + let variant = variant_info_for(variant_idx); + let name = variant.variant_name(); + + Some(unsafe { + llvm::LLVMRustDIBuilderCreateEnumerator( + DIB(cx), + name.as_ptr().cast(), + name.len(), + tag as i64, + !discr_enum_ty.is_signed(), + ) + }) + }) + }) + .collect(); + + let discr_enum = unsafe { + llvm::LLVMRustDIBuilderCreateEnumerationType( + DIB(cx), + self_metadata, + "tag$".as_ptr().cast(), + "tag$".len(), + unknown_file_metadata(cx), + UNKNOWN_LINE_NUMBER, + tag.value.size(cx).bits(), + tag.value.align(cx).abi.bits() as u32, + create_DIArray(DIB(cx), &tags), + type_metadata(cx, discr_enum_ty, self.span), + true, + ) + }; + + let (size, align) = + cx.size_and_align_of(dataful_variant_layout.field(cx, tag_field).ty); + let members = vec![MemberDescription { + name: "discriminant".to_string(), + type_metadata: discr_enum, + offset: dataful_variant_layout.fields.offset(tag_field), + size, + align, + flags: DIFlags::FlagArtificial, + discriminant: None, + source_info: None, + }]; + + set_members_of_composite_type(cx, self.enum_type, variant_metadata, members, None); + + let variant_info = variant_info_for(dataful_variant); + let (variant_type_metadata, member_desc_factory) = describe_enum_variant( cx, - variant, - variant_info_for(dataful_variant), + dataful_variant_layout, + variant_info, Some(NicheTag), self_metadata, self.span, ); - let variant_member_descriptions = - member_description_factory.create_member_descriptions(cx); + let member_descriptions = member_desc_factory.create_member_descriptions(cx); set_members_of_composite_type( cx, self.enum_type, variant_type_metadata, - variant_member_descriptions, + member_descriptions, Some(&self.common_members), ); - // 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: TyAndLayout<'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(tag_field), - self.layout.field(cx, tag_field).size, - ); - let variant_info = variant_info_for(*niche_variants.start()); - variant_info.map_struct_name(|variant_name| { - name.push_str(variant_name); - }); - - // 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.abi, - flags: DIFlags::FlagZero, - discriminant: None, - source_info: variant_info.source_info(cx), - }] + vec![ + MemberDescription { + // Name the dataful variant so that we can identify it for natvis + name: "dataful_variant".to_string(), + type_metadata: variant_type_metadata, + offset: Size::ZERO, + size: self.layout.size, + align: self.layout.align.abi, + flags: DIFlags::FlagZero, + discriminant: None, + source_info: variant_info.source_info(cx), + }, + MemberDescription { + name: "discriminant$".into(), + type_metadata: variant_metadata, + offset: Size::ZERO, + size: self.layout.size, + align: self.layout.align.abi, + flags: DIFlags::FlagZero, + discriminant: None, + source_info: None, + }, + ] } else { variants .iter_enumerated() @@ -1692,19 +1759,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { Some(&self.common_members), ); - let niche_value = if i == dataful_variant { - None - } else { - let value = (i.as_u32() as u128) - .wrapping_sub(niche_variants.start().as_u32() as u128) - .wrapping_add(niche_start); - let value = tag.value.size(cx).truncate(value); - // NOTE(eddyb) do *NOT* remove this assert, until - // we pass the full 128-bit value to LLVM, otherwise - // truncation will be silent and remain undetected. - assert_eq!(value as u64 as u128, value); - Some(value as u64) - }; + let niche_value = calculate_niche_value(i); MemberDescription { name: variant_info.variant_name(), @@ -2040,9 +2095,9 @@ fn prepare_enum_metadata( if use_enum_fallback(cx) { let discriminant_type_metadata = match layout.variants { - Variants::Single { .. } - | Variants::Multiple { tag_encoding: TagEncoding::Niche { .. }, .. } => None, - Variants::Multiple { tag_encoding: TagEncoding::Direct, ref tag, .. } => { + Variants::Single { .. } => None, + Variants::Multiple { tag_encoding: TagEncoding::Niche { .. }, ref tag, .. } + | Variants::Multiple { tag_encoding: TagEncoding::Direct, ref tag, .. } => { Some(discriminant_type_metadata(tag.value)) } }; diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index 626c71abf63d9..1f3e94933188d 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -3,7 +3,8 @@ use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; use rustc_hir::def_id::DefId; -use rustc_middle::ty::{self, subst::SubstsRef, Ty, TyCtxt}; +use rustc_middle::ty::{self, subst::SubstsRef, AdtDef, Ty, TyCtxt}; +use rustc_target::abi::{TagEncoding, Variants}; use std::fmt::Write; @@ -46,14 +47,10 @@ pub fn push_debuginfo_type_name<'tcx>( ty::Foreign(def_id) => push_item_name(tcx, def_id, qualified, output), ty::Adt(def, substs) => { if def.is_enum() && cpp_like_names { - output.push_str("_enum<"); - } - - push_item_name(tcx, def.did, qualified, output); - push_type_params(tcx, substs, output, visited); - - if def.is_enum() && cpp_like_names { - output.push('>'); + msvc_enum_fallback(tcx, t, def, substs, output, visited); + } else { + push_item_name(tcx, def.did, qualified, output); + push_type_params(tcx, substs, output, visited); } } ty::Tuple(component_types) => { @@ -241,6 +238,50 @@ pub fn push_debuginfo_type_name<'tcx>( } } + fn msvc_enum_fallback( + tcx: TyCtxt<'tcx>, + ty: Ty<'tcx>, + def: &AdtDef, + substs: SubstsRef<'tcx>, + output: &mut String, + visited: &mut FxHashSet>, + ) { + let layout = tcx.layout_of(tcx.param_env(def.did).and(ty)).expect("layout error"); + + if let Variants::Multiple { + tag_encoding: TagEncoding::Niche { dataful_variant, .. }, + tag, + variants, + .. + } = &layout.variants + { + let dataful_variant_layout = &variants[*dataful_variant]; + + // calculate the range of values for the dataful variant + let dataful_discriminant_range = + &dataful_variant_layout.largest_niche.as_ref().unwrap().scalar.valid_range; + + let min = dataful_discriminant_range.start(); + let min = tag.value.size(&tcx).truncate(*min); + + let max = dataful_discriminant_range.end(); + let max = tag.value.size(&tcx).truncate(*max); + + output.push_str("_enum<"); + push_item_name(tcx, def.did, true, output); + push_type_params(tcx, substs, output, visited); + + let dataful_variant_name = def.variants[*dataful_variant].ident.as_str(); + + output.push_str(&format!(", {}, {}, {}>", min, max, dataful_variant_name)); + } else { + output.push_str("_enum<"); + push_item_name(tcx, def.did, true, output); + push_type_params(tcx, substs, output, visited); + output.push('>'); + } + } + fn push_item_name(tcx: TyCtxt<'tcx>, def_id: DefId, qualified: bool, output: &mut String) { if qualified { output.push_str(&tcx.crate_name(def_id.krate).as_str()); diff --git a/src/etc/natvis/intrinsic.natvis b/src/etc/natvis/intrinsic.natvis index 45e36f929b4d1..82d3ab15a7048 100644 --- a/src/etc/natvis/intrinsic.natvis +++ b/src/etc/natvis/intrinsic.natvis @@ -187,4 +187,18 @@ Variant15 + + + + + + {"$T4",sb}({dataful_variant}) + {discriminant$.discriminant,en} + + dataful_variant + + {"$T4",sb} + + + From 2a9fa202fef9b1717fff3cbbe59f980c9f762c54 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 13 May 2021 12:56:33 -0400 Subject: [PATCH 05/10] Add/update tests --- src/test/debuginfo/msvc-pretty-enums.rs | 104 ++++++++++++++++++++++++ src/test/debuginfo/pretty-std.rs | 6 +- 2 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 src/test/debuginfo/msvc-pretty-enums.rs diff --git a/src/test/debuginfo/msvc-pretty-enums.rs b/src/test/debuginfo/msvc-pretty-enums.rs new file mode 100644 index 0000000000000..146be23bbb3c0 --- /dev/null +++ b/src/test/debuginfo/msvc-pretty-enums.rs @@ -0,0 +1,104 @@ +// only-cdb +// compile-flags:-g + +// cdb-command: g + +// Note: The natvis used to visualize niche-layout enums don't work correctly in cdb +// so the best we can do is to make sure we are generating the right debuginfo + +// cdb-command: dx -r2 a,! +// cdb-check:a,! [Type: _enum>, 2, 16, Some>] +// cdb-check: [+0x000] dataful_variant [Type: _enum>, 2, 16, Some>::Some] +// cdb-check: [+0x000] __0 : Low (0x2) [Type: msvc_pretty_enums::CStyleEnum] +// cdb-check: [+0x000] discriminant$ [Type: _enum>, 2, 16, Some>::discriminant$] +// cdb-check: [+0x000] discriminant : 0x2 [Type: _enum>, 2, 16, Some>::tag$] + +// cdb-command: dx -r2 b,! +// cdb-check:b,! [Type: _enum>, 2, 16, Some>] +// cdb-check: [+0x000] dataful_variant [Type: _enum>, 2, 16, Some>::Some] +// cdb-check: [+0x000] __0 : 0x11 [Type: msvc_pretty_enums::CStyleEnum] +// cdb-check: [+0x000] discriminant$ [Type: _enum>, 2, 16, Some>::discriminant$] +// cdb-check: [+0x000] discriminant : None (0x11) [Type: _enum>, 2, 16, Some>::tag$] + +// cdb-command: dx -r2 c,! +// cdb-check:c,! [Type: _enum] +// cdb-check: [+0x000] dataful_variant [Type: _enum::Data] +// cdb-check: [+0x000] my_data : 0x11 [Type: msvc_pretty_enums::CStyleEnum] +// cdb-check: [+0x000] discriminant$ [Type: _enum::discriminant$] +// cdb-check: [+0x000] discriminant : Tag1 (0x11) [Type: _enum::tag$] + +// cdb-command: dx -r2 d,! +// cdb-check:d,! [Type: _enum] +// cdb-check: [+0x000] dataful_variant [Type: _enum::Data] +// cdb-check: [+0x000] my_data : High (0x10) [Type: msvc_pretty_enums::CStyleEnum] +// cdb-check: [+0x000] discriminant$ [Type: _enum::discriminant$] +// cdb-check: [+0x000] discriminant : 0x10 [Type: _enum::tag$] + +// cdb-command: dx -r2 e,! +// cdb-check:e,! [Type: _enum] +// cdb-check: [+0x000] dataful_variant [Type: _enum::Data] +// cdb-check: [+0x000] my_data : 0x13 [Type: msvc_pretty_enums::CStyleEnum] +// cdb-check: [+0x000] discriminant$ [Type: _enum::discriminant$] +// cdb-check: [+0x000] discriminant : Tag2 (0x13) [Type: _enum::tag$] + +// cdb-command: dx -r2 f,! +// cdb-check:f,! [Type: _enum, 1, [...], Some>] +// cdb-check: [+0x000] dataful_variant [Type: _enum, 1, [...], Some>::Some] +// cdb-check: [+0x000] __0 : 0x[...] : 0x1 [Type: unsigned int *] +// cdb-check: [+0x000] discriminant$ [Type: _enum, 1, [...], Some>::discriminant$] +// cdb-check: [+0x000] discriminant : 0x[...] [Type: _enum, 1, [...], Some>::tag$] + +// cdb-command: dx -r2 g,! +// cdb-check:g,! [Type: _enum, 1, [...], Some>] +// cdb-check: [+0x000] dataful_variant [Type: _enum, 1, [...], Some>::Some] +// cdb-check: [+0x000] __0 : 0x0 [Type: unsigned int *] +// cdb-check: [+0x000] discriminant$ [Type: _enum, 1, [...], Some>::discriminant$] +// cdb-check: [+0x000] discriminant : None (0x0) [Type: _enum, 1, [...], Some>::tag$] + +// cdb-command: dx h +// cdb-check:h : Some [Type: _enum>] +// cdb-check: [+0x000] variant$ : Some (0x1) [Type: core::option::Option] +// cdb-check: [+0x004] __0 : 0xc [Type: unsigned int] + +// cdb-command: dx i +// cdb-check:i : None [Type: _enum>] +// cdb-check: [+0x000] variant$ : None (0x0) [Type: core::option::Option] + +// cdb-command: dx j +// cdb-check:j : High (0x10) [Type: msvc_pretty_enums::CStyleEnum] + +// cdb-command: dx -r2 k,! +// cdb-check:k,! [Type: _enum, 1, [...], Some>] +// cdb-check: [+0x000] dataful_variant [Type: _enum, 1, [...], Some>::Some] +// cdb-check: [+0x000] __0 [Type: alloc::string::String] +// cdb-check: [+0x000] discriminant$ [Type: _enum, 1, [...], Some>::discriminant$] +// cdb-check: [+0x000] discriminant : 0x[...] [Type: _enum, 1, [...], Some>::tag$] + +pub enum CStyleEnum { + Low = 2, + High = 16, +} + +pub enum NicheLayoutEnum { + Tag1, + Data { my_data: CStyleEnum }, + Tag2, +} + +fn main() { + let a = Some(CStyleEnum::Low); + let b = Option::::None; + let c = NicheLayoutEnum::Tag1; + let d = NicheLayoutEnum::Data { my_data: CStyleEnum::High }; + let e = NicheLayoutEnum::Tag2; + let f = Some(&1u32); + let g = Option::<&'static u32>::None; + let h = Some(12u32); + let i = Option::::None; + let j = CStyleEnum::High; + let k = Some("IAMA optional string!".to_string()); + + zzz(); // #break +} + +fn zzz() { () } \ No newline at end of file diff --git a/src/test/debuginfo/pretty-std.rs b/src/test/debuginfo/pretty-std.rs index 1a99f8412504a..aad8ae5cd9fbe 100644 --- a/src/test/debuginfo/pretty-std.rs +++ b/src/test/debuginfo/pretty-std.rs @@ -111,11 +111,11 @@ // NOTE: OsString doesn't have a .natvis entry yet. // cdb-command: dx some -// cdb-check:some : Some(8) [Type: [...]::Option] +// cdb-check:some : Some [Type: _enum>] // cdb-command: dx none -// cdb-check:none : None [Type: [...]::Option] +// cdb-check:none : None [Type: _enum>] // cdb-command: dx some_string -// cdb-check:some_string : Some("IAMA optional string!") [[...]::Option<[...]::String>] +// cdb-check:some_string [Type: _enum, 1, 18446744073709551615, Some>] #![allow(unused_variables)] use std::ffi::OsString; From d65009111734c2a4d89cad5d2fb7bbf08c813b7c Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Fri, 14 May 2021 11:17:26 -0400 Subject: [PATCH 06/10] Make tidy happy --- .../rustc_codegen_llvm/src/debuginfo/metadata.rs | 14 ++++++++++---- src/test/debuginfo/msvc-pretty-enums.rs | 3 ++- src/test/debuginfo/pretty-std.rs | 3 ++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 1f268bd187863..0f512fcaaaf25 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -1687,7 +1687,13 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { source_info: None, }]; - set_members_of_composite_type(cx, self.enum_type, variant_metadata, members, None); + set_members_of_composite_type( + cx, + self.enum_type, + variant_metadata, + members, + None, + ); let variant_info = variant_info_for(dataful_variant); let (variant_type_metadata, member_desc_factory) = describe_enum_variant( @@ -1932,9 +1938,9 @@ fn describe_enum_variant( // We have the layout of an enum variant, we need the layout of the outer enum let enum_layout = cx.layout_of(layout.ty); let offset = enum_layout.fields.offset(tag_field.as_usize()); - let tag_name = if cx.tcx.sess.target.is_like_msvc { "variant$" } else { "RUST$ENUM$DISR" }; - let args = - (tag_name.to_owned(), enum_layout.field(cx, tag_field.as_usize()).ty); + let tag_name = + if cx.tcx.sess.target.is_like_msvc { "variant$" } else { "RUST$ENUM$DISR" }; + let args = (tag_name.to_owned(), enum_layout.field(cx, tag_field.as_usize()).ty); (Some(offset), Some(args)) } _ => (None, None), diff --git a/src/test/debuginfo/msvc-pretty-enums.rs b/src/test/debuginfo/msvc-pretty-enums.rs index 146be23bbb3c0..ad1b45a7bc275 100644 --- a/src/test/debuginfo/msvc-pretty-enums.rs +++ b/src/test/debuginfo/msvc-pretty-enums.rs @@ -1,4 +1,5 @@ // only-cdb +// ignore-tidy-linelength // compile-flags:-g // cdb-command: g @@ -101,4 +102,4 @@ fn main() { zzz(); // #break } -fn zzz() { () } \ No newline at end of file +fn zzz() { () } diff --git a/src/test/debuginfo/pretty-std.rs b/src/test/debuginfo/pretty-std.rs index aad8ae5cd9fbe..e9f690fac2e19 100644 --- a/src/test/debuginfo/pretty-std.rs +++ b/src/test/debuginfo/pretty-std.rs @@ -1,6 +1,7 @@ // ignore-freebsd: gdb package too new // only-cdb // "Temporarily" ignored on GDB/LLDB due to debuginfo tests being disabled, see PR 47155 // ignore-android: FIXME(#10381) +// ignore-tidy-linelength // compile-flags:-g // min-gdb-version: 7.7 // min-lldb-version: 310 @@ -115,7 +116,7 @@ // cdb-command: dx none // cdb-check:none : None [Type: _enum>] // cdb-command: dx some_string -// cdb-check:some_string [Type: _enum, 1, 18446744073709551615, Some>] +// cdb-check:some_string [Type: _enum, 1, [...], Some>] #![allow(unused_variables)] use std::ffi::OsString; From d2d6fa852d22eb4e9259cd708e33e7afaa9211d0 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Mon, 17 May 2021 11:09:13 -0400 Subject: [PATCH 07/10] Respond to review feedback --- .../src/debuginfo/metadata.rs | 36 +++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 0f512fcaaaf25..1d81d880b8ded 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -1534,6 +1534,19 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { .. } => { let tag_info = if fallback { + // For MSVC, we generate a union of structs for each variant with an explicit + // discriminant field roughly equivalent to the following C: + // ```c + // union _enum<{name}> { + // struct {variant 0 name} { + // tag$ variant$; + // + // } Variant0; + // + // } + // ``` + // The natvis in `intrinsic.nativs` then matches on `this.Variant0.variant$` to + // determine which variant is active and then displays it. Some(DirectTag { tag_field: Field::from(tag_field), tag_type_metadata: self.tag_type_metadata.unwrap(), @@ -1613,6 +1626,25 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { // For MSVC, we will generate a union of two structs, one for the dataful variant and one that just points to // the discriminant field. We also create an enum that contains tag values for the non-dataful variants and // make the discriminant field that type. We then use natvis to render the enum type correctly in Windbg/VS. + // This will generate debuginfo roughly equivalent to the following C: + // ```c + // union _enum<{name}, {min niche}, {max niche}, {dataful variant name} { + // struct dataful_variant { + // + // }, + // struct discriminant$ { + // enum tag$ { + // + // } discriminant; + // } + // } + // ``` + // The natvis in `intrinsic.natvis` matches on the type name `_enum<*, *, *, *>` + // and evaluates `this.discriminant$.discriminant`. If the value is between + // the min niche and max niche, then the enum is in the dataful variant and + // `this.dataful_variant` is rendered. Otherwise, the enum is in one of the + // non-dataful variants. In that case, we just need to render the name of the + // `this.discriminant$.discriminant` enum. if fallback { let unique_type_id = debug_context(cx) .type_map @@ -1938,9 +1970,7 @@ fn describe_enum_variant( // We have the layout of an enum variant, we need the layout of the outer enum let enum_layout = cx.layout_of(layout.ty); let offset = enum_layout.fields.offset(tag_field.as_usize()); - let tag_name = - if cx.tcx.sess.target.is_like_msvc { "variant$" } else { "RUST$ENUM$DISR" }; - let args = (tag_name.to_owned(), enum_layout.field(cx, tag_field.as_usize()).ty); + let args = ("variant$".to_owned(), enum_layout.field(cx, tag_field.as_usize()).ty); (Some(offset), Some(args)) } _ => (None, None), From ef053fd6f0faa848097ff8b924ac859c667c8d15 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 20 May 2021 13:58:13 -0400 Subject: [PATCH 08/10] Change the type name from `_enum<..>` to `enum$<..>` This makes the type name inline with the proposed standard in #85269. --- .../src/debuginfo/metadata.rs | 6 +- .../src/debuginfo/type_names.rs | 4 +- src/etc/natvis/intrinsic.natvis | 4 +- src/test/debuginfo/msvc-pretty-enums.rs | 68 +++++++++---------- src/test/debuginfo/pretty-std.rs | 6 +- 5 files changed, 44 insertions(+), 44 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 1d81d880b8ded..f56dace0d3362 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -1537,7 +1537,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { // For MSVC, we generate a union of structs for each variant with an explicit // discriminant field roughly equivalent to the following C: // ```c - // union _enum<{name}> { + // union enum$<{name}> { // struct {variant 0 name} { // tag$ variant$; // @@ -1628,7 +1628,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { // make the discriminant field that type. We then use natvis to render the enum type correctly in Windbg/VS. // This will generate debuginfo roughly equivalent to the following C: // ```c - // union _enum<{name}, {min niche}, {max niche}, {dataful variant name} { + // union enum$<{name}, {min niche}, {max niche}, {dataful variant name} { // struct dataful_variant { // // }, @@ -1639,7 +1639,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { // } // } // ``` - // The natvis in `intrinsic.natvis` matches on the type name `_enum<*, *, *, *>` + // The natvis in `intrinsic.natvis` matches on the type name `enum$<*, *, *, *>` // and evaluates `this.discriminant$.discriminant`. If the value is between // the min niche and max niche, then the enum is in the dataful variant and // `this.dataful_variant` is rendered. Otherwise, the enum is in one of the diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index 1f3e94933188d..0f8dc06f4ca70 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -267,7 +267,7 @@ pub fn push_debuginfo_type_name<'tcx>( let max = dataful_discriminant_range.end(); let max = tag.value.size(&tcx).truncate(*max); - output.push_str("_enum<"); + output.push_str("enum$<"); push_item_name(tcx, def.did, true, output); push_type_params(tcx, substs, output, visited); @@ -275,7 +275,7 @@ pub fn push_debuginfo_type_name<'tcx>( output.push_str(&format!(", {}, {}, {}>", min, max, dataful_variant_name)); } else { - output.push_str("_enum<"); + output.push_str("enum$<"); push_item_name(tcx, def.did, true, output); push_type_params(tcx, substs, output, visited); output.push('>'); diff --git a/src/etc/natvis/intrinsic.natvis b/src/etc/natvis/intrinsic.natvis index 82d3ab15a7048..09b5c9f091d70 100644 --- a/src/etc/natvis/intrinsic.natvis +++ b/src/etc/natvis/intrinsic.natvis @@ -149,7 +149,7 @@ ... - + {tag(),en} {tag(),en} @@ -189,7 +189,7 @@ - + {"$T4",sb}({dataful_variant}) diff --git a/src/test/debuginfo/msvc-pretty-enums.rs b/src/test/debuginfo/msvc-pretty-enums.rs index ad1b45a7bc275..e2a331ca17adb 100644 --- a/src/test/debuginfo/msvc-pretty-enums.rs +++ b/src/test/debuginfo/msvc-pretty-enums.rs @@ -8,72 +8,72 @@ // so the best we can do is to make sure we are generating the right debuginfo // cdb-command: dx -r2 a,! -// cdb-check:a,! [Type: _enum>, 2, 16, Some>] -// cdb-check: [+0x000] dataful_variant [Type: _enum>, 2, 16, Some>::Some] +// cdb-check:a,! [Type: enum$>, 2, 16, Some>] +// cdb-check: [+0x000] dataful_variant [Type: enum$>, 2, 16, Some>::Some] // cdb-check: [+0x000] __0 : Low (0x2) [Type: msvc_pretty_enums::CStyleEnum] -// cdb-check: [+0x000] discriminant$ [Type: _enum>, 2, 16, Some>::discriminant$] -// cdb-check: [+0x000] discriminant : 0x2 [Type: _enum>, 2, 16, Some>::tag$] +// cdb-check: [+0x000] discriminant$ [Type: enum$>, 2, 16, Some>::discriminant$] +// cdb-check: [+0x000] discriminant : 0x2 [Type: enum$>, 2, 16, Some>::tag$] // cdb-command: dx -r2 b,! -// cdb-check:b,! [Type: _enum>, 2, 16, Some>] -// cdb-check: [+0x000] dataful_variant [Type: _enum>, 2, 16, Some>::Some] +// cdb-check:b,! [Type: enum$>, 2, 16, Some>] +// cdb-check: [+0x000] dataful_variant [Type: enum$>, 2, 16, Some>::Some] // cdb-check: [+0x000] __0 : 0x11 [Type: msvc_pretty_enums::CStyleEnum] -// cdb-check: [+0x000] discriminant$ [Type: _enum>, 2, 16, Some>::discriminant$] -// cdb-check: [+0x000] discriminant : None (0x11) [Type: _enum>, 2, 16, Some>::tag$] +// cdb-check: [+0x000] discriminant$ [Type: enum$>, 2, 16, Some>::discriminant$] +// cdb-check: [+0x000] discriminant : None (0x11) [Type: enum$>, 2, 16, Some>::tag$] // cdb-command: dx -r2 c,! -// cdb-check:c,! [Type: _enum] -// cdb-check: [+0x000] dataful_variant [Type: _enum::Data] +// cdb-check:c,! [Type: enum$] +// cdb-check: [+0x000] dataful_variant [Type: enum$::Data] // cdb-check: [+0x000] my_data : 0x11 [Type: msvc_pretty_enums::CStyleEnum] -// cdb-check: [+0x000] discriminant$ [Type: _enum::discriminant$] -// cdb-check: [+0x000] discriminant : Tag1 (0x11) [Type: _enum::tag$] +// cdb-check: [+0x000] discriminant$ [Type: enum$::discriminant$] +// cdb-check: [+0x000] discriminant : Tag1 (0x11) [Type: enum$::tag$] // cdb-command: dx -r2 d,! -// cdb-check:d,! [Type: _enum] -// cdb-check: [+0x000] dataful_variant [Type: _enum::Data] +// cdb-check:d,! [Type: enum$] +// cdb-check: [+0x000] dataful_variant [Type: enum$::Data] // cdb-check: [+0x000] my_data : High (0x10) [Type: msvc_pretty_enums::CStyleEnum] -// cdb-check: [+0x000] discriminant$ [Type: _enum::discriminant$] -// cdb-check: [+0x000] discriminant : 0x10 [Type: _enum::tag$] +// cdb-check: [+0x000] discriminant$ [Type: enum$::discriminant$] +// cdb-check: [+0x000] discriminant : 0x10 [Type: enum$::tag$] // cdb-command: dx -r2 e,! -// cdb-check:e,! [Type: _enum] -// cdb-check: [+0x000] dataful_variant [Type: _enum::Data] +// cdb-check:e,! [Type: enum$] +// cdb-check: [+0x000] dataful_variant [Type: enum$::Data] // cdb-check: [+0x000] my_data : 0x13 [Type: msvc_pretty_enums::CStyleEnum] -// cdb-check: [+0x000] discriminant$ [Type: _enum::discriminant$] -// cdb-check: [+0x000] discriminant : Tag2 (0x13) [Type: _enum::tag$] +// cdb-check: [+0x000] discriminant$ [Type: enum$::discriminant$] +// cdb-check: [+0x000] discriminant : Tag2 (0x13) [Type: enum$::tag$] // cdb-command: dx -r2 f,! -// cdb-check:f,! [Type: _enum, 1, [...], Some>] -// cdb-check: [+0x000] dataful_variant [Type: _enum, 1, [...], Some>::Some] +// cdb-check:f,! [Type: enum$, 1, [...], Some>] +// cdb-check: [+0x000] dataful_variant [Type: enum$, 1, [...], Some>::Some] // cdb-check: [+0x000] __0 : 0x[...] : 0x1 [Type: unsigned int *] -// cdb-check: [+0x000] discriminant$ [Type: _enum, 1, [...], Some>::discriminant$] -// cdb-check: [+0x000] discriminant : 0x[...] [Type: _enum, 1, [...], Some>::tag$] +// cdb-check: [+0x000] discriminant$ [Type: enum$, 1, [...], Some>::discriminant$] +// cdb-check: [+0x000] discriminant : 0x[...] [Type: enum$, 1, [...], Some>::tag$] // cdb-command: dx -r2 g,! -// cdb-check:g,! [Type: _enum, 1, [...], Some>] -// cdb-check: [+0x000] dataful_variant [Type: _enum, 1, [...], Some>::Some] +// cdb-check:g,! [Type: enum$, 1, [...], Some>] +// cdb-check: [+0x000] dataful_variant [Type: enum$, 1, [...], Some>::Some] // cdb-check: [+0x000] __0 : 0x0 [Type: unsigned int *] -// cdb-check: [+0x000] discriminant$ [Type: _enum, 1, [...], Some>::discriminant$] -// cdb-check: [+0x000] discriminant : None (0x0) [Type: _enum, 1, [...], Some>::tag$] +// cdb-check: [+0x000] discriminant$ [Type: enum$, 1, [...], Some>::discriminant$] +// cdb-check: [+0x000] discriminant : None (0x0) [Type: enum$, 1, [...], Some>::tag$] // cdb-command: dx h -// cdb-check:h : Some [Type: _enum>] +// cdb-check:h : Some [Type: enum$>] // cdb-check: [+0x000] variant$ : Some (0x1) [Type: core::option::Option] // cdb-check: [+0x004] __0 : 0xc [Type: unsigned int] // cdb-command: dx i -// cdb-check:i : None [Type: _enum>] +// cdb-check:i : None [Type: enum$>] // cdb-check: [+0x000] variant$ : None (0x0) [Type: core::option::Option] // cdb-command: dx j // cdb-check:j : High (0x10) [Type: msvc_pretty_enums::CStyleEnum] // cdb-command: dx -r2 k,! -// cdb-check:k,! [Type: _enum, 1, [...], Some>] -// cdb-check: [+0x000] dataful_variant [Type: _enum, 1, [...], Some>::Some] +// cdb-check:k,! [Type: enum$, 1, [...], Some>] +// cdb-check: [+0x000] dataful_variant [Type: enum$, 1, [...], Some>::Some] // cdb-check: [+0x000] __0 [Type: alloc::string::String] -// cdb-check: [+0x000] discriminant$ [Type: _enum, 1, [...], Some>::discriminant$] -// cdb-check: [+0x000] discriminant : 0x[...] [Type: _enum, 1, [...], Some>::tag$] +// cdb-check: [+0x000] discriminant$ [Type: enum$, 1, [...], Some>::discriminant$] +// cdb-check: [+0x000] discriminant : 0x[...] [Type: enum$, 1, [...], Some>::tag$] pub enum CStyleEnum { Low = 2, diff --git a/src/test/debuginfo/pretty-std.rs b/src/test/debuginfo/pretty-std.rs index e9f690fac2e19..68e73b5f38da9 100644 --- a/src/test/debuginfo/pretty-std.rs +++ b/src/test/debuginfo/pretty-std.rs @@ -112,11 +112,11 @@ // NOTE: OsString doesn't have a .natvis entry yet. // cdb-command: dx some -// cdb-check:some : Some [Type: _enum>] +// cdb-check:some : Some [Type: enum$>] // cdb-command: dx none -// cdb-check:none : None [Type: _enum>] +// cdb-check:none : None [Type: enum$>] // cdb-command: dx some_string -// cdb-check:some_string [Type: _enum, 1, [...], Some>] +// cdb-check:some_string [Type: enum$, 1, [...], Some>] #![allow(unused_variables)] use std::ffi::OsString; From 3127419e2b2cb4ad2e44a06d30af6a015daf4557 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Wed, 26 May 2021 12:02:07 -0400 Subject: [PATCH 09/10] Respond to review feedback --- .../src/debuginfo/metadata.rs | 90 ++++++------------- .../src/debuginfo/type_names.rs | 4 + src/etc/natvis/intrinsic.natvis | 41 ++++----- src/test/debuginfo/msvc-pretty-enums.rs | 24 ++--- 4 files changed, 61 insertions(+), 98 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index f56dace0d3362..1e70664e64d70 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -1485,8 +1485,8 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { _ => bug!(), }; - // This will always find the metadata in the type map. let fallback = use_enum_fallback(cx); + // This will always find the metadata in the type map. let self_metadata = type_metadata(cx, self.enum_type, self.span); match self.layout.variants { @@ -1541,11 +1541,11 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { // struct {variant 0 name} { // tag$ variant$; // - // } Variant0; + // } variant0; // // } // ``` - // The natvis in `intrinsic.nativs` then matches on `this.Variant0.variant$` to + // The natvis in `intrinsic.nativs` then matches on `this.variant0.variant$` to // determine which variant is active and then displays it. Some(DirectTag { tag_field: Field::from(tag_field), @@ -1582,7 +1582,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { MemberDescription { name: if fallback { - format!("Variant{}", i.as_u32()) + format!("variant{}", i.as_u32()) } else { variant_info.variant_name() }, @@ -1623,43 +1623,27 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { } }; - // For MSVC, we will generate a union of two structs, one for the dataful variant and one that just points to - // the discriminant field. We also create an enum that contains tag values for the non-dataful variants and - // make the discriminant field that type. We then use natvis to render the enum type correctly in Windbg/VS. + // For MSVC, we will generate a union of two fields, one for the dataful variant + // and one that just points to the discriminant. We also create an enum that + // contains tag values for the non-dataful variants and make the discriminant field + // that type. We then use natvis to render the enum type correctly in Windbg/VS. // This will generate debuginfo roughly equivalent to the following C: // ```c - // union enum$<{name}, {min niche}, {max niche}, {dataful variant name} { - // struct dataful_variant { + // union enum$<{name}, {min niche}, {max niche}, {dataful variant name}> { + // struct { // - // }, - // struct discriminant$ { - // enum tag$ { - // - // } discriminant; - // } + // } dataful_variant; + // enum Discriminant$ { + // + // } discriminant; // } // ``` // The natvis in `intrinsic.natvis` matches on the type name `enum$<*, *, *, *>` - // and evaluates `this.discriminant$.discriminant`. If the value is between - // the min niche and max niche, then the enum is in the dataful variant and - // `this.dataful_variant` is rendered. Otherwise, the enum is in one of the - // non-dataful variants. In that case, we just need to render the name of the - // `this.discriminant$.discriminant` enum. + // and evaluates `this.discriminant`. If the value is between the min niche and max + // niche, then the enum is in the dataful variant and `this.dataful_variant` is + // rendered. Otherwise, the enum is in one of the non-dataful variants. In that + // case, we just need to render the name of the `this.discriminant` enum. if fallback { - let unique_type_id = debug_context(cx) - .type_map - .borrow_mut() - .get_unique_type_id_of_enum_variant(cx, self.enum_type, "discriminant$"); - - let variant_metadata = create_struct_stub( - cx, - self.layout.ty, - &"discriminant$", - unique_type_id, - Some(self_metadata), - DIFlags::FlagArtificial, - ); - let dataful_variant_layout = self.layout.for_variant(cx, dataful_variant); let mut discr_enum_ty = tag.value.to_ty(cx.tcx); @@ -1694,8 +1678,8 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { llvm::LLVMRustDIBuilderCreateEnumerationType( DIB(cx), self_metadata, - "tag$".as_ptr().cast(), - "tag$".len(), + "Discriminant$".as_ptr().cast(), + "Discriminant$".len(), unknown_file_metadata(cx), UNKNOWN_LINE_NUMBER, tag.value.size(cx).bits(), @@ -1706,27 +1690,6 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { ) }; - let (size, align) = - cx.size_and_align_of(dataful_variant_layout.field(cx, tag_field).ty); - let members = vec![MemberDescription { - name: "discriminant".to_string(), - type_metadata: discr_enum, - offset: dataful_variant_layout.fields.offset(tag_field), - size, - align, - flags: DIFlags::FlagArtificial, - discriminant: None, - source_info: None, - }]; - - set_members_of_composite_type( - cx, - self.enum_type, - variant_metadata, - members, - None, - ); - let variant_info = variant_info_for(dataful_variant); let (variant_type_metadata, member_desc_factory) = describe_enum_variant( cx, @@ -1747,6 +1710,9 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { Some(&self.common_members), ); + let (size, align) = + cx.size_and_align_of(dataful_variant_layout.field(cx, tag_field).ty); + vec![ MemberDescription { // Name the dataful variant so that we can identify it for natvis @@ -1760,11 +1726,11 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { source_info: variant_info.source_info(cx), }, MemberDescription { - name: "discriminant$".into(), - type_metadata: variant_metadata, - offset: Size::ZERO, - size: self.layout.size, - align: self.layout.align.abi, + name: "discriminant".into(), + type_metadata: discr_enum, + offset: dataful_variant_layout.fields.offset(tag_field), + size, + align, flags: DIFlags::FlagZero, discriminant: None, source_info: None, diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index 0f8dc06f4ca70..7b4b0821c4be8 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -238,6 +238,10 @@ pub fn push_debuginfo_type_name<'tcx>( } } + /// MSVC names enums differently than other platforms so that the debugging visualization + // format (natvis) is able to understand enums and render the active variant correctly in the + // debugger. For more information, look in `src/etc/natvis/intrinsic.natvis` and + // `EnumMemberDescriptionFactor::create_member_descriptions`. fn msvc_enum_fallback( tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, diff --git a/src/etc/natvis/intrinsic.natvis b/src/etc/natvis/intrinsic.natvis index 09b5c9f091d70..89280149a0351 100644 --- a/src/etc/natvis/intrinsic.natvis +++ b/src/etc/natvis/intrinsic.natvis @@ -150,7 +150,7 @@ - + {tag(),en} {tag(),en} {tag(),en} @@ -169,31 +169,32 @@ {tag(),en} - Variant0 - Variant1 - Variant2 - Variant3 - Variant4 - Variant5 - Variant6 - Variant7 - Variant8 - Variant9 - Variant10 - Variant11 - Variant12 - Variant13 - Variant14 - Variant15 + variant0 + variant1 + variant2 + variant3 + variant4 + variant5 + variant6 + variant7 + variant8 + variant9 + variant10 + variant11 + variant12 + variant13 + variant14 + variant15 - + - + {"$T4",sb}({dataful_variant}) - {discriminant$.discriminant,en} + {discriminant,en} dataful_variant diff --git a/src/test/debuginfo/msvc-pretty-enums.rs b/src/test/debuginfo/msvc-pretty-enums.rs index e2a331ca17adb..550cc66f3899c 100644 --- a/src/test/debuginfo/msvc-pretty-enums.rs +++ b/src/test/debuginfo/msvc-pretty-enums.rs @@ -11,50 +11,43 @@ // cdb-check:a,! [Type: enum$>, 2, 16, Some>] // cdb-check: [+0x000] dataful_variant [Type: enum$>, 2, 16, Some>::Some] // cdb-check: [+0x000] __0 : Low (0x2) [Type: msvc_pretty_enums::CStyleEnum] -// cdb-check: [+0x000] discriminant$ [Type: enum$>, 2, 16, Some>::discriminant$] -// cdb-check: [+0x000] discriminant : 0x2 [Type: enum$>, 2, 16, Some>::tag$] +// cdb-check: [+0x000] discriminant : 0x2 [Type: enum$>, 2, 16, Some>::Discriminant$] // cdb-command: dx -r2 b,! // cdb-check:b,! [Type: enum$>, 2, 16, Some>] // cdb-check: [+0x000] dataful_variant [Type: enum$>, 2, 16, Some>::Some] // cdb-check: [+0x000] __0 : 0x11 [Type: msvc_pretty_enums::CStyleEnum] -// cdb-check: [+0x000] discriminant$ [Type: enum$>, 2, 16, Some>::discriminant$] -// cdb-check: [+0x000] discriminant : None (0x11) [Type: enum$>, 2, 16, Some>::tag$] +// cdb-check: [+0x000] discriminant : None (0x11) [Type: enum$>, 2, 16, Some>::Discriminant$] // cdb-command: dx -r2 c,! // cdb-check:c,! [Type: enum$] // cdb-check: [+0x000] dataful_variant [Type: enum$::Data] // cdb-check: [+0x000] my_data : 0x11 [Type: msvc_pretty_enums::CStyleEnum] -// cdb-check: [+0x000] discriminant$ [Type: enum$::discriminant$] -// cdb-check: [+0x000] discriminant : Tag1 (0x11) [Type: enum$::tag$] +// cdb-check: [+0x000] discriminant : Tag1 (0x11) [Type: enum$::Discriminant$] // cdb-command: dx -r2 d,! // cdb-check:d,! [Type: enum$] // cdb-check: [+0x000] dataful_variant [Type: enum$::Data] // cdb-check: [+0x000] my_data : High (0x10) [Type: msvc_pretty_enums::CStyleEnum] -// cdb-check: [+0x000] discriminant$ [Type: enum$::discriminant$] -// cdb-check: [+0x000] discriminant : 0x10 [Type: enum$::tag$] +// cdb-check: [+0x000] discriminant : 0x10 [Type: enum$::Discriminant$] // cdb-command: dx -r2 e,! // cdb-check:e,! [Type: enum$] // cdb-check: [+0x000] dataful_variant [Type: enum$::Data] // cdb-check: [+0x000] my_data : 0x13 [Type: msvc_pretty_enums::CStyleEnum] -// cdb-check: [+0x000] discriminant$ [Type: enum$::discriminant$] -// cdb-check: [+0x000] discriminant : Tag2 (0x13) [Type: enum$::tag$] +// cdb-check: [+0x000] discriminant : Tag2 (0x13) [Type: enum$::Discriminant$] // cdb-command: dx -r2 f,! // cdb-check:f,! [Type: enum$, 1, [...], Some>] // cdb-check: [+0x000] dataful_variant [Type: enum$, 1, [...], Some>::Some] // cdb-check: [+0x000] __0 : 0x[...] : 0x1 [Type: unsigned int *] -// cdb-check: [+0x000] discriminant$ [Type: enum$, 1, [...], Some>::discriminant$] -// cdb-check: [+0x000] discriminant : 0x[...] [Type: enum$, 1, [...], Some>::tag$] +// cdb-check: [+0x000] discriminant : 0x[...] [Type: enum$, 1, [...], Some>::Discriminant$] // cdb-command: dx -r2 g,! // cdb-check:g,! [Type: enum$, 1, [...], Some>] // cdb-check: [+0x000] dataful_variant [Type: enum$, 1, [...], Some>::Some] // cdb-check: [+0x000] __0 : 0x0 [Type: unsigned int *] -// cdb-check: [+0x000] discriminant$ [Type: enum$, 1, [...], Some>::discriminant$] -// cdb-check: [+0x000] discriminant : None (0x0) [Type: enum$, 1, [...], Some>::tag$] +// cdb-check: [+0x000] discriminant : None (0x0) [Type: enum$, 1, [...], Some>::Discriminant$] // cdb-command: dx h // cdb-check:h : Some [Type: enum$>] @@ -72,8 +65,7 @@ // cdb-check:k,! [Type: enum$, 1, [...], Some>] // cdb-check: [+0x000] dataful_variant [Type: enum$, 1, [...], Some>::Some] // cdb-check: [+0x000] __0 [Type: alloc::string::String] -// cdb-check: [+0x000] discriminant$ [Type: enum$, 1, [...], Some>::discriminant$] -// cdb-check: [+0x000] discriminant : 0x[...] [Type: enum$, 1, [...], Some>::tag$] +// cdb-check: [+0x000] discriminant : 0x[...] [Type: enum$, 1, [...], Some>::Discriminant$] pub enum CStyleEnum { Low = 2, From 94c45ef1089bfd9fa4304cf67dedce4788883051 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Wed, 2 Jun 2021 16:09:23 -0400 Subject: [PATCH 10/10] Update generator tests --- src/test/codegen/async-fn-debug-msvc.rs | 16 ++++++++-------- src/test/codegen/generator-debug-msvc.rs | 16 ++++++++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/test/codegen/async-fn-debug-msvc.rs b/src/test/codegen/async-fn-debug-msvc.rs index f2641404aae21..e410180bfff6f 100644 --- a/src/test/codegen/async-fn-debug-msvc.rs +++ b/src/test/codegen/async-fn-debug-msvc.rs @@ -17,33 +17,33 @@ async fn async_fn_test() { // FIXME: No way to reliably check the filename. // CHECK-DAG: [[ASYNC_FN:!.*]] = !DINamespace(name: "async_fn_test" -// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0", scope: [[ASYNC_FN]] -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0" +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant0", scope: [[GEN]], // For brevity, we only check the struct name and members of the last variant. // CHECK-SAME: file: [[FILE:![0-9]*]], line: 11, // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant1", scope: [[GEN]], // CHECK-SAME: file: [[FILE]], line: 15, // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant2", scope: [[GEN]], // CHECK-SAME: file: [[FILE]], line: 15, // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant3", scope: [[GEN]], // CHECK-SAME: file: [[FILE]], line: 12, // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant4", scope: [[GEN]], // CHECK-SAME: file: [[FILE]], line: 14, // CHECK-SAME: baseType: [[VARIANT:![0-9]*]] // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[ASYNC_FN]], +// CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN]], // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "RUST$ENUM$DISR", scope: [[S1]], +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant$", scope: [[S1]], // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]] // CHECK-NOT: flags: DIFlagArtificial diff --git a/src/test/codegen/generator-debug-msvc.rs b/src/test/codegen/generator-debug-msvc.rs index 44be71f3b9b80..7edb07d224c36 100644 --- a/src/test/codegen/generator-debug-msvc.rs +++ b/src/test/codegen/generator-debug-msvc.rs @@ -21,33 +21,33 @@ fn generator_test() -> impl Generator { // FIXME: No way to reliably check the filename. // CHECK-DAG: [[GEN_FN:!.*]] = !DINamespace(name: "generator_test" -// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0", scope: [[GEN_FN]] -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0" +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant0", scope: [[GEN]], // For brevity, we only check the struct name and members of the last variant. // CHECK-SAME: file: [[FILE:![0-9]*]], line: 14, // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant1", scope: [[GEN]], // CHECK-SAME: file: [[FILE]], line: 18, // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant2", scope: [[GEN]], // CHECK-SAME: file: [[FILE]], line: 18, // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant3", scope: [[GEN]], // CHECK-SAME: file: [[FILE]], line: 15, // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant4", scope: [[GEN]], // CHECK-SAME: file: [[FILE]], line: 17, // CHECK-SAME: baseType: [[VARIANT:![0-9]*]] // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN_FN]], +// CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN]], // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "RUST$ENUM$DISR", scope: [[S1]], +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant$", scope: [[S1]], // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]] // CHECK-NOT: flags: DIFlagArtificial