-
Notifications
You must be signed in to change notification settings - Fork 12.3k
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
[llvm][DebugInfo] DWARFv5: static data members declarations are DW_TAG_variable #72234
[llvm][DebugInfo] DWARFv5: static data members declarations are DW_TAG_variable #72234
Conversation
…G_variable This patch adds the LLVM-side infrastructure to implement DWARFv5 issue 161118.1: "DW_TAG for C++ static data members". The clang-side of this patch will simply construct the DIDerivedType with a different DW_TAG.
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-clang Author: Michael Buch (Michael137) ChangesThis patch adds the LLVM-side infrastructure to implement DWARFv5 issue 161118.1: "DW_TAG for C++ static data members". The clang-side of this patch will simply construct the DIDerivedType with a different DW_TAG. Full diff: https://github.com/llvm/llvm-project/pull/72234.diff 8 Files Affected:
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 04ca02cfe858579..b8acef67dee4c7c 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1681,7 +1681,8 @@ CGDebugInfo::CreateRecordStaticField(const VarDecl *Var, llvm::DIType *RecordTy,
llvm::DINode::DIFlags Flags = getAccessFlag(Var->getAccess(), RD);
auto Align = getDeclAlignIfRequired(Var, CGM.getContext());
llvm::DIDerivedType *GV = DBuilder.createStaticMemberType(
- RecordTy, VName, VUnit, LineNumber, VTy, Flags, /* Val */ nullptr, Align);
+ RecordTy, VName, VUnit, LineNumber, VTy, Flags, /* Val */ nullptr,
+ DW_TAG_member, Align);
StaticDataMemberCache[Var->getCanonicalDecl()].reset(GV);
StaticDataMemberDefinitionsToEmit.push_back(Var->getCanonicalDecl());
return GV;
diff --git a/llvm/include/llvm-c/DebugInfo.h b/llvm/include/llvm-c/DebugInfo.h
index 5924294708cc354..61659d514b520e5 100644
--- a/llvm/include/llvm-c/DebugInfo.h
+++ b/llvm/include/llvm-c/DebugInfo.h
@@ -785,14 +785,14 @@ LLVMMetadataRef LLVMDIBuilderCreateMemberType(
* \param Type Type of the static member.
* \param Flags Flags to encode member attribute, e.g. private.
* \param ConstantVal Const initializer of the member.
+ * \param Tag DWARF tag of the static member.
* \param AlignInBits Member alignment.
*/
-LLVMMetadataRef
-LLVMDIBuilderCreateStaticMemberType(
+LLVMMetadataRef LLVMDIBuilderCreateStaticMemberType(
LLVMDIBuilderRef Builder, LLVMMetadataRef Scope, const char *Name,
size_t NameLen, LLVMMetadataRef File, unsigned LineNumber,
LLVMMetadataRef Type, LLVMDIFlags Flags, LLVMValueRef ConstantVal,
- uint32_t AlignInBits);
+ unsigned Tag, uint32_t AlignInBits);
/**
* Create debugging information entry for a pointer to member.
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index ecd6dd7b0a4f822..2be133e85e8c1e4 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -376,11 +376,12 @@ namespace llvm {
/// \param Ty Type of the static member.
/// \param Flags Flags to encode member attribute, e.g. private.
/// \param Val Const initializer of the member.
+ /// \param Tag DWARF tag of the static member.
/// \param AlignInBits Member alignment.
DIDerivedType *createStaticMemberType(DIScope *Scope, StringRef Name,
DIFile *File, unsigned LineNo,
DIType *Ty, DINode::DIFlags Flags,
- Constant *Val,
+ Constant *Val, unsigned Tag,
uint32_t AlignInBits = 0);
/// Create debugging information entry for Objective-C
diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
index 1ce8c17f8a880f6..58a7e07d9b58d86 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -405,12 +405,11 @@ DIDerivedType *
DIBuilder::createStaticMemberType(DIScope *Scope, StringRef Name, DIFile *File,
unsigned LineNumber, DIType *Ty,
DINode::DIFlags Flags, llvm::Constant *Val,
- uint32_t AlignInBits) {
+ unsigned Tag, uint32_t AlignInBits) {
Flags |= DINode::FlagStaticMember;
- return DIDerivedType::get(VMContext, dwarf::DW_TAG_member, Name, File,
- LineNumber, getNonCompileUnitScope(Scope), Ty, 0,
- AlignInBits, 0, std::nullopt, Flags,
- getConstantOrNull(Val));
+ return DIDerivedType::get(VMContext, Tag, Name, File, LineNumber,
+ getNonCompileUnitScope(Scope), Ty, 0, AlignInBits,
+ 0, std::nullopt, Flags, getConstantOrNull(Val));
}
DIDerivedType *
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 390a27c4bc0c4dd..0f3590ec66bf2b9 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -1315,17 +1315,15 @@ LLVMDIBuilderCreateUnspecifiedType(LLVMDIBuilderRef Builder, const char *Name,
return wrap(unwrap(Builder)->createUnspecifiedType({Name, NameLen}));
}
-LLVMMetadataRef
-LLVMDIBuilderCreateStaticMemberType(
+LLVMMetadataRef LLVMDIBuilderCreateStaticMemberType(
LLVMDIBuilderRef Builder, LLVMMetadataRef Scope, const char *Name,
size_t NameLen, LLVMMetadataRef File, unsigned LineNumber,
LLVMMetadataRef Type, LLVMDIFlags Flags, LLVMValueRef ConstantVal,
- uint32_t AlignInBits) {
+ unsigned Tag, uint32_t AlignInBits) {
return wrap(unwrap(Builder)->createStaticMemberType(
- unwrapDI<DIScope>(Scope), {Name, NameLen},
- unwrapDI<DIFile>(File), LineNumber, unwrapDI<DIType>(Type),
- map_from_llvmDIFlags(Flags), unwrap<Constant>(ConstantVal),
- AlignInBits));
+ unwrapDI<DIScope>(Scope), {Name, NameLen}, unwrapDI<DIFile>(File),
+ LineNumber, unwrapDI<DIType>(Type), map_from_llvmDIFlags(Flags),
+ unwrap<Constant>(ConstantVal), Tag, AlignInBits));
}
LLVMMetadataRef
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index 0e14ec90b51f1fb..943826c6ac89df9 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -711,7 +711,9 @@ Constant *DIDerivedType::getStorageOffsetInBits() const {
}
Constant *DIDerivedType::getConstant() const {
- assert(getTag() == dwarf::DW_TAG_member && isStaticMember());
+ assert((getTag() == dwarf::DW_TAG_member ||
+ getTag() == dwarf::DW_TAG_variable) &&
+ isStaticMember());
if (auto *C = cast_or_null<ConstantAsMetadata>(getExtraData()))
return C->getValue();
return nullptr;
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index b1d1075285c2210..25981d8dccb11e7 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -1172,6 +1172,7 @@ void Verifier::visitDIDerivedType(const DIDerivedType &N) {
N.getTag() == dwarf::DW_TAG_restrict_type ||
N.getTag() == dwarf::DW_TAG_atomic_type ||
N.getTag() == dwarf::DW_TAG_member ||
+ (N.getTag() == dwarf::DW_TAG_variable && N.isStaticMember()) ||
N.getTag() == dwarf::DW_TAG_inheritance ||
N.getTag() == dwarf::DW_TAG_friend ||
N.getTag() == dwarf::DW_TAG_set_type,
diff --git a/llvm/test/DebugInfo/Generic/dwarf5-debug-info-static-member.ll b/llvm/test/DebugInfo/Generic/dwarf5-debug-info-static-member.ll
new file mode 100644
index 000000000000000..6e8f48d652e34b2
--- /dev/null
+++ b/llvm/test/DebugInfo/Generic/dwarf5-debug-info-static-member.ll
@@ -0,0 +1,109 @@
+; RUN: %llc_dwarf -filetype=obj -o %t.o < %s
+; RUN: llvm-dwarfdump -v -debug-info %t.o | FileCheck %s -check-prefix=CHECK
+
+; LLVM IR generated using: clang -emit-llvm -S -g -gdwarf-5
+;
+; struct C
+; {
+; static int a;
+; const static bool b = true;
+; static constexpr int c = 15;
+; };
+;
+; int C::a = 10;
+;
+; int main()
+; {
+; C instance_C;
+; return C::a + (int)C::b + C::c;
+; }
+
+source_filename = "llvm/test/DebugInfo/X86/dwarf5-debug-info-static-member.ll"
+
+%struct.C = type { i8 }
+
+@_ZN1C1aE = global i32 10, align 4, !dbg !0
+
+; Function Attrs: mustprogress noinline norecurse nounwind optnone ssp uwtable(sync)
+define noundef i32 @main() #0 !dbg !26 {
+entry:
+ %retval = alloca i32, align 4
+ %instance_C = alloca %struct.C, align 1
+ store i32 0, ptr %retval, align 4
+ call void @llvm.dbg.declare(metadata ptr %instance_C, metadata !30, metadata !DIExpression()), !dbg !31
+ %0 = load i32, ptr @_ZN1C1aE, align 4, !dbg !32
+ %add = add nsw i32 %0, 1, !dbg !33
+ %add1 = add nsw i32 %add, 15, !dbg !34
+ ret i32 %add1, !dbg !35
+}
+
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { nounwind uwtable }
+attributes #1 = { nounwind readnone }
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!19, !20, !21, !22, !23, !24}
+!llvm.ident = !{!25}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "a", linkageName: "_ZN1C1aE", scope: !2, file: !3, line: 8, type: !5, isLocal: false, isDefinition: true, declaration: !8)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !3, producer: "clang version 18.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, retainedTypes: !4, globals: !14, splitDebugInlining: false, nameTableKind: Apple, sysroot: "/")
+!3 = !DIFile(filename: "main.cpp", directory: "/tmp", checksumkind: CSK_MD5, checksum: "b2547f7df8f54777c012dbfbd626f155")
+!4 = !{!5, !6}
+!5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!6 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "C", file: !3, line: 1, size: 8, flags: DIFlagTypePassByValue, elements: !7, identifier: "_ZTS1C")
+!7 = !{!8, !9, !12}
+!8 = !DIDerivedType(tag: DW_TAG_variable, name: "a", scope: !6, file: !3, line: 3, baseType: !5, flags: DIFlagStaticMember)
+!9 = !DIDerivedType(tag: DW_TAG_variable, name: "b", scope: !6, file: !3, line: 4, baseType: !10, flags: DIFlagStaticMember)
+!10 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !11)
+!11 = !DIBasicType(name: "bool", size: 8, encoding: DW_ATE_boolean)
+!12 = !DIDerivedType(tag: DW_TAG_variable, name: "c", scope: !6, file: !3, line: 5, baseType: !13, flags: DIFlagStaticMember)
+!13 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !5)
+!14 = !{!0, !15, !17}
+!15 = !DIGlobalVariableExpression(var: !16, expr: !DIExpression(DW_OP_constu, 1, DW_OP_stack_value))
+!16 = distinct !DIGlobalVariable(name: "b", scope: !2, file: !3, line: 4, type: !10, isLocal: true, isDefinition: true, declaration: !9)
+!17 = !DIGlobalVariableExpression(var: !18, expr: !DIExpression(DW_OP_constu, 15, DW_OP_stack_value))
+!18 = distinct !DIGlobalVariable(name: "c", scope: !2, file: !3, line: 5, type: !13, isLocal: true, isDefinition: true, declaration: !12)
+!19 = !{i32 7, !"Dwarf Version", i32 5}
+!20 = !{i32 2, !"Debug Info Version", i32 3}
+!21 = !{i32 1, !"wchar_size", i32 4}
+!22 = !{i32 8, !"PIC Level", i32 2}
+!23 = !{i32 7, !"uwtable", i32 1}
+!24 = !{i32 7, !"frame-pointer", i32 1}
+!25 = !{!"clang version 18.0.0"}
+!26 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 10, type: !27, scopeLine: 11, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !2, retainedNodes: !29)
+!27 = !DISubroutineType(types: !28)
+!28 = !{!5}
+!29 = !{}
+!30 = !DILocalVariable(name: "instance_C", scope: !26, file: !3, line: 12, type: !6)
+!31 = !DILocation(line: 12, column: 5, scope: !26)
+!32 = !DILocation(line: 13, column: 10, scope: !26)
+!33 = !DILocation(line: 13, column: 15, scope: !26)
+!34 = !DILocation(line: 13, column: 27, scope: !26)
+!35 = !DILocation(line: 13, column: 3, scope: !26)
+
+; CHECK: .debug_info contents:
+; CHECK: DW_TAG_variable
+; CHECK-NEXT: DW_AT_specification {{.*}} "a"
+; CHECK: DW_TAG_structure_type
+; CHECK: DW_AT_name {{.*}} "C"
+; CHECK: DW_TAG_variable
+; CHECK-NEXT: DW_AT_name {{.*}} "a"
+; CHECK: DW_AT_external
+; CHECK: DW_AT_declaration
+; CHECK: DW_TAG_variable
+; CHECK-NEXT: DW_AT_name {{.*}} "b"
+; CHECK: DW_AT_external
+; CHECK: DW_AT_declaration
+; CHECK: DW_TAG_variable
+; CHECK-NEXT: DW_AT_name {{.*}} "c"
+; CHECK: DW_AT_external
+; CHECK: DW_AT_declaration
+; CHECK: NULL
+; CHECK: DW_TAG_variable
+; CHECK-NEXT: DW_AT_specification {{.*}} "b"
+; CHECK-NEXT: DW_AT_const_value
+; CHECK: DW_TAG_variable
+; CHECK-NEXT: DW_AT_specification {{.*}} "c"
|
LLVMDIBuilderRef Builder, LLVMMetadataRef Scope, const char *Name, | ||
size_t NameLen, LLVMMetadataRef File, unsigned LineNumber, | ||
LLVMMetadataRef Type, LLVMDIFlags Flags, LLVMValueRef ConstantVal, | ||
uint32_t AlignInBits) { | ||
unsigned Tag, uint32_t AlignInBits) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somebody might complain if we change the C API, but I don't think we guarantee compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is missing a test in clang/test/CodeGenCXX that verifies Clang generates the expected LLVM IR.
@@ -1681,7 +1681,8 @@ CGDebugInfo::CreateRecordStaticField(const VarDecl *Var, llvm::DIType *RecordTy, | |||
llvm::DINode::DIFlags Flags = getAccessFlag(Var->getAccess(), RD); | |||
auto Align = getDeclAlignIfRequired(Var, CGM.getContext()); | |||
llvm::DIDerivedType *GV = DBuilder.createStaticMemberType( | |||
RecordTy, VName, VUnit, LineNumber, VTy, Flags, /* Val */ nullptr, Align); | |||
RecordTy, VName, VUnit, LineNumber, VTy, Flags, /* Val */ nullptr, | |||
llvm::dwarf::DW_TAG_member, Align); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming this changes the IR.
I added those tests here: #72235 The IR doesn't change with this patch. It merely adds the necessary parameter to the DIBuilder API to be used from clang in the subsequent patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, then
Looks like the OCaml bindings are using the C-API that was changed in this PR, causing some buildbots to fail. I'll revert the change to the C-bindings for now |
…re DW_TAG_variable (#72234)" This reverts commit 9a9933f. The OCaml bindings were using `LLVMDIBuilderCreateStaticMemberType`, causing the API change in `9a9933fae23249fbf6cf5b3c090e630f578b7f98` to break buildbots that built the bindings. Revert until we figure out whether to fixup the bindings or just not change the C-API
…re DW_TAG_variable (#72234)" This was reverted because it broke the OCaml LLVM bindings. Relanding the original patch but without changing the C-API. They'll continue to work just fine as they do today. If in the future there is a need to pass a new tag to the C-API for creating static members, then we'll make the change to the OCaml bindings at that time. Original commit message: """ This patch adds the LLVM-side infrastructure to implement DWARFv5 issue 161118.1: "DW_TAG for C++ static data members". The clang-side of this patch will simply construct the DIDerivedType with a different DW_TAG. """
…AG_variable (#72235) This patch implements the DWARFv5 issue 161118.1: "DW_TAG for C++ static data members". This will simplify LLDB's handling of static data members greatly in the long term since we no longer need to differentiate non-static from static data member declarations using non-portable heuristics. Depends on: * #72234
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adrian-prantl We do have a pretty strong guarantee of compatibility in the llvm-c API. A new parameter pretty much means you need to define a new function.
Ended up not changing the C-API in the relanded version |
Makes llpc compatible with the change in llvm/llvm-project#72234
Makes llpc compatible with the change in llvm/llvm-project#72234
Makes llpc compatible with the change in llvm/llvm-project#72234
we're seeing debug info verifier issues in Rust with this change, e.g.
is this expected? |
Hmmm yea that's problematic. Feel free to revert since I'm currently not at my PC. It looks like the Tag parameter wouldve been better off strongly typed since I suspect there are some callers that I missed that now pass alignment to where now the tag is. Thought I had checked each callsite. Please make sure you revert the two dependent clang/lldb patches too |
ah I see, that definitely sounds plausible I've also recently done some NFC changes to better catch issues like this in #66295 |
actually I think the caller is in Rust, so perhaps we should just update Rust instead. but making some of these params more strongly typed would be good |
Is it possible that this is to be fixed at the callsites in the rust frontend? Dont think i missed any callers in LLVM (then again I'm not at a PC to confirm this atm) |
…rType() This was added in llvm/llvm-project#72234. DW_TAG_member was the implicit default before.
llvm-wrapper: Pass newly added param to DIBuilder::createStaticMemberType() This was added in llvm/llvm-project#72234. DW_TAG_member was the implicit default before. The LLVM change is quite sinister since due to weakly typed ints and default params, this was still successfully compiling against LLVM but was passing the wrong parameters.
llvm-wrapper: Pass newly added param to DIBuilder::createStaticMemberType() This was added in llvm/llvm-project#72234. DW_TAG_member was the implicit default before. The LLVM change is quite sinister since due to weakly typed ints and default params, this was still successfully compiling against LLVM but was passing the wrong parameters.
Makes llpc compatible with the change in llvm/llvm-project#72234
…G_variable (llvm#72234) This patch adds the LLVM-side infrastructure to implement DWARFv5 issue 161118.1: "DW_TAG for C++ static data members". The clang-side of this patch will simply construct the DIDerivedType with a different DW_TAG.
…re DW_TAG_variable (llvm#72234)" This reverts commit 9a9933f. The OCaml bindings were using `LLVMDIBuilderCreateStaticMemberType`, causing the API change in `9a9933fae23249fbf6cf5b3c090e630f578b7f98` to break buildbots that built the bindings. Revert until we figure out whether to fixup the bindings or just not change the C-API
…re DW_TAG_variable (llvm#72234)" This was reverted because it broke the OCaml LLVM bindings. Relanding the original patch but without changing the C-API. They'll continue to work just fine as they do today. If in the future there is a need to pass a new tag to the C-API for creating static members, then we'll make the change to the OCaml bindings at that time. Original commit message: """ This patch adds the LLVM-side infrastructure to implement DWARFv5 issue 161118.1: "DW_TAG for C++ static data members". The clang-side of this patch will simply construct the DIDerivedType with a different DW_TAG. """
…AG_variable (llvm#72235) This patch implements the DWARFv5 issue 161118.1: "DW_TAG for C++ static data members". This will simplify LLDB's handling of static data members greatly in the long term since we no longer need to differentiate non-static from static data member declarations using non-portable heuristics. Depends on: * llvm#72234
…ata members declarations (llvm#72236) The accepted DWARFv5 issue 161118.1: "DW_TAG for C++ static data members" specifies that static data member declaration be described by DW_TAG_variable. Make sure we recognize such members. Depends on: * llvm#72234 * llvm#72235
This patch adds the LLVM-side infrastructure to implement DWARFv5 issue 161118.1: "DW_TAG for C++ static data members".
The clang-side of this patch will simply construct the DIDerivedType with a different DW_TAG.