[clang][DebugInfo] Emit DW_AT_object_pointer on function declarations with explicit this
#122928
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Will be rebased on top of #122897 |
…with explicit `this` We currently don't emit `DW_AT_object_pointer` on function declarations or definitions. The DWARFv5 spec doesn't mandate this attribute be present *only* for implicit object parameters: ``` If the member function entry describes a non-static member function, then that entry has a DW_AT_object_pointer attribute whose value is a reference to the formal parameter entry that corresponds to the object for which the function is called. That parameter also has a DW_AT_artificial attribute whose value is true. ``` The part about `DW_AT_artificial` seems overly restrictive, and not true for explicit object parameters. We probably should relax this part of the DWARF spec. This will help LLDB identify static vs. non-static member functions (see llvm#120856). GCC suffers from the same issue: https://godbolt.org/z/h4jeT54G5 Partially fixes llvm#120974
Michael137
force-pushed
the
clang/explicit-object-pointer-on-declaration
branch
from
b1a937b to
88125a0
Compare
llvmbot
added
clang
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-clang Author: Michael Buch (Michael137) ChangesIn #122897 we started attaching Fixes #120974 Full diff: https://github.com/llvm/llvm-project/pull/122928.diff 6 Files Affected:
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index f88f56c98186da..6cbcaf03844102 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2016,13 +2016,15 @@ llvm::DISubroutineType *CGDebugInfo::getOrCreateInstanceMethodType(
// First element is always return type. For 'void' functions it is NULL.
Elts.push_back(Args[0]);
- // "this" pointer is always first argument.
- // ThisPtr may be null if the member function has an explicit 'this'
- // parameter.
- if (!ThisPtr.isNull()) {
+ const bool HasExplicitObjectParameter = ThisPtr.isNull();
+
+ // "this" pointer is always first argument. For explicit "this"
+ // parameters, it will already be in Args[1].
+ if (!HasExplicitObjectParameter) {
llvm::DIType *ThisPtrType = getOrCreateType(ThisPtr, Unit);
TypeCache[ThisPtr.getAsOpaquePtr()].reset(ThisPtrType);
- ThisPtrType = DBuilder.createObjectPointerType(ThisPtrType);
+ ThisPtrType =
+ DBuilder.createObjectPointerType(ThisPtrType, /*Implicit=*/true);
Elts.push_back(ThisPtrType);
}
@@ -2030,6 +2032,13 @@ llvm::DISubroutineType *CGDebugInfo::getOrCreateInstanceMethodType(
for (unsigned i = 1, e = Args.size(); i != e; ++i)
Elts.push_back(Args[i]);
+ // Attach FlagObjectPointer to the explicit "this" parameter.
+ if (HasExplicitObjectParameter) {
+ assert(Elts.size() >= 2 && Args.size() >= 2 &&
+ "Expected at least return type and object parameter.");
+ Elts[1] = DBuilder.createObjectPointerType(Args[1], /*Implicit=*/false);
+ }
+
llvm::DITypeRefArray EltTypeArray = DBuilder.getOrCreateTypeArray(Elts);
return DBuilder.createSubroutineType(EltTypeArray, OriginalFunc->getFlags(),
@@ -5118,7 +5127,7 @@ llvm::DIType *CGDebugInfo::CreateSelfType(const QualType &QualTy,
llvm::DIType *CachedTy = getTypeOrNull(QualTy);
if (CachedTy)
Ty = CachedTy;
- return DBuilder.createObjectPointerType(Ty);
+ return DBuilder.createObjectPointerType(Ty, /*Implicit=*/true);
}
void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable(
diff --git a/clang/test/CodeGenCXX/debug-info-object-pointer.cpp b/clang/test/CodeGenCXX/debug-info-object-pointer.cpp
index 594d4da791ee84..49079f59909968 100644
--- a/clang/test/CodeGenCXX/debug-info-object-pointer.cpp
+++ b/clang/test/CodeGenCXX/debug-info-object-pointer.cpp
@@ -5,12 +5,11 @@
// CHECK: !DIDerivedType(tag: DW_TAG_pointer_type
// CHECK-SAME: flags: DIFlagArtificial | DIFlagObjectPointer
//
-// // FIXME: DIFlagObjectPointer not attached to the explicit object
-// // argument in the subprogram declaration.
// CHECK: !DISubprogram(name: "explicit_this",
// flags: DIFlagPrototyped
-// CHECK-NOT: DIFlagObjectPointer
-// CHECK-NOT: DIFlagArtificial
+//
+// CHECK: !DIDerivedType(tag: DW_TAG_rvalue_reference_type
+// CHECK-SAME: flags: DIFlagObjectPointer)
//
// CHECK: !DILocalVariable(name: "this", arg: 1
// CHECK-SAME: flags: DIFlagArtificial | DIFlagObjectPointer
diff --git a/llvm/include/llvm-c/DebugInfo.h b/llvm/include/llvm-c/DebugInfo.h
index 07f87d44088e7e..ac7ee5a7cc9a19 100644
--- a/llvm/include/llvm-c/DebugInfo.h
+++ b/llvm/include/llvm-c/DebugInfo.h
@@ -870,13 +870,16 @@ LLVMDIBuilderCreateObjCProperty(LLVMDIBuilderRef Builder,
LLVMMetadataRef Ty);
/**
- * Create a uniqued DIType* clone with FlagObjectPointer and FlagArtificial set.
+ * Create a uniqued DIType* clone with FlagObjectPointer. If \c Implicit
+ * is true, then also set FlagArtificial.
* \param Builder The DIBuilder.
* \param Type The underlying type to which this pointer points.
+ * \param Implicit Indicates whether this pointer was implicitly generated
+ * (i.e., not spelled out in source).
*/
-LLVMMetadataRef
-LLVMDIBuilderCreateObjectPointerType(LLVMDIBuilderRef Builder,
- LLVMMetadataRef Type);
+LLVMMetadataRef LLVMDIBuilderCreateObjectPointerType(LLVMDIBuilderRef Builder,
+ LLVMMetadataRef Type,
+ LLVMBool Implicit);
/**
* Create debugging information entry for a qualified
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index cb1150c269a1da..6c479415b9ed27 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -662,9 +662,9 @@ namespace llvm {
/// Create a uniqued clone of \p Ty with FlagArtificial set.
static DIType *createArtificialType(DIType *Ty);
- /// Create a uniqued clone of \p Ty with FlagObjectPointer and
- /// FlagArtificial set.
- static DIType *createObjectPointerType(DIType *Ty);
+ /// Create a uniqued clone of \p Ty with FlagObjectPointer set.
+ /// If \p Implicit is true, also set FlagArtificial.
+ static DIType *createObjectPointerType(DIType *Ty, bool Implicit);
/// Create a permanent forward-declared type.
DICompositeType *createForwardDecl(unsigned Tag, StringRef Name,
diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
index b240a2a39de362..d9bd4f11e89a39 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -644,11 +644,15 @@ DIType *DIBuilder::createArtificialType(DIType *Ty) {
return createTypeWithFlags(Ty, DINode::FlagArtificial);
}
-DIType *DIBuilder::createObjectPointerType(DIType *Ty) {
+DIType *DIBuilder::createObjectPointerType(DIType *Ty, bool Implicit) {
// FIXME: Restrict this to the nodes where it's valid.
if (Ty->isObjectPointer())
return Ty;
- DINode::DIFlags Flags = DINode::FlagObjectPointer | DINode::FlagArtificial;
+ DINode::DIFlags Flags = DINode::FlagObjectPointer;
+
+ if (Implicit)
+ Flags |= DINode::FlagArtificial;
+
return createTypeWithFlags(Ty, Flags);
}
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index e5b45e0082a823..4ce518009bd3ea 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -1432,10 +1432,11 @@ LLVMDIBuilderCreateObjCProperty(LLVMDIBuilderRef Builder,
PropertyAttributes, unwrapDI<DIType>(Ty)));
}
-LLVMMetadataRef
-LLVMDIBuilderCreateObjectPointerType(LLVMDIBuilderRef Builder,
- LLVMMetadataRef Type) {
- return wrap(unwrap(Builder)->createObjectPointerType(unwrapDI<DIType>(Type)));
+LLVMMetadataRef LLVMDIBuilderCreateObjectPointerType(LLVMDIBuilderRef Builder,
+ LLVMMetadataRef Type,
+ LLVMBool Implicit) {
+ return wrap(unwrap(Builder)->createObjectPointerType(unwrapDI<DIType>(Type),
+ Implicit));
}
LLVMMetadataRef
|
|
@llvm/pr-subscribers-clang-codegen Author: Michael Buch (Michael137) ChangesIn #122897 we started attaching Fixes #120974 Full diff: https://github.com/llvm/llvm-project/pull/122928.diff 6 Files Affected:
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index f88f56c98186da..6cbcaf03844102 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2016,13 +2016,15 @@ llvm::DISubroutineType *CGDebugInfo::getOrCreateInstanceMethodType(
// First element is always return type. For 'void' functions it is NULL.
Elts.push_back(Args[0]);
- // "this" pointer is always first argument.
- // ThisPtr may be null if the member function has an explicit 'this'
- // parameter.
- if (!ThisPtr.isNull()) {
+ const bool HasExplicitObjectParameter = ThisPtr.isNull();
+
+ // "this" pointer is always first argument. For explicit "this"
+ // parameters, it will already be in Args[1].
+ if (!HasExplicitObjectParameter) {
llvm::DIType *ThisPtrType = getOrCreateType(ThisPtr, Unit);
TypeCache[ThisPtr.getAsOpaquePtr()].reset(ThisPtrType);
- ThisPtrType = DBuilder.createObjectPointerType(ThisPtrType);
+ ThisPtrType =
+ DBuilder.createObjectPointerType(ThisPtrType, /*Implicit=*/true);
Elts.push_back(ThisPtrType);
}
@@ -2030,6 +2032,13 @@ llvm::DISubroutineType *CGDebugInfo::getOrCreateInstanceMethodType(
for (unsigned i = 1, e = Args.size(); i != e; ++i)
Elts.push_back(Args[i]);
+ // Attach FlagObjectPointer to the explicit "this" parameter.
+ if (HasExplicitObjectParameter) {
+ assert(Elts.size() >= 2 && Args.size() >= 2 &&
+ "Expected at least return type and object parameter.");
+ Elts[1] = DBuilder.createObjectPointerType(Args[1], /*Implicit=*/false);
+ }
+
llvm::DITypeRefArray EltTypeArray = DBuilder.getOrCreateTypeArray(Elts);
return DBuilder.createSubroutineType(EltTypeArray, OriginalFunc->getFlags(),
@@ -5118,7 +5127,7 @@ llvm::DIType *CGDebugInfo::CreateSelfType(const QualType &QualTy,
llvm::DIType *CachedTy = getTypeOrNull(QualTy);
if (CachedTy)
Ty = CachedTy;
- return DBuilder.createObjectPointerType(Ty);
+ return DBuilder.createObjectPointerType(Ty, /*Implicit=*/true);
}
void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable(
diff --git a/clang/test/CodeGenCXX/debug-info-object-pointer.cpp b/clang/test/CodeGenCXX/debug-info-object-pointer.cpp
index 594d4da791ee84..49079f59909968 100644
--- a/clang/test/CodeGenCXX/debug-info-object-pointer.cpp
+++ b/clang/test/CodeGenCXX/debug-info-object-pointer.cpp
@@ -5,12 +5,11 @@
// CHECK: !DIDerivedType(tag: DW_TAG_pointer_type
// CHECK-SAME: flags: DIFlagArtificial | DIFlagObjectPointer
//
-// // FIXME: DIFlagObjectPointer not attached to the explicit object
-// // argument in the subprogram declaration.
// CHECK: !DISubprogram(name: "explicit_this",
// flags: DIFlagPrototyped
-// CHECK-NOT: DIFlagObjectPointer
-// CHECK-NOT: DIFlagArtificial
+//
+// CHECK: !DIDerivedType(tag: DW_TAG_rvalue_reference_type
+// CHECK-SAME: flags: DIFlagObjectPointer)
//
// CHECK: !DILocalVariable(name: "this", arg: 1
// CHECK-SAME: flags: DIFlagArtificial | DIFlagObjectPointer
diff --git a/llvm/include/llvm-c/DebugInfo.h b/llvm/include/llvm-c/DebugInfo.h
index 07f87d44088e7e..ac7ee5a7cc9a19 100644
--- a/llvm/include/llvm-c/DebugInfo.h
+++ b/llvm/include/llvm-c/DebugInfo.h
@@ -870,13 +870,16 @@ LLVMDIBuilderCreateObjCProperty(LLVMDIBuilderRef Builder,
LLVMMetadataRef Ty);
/**
- * Create a uniqued DIType* clone with FlagObjectPointer and FlagArtificial set.
+ * Create a uniqued DIType* clone with FlagObjectPointer. If \c Implicit
+ * is true, then also set FlagArtificial.
* \param Builder The DIBuilder.
* \param Type The underlying type to which this pointer points.
+ * \param Implicit Indicates whether this pointer was implicitly generated
+ * (i.e., not spelled out in source).
*/
-LLVMMetadataRef
-LLVMDIBuilderCreateObjectPointerType(LLVMDIBuilderRef Builder,
- LLVMMetadataRef Type);
+LLVMMetadataRef LLVMDIBuilderCreateObjectPointerType(LLVMDIBuilderRef Builder,
+ LLVMMetadataRef Type,
+ LLVMBool Implicit);
/**
* Create debugging information entry for a qualified
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index cb1150c269a1da..6c479415b9ed27 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -662,9 +662,9 @@ namespace llvm {
/// Create a uniqued clone of \p Ty with FlagArtificial set.
static DIType *createArtificialType(DIType *Ty);
- /// Create a uniqued clone of \p Ty with FlagObjectPointer and
- /// FlagArtificial set.
- static DIType *createObjectPointerType(DIType *Ty);
+ /// Create a uniqued clone of \p Ty with FlagObjectPointer set.
+ /// If \p Implicit is true, also set FlagArtificial.
+ static DIType *createObjectPointerType(DIType *Ty, bool Implicit);
/// Create a permanent forward-declared type.
DICompositeType *createForwardDecl(unsigned Tag, StringRef Name,
diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
index b240a2a39de362..d9bd4f11e89a39 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -644,11 +644,15 @@ DIType *DIBuilder::createArtificialType(DIType *Ty) {
return createTypeWithFlags(Ty, DINode::FlagArtificial);
}
-DIType *DIBuilder::createObjectPointerType(DIType *Ty) {
+DIType *DIBuilder::createObjectPointerType(DIType *Ty, bool Implicit) {
// FIXME: Restrict this to the nodes where it's valid.
if (Ty->isObjectPointer())
return Ty;
- DINode::DIFlags Flags = DINode::FlagObjectPointer | DINode::FlagArtificial;
+ DINode::DIFlags Flags = DINode::FlagObjectPointer;
+
+ if (Implicit)
+ Flags |= DINode::FlagArtificial;
+
return createTypeWithFlags(Ty, Flags);
}
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index e5b45e0082a823..4ce518009bd3ea 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -1432,10 +1432,11 @@ LLVMDIBuilderCreateObjCProperty(LLVMDIBuilderRef Builder,
PropertyAttributes, unwrapDI<DIType>(Ty)));
}
-LLVMMetadataRef
-LLVMDIBuilderCreateObjectPointerType(LLVMDIBuilderRef Builder,
- LLVMMetadataRef Type) {
- return wrap(unwrap(Builder)->createObjectPointerType(unwrapDI<DIType>(Type)));
+LLVMMetadataRef LLVMDIBuilderCreateObjectPointerType(LLVMDIBuilderRef Builder,
+ LLVMMetadataRef Type,
+ LLVMBool Implicit) {
+ return wrap(unwrap(Builder)->createObjectPointerType(unwrapDI<DIType>(Type),
+ Implicit));
}
LLVMMetadataRef
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/16520 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/12191 Here is the relevant piece of the build log for the reference |
|
I'm going to revert because of the buildbot failures. |
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 18, 2025
…nction declarations with explicit `this`" (#123455) Reverts llvm/llvm-project#122928
|
Thanks! |
junlarsen
added a commit
to junlarsen/llvm-project
that referenced
this pull request
Jan 23, 2025
…compatability llvm#122928 Introduces a change to the C DebugInfo API which breaks compatability with older versions. This patch proposes the same functionality through a new function instead. I'm not that familiar with Clang development, but from how things are described in the original patch (and its parent) it seems like it should be an avoidable breakage.
|
This causes a breaking change in the C-API which I believe can easily be avoided. I've submitted #124144 as a suggestion to keep compatability |
Yea that's not unexpected, since the APIs are not stable/are experimental. I commented on the PR and CC'd Paul who might have an opinion on this |
|
Thanks, I'm not very opinionated on it, but since a lot of libraries in various languages rely on the C API I think it's fair to keep compatible if it makes sense |
|
The C API must remain stable and backward compatible. It's the C++ APIs that we are willing to change without worrying. |
In that case should we adjust the documentation in the header? llvm-project/llvm/include/llvm-c/DebugInfo.h Lines 7 to 14 in 1c28b92 I was changing these without worrying about compatibility precisely due to this comment |
|
yeah, I was going to say - thought we carved out part (the debug info part, maybe some other things - ORC?) of the C API as unstable, so I'm OK continuing with that (lack of) guarantee... |
|
Aha. I didn't know or had forgotten about the exception. The breakage isn't great but is allowed. |
|
I wasn't aware of that exception either, so my apologies there. I still think it's fair policy to keep things as compatible if easily possible though. |
abidh
pushed a commit
to abidh/llvm-project
that referenced
this pull request
Feb 4, 2025
… with explicit `this` (llvm#122928) In llvm#122897 we started attaching `DW_AT_object_pointer` to function definitions. This patch does the same but for function declarations (which we do for implicit object pointers already). Fixes llvm#120974
abidh
pushed a commit
to abidh/llvm-project
that referenced
this pull request
Feb 4, 2025
…erm/restore-10fdd09c3bda reland: 10fdd09 [clang][DebugInfo] Emit DW_AT_object_pointer on function declarations with explicit `this` (llvm#122928)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #122897 we started attaching
DW_AT_object_pointerto function definitions. This patch does the same but for function declarations (which we do for implicit object pointers already).Fixes #120974