Skip to content

[Clang,debuginfo] added vtt parameter in destructor DISubroutineType #130674

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.

Sign up for GitHub

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 5 commits into from
Apr 16, 2025
Merged

[Clang,debuginfo] added vtt parameter in destructor DISubroutineType #130674

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
017a07e
[Clang,debuginfo] added vtt parameter in DISubroutineType for virtual…
mgschossmann Mar 10, 2025
cdd8eb8
fixed test
mgschossmann Mar 10, 2025
803460d
test: fixed target triple to x86_64-none-linux-gnu (vtt not generated…
mgschossmann Mar 10, 2025
2853c57
added test for msvc
mgschossmann Apr 14, 2025
909d340
added comment
mgschossmann Apr 14, 2025
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
19 changes: 16 additions & 3 deletions clang/lib/CodeGen/CGDebugInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2003,8 +2003,17 @@ CGDebugInfo::getOrCreateMethodType(const CXXMethodDecl *Method,
return getOrCreateInstanceMethodType(ThisType, Func, Unit);
}

llvm::DISubroutineType *CGDebugInfo::getOrCreateInstanceMethodType(
QualType ThisPtr, const FunctionProtoType *Func, llvm::DIFile *Unit) {
llvm::DISubroutineType *CGDebugInfo::getOrCreateMethodTypeForDestructor(
const CXXMethodDecl *Method, llvm::DIFile *Unit, QualType FNType) {
const FunctionProtoType *Func = FNType->getAs<FunctionProtoType>();
// skip the first param since it is also this
return getOrCreateInstanceMethodType(Method->getThisType(), Func, Unit, true);
}

llvm::DISubroutineType *
CGDebugInfo::getOrCreateInstanceMethodType(QualType ThisPtr,
const FunctionProtoType *Func,
llvm::DIFile *Unit, bool SkipFirst) {
FunctionProtoType::ExtProtoInfo EPI = Func->getExtProtoInfo();
Qualifiers &Qc = EPI.TypeQuals;
Qc.removeConst();
Expand Down Expand Up @@ -2044,7 +2053,7 @@ llvm::DISubroutineType *CGDebugInfo::getOrCreateInstanceMethodType(
}

// Copy rest of the arguments.
for (unsigned i = 1, e = Args.size(); i != e; ++i)
for (unsigned i = (SkipFirst ? 2 : 1), e = Args.size(); i != e; ++i)
Elts.push_back(Args[i]);

// Attach FlagObjectPointer to the explicit "this" parameter.
Expand Down Expand Up @@ -4352,6 +4361,10 @@ llvm::DISubroutineType *CGDebugInfo::getOrCreateFunctionType(const Decl *D,
// subprogram DIE will miss DW_AT_decl_file and DW_AT_decl_line fields.
return DBuilder.createSubroutineType(DBuilder.getOrCreateTypeArray({}));

if (const auto *Method = dyn_cast<CXXDestructorDecl>(D)) {
return getOrCreateMethodTypeForDestructor(Method, F, FnType);
}

if (const auto *Method = dyn_cast<CXXMethodDecl>(D))
return getOrCreateMethodType(Method, F);

Expand Down
7 changes: 6 additions & 1 deletion clang/lib/CodeGen/CGDebugInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,14 @@ class CGDebugInfo {
/// to get a method type which includes \c this pointer.
llvm::DISubroutineType *getOrCreateMethodType(const CXXMethodDecl *Method,
llvm::DIFile *F);

llvm::DISubroutineType *
getOrCreateMethodTypeForDestructor(const CXXMethodDecl *Method,
llvm::DIFile *F, QualType FNType);

llvm::DISubroutineType *
getOrCreateInstanceMethodType(QualType ThisPtr, const FunctionProtoType *Func,
llvm::DIFile *Unit);
llvm::DIFile *Unit, bool SkipFirst = false);
llvm::DISubroutineType *
getOrCreateFunctionType(const Decl *D, QualType FnType, llvm::DIFile *F);
/// \return debug info descriptor for vtable.
Expand Down
16 changes: 16 additions & 0 deletions clang/test/CodeGenCXX/debug-info-vtt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// RUN: %clang_cc1 -triple x86_64-none-linux-gnu -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a test for the MSVC ABI, since I don't think it has VTT parameters, and I'm not sure the code you added does the right thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your comment. Indeed, my code changes behavior when compiling for i686-pc-windows-msvc, which I did not expect.
However, there seems to be the same problem: There is an implicit parameter should_call_delete, whose DILocalVariable has arg: 2 even though there is no second argument in the function type's types array. Similarly to vtt, the patched code adds this parameter to the array.

Additionally, it changes the return type of the destructor to a void pointer. I am unsure, why this happens, since that does not happen on ItaniumABI. Does the MSVC ABI feature an implicit void return? Unfortunately, I am lacking knowledge of the MSVC ABI to be able to judge what the correct behavior should be.

Following are clang outputs for debug-info-vtt.cpp:
clang_outputs.zip

PS: I am unsure whether the bug is that the AST is not correctly updated in clang/lib/CodeGen/ItaniumCXXABI.cpp (and clang/lib/CodeGen/MicrosoftCXXABI.cpp) or that the AST is not correctly read in clang/lib/CodeGen/CGDebugInfo.cpp. In the former case, my patch feels like a workaround.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, I looked at the files, and I think the new debug info looks good, it adds a DISubroutineType int parameter.

However, I don't understand why the patch you've posted here has that effect. Do you have more local changes? Why does skipping more arguments in this loop create more subroutine parameter types? I'm struggling to understand, and I just assumed adding more testing would provide answers to these questions.

Please do create a test case out of the IR you shared, it does seem relevant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The key change of this pull request is that debug info for destructors is now taken from FNType instead of D->getType() / Method->getType(). Method->getType() does not include the implicit arguments, while FNType does. FNType however also includes the this argument. In order to prevent duplicate emission of this, the pull request introduces the SkipFirst argument in getOrCreateInstanceMethodType.

I now also added a test for MSVC, which checks for presence of the should_call_delete arg. It also enforces that the return value be void*. The new test seems to pass, however there seems to be an issue in the CI, so it is reported as failed.


struct B {
virtual ~B() {}
};

struct A : virtual B {
};

A a;


// CHECK-DAG: !{{[0-9]+}} = !DILocalVariable(name: "vtt", arg: 2, scope: ![[destructor:[0-9]+]], type: ![[vtttype:[0-9]+]], flags: DIFlagArtificial)
// CHECK-DAG: ![[destructor]] = distinct !DISubprogram(name: "~A", {{.*}}, type: ![[subroutinetype:[0-9]+]]
// CHECK-DAG: ![[subroutinetype]] = !DISubroutineType(types: ![[types:[0-9]+]])
// CHECK-DAG: [[types]] = !{null, !{{[0-9]+}}, ![[vtttype]]}