[clang-tidy] Fix crash in modernize-use-designated-initializers check

[z1nke]

Fixed #113652
When calling Node.isAggregate() and Node.isPOD(), if Node is declared but not defined, it will result in null pointer dereference (and if assertions are enabled, it will cause an assertion failure).

[llvmbot]

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: None (z1nke)

Changes

Fixed #113652
When calling Node.isAggregate() and Node.isPOD(), if Node is declared but not defined, it will result in null pointer dereference (and if assertions are enabled, it will cause an assertion failure).

---

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

2 Files Affected:

• (modified) clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp (+6-2)
• (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp (+8)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp
index 2a0cc403b726e8..3132067f3d5ece 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp
@@ -80,9 +80,13 @@ unsigned getNumberOfDesignated(const InitListExpr *SyntacticInitList) {
   });
 }
 
-AST_MATCHER(CXXRecordDecl, isAggregate) { return Node.isAggregate(); }
+AST_MATCHER(CXXRecordDecl, isAggregate) {
+  return Node.hasDefinition() && Node.isAggregate();
+}
 
-AST_MATCHER(CXXRecordDecl, isPOD) { return Node.isPOD(); }
+AST_MATCHER(CXXRecordDecl, isPOD) {
+  return Node.hasDefinition() && Node.isPOD();
+}
 
 AST_MATCHER(InitListExpr, isFullyDesignated) {
   if (const InitListExpr *SyntacticForm =
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp
index 9b769ad0be23ca..db1fe3baa99d7c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp
@@ -201,3 +201,11 @@ DECLARE_S93;
 // CHECK-MESSAGES-MACROS: :[[@LINE-1]]:1: warning: use designated initializer list to initialize 'S9' [modernize-use-designated-initializers]
 // CHECK-MESSAGES-MACROS: :[[@LINE-4]]:28: note: expanded from macro 'DECLARE_S93'
 // CHECK-MESSAGES-MACROS: :[[@LINE-71]]:1: note: aggregate type is defined here
+
+// Issus #113652.
+struct S14;
+
+struct S15{
+  S15(S14& d):d{d}{}
+  S14& d;
+};
\ No newline at end of file

[nicovank]

I fixed a similar issue in 0aaac4f. There's an argument to be made whether this should be in the matchers (isPOD, isAggregate like here) versus an hasDefinition call in the parent before calling those matchers. I am slightly leaning towards the latter but this is not a strong opinion.

Add a small release notes entry?

[z1nke]

@nicovank Thanks! Release note has been added.
What do others think about the argument?

[5chmidti]

Either way is fine with me.

[nicovank]

Looking at it again, this way is also good, no preference. Good to merge 👍

[z1nke]

Hi, I don’t have write access to merge code, and this is my first commit to clang-tidy. What else needs to be done to merge this PR? Thanks!

[nicovank]

@z1nke I will go ahead and click the merge button. Thanks!

[mommsen]

Will this fix be added to an upcoming patch release for 19.1? We suffer from this issue using version 19.1.5.