From d11abb4917114eb7e96e7272c9182bc541d39156 Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Thu, 28 May 2015 00:14:02 +0000 Subject: [PATCH] Update -Winvalid-noreturn to handle destructors better. When checking if a function is noreturn, consider a codepath to be noreturn if the path destroys a class and the class destructor, base class destructors, or member field destructors are marked noreturn. Differential Revision: http://reviews.llvm.org/D9454 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@238382 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/DeclCXX.h | 4 + lib/AST/DeclCXX.cpp | 22 +++++ lib/Analysis/CFG.cpp | 5 +- test/SemaCXX/attr-noreturn.cpp | 148 +++++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 3 deletions(-) diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h index f7cb4624cb..537ad4640c 100644 --- a/include/clang/AST/DeclCXX.h +++ b/include/clang/AST/DeclCXX.h @@ -1392,6 +1392,10 @@ public: /// \brief Returns the destructor decl for this class. CXXDestructorDecl *getDestructor() const; + /// \brief Returns true if the class destructor, or any implicitly invoked + /// destructors are marked noreturn. + bool isAnyDestructorNoReturn() const; + /// \brief If the class is a local class [class.local], returns /// the enclosing function declaration. const FunctionDecl *isLocalClass() const { diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp index 8dc62dd07f..b00b8a0ea9 100644 --- a/lib/AST/DeclCXX.cpp +++ b/lib/AST/DeclCXX.cpp @@ -1315,6 +1315,28 @@ CXXDestructorDecl *CXXRecordDecl::getDestructor() const { return Dtor; } +bool CXXRecordDecl::isAnyDestructorNoReturn() const { + // Destructor is noreturn. + if (const CXXDestructorDecl *Destructor = getDestructor()) + if (Destructor->isNoReturn()) + return true; + + // Check base classes destructor for noreturn. + for (const auto &Base : bases()) + if (Base.getType()->getAsCXXRecordDecl()->isAnyDestructorNoReturn()) + return true; + + // Check fields for noreturn. + for (const auto *Field : fields()) + if (const CXXRecordDecl *RD = + Field->getType()->getBaseElementTypeUnsafe()->getAsCXXRecordDecl()) + if (RD->isAnyDestructorNoReturn()) + return true; + + // All destructors are not noreturn. + return false; +} + void CXXRecordDecl::completeDefinition() { completeDefinition(nullptr); } diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp index 2744c5fbe7..6f624ee406 100644 --- a/lib/Analysis/CFG.cpp +++ b/lib/Analysis/CFG.cpp @@ -1179,8 +1179,7 @@ void CFGBuilder::addAutomaticObjDtors(LocalScope::const_iterator B, } Ty = Context->getBaseElementType(Ty); - const CXXDestructorDecl *Dtor = Ty->getAsCXXRecordDecl()->getDestructor(); - if (Dtor->isNoReturn()) + if (Ty->getAsCXXRecordDecl()->isAnyDestructorNoReturn()) Block = createNoReturnBlock(); else autoCreateBlock(); @@ -3682,7 +3681,7 @@ CFGBlock *CFGBuilder::VisitCXXBindTemporaryExprForTemporaryDtors( const CXXDestructorDecl *Dtor = E->getTemporary()->getDestructor(); - if (Dtor->isNoReturn()) { + if (Dtor->getParent()->isAnyDestructorNoReturn()) { // If the destructor is marked as a no-return destructor, we need to // create a new block for the destructor which does not have as a // successor anything built thus far. Control won't flow out of this diff --git a/test/SemaCXX/attr-noreturn.cpp b/test/SemaCXX/attr-noreturn.cpp index 41214c4f4b..6df008dffa 100644 --- a/test/SemaCXX/attr-noreturn.cpp +++ b/test/SemaCXX/attr-noreturn.cpp @@ -17,6 +17,154 @@ namespace test5 { } } +namespace destructor_tests { + __attribute__((noreturn)) void fail(); + + struct A { + ~A() __attribute__((noreturn)) { fail(); } + }; + struct B { + B() {} + ~B() __attribute__((noreturn)) { fail(); } + }; + struct C : A {}; + struct D : B {}; + struct E : virtual A {}; + struct F : A, virtual B {}; + struct G : E {}; + struct H : virtual D {}; + struct I : A {}; + struct J : I {}; + struct K : virtual A {}; + struct L : K {}; + struct M : virtual C {}; + struct N : M {}; + struct O { N n; }; + + __attribute__((noreturn)) void test_1() { A a; } + __attribute__((noreturn)) void test_2() { B b; } + __attribute__((noreturn)) void test_3() { C c; } + __attribute__((noreturn)) void test_4() { D d; } + __attribute__((noreturn)) void test_5() { E e; } + __attribute__((noreturn)) void test_6() { F f; } + __attribute__((noreturn)) void test_7() { G g; } + __attribute__((noreturn)) void test_8() { H h; } + __attribute__((noreturn)) void test_9() { I i; } + __attribute__((noreturn)) void test_10() { J j; } + __attribute__((noreturn)) void test_11() { K k; } + __attribute__((noreturn)) void test_12() { L l; } + __attribute__((noreturn)) void test_13() { M m; } + __attribute__((noreturn)) void test_14() { N n; } + __attribute__((noreturn)) void test_15() { O o; } + + __attribute__((noreturn)) void test_16() { const A& a = A(); } + __attribute__((noreturn)) void test_17() { const B& b = B(); } + __attribute__((noreturn)) void test_18() { const C& c = C(); } + __attribute__((noreturn)) void test_19() { const D& d = D(); } + __attribute__((noreturn)) void test_20() { const E& e = E(); } + __attribute__((noreturn)) void test_21() { const F& f = F(); } + __attribute__((noreturn)) void test_22() { const G& g = G(); } + __attribute__((noreturn)) void test_23() { const H& h = H(); } + __attribute__((noreturn)) void test_24() { const I& i = I(); } + __attribute__((noreturn)) void test_25() { const J& j = J(); } + __attribute__((noreturn)) void test_26() { const K& k = K(); } + __attribute__((noreturn)) void test_27() { const L& l = L(); } + __attribute__((noreturn)) void test_28() { const M& m = M(); } + __attribute__((noreturn)) void test_29() { const N& n = N(); } + __attribute__((noreturn)) void test_30() { const O& o = O(); } + + struct AA {}; + struct BB { BB() {} ~BB() {} }; + struct CC : AA {}; + struct DD : BB {}; + struct EE : virtual AA {}; + struct FF : AA, virtual BB {}; + struct GG : EE {}; + struct HH : virtual DD {}; + struct II : AA {}; + struct JJ : II {}; + struct KK : virtual AA {}; + struct LL : KK {}; + struct MM : virtual CC {}; + struct NN : MM {}; + struct OO { NN n; }; + + __attribute__((noreturn)) void test_31() { + AA a; + BB b; + CC c; + DD d; + EE e; + FF f; + GG g; + HH h; + II i; + JJ j; + KK k; + LL l; + MM m; + NN n; + OO o; + + const AA& aa = AA(); + const BB& bb = BB(); + const CC& cc = CC(); + const DD& dd = DD(); + const EE& ee = EE(); + const FF& ff = FF(); + const GG& gg = GG(); + const HH& hh = HH(); + const II& ii = II(); + const JJ& jj = JJ(); + const KK& kk = KK(); + const LL& ll = LL(); + const MM& mm = MM(); + const NN& nn = NN(); + const OO& oo = OO(); + } // expected-warning {{function declared 'noreturn' should not return}} + + struct P { + ~P() __attribute__((noreturn)) { fail(); } + void foo() {} + }; + struct Q : P { }; + __attribute__((noreturn)) void test31() { + P().foo(); + } + __attribute__((noreturn)) void test32() { + Q().foo(); + } + + struct R { + A a[5]; + }; + __attribute__((noreturn)) void test33() { + R r; + } + + // FIXME: Code flow analysis does not preserve information about non-null + // pointers, so it can't determine that this function is noreturn. + __attribute__((noreturn)) void test34() { + A *a = new A; + delete a; + } // expected-warning {{function declared 'noreturn' should not return}} + + struct S { + virtual ~S(); + }; + struct T : S { + __attribute__((noreturn)) ~T(); + }; + + // FIXME: Code flow analysis does not preserve information about non-null + // pointers or derived class pointers, so it can't determine that this + // function is noreturn. + __attribute__((noreturn)) void test35() { + S *s = new T; + delete s; + } // expected-warning {{function declared 'noreturn' should not return}} +} + // PR5620 void f0() __attribute__((__noreturn__)); void f1(void (*)()); -- 2.40.0