Skip to content

codegen_llvm: set DW_AT_accessibility #115165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,27 @@ fn build_field_di_node<'ll, 'tcx>(
}
}

/// Returns the `DIFlags` corresponding to the visibility of the item identified by `did`.
///
/// `DIFlags::Flag{Public,Protected,Private}` correspond to `DW_AT_accessibility`
/// (public/protected/private) aren't exactly right for Rust, but neither is `DW_AT_visibility`
/// (local/exported/qualified), and there's no way to set `DW_AT_visibility` in LLVM's API.
fn visibility_di_flags<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
did: DefId,
type_did: DefId,
) -> DIFlags {
let parent_did = cx.tcx.parent(type_did);
let visibility = cx.tcx.visibility(did);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to comment that it might make more sense to calculate the visibility from the perspective of the crate root (or, rather, as if you were using the crate as a dependency?) but I don't see why that necessarily be better and the current implementation is simpler and matches the visibility of the declaration site (even if the effective visibility ends up being more hidden due to the visibility/exports of parent modules).

Just noting that is another possible way to implement this but I don't see a strong reason to do that at this time.

match visibility {
Visibility::Public => DIFlags::FlagPublic,
// Private fields have a restricted visibility of the module containing the type.
Visibility::Restricted(did) if did == parent_did => DIFlags::FlagPrivate,
// `pub(crate)`/`pub(super)` visibilities are any other restricted visibility.
Visibility::Restricted(..) => DIFlags::FlagProtected,
}
}

/// Creates the debuginfo node for a Rust struct type. Maybe be a regular struct or a tuple-struct.
fn build_struct_type_di_node<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
Expand All @@ -1000,7 +1021,7 @@ fn build_struct_type_di_node<'ll, 'tcx>(
&compute_debuginfo_type_name(cx.tcx, struct_type, false),
size_and_align_of(struct_type_and_layout),
Some(containing_scope),
DIFlags::FlagZero,
visibility_di_flags(cx, adt_def.did(), adt_def.did()),
),
// Fields:
|cx, owner| {
Expand All @@ -1023,7 +1044,7 @@ fn build_struct_type_di_node<'ll, 'tcx>(
&field_name[..],
(field_layout.size, field_layout.align.abi),
struct_type_and_layout.fields.offset(i),
DIFlags::FlagZero,
visibility_di_flags(cx, f.did, adt_def.did()),
type_di_node(cx, field_layout.ty),
)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ use crate::{
enums::{tag_base_type, DiscrResult},
file_metadata, size_and_align_of, type_di_node,
type_map::{self, Stub, UniqueTypeId},
unknown_file_metadata, DINodeCreationResult, SmallVec, NO_GENERICS, NO_SCOPE_METADATA,
UNKNOWN_LINE_NUMBER,
unknown_file_metadata, visibility_di_flags, DINodeCreationResult, SmallVec,
NO_GENERICS, NO_SCOPE_METADATA, UNKNOWN_LINE_NUMBER,
},
utils::DIB,
},
Expand Down Expand Up @@ -215,7 +215,7 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
&enum_type_name,
cx.size_and_align_of(enum_type),
NO_SCOPE_METADATA,
DIFlags::FlagZero,
visibility_di_flags(cx, enum_adt_def.did(), enum_adt_def.did()),
),
|cx, enum_type_di_node| {
match enum_type_and_layout.variants {
Expand Down Expand Up @@ -320,13 +320,15 @@ fn build_single_variant_union_fields<'ll, 'tcx>(
variant_index: VariantIdx,
) -> SmallVec<&'ll DIType> {
let variant_layout = enum_type_and_layout.for_variant(cx, variant_index);
let visibility_flags = visibility_di_flags(cx, enum_adt_def.did(), enum_adt_def.did());
let variant_struct_type_di_node = super::build_enum_variant_struct_type_di_node(
cx,
enum_type_and_layout,
enum_type_di_node,
variant_index,
enum_adt_def.variant(variant_index),
variant_layout,
visibility_flags,
);

let tag_base_type = cx.tcx.types.u32;
Expand Down Expand Up @@ -364,7 +366,7 @@ fn build_single_variant_union_fields<'ll, 'tcx>(
// since the later is sometimes smaller (if it has fewer fields).
size_and_align_of(enum_type_and_layout),
Size::ZERO,
DIFlags::FlagZero,
visibility_flags,
variant_struct_type_wrapper_di_node,
),
unsafe {
Expand All @@ -376,7 +378,7 @@ fn build_single_variant_union_fields<'ll, 'tcx>(
unknown_file_metadata(cx),
UNKNOWN_LINE_NUMBER,
variant_names_type_di_node,
DIFlags::FlagZero,
visibility_flags,
Some(cx.const_u64(SINGLE_VARIANT_VIRTUAL_DISR)),
tag_base_type_align.bits() as u32,
)
Expand All @@ -403,6 +405,7 @@ fn build_union_fields_for_enum<'ll, 'tcx>(
(variant_index, variant_name)
}),
);
let visibility_flags = visibility_di_flags(cx, enum_adt_def.did(), enum_adt_def.did());

let variant_field_infos: SmallVec<VariantFieldInfo<'ll>> = variant_indices
.map(|variant_index| {
Expand All @@ -417,6 +420,7 @@ fn build_union_fields_for_enum<'ll, 'tcx>(
variant_index,
variant_def,
variant_layout,
visibility_flags,
);

VariantFieldInfo {
Expand All @@ -437,6 +441,7 @@ fn build_union_fields_for_enum<'ll, 'tcx>(
tag_base_type,
tag_field,
untagged_variant_index,
visibility_flags,
)
}

Expand Down Expand Up @@ -744,6 +749,7 @@ fn build_union_fields_for_direct_tag_coroutine<'ll, 'tcx>(
tag_base_type,
tag_field,
None,
DIFlags::FlagZero,
)
}

Expand All @@ -758,6 +764,7 @@ fn build_union_fields_for_direct_tag_enum_or_coroutine<'ll, 'tcx>(
tag_base_type: Ty<'tcx>,
tag_field: usize,
untagged_variant_index: Option<VariantIdx>,
di_flags: DIFlags,
) -> SmallVec<&'ll DIType> {
let tag_base_type_di_node = type_di_node(cx, tag_base_type);
let mut unions_fields = SmallVec::with_capacity(variant_field_infos.len() + 1);
Expand Down Expand Up @@ -801,7 +808,7 @@ fn build_union_fields_for_direct_tag_enum_or_coroutine<'ll, 'tcx>(
align.bits() as u32,
// Union fields are always at offset zero
Size::ZERO.bits(),
DIFlags::FlagZero,
di_flags,
variant_struct_type_wrapper,
)
}
Expand Down Expand Up @@ -835,7 +842,7 @@ fn build_union_fields_for_direct_tag_enum_or_coroutine<'ll, 'tcx>(
TAG_FIELD_NAME_128_LO,
size_and_align,
lo_offset,
DIFlags::FlagZero,
di_flags,
type_di_node,
));

Expand All @@ -855,7 +862,7 @@ fn build_union_fields_for_direct_tag_enum_or_coroutine<'ll, 'tcx>(
TAG_FIELD_NAME,
cx.size_and_align_of(enum_type_and_layout.field(cx, tag_field).ty),
enum_type_and_layout.fields.offset(tag_field),
DIFlags::FlagZero,
di_flags,
tag_base_type_di_node,
));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ fn build_enum_variant_struct_type_di_node<'ll, 'tcx>(
variant_index: VariantIdx,
variant_def: &VariantDef,
variant_layout: TyAndLayout<'tcx>,
di_flags: DIFlags,
) -> &'ll DIType {
debug_assert_eq!(variant_layout.ty, enum_type_and_layout.ty);

Expand All @@ -267,7 +268,7 @@ fn build_enum_variant_struct_type_di_node<'ll, 'tcx>(
// NOTE: We use size and align of enum_type, not from variant_layout:
size_and_align_of(enum_type_and_layout),
Some(enum_type_di_node),
DIFlags::FlagZero,
di_flags,
),
|cx, struct_type_di_node| {
(0..variant_layout.fields.count())
Expand All @@ -289,7 +290,7 @@ fn build_enum_variant_struct_type_di_node<'ll, 'tcx>(
&field_name,
(field_layout.size, field_layout.align.abi),
variant_layout.fields.offset(field_index),
DIFlags::FlagZero,
di_flags,
type_di_node(cx, field_layout.ty),
)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use crate::{
enums::tag_base_type,
file_metadata, size_and_align_of, type_di_node,
type_map::{self, Stub, StubInfo, UniqueTypeId},
unknown_file_metadata, DINodeCreationResult, SmallVec, NO_GENERICS,
UNKNOWN_LINE_NUMBER,
unknown_file_metadata, visibility_di_flags, DINodeCreationResult, SmallVec,
NO_GENERICS, UNKNOWN_LINE_NUMBER,
},
utils::{create_DIArray, get_namespace_for_item, DIB},
},
Expand Down Expand Up @@ -63,6 +63,8 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
let enum_type_and_layout = cx.layout_of(enum_type);
let enum_type_name = compute_debuginfo_type_name(cx.tcx, enum_type, false);

let visibility_flags = visibility_di_flags(cx, enum_adt_def.did(), enum_adt_def.did());

debug_assert!(!wants_c_like_enum_debuginfo(enum_type_and_layout));

type_map::build_type_with_children(
Expand All @@ -74,7 +76,7 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
&enum_type_name,
size_and_align_of(enum_type_and_layout),
Some(containing_scope),
DIFlags::FlagZero,
visibility_flags,
),
|cx, enum_type_di_node| {
// Build the struct type for each variant. These will be referenced by the
Expand All @@ -92,6 +94,7 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
variant_index,
enum_adt_def.variant(variant_index),
enum_type_and_layout.for_variant(cx, variant_index),
visibility_flags,
),
source_info: None,
})
Expand Down
26 changes: 26 additions & 0 deletions tests/codegen/debug-accessibility/crate-enum.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// compile-flags: -C debuginfo=2
// ignore-tidy-linelength

#![allow(dead_code)]

// Checks that visibility information is present in the debuginfo for crate-visibility enums.

mod module {
use std::hint::black_box;

pub(crate) enum CrateFooEnum {
A,
B(u32),
C { x: u32 },
}

// NONMSVC: {{!.*}} = !DICompositeType(tag: DW_TAG_structure_type, name: "CrateFooEnum"{{.*}}flags: DIFlagProtected{{.*}})
// MSVC: {{!.*}} = !DICompositeType(tag: DW_TAG_union_type, name: "enum2$<crate_enum::module::CrateFooEnum>"{{.*}}flags: DIFlagProtected{{.*}})
pub fn use_everything() {
black_box(CrateFooEnum::A);
}
}

fn main() {
module::use_everything();
}
23 changes: 23 additions & 0 deletions tests/codegen/debug-accessibility/crate-struct.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// compile-flags: -C debuginfo=2

#![allow(dead_code)]

// Checks that visibility information is present in the debuginfo for crate-visibility structs.

mod module {
use std::hint::black_box;

pub(crate) struct CrateFooStruct {
x: u32,
}

// CHECK: {{!.*}} = !DICompositeType(tag: DW_TAG_structure_type, name: "CrateFooStruct"{{.*}}flags: DIFlagProtected{{.*}})

pub fn use_everything() {
black_box(CrateFooStruct { x: 2 });
}
}

fn main() {
module::use_everything();
}
21 changes: 21 additions & 0 deletions tests/codegen/debug-accessibility/private-enum.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// compile-flags: -C debuginfo=2
// ignore-tidy-linelength

#![allow(dead_code)]

// Checks that visibility information is present in the debuginfo for private enums.

use std::hint::black_box;

enum PrivateFooEnum {
A,
B(u32),
C { x: u32 },
}

// NONMSVC: {{!.*}} = !DICompositeType(tag: DW_TAG_structure_type, name: "PrivateFooEnum"{{.*}}flags: DIFlagPrivate{{.*}})
// MSVC: {{!.*}} = !DICompositeType(tag: DW_TAG_union_type, name: "enum2$<private_enum::PrivateFooEnum>"{{.*}}flags: DIFlagPrivate{{.*}})

fn main() {
black_box(PrivateFooEnum::A);
}
17 changes: 17 additions & 0 deletions tests/codegen/debug-accessibility/private-struct.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// compile-flags: -C debuginfo=2

#![allow(dead_code)]

// Checks that visibility information is present in the debuginfo for private structs.

use std::hint::black_box;

struct PrivateFooStruct {
x: u32,
}

// CHECK: {{!.*}} = !DICompositeType(tag: DW_TAG_structure_type, name: "PrivateFooStruct"{{.*}}flags: DIFlagPrivate{{.*}})

fn main() {
black_box(PrivateFooStruct { x: 1 });
}
21 changes: 21 additions & 0 deletions tests/codegen/debug-accessibility/public-enum.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// compile-flags: -C debuginfo=2
// ignore-tidy-linelength

#![allow(dead_code)]

// Checks that visibility information is present in the debuginfo for types and their fields.

use std::hint::black_box;

pub enum PublicFooEnum {
A,
B(u32),
C { x: u32 },
}

// NONMSVC: {{!.*}} = !DICompositeType(tag: DW_TAG_structure_type, name: "PublicFooEnum"{{.*}}flags: DIFlagPublic{{.*}})
// MSVC: {{!.*}} = !DICompositeType(tag: DW_TAG_union_type, name: "enum2$<public_enum::PublicFooEnum>"{{.*}}flags: DIFlagPublic{{.*}})

fn main() {
black_box(PublicFooEnum::A);
}
17 changes: 17 additions & 0 deletions tests/codegen/debug-accessibility/public-struct.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// compile-flags: -C debuginfo=2

#![allow(dead_code)]

// Checks that visibility information is present in the debuginfo for public structs.

use std::hint::black_box;

pub struct PublicFooStruct {
x: u32,
}

// CHECK: {{!.*}} = !DICompositeType(tag: DW_TAG_structure_type, name: "PublicFooStruct"{{.*}}flags: DIFlagPublic{{.*}})

fn main() {
black_box(PublicFooStruct { x: 4 });
}
30 changes: 30 additions & 0 deletions tests/codegen/debug-accessibility/struct-fields.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// compile-flags: -C debuginfo=2

#![allow(dead_code)]

// Checks that visibility information is present in the debuginfo for struct fields.

mod module {
use std::hint::black_box;

struct StructFields {
a: u32,
pub(crate) b: u32,
pub(super) c: u32,
pub d: u32,
}

// CHECK: [[StructFields:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "StructFields"{{.*}}flags: DIFlagPrivate{{.*}})
// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "a", scope: [[StructFields]]{{.*}}flags: DIFlagPrivate{{.*}})
// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "b", scope: [[StructFields]]{{.*}}flags: DIFlagProtected{{.*}})
// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "c", scope: [[StructFields]]{{.*}}flags: DIFlagProtected{{.*}})
// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "d", scope: [[StructFields]]{{.*}}flags: DIFlagPublic{{.*}})

pub fn use_everything() {
black_box(StructFields { a: 1, b: 2, c: 3, d: 4 });
}
}

fn main() {
module::use_everything();
}
Loading