[clang] adab08e - [clang][Sema] Fix crash on invalid base destructor

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2022-10-05T15:23:15+02:00
New Revision: adab08ecf2bdeb84dc7d2009d4c0c13617db9c58

URL: https://github.com/llvm/llvm-project/commit/adab08ecf2bdeb84dc7d2009d4c0c13617db9c58
DIFF: https://github.com/llvm/llvm-project/commit/adab08ecf2bdeb84dc7d2009d4c0c13617db9c58.diff

LOG: [clang][Sema] Fix crash on invalid base destructor

LookupSpecialMember might fail, so changes the cast to cast_or_null.
Inside Sema, skip a particular base, similar to other cases, rather than
asserting on dtor showing up.

Other option would be to mark classes with invalid destructors as invalid, but
that seems like a lot more invasive and we do lose lots of diagnostics that
currently work on classes with broken members.

Differential Revision: https://reviews.llvm.org/D135254

Added: 
    

Modified: 
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/lib/Sema/SemaLookup.cpp
    clang/test/SemaCXX/destructor.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 5cc5dc1c449a6..a490f1e59dd10 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/ComparisonCategories.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/ExprCXX.h"
@@ -5650,7 +5651,9 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
       continue;
 
     CXXDestructorDecl *Dtor = LookupDestructor(FieldClassDecl);
-    assert(Dtor && "No dtor found for FieldClassDecl!");
+    // Dtor might still be missing, e.g because it's invalid.
+    if (!Dtor)
+      continue;
     CheckDestructorAccess(Field->getLocation(), Dtor,
                           PDiag(diag::err_access_dtor_field)
                             << Field->getDeclName()
@@ -5696,7 +5699,9 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
       continue;
 
     CXXDestructorDecl *Dtor = LookupDestructor(BaseClassDecl);
-    assert(Dtor && "No dtor found for BaseClassDecl!");
+    // Dtor might still be missing, e.g because it's invalid.
+    if (!Dtor)
+      continue;
 
     // FIXME: caret should be on the start of the class name
     CheckDestructorAccess(Base.getBeginLoc(), Dtor,
@@ -5733,7 +5738,9 @@ void Sema::MarkVirtualBaseDestructorsReferenced(
       continue;
 
     CXXDestructorDecl *Dtor = LookupDestructor(BaseClassDecl);
-    assert(Dtor && "No dtor found for BaseClassDecl!");
+    // Dtor might still be missing, e.g because it's invalid.
+    if (!Dtor)
+      continue;
     if (CheckDestructorAccess(
             ClassDecl->getLocation(), Dtor,
             PDiag(diag::err_access_dtor_vbase)

diff  --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 70d0ce0baae06..a144cb40b7abc 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -40,6 +40,7 @@
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/ADT/edit_distance.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <algorithm>
 #include <iterator>
@@ -3616,9 +3617,10 @@ CXXMethodDecl *Sema::LookupMovingAssignment(CXXRecordDecl *Class,
 ///
 /// \returns The destructor for this class.
 CXXDestructorDecl *Sema::LookupDestructor(CXXRecordDecl *Class) {
-  return cast<CXXDestructorDecl>(LookupSpecialMember(Class, CXXDestructor,
-                                                     false, false, false,
-                                                     false, false).getMethod());
+  return cast_or_null<CXXDestructorDecl>(
+      LookupSpecialMember(Class, CXXDestructor, false, false, false, false,
+                          false)
+          .getMethod());
 }
 
 /// LookupLiteralOperator - Determine which literal operator should be used for

diff  --git a/clang/test/SemaCXX/destructor.cpp b/clang/test/SemaCXX/destructor.cpp
index 040d13ed99143..971cea621e865 100644
--- a/clang/test/SemaCXX/destructor.cpp
+++ b/clang/test/SemaCXX/destructor.cpp
@@ -550,4 +550,20 @@ namespace PR44978 {
   template<typename T> void f(T *p) { p->~U(); } // expected-error {{ambiguous}}
 }
 
+namespace crash_on_invalid_base_dtor {
+struct Test {
+  virtual ~Test();
+};
+struct Baz : public Test { // expected-warning {{non-virtual destructor}}
+  Baz() {}
+  ~Baz() = defaul; // expected-error {{undeclared identifier 'defaul'}} \
+                   // expected-error {{initializer on function}} \
+                   // expected-note {{overridden virtual function is here}}
+};
+struct Foo : public Baz { // expected-error {{cannot override a non-deleted function}} \
+                          // expected-note {{destructor of 'Foo' is implicitly deleted}}
+  Foo() {}
+};
+}
+
 #endif // BE_THE_HEADER