<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 23 June 2017 at 13:22, Erich Keane via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: erichkeane<br>
Date: Fri Jun 23 15:22:19 2017<br>
New Revision: 306149<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=306149&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=306149&view=rev</a><br>
Log:<br>
Emit warning when throw exception in destruct or dealloc functions which has a<br>
(possible implicit) noexcept specifier<br>
<br>
Throwing in the destructor is not good (C++11 change try to not allow see below).<br>
But in reality, those codes are exist.<br>
C++11 [class.dtor]p3:<br>
<br>
A declaration of a destructor that does not have an exception-specification is<br>
implicitly considered to have the same exception specification as an implicit<br>
declaration.<br>
<br>
With this change, the application worked before may now run into runtime<br>
termination. My goal here is to emit a warning to provide only possible info to<br>
where the code may need to be changed.<br>
<br>
First there is no way, in compile time to identify the “throw” really throw out<br>
of the function. Things like the call which throw out… To keep this simple,<br>
when “throw” is seen, checking its enclosing function(only destructor and<br>
dealloc functions) with noexcept(true) specifier emit warning.<br>
<br>
Here is implementation detail:<br>
A new member function CheckCXXThrowInNonThrowingFunc is added for class Sema<br>
in Sema.h. It is used in the call to both BuildCXXThrow and<br>
TransformCXXThrowExpr.<br>
<br>
The function basic check if the enclosing function with non-throwing noexcept<br>
specifer, if so emit warning for it.<br>
<br>
The example of warning message like:<br>
k1.cpp:18:3: warning: ''~dependent_warn'' has a (possible implicit) non-throwing<br>
<br>
noexcept specifier. Throwing exception may cause termination.<br>
[-Wthrow-in-dtor]<br>
throw 1;<br>
^<br>
<br>
k1.cpp:43:30: note: in instantiation of member function<br>
<br>
'dependent_warn<noexcept_fun>:<wbr>:~dependent_warn' requested here<br>
<br>
dependent_warn<noexcept_fun> f; // cause warning<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D33333" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D33333</a><br>
<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/<wbr>DiagnosticSemaKinds.td<br>
cfe/trunk/lib/Sema/<wbr>AnalysisBasedWarnings.cpp<br>
cfe/trunk/test/CXX/except/<wbr>except.spec/p11.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/<wbr>DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=306149&r1=306148&r2=306149&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/Basic/<wbr>DiagnosticSemaKinds.td?rev=<wbr>306149&r1=306148&r2=306149&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/Basic/<wbr>DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/<wbr>DiagnosticSemaKinds.td Fri Jun 23 15:22:19 2017<br>
@@ -6351,6 +6351,15 @@ def err_exceptions_disabled : Error<<br>
"cannot use '%0' with exceptions disabled">;<br>
def err_objc_exceptions_disabled : Error<<br>
"cannot use '%0' with Objective-C exceptions disabled">;<br>
+def warn_throw_in_noexcept_func<br>
+ : Warning<"%0 has a non-throwing exception specification but can still "<br>
+ "throw, resulting in unexpected program termination">,<br></blockquote><div><br></div><div>How do you know it's unexpected? :) You also don't know that this leads to program termination: a set_unexpected handler could do something else, in principle. I would just delete the ", resulting in unexpected program termination" part here.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ InGroup<Exceptions>;<br>
+def note_throw_in_dtor<br>
+ : Note<"destructor or deallocator has a (possibly implicit) non-throwing "<br>
+ "excepton specification">;<br></blockquote><div><br></div><div>Please figure out which case we're actually in, and just mention that one. You can use "hasImplicitExceptionSpec" in SemaExceptionSpec.cpp to determine whether the exception specification is implicit.</div><div><br></div><div>Also, typo "excepton".</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+def note_throw_in_function<br>
+ : Note<"non-throwing function declare here">;<br></blockquote><div><br></div><div>declare -> declared, but something like "function declared non-throwing here" would be preferable</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
def err_seh_try_outside_functions : Error<<br>
"cannot use SEH '__try' in blocks, captured regions, or Obj-C method decls">;<br>
def err_mixing_cxx_try_seh_try : Error<<br>
<br>
Modified: cfe/trunk/lib/Sema/<wbr>AnalysisBasedWarnings.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=306149&r1=306148&r2=306149&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Sema/<wbr>AnalysisBasedWarnings.cpp?rev=<wbr>306149&r1=306148&r2=306149&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Sema/<wbr>AnalysisBasedWarnings.cpp (original)<br>
+++ cfe/trunk/lib/Sema/<wbr>AnalysisBasedWarnings.cpp Fri Jun 23 15:22:19 2017<br>
@@ -279,6 +279,150 @@ static void checkRecursiveFunction(Sema<br>
}<br>
<br>
//===-------------------------<wbr>------------------------------<wbr>---------------===//<br>
+// Check for throw in a non-throwing function.<br>
+//===------------------------<wbr>------------------------------<wbr>----------------===//<br>
+enum ThrowState {<br>
+ FoundNoPathForThrow,<br>
+ FoundPathForThrow,<br>
+ FoundPathWithNoThrowOutFunctio<wbr>n,<br>
+};<br>
+<br>
+static bool isThrowCaught(const CXXThrowExpr *Throw,<br>
+ const CXXCatchStmt *Catch) {<br>
+ const Type *ThrowType = nullptr;<br>
+ if (Throw->getSubExpr())<br>
+ ThrowType = Throw->getSubExpr()->getType()<wbr>.getTypePtrOrNull();<br>
+ if (!ThrowType)<br>
+ return false;<br>
+ const Type *CaughtType = Catch->getCaughtType().<wbr>getTypePtrOrNull();<br>
+ if (!CaughtType)<br>
+ return true;<br>
+ if (ThrowType->isReferenceType())<br>
+ ThrowType = ThrowType->castAs<<wbr>ReferenceType>()<br>
+ ->getPointeeType()<br>
+ ->getUnqualifiedDesugaredType(<wbr>);<br>
+ if (CaughtType->isReferenceType()<wbr>)<br>
+ CaughtType = CaughtType->castAs<<wbr>ReferenceType>()<br>
+ ->getPointeeType()<br>
+ ->getUnqualifiedDesugaredType(<wbr>);<br>
+ if (CaughtType == ThrowType)<br>
+ return true;<br>
+ const CXXRecordDecl *CaughtAsRecordType =<br>
+ CaughtType-><wbr>getPointeeCXXRecordDecl();<br>
+ const CXXRecordDecl *ThrowTypeAsRecordType = ThrowType->getAsCXXRecordDecl(<wbr>);<br>
+ if (CaughtAsRecordType && ThrowTypeAsRecordType)<br>
+ return ThrowTypeAsRecordType-><wbr>isDerivedFrom(<wbr>CaughtAsRecordType);<br>
+ return false;<br>
+}<br>
+<br>
+static bool isThrowCaughtByHandlers(const CXXThrowExpr *CE,<br>
+ const CXXTryStmt *TryStmt) {<br>
+ for (unsigned H = 0, E = TryStmt->getNumHandlers(); H < E; ++H) {<br>
+ if (isThrowCaught(CE, TryStmt->getHandler(H)))<br>
+ return true;<br>
+ }<br>
+ return false;<br>
+}<br>
+<br>
+static bool doesThrowEscapePath(CFGBlock Block, SourceLocation &OpLoc) {<br>
+ for (const auto &B : Block) {<br>
+ if (B.getKind() != CFGElement::Statement)<br>
+ continue;<br>
+ const auto *CE = dyn_cast<CXXThrowExpr>(B.<wbr>getAs<CFGStmt>()->getStmt());<br>
+ if (!CE)<br>
+ continue;<br>
+<br>
+ OpLoc = CE->getThrowLoc();<br>
+ for (const auto &I : Block.succs()) {<br>
+ if (!I.isReachable())<br>
+ continue;<br>
+ if (const auto *Terminator =<br>
+ dyn_cast_or_null<CXXTryStmt>(<wbr>I->getTerminator()))<br>
+ if (isThrowCaughtByHandlers(CE, Terminator))<br>
+ return false;<br>
+ }<br>
+ return true;<br>
+ }<br>
+ return false;<br>
+}<br>
+<br>
+static bool hasThrowOutNonThrowingFunc(<wbr>SourceLocation &OpLoc, CFG *BodyCFG) {<br>
+<br>
+ unsigned ExitID = BodyCFG->getExit().getBlockID(<wbr>);<br>
+<br>
+ SmallVector<ThrowState, 16> States(BodyCFG-><wbr>getNumBlockIDs(),<br>
+ FoundNoPathForThrow);<br>
+ States[BodyCFG->getEntry().<wbr>getBlockID()] = FoundPathWithNoThrowOutFunctio<wbr>n;<br>
+<br>
+ SmallVector<CFGBlock *, 16> Stack;<br>
+ Stack.push_back(&BodyCFG-><wbr>getEntry());<br>
+ while (!Stack.empty()) {<br>
+ CFGBlock *CurBlock = Stack.back();<br>
+ Stack.pop_back();<br>
+<br>
+ unsigned ID = CurBlock->getBlockID();<br>
+ ThrowState CurState = States[ID];<br>
+ if (CurState == FoundPathWithNoThrowOutFunctio<wbr>n) {<br>
+ if (ExitID == ID)<br>
+ continue;<br>
+<br>
+ if (doesThrowEscapePath(*<wbr>CurBlock, OpLoc))<br>
+ CurState = FoundPathForThrow;<br>
+ }<br>
+<br>
+ // Loop over successor blocks and add them to the Stack if their state<br>
+ // changes.<br>
+ for (const auto &I : CurBlock->succs())<br>
+ if (I.isReachable()) {<br>
+ unsigned NextID = I->getBlockID();<br>
+ if (NextID == ExitID && CurState == FoundPathForThrow) {<br>
+ States[NextID] = CurState;<br>
+ } else if (States[NextID] < CurState) {<br>
+ States[NextID] = CurState;<br>
+ Stack.push_back(I);<br>
+ }<br>
+ }<br>
+ }<br>
+ // Return true if the exit node is reachable, and only reachable through<br>
+ // a throw expression.<br>
+ return States[ExitID] == FoundPathForThrow;<br>
+}<br>
+<br>
+static void EmitDiagForCXXThrowInNonThrowi<wbr>ngFunc(Sema &S, SourceLocation OpLoc,<br>
+ const FunctionDecl *FD) {<br>
+ if (!S.getSourceManager().<wbr>isInSystemHeader(OpLoc)) {<br>
+ S.Diag(OpLoc, diag::warn_throw_in_noexcept_<wbr>func) << FD;<br>
+ if (S.getLangOpts().CPlusPlus11 &&<br>
+ (isa<CXXDestructorDecl>(FD) ||<br>
+ FD->getDeclName().<wbr>getCXXOverloadedOperator() == OO_Delete ||<br>
+ FD->getDeclName().<wbr>getCXXOverloadedOperator() == OO_Array_Delete))<br>
+ S.Diag(FD->getLocation(), diag::note_throw_in_dtor);<br>
+ else<br>
+ S.Diag(FD->getLocation(), diag::note_throw_in_function);<br></blockquote><div><br></div><div>Please point this diagnostic at the exception specification when its location can be computed (use FD->getExceptionSpecSourceRange()).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ }<br>
+}<br>
+<br>
+static void checkThrowInNonThrowingFunc(<wbr>Sema &S, const FunctionDecl *FD,<br>
+ AnalysisDeclContext &AC) {<br>
+ CFG *BodyCFG = AC.getCFG();<br>
+ if (!BodyCFG)<br>
+ return;<br>
+ if (BodyCFG->getExit().pred_<wbr>empty())<br>
+ return;<br>
+ SourceLocation OpLoc;<br>
+ if (hasThrowOutNonThrowingFunc(<wbr>OpLoc, BodyCFG))<br>
+ EmitDiagForCXXThrowInNonThrowi<wbr>ngFunc(S, OpLoc, FD);<br>
+}<br>
+<br>
+static bool isNoexcept(const FunctionDecl *FD) {<br>
+ const auto *FPT = FD->getType()->castAs<<wbr>FunctionProtoType>();<br>
+ if (FPT->getExceptionSpecType() != EST_None &&<br>
+ FPT->isNothrow(FD-><wbr>getASTContext()))<br></blockquote><div><br></div><div>Why the EST_None special case here? The isNothrow check would handle that just fine.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ return true;<br>
+ return false;<br>
+}<br>
+<br>
+//===------------------------<wbr>------------------------------<wbr>----------------===//<br>
// Check for missing return value.<br>
//===-------------------------<wbr>------------------------------<wbr>---------------===//<br>
<br>
@@ -2127,6 +2271,12 @@ AnalysisBasedWarnings::<wbr>IssueWarnings(sem<br>
}<br>
}<br>
<br>
+ // Check for throw out of non-throwing function.<br>
+ if (!Diags.isIgnored(diag::warn_<wbr>throw_in_noexcept_func, D->getLocStart()))<br>
+ if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))<br>
+ if (S.getLangOpts().CPlusPlus && isNoexcept(FD))<br>
+ checkThrowInNonThrowingFunc(S, FD, AC);<br>
+<br>
// If none of the previous checks caused a CFG build, trigger one here<br>
// for -Wtautological-overlap-compare<br>
if (!Diags.isIgnored(diag::warn_<wbr>tautological_overlap_<wbr>comparison,<br>
<br>
Modified: cfe/trunk/test/CXX/except/<wbr>except.spec/p11.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p11.cpp?rev=306149&r1=306148&r2=306149&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/CXX/<wbr>except/except.spec/p11.cpp?<wbr>rev=306149&r1=306148&r2=<wbr>306149&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/CXX/except/<wbr>except.spec/p11.cpp (original)<br>
+++ cfe/trunk/test/CXX/except/<wbr>except.spec/p11.cpp Fri Jun 23 15:22:19 2017<br>
@@ -1,12 +1,11 @@<br>
// RUN: %clang_cc1 -std=c++11 -fexceptions -fcxx-exceptions -fsyntax-only -verify %s<br>
-// expected-no-diagnostics<br>
<br>
// This is the "let the user shoot themselves in the foot" clause.<br>
-void f() noexcept {<br>
- throw 0; // no-error<br>
+void f() noexcept { // expected-note {{non-throwing function declare here}}<br>
+ throw 0; // expected-warning {{has a non-throwing exception specification but}}<br>
}<br>
-void g() throw() {<br>
- throw 0; // no-error<br>
+void g() throw() { // expected-note {{non-throwing function declare here}}<br>
+ throw 0; // expected-warning {{has a non-throwing exception specification but}}<br>
}<br>
void h() throw(int) {<br>
throw 0.0; // no-error<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>