-
Notifications
You must be signed in to change notification settings - Fork 14.5k
fix-149477-- Emit correct closure members for lambdas in CreateLimitedType #149674
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
base: main
Are you sure you want to change the base?
Conversation
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
@llvm/pr-subscribers-clang Author: Ayush Pareek (ayushpareek2003) ChangesFix an issue where local lambdas caused a crash when importing the std module in LLDB due to missing capture fields in debug info. Full diff: https://github.com/llvm/llvm-project/pull/149674.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index e24c68ed02865..a8f9e3e028e9f 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4187,6 +4187,32 @@ llvm::DICompositeType *CGDebugInfo::CreateLimitedType(const RecordType *Ty) {
RegionMap[Ty->getDecl()].reset(RealDecl);
TypeCache[QualType(Ty, 0).getAsOpaquePtr()].reset(RealDecl);
+ if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+ SmallVector<llvm::Metadata *, 16> EltTys;
+ const ASTRecordLayout &Layout = CGM.getContext().getASTRecordLayout(CXXRD);
+
+ for (const FieldDecl *Field : CXXRD->fields()) {
+ if (!CXXRD->isLambda() && Field->isImplicit())
+ continue;
+
+ llvm::DIType *FieldType = getOrCreateType(Field->getType(), DefUnit);
+ unsigned FieldLine = getLineNumber(Field->getLocation());
+ uint64_t FieldOffset = Layout.getFieldOffset(Field->getFieldIndex());
+ llvm::DIFile *FieldFile = DefUnit;
+
+ llvm::DIDerivedType *Elem = DBuilder.createMemberType(
+ RealDecl, Field->getName(), FieldFile, FieldLine,
+ CGM.getContext().getTypeSize(Field->getType()),
+ getTypeAlignIfRequired(Field->getType(), CGM.getContext()),
+ FieldOffset,
+ getAccessFlag(Field->getAccess()), FieldType);
+
+ EltTys.push_back(Elem);
+ }
+ // Attach the fields.
+ DBuilder.replaceArrays(RealDecl, DBuilder.getOrCreateArray(EltTys));
+ }
+
if (const auto *TSpecial = dyn_cast<ClassTemplateSpecializationDecl>(RD))
DBuilder.replaceArrays(RealDecl, llvm::DINodeArray(),
CollectCXXTemplateParams(TSpecial, DefUnit));
|
@llvm/pr-subscribers-debuginfo Author: Ayush Pareek (ayushpareek2003) ChangesFix an issue where local lambdas caused a crash when importing the std module in LLDB due to missing capture fields in debug info. Full diff: https://github.com/llvm/llvm-project/pull/149674.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index e24c68ed02865..a8f9e3e028e9f 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4187,6 +4187,32 @@ llvm::DICompositeType *CGDebugInfo::CreateLimitedType(const RecordType *Ty) {
RegionMap[Ty->getDecl()].reset(RealDecl);
TypeCache[QualType(Ty, 0).getAsOpaquePtr()].reset(RealDecl);
+ if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+ SmallVector<llvm::Metadata *, 16> EltTys;
+ const ASTRecordLayout &Layout = CGM.getContext().getASTRecordLayout(CXXRD);
+
+ for (const FieldDecl *Field : CXXRD->fields()) {
+ if (!CXXRD->isLambda() && Field->isImplicit())
+ continue;
+
+ llvm::DIType *FieldType = getOrCreateType(Field->getType(), DefUnit);
+ unsigned FieldLine = getLineNumber(Field->getLocation());
+ uint64_t FieldOffset = Layout.getFieldOffset(Field->getFieldIndex());
+ llvm::DIFile *FieldFile = DefUnit;
+
+ llvm::DIDerivedType *Elem = DBuilder.createMemberType(
+ RealDecl, Field->getName(), FieldFile, FieldLine,
+ CGM.getContext().getTypeSize(Field->getType()),
+ getTypeAlignIfRequired(Field->getType(), CGM.getContext()),
+ FieldOffset,
+ getAccessFlag(Field->getAccess()), FieldType);
+
+ EltTys.push_back(Elem);
+ }
+ // Attach the fields.
+ DBuilder.replaceArrays(RealDecl, DBuilder.getOrCreateArray(EltTys));
+ }
+
if (const auto *TSpecial = dyn_cast<ClassTemplateSpecializationDecl>(RD))
DBuilder.replaceArrays(RealDecl, llvm::DINodeArray(),
CollectCXXTemplateParams(TSpecial, DefUnit));
|
@llvm/pr-subscribers-clang-codegen Author: Ayush Pareek (ayushpareek2003) ChangesFix an issue where local lambdas caused a crash when importing the std module in LLDB due to missing capture fields in debug info. Full diff: https://github.com/llvm/llvm-project/pull/149674.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index e24c68ed02865..a8f9e3e028e9f 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4187,6 +4187,32 @@ llvm::DICompositeType *CGDebugInfo::CreateLimitedType(const RecordType *Ty) {
RegionMap[Ty->getDecl()].reset(RealDecl);
TypeCache[QualType(Ty, 0).getAsOpaquePtr()].reset(RealDecl);
+ if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+ SmallVector<llvm::Metadata *, 16> EltTys;
+ const ASTRecordLayout &Layout = CGM.getContext().getASTRecordLayout(CXXRD);
+
+ for (const FieldDecl *Field : CXXRD->fields()) {
+ if (!CXXRD->isLambda() && Field->isImplicit())
+ continue;
+
+ llvm::DIType *FieldType = getOrCreateType(Field->getType(), DefUnit);
+ unsigned FieldLine = getLineNumber(Field->getLocation());
+ uint64_t FieldOffset = Layout.getFieldOffset(Field->getFieldIndex());
+ llvm::DIFile *FieldFile = DefUnit;
+
+ llvm::DIDerivedType *Elem = DBuilder.createMemberType(
+ RealDecl, Field->getName(), FieldFile, FieldLine,
+ CGM.getContext().getTypeSize(Field->getType()),
+ getTypeAlignIfRequired(Field->getType(), CGM.getContext()),
+ FieldOffset,
+ getAccessFlag(Field->getAccess()), FieldType);
+
+ EltTys.push_back(Elem);
+ }
+ // Attach the fields.
+ DBuilder.replaceArrays(RealDecl, DBuilder.getOrCreateArray(EltTys));
+ }
+
if (const auto *TSpecial = dyn_cast<ClassTemplateSpecializationDecl>(RD))
DBuilder.replaceArrays(RealDecl, llvm::DINodeArray(),
CollectCXXTemplateParams(TSpecial, DefUnit));
|
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.
Thanks for trying to fix that LLDB issue. Could you elaborate on what you think the root cause is and how this fixes it? Not saying it's impossible, but I'm a bit surprised that debug-info is involved in this crash at all since we're supposedly getting the decls from the std
module.
Also, this patch fails to build for me locally. And when I fix that build failure the crash still reproduces.
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions cpp -- clang/lib/CodeGen/CGDebugInfo.cpp View the diff from clang-format here.diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index a8f9e3e02..081b038cc 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4204,8 +4204,7 @@ llvm::DICompositeType *CGDebugInfo::CreateLimitedType(const RecordType *Ty) {
RealDecl, Field->getName(), FieldFile, FieldLine,
CGM.getContext().getTypeSize(Field->getType()),
getTypeAlignIfRequired(Field->getType(), CGM.getContext()),
- FieldOffset,
- getAccessFlag(Field->getAccess()), FieldType);
+ FieldOffset, getAccessFlag(Field->getAccess()), FieldType);
EltTys.push_back(Elem);
}
|
Some Clang test coverage would be nice - showing the change in behavior (& not a committed test, but to @Michael137's point - showing the difference in the resulting DWARF would be good to look through too) |
According to my understanding, this issue happens because when a standard library header like uses a function-local lambda, the compiler generates a closure type for that lambda when LLDB runs import std and tries to inspect types that indirectly depend on that header (for ex- std::list), it needs the debug info for all these I made CreateLimitedType keep all fields for lambdas, so their captures aren’t skipped by the isImplicit check am I missing something here or am I looking at this from the wrong angle? |
I think we need some more detail - generally we try not to emit definitions of classes (implicitly, including the closure type of a lambda) if the definition isn't needed (essentially, if the code doesn't do something that would fail to compile if the class were declared but not defined, we skip the definition in the DWARF). Most issues that could apply to lambda closure types would apply to local class types - so mostly any bug that would apply to a lambda, would apply to a local class type - so I'm suspicious of any change that's lambda specific like this one. Feels like we might not know enough about the problem yet. Can you provide a specific example? (source code, compilation commands, platform, debugger, debugger commands, failure mode/crash/etc) |
Fix an issue where local lambdas caused a crash when importing the std module in LLDB due to missing capture fields in debug info.
Now closures always emit all fields for lambdas by skipping the
isImplicit
check.This unblocks libc++ from using local lambdas freely