From 8b3f93869772ef9e1fc667c3dff78fa5cd8ee492 Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Thu, 13 Sep 2012 00:21:31 +0000 Subject: [PATCH] Refactor logic in ExprEngine for detecting 'noreturn' methods in NSException to a helper object in libAnalysis that can also be used by Sema. Not sure if the predicate name 'isImplicitNoReturn' is the best one, but we can massage that later. No functionality change. llvm-svn: 163759 --- .../Analysis/DomainSpecific/ObjCNoReturn.h | 46 +++++++++++++ .../Core/PathSensitive/ExprEngine.h | 12 ++-- clang/lib/Analysis/CMakeLists.txt | 3 +- clang/lib/Analysis/ObjCNoReturn.cpp | 67 +++++++++++++++++++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 4 +- .../StaticAnalyzer/Core/ExprEngineObjC.cpp | 63 +++-------------- 6 files changed, 129 insertions(+), 66 deletions(-) create mode 100644 clang/include/clang/Analysis/DomainSpecific/ObjCNoReturn.h create mode 100644 clang/lib/Analysis/ObjCNoReturn.cpp diff --git a/clang/include/clang/Analysis/DomainSpecific/ObjCNoReturn.h b/clang/include/clang/Analysis/DomainSpecific/ObjCNoReturn.h new file mode 100644 index 000000000000..930c2bd0925b --- /dev/null +++ b/clang/include/clang/Analysis/DomainSpecific/ObjCNoReturn.h @@ -0,0 +1,46 @@ +//= ObjCNoReturn.h - Handling of Cocoa APIs known not to return --*- C++ -*---// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file implements special handling of recognizing ObjC API hooks that +// do not return but aren't marked as such in API headers. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_ANALYSIS_DS_OBJCNORETURN +#define LLVM_CLANG_ANALYSIS_DS_OBJCNORETURN + +#include "clang/Basic/IdentifierTable.h" + +namespace clang { + +class ASTContext; +class ObjCMessageExpr; + +class ObjCNoReturn { + /// Cached "raise" selector. + Selector RaiseSel; + + /// Cached identifier for "NSException". + IdentifierInfo *NSExceptionII; + + enum { NUM_RAISE_SELECTORS = 2 }; + + /// Cached set of selectors in NSException that are 'noreturn'. + Selector NSExceptionInstanceRaiseSelectors[NUM_RAISE_SELECTORS]; + +public: + ObjCNoReturn(ASTContext &C); + + /// Return true if the given message expression is known to never + /// return. + bool isImplicitNoReturn(const ObjCMessageExpr *ME); +}; +} + +#endif diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index 690ead02a008..b1fe05796f5b 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -16,6 +16,7 @@ #ifndef LLVM_CLANG_GR_EXPRENGINE #define LLVM_CLANG_GR_EXPRENGINE +#include "clang/Analysis/DomainSpecific/ObjCNoReturn.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h" @@ -74,13 +75,10 @@ class ExprEngine : public SubEngine { const Stmt *currStmt; unsigned int currStmtIdx; const NodeBuilderContext *currBldrCtx; - - /// Obj-C Class Identifiers. - IdentifierInfo* NSExceptionII; - - /// Obj-C Selectors. - Selector* NSExceptionInstanceRaiseSelectors; - Selector RaiseSel; + + /// Helper object to determine if an Objective-C message expression + /// implicitly never returns. + ObjCNoReturn ObjCNoRet; /// Whether or not GC is enabled in this analysis. bool ObjCGCEnabled; diff --git a/clang/lib/Analysis/CMakeLists.txt b/clang/lib/Analysis/CMakeLists.txt index d57e4813ca02..7c33d7371487 100644 --- a/clang/lib/Analysis/CMakeLists.txt +++ b/clang/lib/Analysis/CMakeLists.txt @@ -1,13 +1,14 @@ add_clang_library(clangAnalysis AnalysisDeclContext.cpp - CallGraph.cpp CFG.cpp CFGReachabilityAnalysis.cpp CFGStmtMap.cpp + CallGraph.cpp CocoaConventions.cpp Dominators.cpp FormatString.cpp LiveVariables.cpp + ObjCNoReturn.cpp PostOrderCFGView.cpp PrintfFormatString.cpp ProgramPoint.cpp diff --git a/clang/lib/Analysis/ObjCNoReturn.cpp b/clang/lib/Analysis/ObjCNoReturn.cpp new file mode 100644 index 000000000000..850739cc785f --- /dev/null +++ b/clang/lib/Analysis/ObjCNoReturn.cpp @@ -0,0 +1,67 @@ +//= ObjCNoReturn.cpp - Handling of Cocoa APIs known not to return --*- C++ -*--- +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file implements special handling of recognizing ObjC API hooks that +// do not return but aren't marked as such in API headers. +// +//===----------------------------------------------------------------------===// + +#include "clang/AST/ASTContext.h" +#include "clang/AST/ExprObjC.h" +#include "clang/Analysis/DomainSpecific/ObjCNoReturn.h" + +using namespace clang; + +static bool isSubclass(const ObjCInterfaceDecl *Class, IdentifierInfo *II) { + if (!Class) + return false; + if (Class->getIdentifier() == II) + return true; + return isSubclass(Class->getSuperClass(), II); +} + +ObjCNoReturn::ObjCNoReturn(ASTContext &C) + : RaiseSel(GetNullarySelector("raise", C)), + NSExceptionII(&C.Idents.get("NSException")) +{ + // Generate selectors. + SmallVector II; + + // raise:format: + II.push_back(&C.Idents.get("raise")); + II.push_back(&C.Idents.get("format")); + NSExceptionInstanceRaiseSelectors[0] = + C.Selectors.getSelector(II.size(), &II[0]); + + // raise:format:arguments: + II.push_back(&C.Idents.get("arguments")); + NSExceptionInstanceRaiseSelectors[1] = + C.Selectors.getSelector(II.size(), &II[0]); +} + + +bool ObjCNoReturn::isImplicitNoReturn(const ObjCMessageExpr *ME) { + Selector S = ME->getSelector(); + + if (ME->isInstanceMessage()) { + // Check for the "raise" message. + return S == RaiseSel; + } + + if (const ObjCInterfaceDecl *ID = ME->getReceiverInterface()) { + if (isSubclass(ID, NSExceptionII)) { + for (unsigned i = 0; i < NUM_RAISE_SELECTORS; ++i) { + if (S == NSExceptionInstanceRaiseSelectors[i]) + return true; + } + } + } + + return false; +} \ No newline at end of file diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 4d4faf6d0b8b..3e5733f10c07 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -68,8 +68,7 @@ ExprEngine::ExprEngine(AnalysisManager &mgr, bool gcEnabled, svalBuilder(StateMgr.getSValBuilder()), EntryNode(NULL), currStmt(NULL), currStmtIdx(0), currBldrCtx(0), - NSExceptionII(NULL), NSExceptionInstanceRaiseSelectors(NULL), - RaiseSel(GetNullarySelector("raise", getContext())), + ObjCNoRet(mgr.getASTContext()), ObjCGCEnabled(gcEnabled), BR(mgr, *this), VisitedCallees(VisitedCalleesIn) { @@ -81,7 +80,6 @@ ExprEngine::ExprEngine(AnalysisManager &mgr, bool gcEnabled, ExprEngine::~ExprEngine() { BR.FlushReports(); - delete [] NSExceptionInstanceRaiseSelectors; } //===----------------------------------------------------------------------===// diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp index 4f1a76e89ed7..51dda19b5315 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp @@ -132,14 +132,6 @@ void ExprEngine::VisitObjCForCollectionStmt(const ObjCForCollectionStmt *S, getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this); } -static bool isSubclass(const ObjCInterfaceDecl *Class, IdentifierInfo *II) { - if (!Class) - return false; - if (Class->getIdentifier() == II) - return true; - return isSubclass(Class->getSuperClass(), II); -} - void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, ExplodedNode *Pred, ExplodedNodeSet &Dst) { @@ -184,7 +176,7 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, // Check if the "raise" message was sent. assert(notNilState); - if (Msg->getSelector() == RaiseSel) { + if (ObjCNoRet.isImplicitNoReturn(ME)) { // If we raise an exception, for now treat it as a sink. // Eventually we will want to handle exceptions properly. Bldr.generateSink(currStmt, Pred, State); @@ -198,52 +190,13 @@ void ExprEngine::VisitObjCMessage(const ObjCMessageExpr *ME, } } } else { - // Check for special class methods. - if (const ObjCInterfaceDecl *Iface = Msg->getReceiverInterface()) { - if (!NSExceptionII) { - ASTContext &Ctx = getContext(); - NSExceptionII = &Ctx.Idents.get("NSException"); - } - - if (isSubclass(Iface, NSExceptionII)) { - enum { NUM_RAISE_SELECTORS = 2 }; - - // Lazily create a cache of the selectors. - if (!NSExceptionInstanceRaiseSelectors) { - ASTContext &Ctx = getContext(); - NSExceptionInstanceRaiseSelectors = - new Selector[NUM_RAISE_SELECTORS]; - SmallVector II; - unsigned idx = 0; - - // raise:format: - II.push_back(&Ctx.Idents.get("raise")); - II.push_back(&Ctx.Idents.get("format")); - NSExceptionInstanceRaiseSelectors[idx++] = - Ctx.Selectors.getSelector(II.size(), &II[0]); - - // raise:format:arguments: - II.push_back(&Ctx.Idents.get("arguments")); - NSExceptionInstanceRaiseSelectors[idx++] = - Ctx.Selectors.getSelector(II.size(), &II[0]); - } - - Selector S = Msg->getSelector(); - bool RaisesException = false; - for (unsigned i = 0; i < NUM_RAISE_SELECTORS; ++i) { - if (S == NSExceptionInstanceRaiseSelectors[i]) { - RaisesException = true; - break; - } - } - if (RaisesException) { - // If we raise an exception, for now treat it as a sink. - // Eventually we will want to handle exceptions properly. - Bldr.generateSink(currStmt, Pred, Pred->getState()); - continue; - } - - } + // Check for special class methods that are known to not return + // and that we should treat as a sink. + if (ObjCNoRet.isImplicitNoReturn(ME)) { + // If we raise an exception, for now treat it as a sink. + // Eventually we will want to handle exceptions properly. + Bldr.generateSink(currStmt, Pred, Pred->getState()); + continue; } }