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.

llvm-svn: 325545
This commit is contained in:
Richard Smith 2018-02-20 02:32:30 +00:00
parent 711964ddc1
commit 0848210b74
2 changed files with 110 additions and 77 deletions

View File

@ -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;
}
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<CXXThrowExpr>(B.getAs<CFGStmt>()->getStmt());
if (!CE)
continue;
OpLoc = CE->getThrowLoc();
for (const auto &I : Block.succs()) {
if (!I.isReachable())
continue;
if (const auto *Terminator =
dyn_cast_or_null<CXXTryStmt>(I->getTerminator()))
if (isThrowCaughtByHandlers(S, CE, Terminator))
return false;
}
return true;
}
return false;
}
static bool hasThrowOutNonThrowingFunc(Sema &S, SourceLocation &OpLoc,
CFG *BodyCFG) {
unsigned ExitID = BodyCFG->getExit().getBlockID();
SmallVector<ThrowState, 16> States(BodyCFG->getNumBlockIDs(),
FoundNoPathForThrow);
States[BodyCFG->getEntry().getBlockID()] = FoundPathWithNoThrowOutFunction;
/// 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<CFGBlock *, 16> Stack;
Stack.push_back(&BodyCFG->getEntry());
while (!Stack.empty()) {
CFGBlock *CurBlock = Stack.pop_back_val();
llvm::BitVector Queued(Body->getNumBlockIDs());
unsigned ID = CurBlock->getBlockID();
ThrowState CurState = States[ID];
if (CurState == FoundPathWithNoThrowOutFunction) {
if (ExitID == ID)
Stack.push_back(&ThrowBlock);
Queued[ThrowBlock.getBlockID()] = true;
while (!Stack.empty()) {
CFGBlock &UnwindBlock = *Stack.back();
Stack.pop_back();
for (auto &Succ : UnwindBlock.succs()) {
if (!Succ.isReachable() || Queued[Succ->getBlockID()])
continue;
if (doesThrowEscapePath(S, *CurBlock, OpLoc))
CurState = FoundPathForThrow;
}
if (Succ->getBlockID() == Body->getExit().getBlockID())
return true;
// 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);
}
if (auto *Catch =
dyn_cast_or_null<CXXCatchStmt>(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 false;
}
static void visitReachableThrows(
CFG *BodyCFG,
llvm::function_ref<void(const CXXThrowExpr *, CFGBlock &)> 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<CFGStmt> S = E.getAs<CFGStmt>();
if (!S)
continue;
if (auto *Throw = dyn_cast<CXXThrowExpr>(S->getStmt()))
Visit(Throw, *B);
}
}
// 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) {

View File

@ -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}}
}
}
}