Skip to content
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

[Clang] Assert non-null enum definition in CGDebugInfo::CreateTypeDefinition(const EnumType*) #105556

Merged
merged 1 commit into from
Aug 23, 2024

Conversation

smanna12
Copy link
Contributor

@smanna12 smanna12 commented Aug 21, 2024

This commit adds an assert to check for a non-null enum definition in CGDebugInfo::CreateTypeDefinition(const EnumType*), ensuring precondition validity.

Previous discussion on #97105

@smanna12 smanna12 requested a review from tahonermann August 21, 2024 17:59
@smanna12 smanna12 marked this pull request as ready for review August 21, 2024 18:34
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen debuginfo labels Aug 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 21, 2024

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-debuginfo

Author: None (smanna12)

Changes

This commit adds an assertion to the CreateTypeDefinition function in CGDebugInfo.cpp to verify that an enumeration definition (ED) is available before iterating over its enumerators. This prevents potential null pointer dereferences and ensures that the function operates on a valid enumeration definition.


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

1 Files Affected:

  • (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+1)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 7ad3088f0ab756..dc83d596e3cb06 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3561,6 +3561,7 @@ llvm::DIType *CGDebugInfo::CreateTypeDefinition(const EnumType *Ty) {
 
   SmallVector<llvm::Metadata *, 16> Enumerators;
   ED = ED->getDefinition();
+  assert(ED && "An enumeration definition is required");
   for (const auto *Enum : ED->enumerators()) {
     Enumerators.push_back(
         DBuilder.createEnumerator(Enum->getName(), Enum->getInitVal()));

@llvmbot
Copy link
Member

llvmbot commented Aug 21, 2024

@llvm/pr-subscribers-clang

Author: None (smanna12)

Changes

This commit adds an assertion to the CreateTypeDefinition function in CGDebugInfo.cpp to verify that an enumeration definition (ED) is available before iterating over its enumerators. This prevents potential null pointer dereferences and ensures that the function operates on a valid enumeration definition.


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

1 Files Affected:

  • (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+1)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 7ad3088f0ab756..dc83d596e3cb06 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3561,6 +3561,7 @@ llvm::DIType *CGDebugInfo::CreateTypeDefinition(const EnumType *Ty) {
 
   SmallVector<llvm::Metadata *, 16> Enumerators;
   ED = ED->getDefinition();
+  assert(ED && "An enumeration definition is required");
   for (const auto *Enum : ED->enumerators()) {
     Enumerators.push_back(
         DBuilder.createEnumerator(Enum->getName(), Enum->getInitVal()));

@pogo59
Copy link
Collaborator

pogo59 commented Aug 22, 2024

The patch title and description claims more than it actually does. The assertion does not "fix" or prevent a null dereference, it's simply documenting a requirement. Please update the title and description.

Copy link
Contributor

@tahonermann tahonermann left a comment

Choose a reason for hiding this comment

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

Per discussion in #97105, this looks good to me.

However, please do update the PR title and adjust the commit message accordingly when merging the change as requested by @pogo59.

@smanna12 smanna12 changed the title [Clang] Fix null pointer dereference in enum debug info generation [Clang] Assert non-null enum definition in CGDebugInfo Aug 23, 2024
@smanna12
Copy link
Contributor Author

Per discussion in #97105, this looks good to me.

However, please do update the PR title and adjust the commit message accordingly when merging the change as requested by @pogo59.

Thanks @tahonermann and @pogo59 for reviews.

I have updated title and commit messages.

@smanna12 smanna12 changed the title [Clang] Assert non-null enum definition in CGDebugInfo [Clang] Assert non-null enum definition in CGDebugInfo::CreateTypeDefinition(const EnumType*) Aug 23, 2024
@smanna12 smanna12 merged commit 8f08b75 into llvm:main Aug 23, 2024
14 checks passed
@smanna12 smanna12 deleted the FixDebugBug branch August 23, 2024 18:25
5chmidti pushed a commit that referenced this pull request Aug 24, 2024
…inition(const EnumType*) (#105556)

This commit adds an assert to check for a non-null enum definition in
CGDebugInfo::CreateTypeDefinition(const EnumType*), ensuring
precondition validity.

Previous discussion on #97105
@dwblaikie
Copy link
Collaborator

What static analysis tool flagged this? Any idea why this particular null dereference, when LLVM/Clang have loads of assumed-non-null pointers that are dereferenced in a great many places?

dmpolukhin pushed a commit to dmpolukhin/llvm-project that referenced this pull request Sep 2, 2024
…inition(const EnumType*) (llvm#105556)

This commit adds an assert to check for a non-null enum definition in
CGDebugInfo::CreateTypeDefinition(const EnumType*), ensuring
precondition validity.

Previous discussion on llvm#97105
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang Clang issues not falling into any other category debuginfo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants