diff options
| -rw-r--r-- | lib/Checker/NoReturnFunctionChecker.cpp | 82 | ||||
| -rw-r--r-- | test/Analysis/retain-release.m | 9 |
2 files changed, 48 insertions, 43 deletions
diff --git a/lib/Checker/NoReturnFunctionChecker.cpp b/lib/Checker/NoReturnFunctionChecker.cpp index 1455d87665..80fea99c36 100644 --- a/lib/Checker/NoReturnFunctionChecker.cpp +++ b/lib/Checker/NoReturnFunctionChecker.cpp @@ -13,17 +13,17 @@ //===----------------------------------------------------------------------===// #include "GRExprEngineInternalChecks.h" -#include "clang/Checker/PathSensitive/Checker.h" +#include "clang/Checker/PathSensitive/CheckerVisitor.h" #include "llvm/ADT/StringSwitch.h" using namespace clang; namespace { -class NoReturnFunctionChecker : public Checker { +class NoReturnFunctionChecker : public CheckerVisitor<NoReturnFunctionChecker> { public: static void *getTag() { static int tag = 0; return &tag; } - virtual bool EvalCallExpr(CheckerContext &C, const CallExpr *CE); + void PostVisitCallExpr(CheckerContext &C, const CallExpr *CE); }; } @@ -32,48 +32,48 @@ void clang::RegisterNoReturnFunctionChecker(GRExprEngine &Eng) { Eng.registerCheck(new NoReturnFunctionChecker()); } -bool NoReturnFunctionChecker::EvalCallExpr(CheckerContext &C, - const CallExpr *CE) { +void NoReturnFunctionChecker::PostVisitCallExpr(CheckerContext &C, + const CallExpr *CE) { const GRState *state = C.getState(); const Expr *Callee = CE->getCallee(); - SVal L = state->getSVal(Callee); - const FunctionDecl *FD = L.getAsFunctionDecl(); - if (!FD) - return false; - bool BuildSinks = false; + bool BuildSinks = Callee->getType().getNoReturnAttr(); - if (FD->getAttr<NoReturnAttr>() || FD->getAttr<AnalyzerNoReturnAttr>()) - BuildSinks = true; - else if (const IdentifierInfo *II = FD->getIdentifier()) { - // HACK: Some functions are not marked noreturn, and don't return. - // Here are a few hardwired ones. If this takes too long, we can - // potentially cache these results. - BuildSinks - = llvm::StringSwitch<bool>(llvm::StringRef(II->getName())) - .Case("exit", true) - .Case("panic", true) - .Case("error", true) - .Case("Assert", true) - // FIXME: This is just a wrapper around throwing an exception. - // Eventually inter-procedural analysis should handle this easily. - .Case("ziperr", true) - .Case("assfail", true) - .Case("db_error", true) - .Case("__assert", true) - .Case("__assert_rtn", true) - .Case("__assert_fail", true) - .Case("dtrace_assfail", true) - .Case("yy_fatal_error", true) - .Case("_XCAssertionFailureHandler", true) - .Case("_DTAssertionFailureHandler", true) - .Case("_TSAssertionFailureHandler", true) - .Default(false); + if (!BuildSinks) { + SVal L = state->getSVal(Callee); + const FunctionDecl *FD = L.getAsFunctionDecl(); + if (!FD) + return; + + if (FD->getAttr<AnalyzerNoReturnAttr>()) + BuildSinks = true; + else if (const IdentifierInfo *II = FD->getIdentifier()) { + // HACK: Some functions are not marked noreturn, and don't return. + // Here are a few hardwired ones. If this takes too long, we can + // potentially cache these results. + BuildSinks + = llvm::StringSwitch<bool>(llvm::StringRef(II->getName())) + .Case("exit", true) + .Case("panic", true) + .Case("error", true) + .Case("Assert", true) + // FIXME: This is just a wrapper around throwing an exception. + // Eventually inter-procedural analysis should handle this easily. + .Case("ziperr", true) + .Case("assfail", true) + .Case("db_error", true) + .Case("__assert", true) + .Case("__assert_rtn", true) + .Case("__assert_fail", true) + .Case("dtrace_assfail", true) + .Case("yy_fatal_error", true) + .Case("_XCAssertionFailureHandler", true) + .Case("_DTAssertionFailureHandler", true) + .Case("_TSAssertionFailureHandler", true) + .Default(false); + } } - - if (!BuildSinks) - return false; - C.GenerateSink(CE); - return true; + if (BuildSinks) + C.GenerateSink(CE); } diff --git a/test/Analysis/retain-release.m b/test/Analysis/retain-release.m index 675e9a6702..3f79c0c7f4 100644 --- a/test/Analysis/retain-release.m +++ b/test/Analysis/retain-release.m @@ -1268,6 +1268,7 @@ CFDateRef returnsRetainedCFDate() { //===----------------------------------------------------------------------===// void panic() __attribute__((noreturn)); +void panic_not_in_hardcoded_list() __attribute__((noreturn)); void test_panic_negative() { signed z = 1; @@ -1291,9 +1292,13 @@ void test_panic_pos_2(int x) { signed z = 1; CFNumberRef value = CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt32Type, &z); // no-warning if (x) - panic(); - if (!x) panic(); + if (!x) { + // This showed up in <rdar://problem/7796563>, where we silently missed checking + // the function type for noreturn. "panic()" is a hard-coded known panic function + // that isn't always noreturn. + panic_not_in_hardcoded_list(); + } } //===----------------------------------------------------------------------===// |