Fix NoReturnFunctionChecker to properly look at a function's type
when determining if it returns. Fixes <rdar://problem/7796563>.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@99663 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Checker/NoReturnFunctionChecker.cpp b/lib/Checker/NoReturnFunctionChecker.cpp
index 1455d87..80fea99 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 @@
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 675e9a6..3f79c0c 100644
--- a/test/Analysis/retain-release.m
+++ b/test/Analysis/retain-release.m
@@ -1268,6 +1268,7 @@
//===----------------------------------------------------------------------===//
void panic() __attribute__((noreturn));
+void panic_not_in_hardcoded_list() __attribute__((noreturn));
void test_panic_negative() {
signed z = 1;
@@ -1291,9 +1292,13 @@
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();
+ }
}
//===----------------------------------------------------------------------===//