From: Ted Kremenek Date: Thu, 18 Sep 2008 21:25:13 +0000 (+0000) Subject: Change implementation of NSError** coding-style check to be invoked at the end of... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=cfdf9b4edf1172728be97d1ae2d95171975f812b;p=clang Change implementation of NSError** coding-style check to be invoked at the end of the retain/release analysis. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@56312 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/Driver/Analyses.def b/Driver/Analyses.def index 691e4eee32..841f0c1da1 100644 --- a/Driver/Analyses.def +++ b/Driver/Analyses.def @@ -34,10 +34,6 @@ ANALYSIS(WarnObjCDealloc, "warn-objc-missing-dealloc", "Warn about Objective-C classes that lack a correct implementation of -dealloc", ObjCImplementation) -ANALYSIS(WarnObjCNSError, "warn-objc-nserror-methods", - "Check coding rules for 'Creating and Returning NSError Objects'", - ObjCImplementation) - ANALYSIS(WarnObjCUnusedIvars, "warn-objc-unused-ivars", "Warn about private ivars that are never used", ObjCImplementation) diff --git a/Driver/AnalysisConsumer.cpp b/Driver/AnalysisConsumer.cpp index 9e321b6fd1..e22178b73e 100644 --- a/Driver/AnalysisConsumer.cpp +++ b/Driver/AnalysisConsumer.cpp @@ -431,11 +431,6 @@ static void ActionWarnObjCMethSigs(AnalysisManager& mgr) { BR); } -static void ActionWarnObjCNSError(AnalysisManager& mgr) { - BugReporter BR(mgr); - CheckNSError(cast(mgr.getCodeDecl()), BR); -} - //===----------------------------------------------------------------------===// // AnalysisConsumer creation. //===----------------------------------------------------------------------===// diff --git a/include/clang/Analysis/LocalCheckers.h b/include/clang/Analysis/LocalCheckers.h index a27f27b567..23610f9a2d 100644 --- a/include/clang/Analysis/LocalCheckers.h +++ b/include/clang/Analysis/LocalCheckers.h @@ -46,7 +46,6 @@ void CheckObjCDealloc(ObjCImplementationDecl* D, const LangOptions& L, void CheckObjCInstMethSignature(ObjCImplementationDecl* ID, BugReporter& BR); void CheckObjCUnusedIvar(ObjCImplementationDecl* D, BugReporter& BR); -void CheckNSError(ObjCImplementationDecl* D, BugReporter& BR); void RegisterAppleChecks(GRExprEngine& Eng); diff --git a/include/clang/Analysis/PathSensitive/BugReporter.h b/include/clang/Analysis/PathSensitive/BugReporter.h index 0cf6567652..39e67f7798 100644 --- a/include/clang/Analysis/PathSensitive/BugReporter.h +++ b/include/clang/Analysis/PathSensitive/BugReporter.h @@ -223,12 +223,18 @@ public: virtual ~GRBugReporter(); + /// getEngine - Return the analysis engine used to analyze a given + /// function or method. GRExprEngine& getEngine() { return Eng; } + /// getGraph - Get the exploded graph created by the analysis engine + /// for the analyzed method or function. ExplodedGraph& getGraph(); + /// getStateManager - Return the state manager used by the analysis + /// engine. GRStateManager& getStateManager(); virtual void GeneratePathDiagnostic(PathDiagnostic& PD, BugReport& R); @@ -240,7 +246,8 @@ public: bool isNotable(SymbolID Sym) const { return (bool) NotableSymbols.count(Sym); } - + + /// classof - Used by isa<>, cast<>, and dyn_cast<>. static bool classof(const BugReporter* R) { return R->getKind() == GRBugReporterKind; } diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h index b1f4e70d39..d08e442e35 100644 --- a/include/clang/Analysis/PathSensitive/GRExprEngine.h +++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h @@ -216,6 +216,9 @@ public: const_bug_type_iterator bug_types_begin() const { return BugTypes.begin(); } const_bug_type_iterator bug_types_end() const { return BugTypes.end(); } + /// Register - Register a BugType with the analyzer engine. A registered + /// BugType object will have its 'EmitWarnings' method called when the + /// the analyzer finishes analyzing a method or function. void Register(BugType* B) { BugTypes.push_back(B); } diff --git a/lib/Analysis/BasicObjCFoundationChecks.cpp b/lib/Analysis/BasicObjCFoundationChecks.cpp index 2b25e16fca..47785aca20 100644 --- a/lib/Analysis/BasicObjCFoundationChecks.cpp +++ b/lib/Analysis/BasicObjCFoundationChecks.cpp @@ -559,5 +559,8 @@ void clang::RegisterAppleChecks(GRExprEngine& Eng) { Eng.AddCheck(CreateBasicObjCFoundationChecks(Ctx, VMgr), Stmt::ObjCMessageExprClass); - Eng.AddCheck(CreateAuditCFNumberCreate(Ctx, VMgr), Stmt::CallExprClass); + Eng.AddCheck(CreateAuditCFNumberCreate(Ctx, VMgr), + Stmt::CallExprClass); + + Eng.Register(CreateNSErrorCheck()); } diff --git a/lib/Analysis/BasicObjCFoundationChecks.h b/lib/Analysis/BasicObjCFoundationChecks.h index 24415419ca..3ad2ca57aa 100644 --- a/lib/Analysis/BasicObjCFoundationChecks.h +++ b/lib/Analysis/BasicObjCFoundationChecks.h @@ -29,14 +29,16 @@ namespace clang { class GRSimpleAPICheck; class ASTContext; class GRStateManager; +class BugType; GRSimpleAPICheck* CreateBasicObjCFoundationChecks(ASTContext& Ctx, GRStateManager* VMgr); GRSimpleAPICheck* CreateAuditCFNumberCreate(ASTContext& Ctx, GRStateManager* VMgr); - - + +BugType* CreateNSErrorCheck(); + } // end clang namespace #endif diff --git a/lib/Analysis/CheckNSError.cpp b/lib/Analysis/CheckNSError.cpp index 26386bc2bb..4c7f2cf25d 100644 --- a/lib/Analysis/CheckNSError.cpp +++ b/lib/Analysis/CheckNSError.cpp @@ -17,56 +17,90 @@ #include "clang/Analysis/LocalCheckers.h" #include "clang/Analysis/PathSensitive/BugReporter.h" +#include "clang/Analysis/PathSensitive/GRExprEngine.h" +#include "BasicObjCFoundationChecks.h" +#include "llvm/Support/Compiler.h" #include "clang/AST/DeclObjC.h" -#include "clang/AST/Type.h" -#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "llvm/ADT/SmallVector.h" using namespace clang; -void clang::CheckNSError(ObjCImplementationDecl* ID, BugReporter& BR) { - // Look at the @interface for this class. - ObjCInterfaceDecl* D = ID->getClassInterface(); +namespace { +class VISIBILITY_HIDDEN NSErrorCheck : public BugTypeCacheLocation { + + void EmitGRWarnings(GRBugReporter& BR); - // Get the ASTContext. Useful for querying type information. - ASTContext &Ctx = BR.getContext(); + void CheckSignature(ObjCMethodDecl& MD, QualType& ResultTy, + llvm::SmallVectorImpl& Params, + IdentifierInfo* NSErrorII); + + bool CheckArgument(QualType ArgTy, IdentifierInfo* NSErrorII); - // Get the IdentifierInfo* for "NSError". - IdentifierInfo* NSErrorII = &Ctx.Idents.get("NSError"); +public: + void EmitWarnings(BugReporter& BR) { EmitGRWarnings(cast(BR));} + const char* getName() const { return "NSError** null dereference"; } +}; + +} // end anonymous namespace - // Scan the methods. See if any of them have an argument of type NSError**. - for (ObjCInterfaceDecl::instmeth_iterator I=D->instmeth_begin(), - E=D->instmeth_end(); I!=E; ++I) { +BugType* clang::CreateNSErrorCheck() { + return new NSErrorCheck(); +} - // Get the method declaration. - ObjCMethodDecl* M = *I; - - // Check for a non-void return type. - if (M->getResultType() != Ctx.VoidTy) - continue; +void NSErrorCheck::EmitGRWarnings(GRBugReporter& BR) { + // Get the analysis engine and the exploded analysis graph. + GRExprEngine& Eng = BR.getEngine(); + GRExprEngine::GraphTy& G = Eng.getGraph(); + + // Get the declaration of the method/function that was analyzed. + Decl& CodeDecl = G.getCodeDecl(); + + ObjCMethodDecl* MD = dyn_cast(&CodeDecl); + if (!MD) + return; + + // Get the ASTContext, which is useful for querying type information. + ASTContext &Ctx = BR.getContext(); - for (ObjCMethodDecl::param_iterator PI=M->param_begin(), - PE=M->param_end(); PI!=PE; ++PI) { - - const PointerType* PPT = (*PI)->getType()->getAsPointerType(); - if (!PPT) continue; - - const PointerType* PT = PPT->getPointeeType()->getAsPointerType(); - if (!PT) continue; - - const ObjCInterfaceType *IT = - PT->getPointeeType()->getAsObjCInterfaceType(); - - if (!IT) continue; - - // Check if IT is "NSError". - if (IT->getDecl()->getIdentifier() == NSErrorII) { - // Documentation: "Creating and Returning NSError Objects" - BR.EmitBasicReport("Bad return type when passing NSError**", - "Method accepting NSError** argument should have " - "non-void return value to indicate that an error occurred.", - M->getLocStart()); - break; - } - } + QualType ResultTy; + llvm::SmallVector Params; + CheckSignature(*MD, ResultTy, Params, &Ctx.Idents.get("NSError")); + + if (Params.empty()) + return; + + if (ResultTy == Ctx.VoidTy) { + BR.EmitBasicReport("Bad return type when passing NSError**", + "Method accepting NSError** argument should have " + "non-void return value to indicate that an error occurred.", + CodeDecl.getLocation()); + } } + +void NSErrorCheck::CheckSignature(ObjCMethodDecl& M, QualType& ResultTy, + llvm::SmallVectorImpl& Params, + IdentifierInfo* NSErrorII) { + + ResultTy = M.getResultType(); + + for (ObjCMethodDecl::param_iterator I=M.param_begin(), + E=M.param_end(); I!=E; ++I) + if (CheckArgument((*I)->getType(), NSErrorII)) + Params.push_back(*I); +} + +bool NSErrorCheck::CheckArgument(QualType ArgTy, IdentifierInfo* NSErrorII) { + const PointerType* PPT = ArgTy->getAsPointerType(); + if (!PPT) return false; + + const PointerType* PT = PPT->getPointeeType()->getAsPointerType(); + if (!PT) return false; + + const ObjCInterfaceType *IT = + PT->getPointeeType()->getAsObjCInterfaceType(); + + if (!IT) return false; + return IT->getDecl()->getIdentifier() == NSErrorII; +}