From: Richard Smith Date: Tue, 20 Feb 2018 02:32:30 +0000 (+0000) Subject: Fix some -Wexceptions false positives. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0432808d1f27410b69c3fd24e2a65d45a759868c;p=clang Fix some -Wexceptions false positives. Reimplement the "noexcept function actually throws" warning to properly handle nested try-blocks. In passing, change 'throw;' handling to treat any enclosing try block as being sufficient to suppress the warning rather than requiring a 'catch (...)'; the warning is intended to be conservatively-correct. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@325545 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp index e67f94c638..aaed8f9827 100644 --- a/lib/Sema/AnalysisBasedWarnings.cpp +++ b/lib/Sema/AnalysisBasedWarnings.cpp @@ -281,87 +281,62 @@ static void checkRecursiveFunction(Sema &S, const FunctionDecl *FD, //===----------------------------------------------------------------------===// // Check for throw in a non-throwing function. //===----------------------------------------------------------------------===// -enum ThrowState { - FoundNoPathForThrow, - FoundPathForThrow, - FoundPathWithNoThrowOutFunction, -}; -static bool isThrowCaughtByHandlers(Sema &S, - const CXXThrowExpr *CE, - const CXXTryStmt *TryStmt) { - for (unsigned H = 0, E = TryStmt->getNumHandlers(); H < E; ++H) { - QualType Caught = TryStmt->getHandler(H)->getCaughtType(); - if (Caught.isNull() || // catch (...) catches everything - (CE->getSubExpr() && // throw; is only caught by ... - S.handlerCanCatch(Caught, CE->getSubExpr()->getType()))) - return true; - } - return false; -} +/// Determine whether an exception thrown by E, unwinding from ThrowBlock, +/// can reach ExitBlock. +static bool throwEscapes(Sema &S, const CXXThrowExpr *E, CFGBlock &ThrowBlock, + CFG *Body) { + SmallVector Stack; + llvm::BitVector Queued(Body->getNumBlockIDs()); -static bool doesThrowEscapePath(Sema &S, CFGBlock Block, - SourceLocation &OpLoc) { - for (const auto &B : Block) { - if (B.getKind() != CFGElement::Statement) - continue; - const auto *CE = dyn_cast(B.getAs()->getStmt()); - if (!CE) - continue; + Stack.push_back(&ThrowBlock); + Queued[ThrowBlock.getBlockID()] = true; - OpLoc = CE->getThrowLoc(); - for (const auto &I : Block.succs()) { - if (!I.isReachable()) + while (!Stack.empty()) { + CFGBlock &UnwindBlock = *Stack.back(); + Stack.pop_back(); + + for (auto &Succ : UnwindBlock.succs()) { + if (!Succ.isReachable() || Queued[Succ->getBlockID()]) continue; - if (const auto *Terminator = - dyn_cast_or_null(I->getTerminator())) - if (isThrowCaughtByHandlers(S, CE, Terminator)) - return false; + + if (Succ->getBlockID() == Body->getExit().getBlockID()) + return true; + + if (auto *Catch = + dyn_cast_or_null(Succ->getLabel())) { + QualType Caught = Catch->getCaughtType(); + if (Caught.isNull() || // catch (...) catches everything + !E->getSubExpr() || // throw; is considered cuaght by any handler + S.handlerCanCatch(Caught, E->getSubExpr()->getType())) + // Exception doesn't escape via this path. + break; + } else { + Stack.push_back(Succ); + Queued[Succ->getBlockID()] = true; + } } - return true; } + return false; } -static bool hasThrowOutNonThrowingFunc(Sema &S, SourceLocation &OpLoc, - CFG *BodyCFG) { - unsigned ExitID = BodyCFG->getExit().getBlockID(); - - SmallVector States(BodyCFG->getNumBlockIDs(), - FoundNoPathForThrow); - States[BodyCFG->getEntry().getBlockID()] = FoundPathWithNoThrowOutFunction; - - SmallVector Stack; - Stack.push_back(&BodyCFG->getEntry()); - while (!Stack.empty()) { - CFGBlock *CurBlock = Stack.pop_back_val(); - - unsigned ID = CurBlock->getBlockID(); - ThrowState CurState = States[ID]; - if (CurState == FoundPathWithNoThrowOutFunction) { - if (ExitID == ID) +static void visitReachableThrows( + CFG *BodyCFG, + llvm::function_ref Visit) { + llvm::BitVector Reachable(BodyCFG->getNumBlockIDs()); + clang::reachable_code::ScanReachableFromBlock(&BodyCFG->getEntry(), Reachable); + for (CFGBlock *B : *BodyCFG) { + if (!Reachable[B->getBlockID()]) + continue; + for (CFGElement &E : *B) { + Optional S = E.getAs(); + if (!S) continue; - - if (doesThrowEscapePath(S, *CurBlock, OpLoc)) - CurState = FoundPathForThrow; + if (auto *Throw = dyn_cast(S->getStmt())) + Visit(Throw, *B); } - - // Loop over successor blocks and add them to the Stack if their state - // changes. - for (const auto &I : CurBlock->succs()) - if (I.isReachable()) { - unsigned NextID = I->getBlockID(); - if (NextID == ExitID && CurState == FoundPathForThrow) { - States[NextID] = CurState; - } else if (States[NextID] < CurState) { - States[NextID] = CurState; - Stack.push_back(I); - } - } } - // Return true if the exit node is reachable, and only reachable through - // a throw expression. - return States[ExitID] == FoundPathForThrow; } static void EmitDiagForCXXThrowInNonThrowingFunc(Sema &S, SourceLocation OpLoc, @@ -392,8 +367,10 @@ static void checkThrowInNonThrowingFunc(Sema &S, const FunctionDecl *FD, if (BodyCFG->getExit().pred_empty()) return; SourceLocation OpLoc; - if (hasThrowOutNonThrowingFunc(S, OpLoc, BodyCFG)) - EmitDiagForCXXThrowInNonThrowingFunc(S, OpLoc, FD); + visitReachableThrows(BodyCFG, [&](const CXXThrowExpr *Throw, CFGBlock &Block) { + if (throwEscapes(S, Throw, Block, BodyCFG)) + EmitDiagForCXXThrowInNonThrowingFunc(S, Throw->getThrowLoc(), FD); + }); } static bool isNoexcept(const FunctionDecl *FD) { diff --git a/test/SemaCXX/warn-throw-out-noexcept-func.cpp b/test/SemaCXX/warn-throw-out-noexcept-func.cpp index 4ecc3a94eb..646cea446c 100644 --- a/test/SemaCXX/warn-throw-out-noexcept-func.cpp +++ b/test/SemaCXX/warn-throw-out-noexcept-func.cpp @@ -248,9 +248,11 @@ void o_ShouldNotDiag() noexcept { } } -void p_ShouldDiag() noexcept { //expected-note {{function declared non-throwing here}} +void p_ShouldNotDiag() noexcept { + // Don't warn here: it's possible that the user arranges to only call this + // when the active exception is of type 'int'. try { - throw; //expected-warning {{has a non-throwing exception specification but}} + throw; } catch (int){ } } @@ -406,3 +408,57 @@ namespace HandlerSpecialCases { try { throw nullptr; } catch (int) {} // expected-warning {{still throw}} } } + +namespace NestedTry { + void f() noexcept { + try { + try { + throw 0; + } catch (float) {} + } catch (int) {} + } + + struct A { [[noreturn]] ~A(); }; + + void g() noexcept { // expected-note {{here}} + try { + try { + throw 0; // expected-warning {{still throw}} + } catch (float) {} + } catch (const char*) {} + } + + void h() noexcept { // expected-note {{here}} + try { + try { + throw 0; + } catch (float) {} + } catch (int) { + throw; // expected-warning {{still throw}} + } + } + + // FIXME: Ideally, this should still warn; we can track which types are + // potentially thrown by the rethrow. + void i() noexcept { + try { + try { + throw 0; + } catch (int) { + throw; + } + } catch (float) {} + } + + // FIXME: Ideally, this should not warn: the second catch block is + // unreachable. + void j() noexcept { // expected-note {{here}} + try { + try { + throw 0; + } catch (int) {} + } catch (float) { + throw; // expected-warning {{still throw}} + } + } +}