[clang-tools-extra] [clang-tidy] Fix crash and infinite loop on incomplete types (PR #219781)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Purna Chandra (Purna-Chandra-4706)

<details>
<summary>Changes</summary>

Fixes #<!-- -->213948

clang-tidy crashes when a class inherits from a forward-declared base class. This happens because hasMemberName() is called on an incomplete type in readability-identifier-naming check.

There's also an infinite loop in misc-multiple-inheritance check when there's circular inheritance (A inherits B, B inherits A). The isInterface() function keeps calling itself forever.

Fixed by adding hasDefinition() checks before accessing base class members, and by inserting a provisional cache entry to break cycles.

Added regression tests for both cases.

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


4 Files Affected:

- (modified) clang-tools-extra/clang-tidy/misc/MultipleInheritanceCheck.cpp (+5-2) 
- (modified) clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp (+1-1) 
- (added) clang-tools-extra/test/clang-tidy/checkers/misc/multiple-inheritance-incomplete-type.cpp (+7) 
- (added) clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-incomplete-type.cpp (+9) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/misc/MultipleInheritanceCheck.cpp b/clang-tools-extra/clang-tidy/misc/MultipleInheritanceCheck.cpp
index 72e6aa6ac0b47..3f3f4a0086897 100644
--- a/clang-tools-extra/clang-tidy/misc/MultipleInheritanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/MultipleInheritanceCheck.cpp
@@ -26,13 +26,16 @@ bool MultipleInheritanceCheck::isInterface(const CXXBaseSpecifier &Base) {
   if (!Node)
     return true;
 
-  assert(Node->isCompleteDefinition());
+  if (!Node->hasDefinition())
+    return false;
 
   // Short circuit the lookup if we have analyzed this record before.
   if (const auto CachedValue = InterfaceMap.find(Node);
       CachedValue != InterfaceMap.end())
     return CachedValue->second;
 
+  InterfaceMap.try_emplace(Node, false);
+
   // To be an interface, a class must have...
   const bool CurrentClassIsInterface =
       // ...no bases that aren't interfaces...
@@ -47,7 +50,7 @@ bool MultipleInheritanceCheck::isInterface(const CXXBaseSpecifier &Base) {
         return M->isUserProvided() && !M->isPureVirtual() && !M->isStatic();
       });
 
-  InterfaceMap.try_emplace(Node, CurrentClassIsInterface);
+  InterfaceMap[Node] = CurrentClassIsInterface;
   return CurrentClassIsInterface;
 }
 
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index 50644bbf37bce..54cb140b12e80 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -1259,7 +1259,7 @@ StyleKind IdentifierNamingCheck::findStyleKind(
     // necessary even if it's not an override. e.g. CRTP.
     for (const CXXBaseSpecifier &Base : Decl->getParent()->bases())
       if (const auto *RD = Base.getType()->getAsCXXRecordDecl();
-          RD && RD->hasMemberName(Decl->getDeclName()))
+          RD && RD->hasDefinition() && RD->hasMemberName(Decl->getDeclName()))
         return SK_Invalid;
 
     if (Decl->isConstexpr() && NamingStyles[SK_ConstexprMethod])
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/multiple-inheritance-incomplete-type.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/multiple-inheritance-incomplete-type.cpp
new file mode 100644
index 0000000000000..debf6b04c2c4e
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/multiple-inheritance-incomplete-type.cpp
@@ -0,0 +1,7 @@
+// RUN: %check_clang_tidy "%s" misc-multiple-inheritance "%t"
+
+template<class T> struct X {
+  struct B;
+  struct A : public B { virtual void foo() {} };
+};
+template<class T> struct X<T>::B : public A { virtual void foo() {} };
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-incomplete-type.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-incomplete-type.cpp
new file mode 100644
index 0000000000000..5a516fe31f7b5
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-incomplete-type.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy "%s" readability-identifier-naming "%t"
+
+template<class T>
+struct X {
+  struct B;
+  struct A : public B {
+    virtual void foo() { }
+  };
+};

``````````

</details>


https://github.com/llvm/llvm-project/pull/219781