Skip to content

Revert "[clang][DebugInfo] Emit DW_AT_object_pointer on function declarations with explicit this" #123455

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

Conversation

mgorny
Copy link
Member

@mgorny mgorny commented Jan 18, 2025

Reverts #122928

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. debuginfo llvm:ir labels Jan 18, 2025
@mgorny mgorny merged commit c3a935e into main Jan 18, 2025
4 of 7 checks passed
@mgorny mgorny deleted the revert-122928-clang/explicit-object-pointer-on-declaration branch January 18, 2025 07:59
@llvmbot
Copy link
Member

llvmbot commented Jan 18, 2025

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-debuginfo

Author: Michał Górny (mgorny)

Changes

Reverts llvm/llvm-project#122928


Full diff: https://github.com/llvm/llvm-project/pull/123455.diff

6 Files Affected:

  • (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+6-15)
  • (modified) clang/test/CodeGenCXX/debug-info-object-pointer.cpp (+4-3)
  • (modified) llvm/include/llvm-c/DebugInfo.h (+4-7)
  • (modified) llvm/include/llvm/IR/DIBuilder.h (+3-3)
  • (modified) llvm/lib/IR/DIBuilder.cpp (+2-6)
  • (modified) llvm/lib/IR/DebugInfo.cpp (+4-5)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 6cbcaf03844102..f88f56c98186da 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2016,15 +2016,13 @@ llvm::DISubroutineType *CGDebugInfo::getOrCreateInstanceMethodType(
   // First element is always return type. For 'void' functions it is NULL.
   Elts.push_back(Args[0]);
 
-  const bool HasExplicitObjectParameter = ThisPtr.isNull();
-
-  // "this" pointer is always first argument. For explicit "this"
-  // parameters, it will already be in Args[1].
-  if (!HasExplicitObjectParameter) {
+  // "this" pointer is always first argument.
+  // ThisPtr may be null if the member function has an explicit 'this'
+  // parameter.
+  if (!ThisPtr.isNull()) {
     llvm::DIType *ThisPtrType = getOrCreateType(ThisPtr, Unit);
     TypeCache[ThisPtr.getAsOpaquePtr()].reset(ThisPtrType);
-    ThisPtrType =
-        DBuilder.createObjectPointerType(ThisPtrType, /*Implicit=*/true);
+    ThisPtrType = DBuilder.createObjectPointerType(ThisPtrType);
     Elts.push_back(ThisPtrType);
   }
 
@@ -2032,13 +2030,6 @@ 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(),
@@ -5127,7 +5118,7 @@ llvm::DIType *CGDebugInfo::CreateSelfType(const QualType &QualTy,
   llvm::DIType *CachedTy = getTypeOrNull(QualTy);
   if (CachedTy)
     Ty = CachedTy;
-  return DBuilder.createObjectPointerType(Ty, /*Implicit=*/true);
+  return DBuilder.createObjectPointerType(Ty);
 }
 
 void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable(
diff --git a/clang/test/CodeGenCXX/debug-info-object-pointer.cpp b/clang/test/CodeGenCXX/debug-info-object-pointer.cpp
index 49079f59909968..594d4da791ee84 100644
--- a/clang/test/CodeGenCXX/debug-info-object-pointer.cpp
+++ b/clang/test/CodeGenCXX/debug-info-object-pointer.cpp
@@ -5,11 +5,12 @@
 // 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: !DIDerivedType(tag: DW_TAG_rvalue_reference_type
-// CHECK-SAME:           flags: DIFlagObjectPointer)
+// CHECK-NOT: DIFlagObjectPointer
+// CHECK-NOT: DIFlagArtificial
 //
 // 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 ac7ee5a7cc9a19..07f87d44088e7e 100644
--- a/llvm/include/llvm-c/DebugInfo.h
+++ b/llvm/include/llvm-c/DebugInfo.h
@@ -870,16 +870,13 @@ LLVMDIBuilderCreateObjCProperty(LLVMDIBuilderRef Builder,
                                 LLVMMetadataRef Ty);
 
 /**
- * Create a uniqued DIType* clone with FlagObjectPointer. If \c Implicit
- * is true, then also set FlagArtificial.
+ * Create a uniqued DIType* clone with FlagObjectPointer and FlagArtificial set.
  * \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,
-                                                     LLVMBool Implicit);
+LLVMMetadataRef
+LLVMDIBuilderCreateObjectPointerType(LLVMDIBuilderRef Builder,
+                                     LLVMMetadataRef Type);
 
 /**
  * Create debugging information entry for a qualified
diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index 6c479415b9ed27..cb1150c269a1da 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 set.
-    /// If \p Implicit is true, also set FlagArtificial.
-    static DIType *createObjectPointerType(DIType *Ty, bool Implicit);
+    /// Create a uniqued clone of \p Ty with FlagObjectPointer and
+    /// FlagArtificial set.
+    static DIType *createObjectPointerType(DIType *Ty);
 
     /// 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 d9bd4f11e89a39..b240a2a39de362 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -644,15 +644,11 @@ DIType *DIBuilder::createArtificialType(DIType *Ty) {
   return createTypeWithFlags(Ty, DINode::FlagArtificial);
 }
 
-DIType *DIBuilder::createObjectPointerType(DIType *Ty, bool Implicit) {
+DIType *DIBuilder::createObjectPointerType(DIType *Ty) {
   // FIXME: Restrict this to the nodes where it's valid.
   if (Ty->isObjectPointer())
     return Ty;
-  DINode::DIFlags Flags = DINode::FlagObjectPointer;
-
-  if (Implicit)
-    Flags |= DINode::FlagArtificial;
-
+  DINode::DIFlags Flags = DINode::FlagObjectPointer | DINode::FlagArtificial;
   return createTypeWithFlags(Ty, Flags);
 }
 
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 4ce518009bd3ea..e5b45e0082a823 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -1432,11 +1432,10 @@ LLVMDIBuilderCreateObjCProperty(LLVMDIBuilderRef Builder,
                   PropertyAttributes, unwrapDI<DIType>(Ty)));
 }
 
-LLVMMetadataRef LLVMDIBuilderCreateObjectPointerType(LLVMDIBuilderRef Builder,
-                                                     LLVMMetadataRef Type,
-                                                     LLVMBool Implicit) {
-  return wrap(unwrap(Builder)->createObjectPointerType(unwrapDI<DIType>(Type),
-                                                       Implicit));
+LLVMMetadataRef
+LLVMDIBuilderCreateObjectPointerType(LLVMDIBuilderRef Builder,
+                                     LLVMMetadataRef Type) {
+  return wrap(unwrap(Builder)->createObjectPointerType(unwrapDI<DIType>(Type)));
 }
 
 LLVMMetadataRef

Michael137 added a commit that referenced this pull request Jan 18, 2025
…larations with explicit `this`" (#123455)

This reverts commit c3a935e.

The only change to the reverted commit is that this also updates
the OCaml bindings according to the C debug-info API changes.

The build failure originally introduced was:
```
FAILED: bindings/ocaml/debuginfo/debuginfo_ocaml.o /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bindings/ocaml/debuginfo/debuginfo_ocaml.o
cd /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bindings/ocaml/debuginfo && /usr/bin/ocamlfind ocamlc -c /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bindings/ocaml/debuginfo/debuginfo_ocaml.c -ccopt "-I/b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/bindings/ocaml/debuginfo/../llvm -D_GNU_SOURCE -D_DEBUG -D_GLIBCXX_ASSERTIONS -DEXPENSIVE_CHECKS -D_GLIBCXX_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/b/1/llvm-clang-x86_64-expensive-checks-debian/build/include -I/b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/include  -DNDEBUG "
/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bindings/ocaml/debuginfo/debuginfo_ocaml.c: In function ‘llvm_dibuild_create_object_pointer_type’:
/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bindings/ocaml/debuginfo/debuginfo_ocaml.c:620:30: error: too few arguments to function ‘LLVMDIBuilderCreateObjectPointerType’
  620 |   LLVMMetadataRef Metadata = LLVMDIBuilderCreateObjectPointerType(
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bindings/ocaml/debuginfo/debuginfo_ocaml.c:23:
/b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/include/llvm-c/DebugInfo.h:880:17: note: declared here
  880 | LLVMMetadataRef LLVMDIBuilderCreateObjectPointerType(LLVMDIBuilderRef Builder,
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category debuginfo llvm:ir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants