Skip to content

[clang][DebugInfo] Fix iterator invalidation during EmitGlobalVariable #72415

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

Michael137
Copy link
Member

This patch fixes an issue introduced in #71780 where, if EmitGlobalVariable triggered a call to CreateRecordStaticField it would invalidate the StaticDataMemberDefinitionsToEmit iterator that
is currently in-use.

This fixes a crash reported in #71780 (comment)

This patch fixes an issue introduced in
llvm#71780 where,
if `EmitGlobalVariable` triggered a call to
`CreateRecordStaticField` it would invalidate the
`StaticDataMemberDefinitionsToEmit` iterator that
is currently in-use.

This fixes a crash reported in
llvm#71780 (comment)
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. debuginfo labels Nov 15, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 15, 2023

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

@llvm/pr-subscribers-debuginfo

Author: Michael Buch (Michael137)

Changes

This patch fixes an issue introduced in #71780 where, if EmitGlobalVariable triggered a call to CreateRecordStaticField it would invalidate the StaticDataMemberDefinitionsToEmit iterator that
is currently in-use.

This fixes a crash reported in #71780 (comment)


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

1 Files Affected:

  • (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+7-2)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index a201cf03c22f711..d19e23e1392f4b4 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -5830,8 +5830,13 @@ void CGDebugInfo::setDwoId(uint64_t Signature) {
 }
 
 void CGDebugInfo::finalize() {
-  for (auto const *VD : StaticDataMemberDefinitionsToEmit) {
-    assert(VD->isStaticDataMember());
+  // We can't use a for-each here because `EmitGlobalVariable`
+  // may push new decls into `StaticDataMemberDefinitionsToEmit`,
+  // which would invalidate any iterator.
+  for (size_t i = 0; i < StaticDataMemberDefinitionsToEmit.size(); ++i) {
+    auto const *VD = StaticDataMemberDefinitionsToEmit[i];
+
+    assert(VD && VD->isStaticDataMember());
 
     if (DeclCache.contains(VD))
       continue;

Copy link
Contributor

@felipepiovezan felipepiovezan left a comment

Choose a reason for hiding this comment

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

LGTM. If there are others waiting on the fix, and since this is a fairly trivial change, it's probably ok to push it soonish

@Michael137 Michael137 merged commit 14a8451 into llvm:main Nov 15, 2023
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants