[cfe-commits] r56938 - in /cfe/trunk: lib/Analysis/CheckNSError.cpp test/Analysis/CheckNSError.m
Ted Kremenek
kremenek at apple.com
Wed Oct 1 16:24:09 PDT 2008
Author: kremenek
Date: Wed Oct 1 18:24:09 2008
New Revision: 56938
URL: http://llvm.org/viewvc/llvm-project?rev=56938&view=rev
Log:
Enhance NSError** checking with analogous checking for CFErrorRef*.
Expand checking to include functions, not just methods.
Modified:
cfe/trunk/lib/Analysis/CheckNSError.cpp
cfe/trunk/test/Analysis/CheckNSError.m
Modified: cfe/trunk/lib/Analysis/CheckNSError.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CheckNSError.cpp?rev=56938&r1=56937&r2=56938&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CheckNSError.cpp (original)
+++ cfe/trunk/lib/Analysis/CheckNSError.cpp Wed Oct 1 18:24:09 2008
@@ -32,20 +32,32 @@
void EmitGRWarnings(GRBugReporter& BR);
void CheckSignature(ObjCMethodDecl& MD, QualType& ResultTy,
- llvm::SmallVectorImpl<VarDecl*>& Params,
- IdentifierInfo* NSErrorII);
+ llvm::SmallVectorImpl<VarDecl*>& NSErrorParams,
+ llvm::SmallVectorImpl<VarDecl*>& CFErrorParams,
+ IdentifierInfo* NSErrorII,
+ IdentifierInfo* CFErrorII);
+
+ void CheckSignature(FunctionDecl& MD, QualType& ResultTy,
+ llvm::SmallVectorImpl<VarDecl*>& NSErrorParams,
+ llvm::SmallVectorImpl<VarDecl*>& CFErrorParams,
+ IdentifierInfo* NSErrorII,
+ IdentifierInfo* CFErrorII);
- bool CheckArgument(QualType ArgTy, IdentifierInfo* NSErrorII);
+ bool CheckNSErrorArgument(QualType ArgTy, IdentifierInfo* NSErrorII);
+ bool CheckCFErrorArgument(QualType ArgTy, IdentifierInfo* CFErrorII);
void CheckParamDeref(VarDecl* V, GRStateRef state, GRExprEngine& Eng,
- GRBugReporter& BR);
+ GRBugReporter& BR, bool isNErrorWarning);
+
+ void EmitRetTyWarning(BugReporter& BR, Decl& CodeDecl, bool isNSErrorWarning);
const char* desc;
+ const char* name;
public:
NSErrorCheck() : desc(0) {}
void EmitWarnings(BugReporter& BR) { EmitGRWarnings(cast<GRBugReporter>(BR));}
- const char* getName() const { return "NSError** null dereference"; }
+ const char* getName() const { return name; }
const char* getDescription() const { return desc; }
const char* getCategory() const { return "Coding Conventions (Apple)"; }
};
@@ -63,52 +75,118 @@
// Get the declaration of the method/function that was analyzed.
Decl& CodeDecl = G.getCodeDecl();
-
- ObjCMethodDecl* MD = dyn_cast<ObjCMethodDecl>(&CodeDecl);
- if (!MD)
- return;
-
+
// Get the ASTContext, which is useful for querying type information.
ASTContext &Ctx = BR.getContext();
QualType ResultTy;
- llvm::SmallVector<VarDecl*, 5> Params;
- CheckSignature(*MD, ResultTy, Params, &Ctx.Idents.get("NSError"));
+ llvm::SmallVector<VarDecl*, 5> NSErrorParams;
+ llvm::SmallVector<VarDecl*, 5> CFErrorParams;
+
+ if (ObjCMethodDecl* MD = dyn_cast<ObjCMethodDecl>(&CodeDecl))
+ CheckSignature(*MD, ResultTy, NSErrorParams, CFErrorParams,
+ &Ctx.Idents.get("NSError"), &Ctx.Idents.get("CFErrorRef"));
+ else if (FunctionDecl* FD = dyn_cast<FunctionDecl>(&CodeDecl))
+ CheckSignature(*FD, ResultTy, NSErrorParams, CFErrorParams,
+ &Ctx.Idents.get("NSError"), &Ctx.Idents.get("CFErrorRef"));
+ else
+ return;
- if (Params.empty())
+ if (NSErrorParams.empty() && CFErrorParams.empty())
return;
- if (ResultTy == Ctx.VoidTy) {
- BR.EmitBasicReport("Bad return type when passing NSError**",
- getCategory(),
- "Method accepting NSError** argument should have "
- "non-void return value to indicate that an error occurred.",
- CodeDecl.getLocation());
-
+ if (ResultTy == Ctx.VoidTy) {
+ if (!NSErrorParams.empty())
+ EmitRetTyWarning(BR, CodeDecl, true);
+ if (!CFErrorParams.empty())
+ EmitRetTyWarning(BR, CodeDecl, false);
}
- // Scan the NSError** parameters for an implicit null dereference.
- for (llvm::SmallVectorImpl<VarDecl*>::iterator I=Params.begin(),
- E=Params.end(); I!=E; ++I)
- for (GRExprEngine::GraphTy::roots_iterator RI=G.roots_begin(),
- RE=G.roots_end(); RI!=RE; ++RI)
+ for (GRExprEngine::GraphTy::roots_iterator RI=G.roots_begin(),
+ RE=G.roots_end(); RI!=RE; ++RI) {
+
+ // Scan the NSError** parameters for an implicit null dereference.
+ for (llvm::SmallVectorImpl<VarDecl*>::iterator I=NSErrorParams.begin(),
+ E=NSErrorParams.end(); I!=E; ++I)
+ CheckParamDeref(*I, GRStateRef((*RI)->getState(), Eng.getStateManager()),
+ Eng, BR, true);
+
+ // Scan the CFErrorRef* parameters for an implicit null dereference.
+ for (llvm::SmallVectorImpl<VarDecl*>::iterator I=CFErrorParams.begin(),
+ E=CFErrorParams.end(); I!=E; ++I)
CheckParamDeref(*I, GRStateRef((*RI)->getState(), Eng.getStateManager()),
- Eng, BR);
+ Eng, BR, false);
+ }
+}
+
+void NSErrorCheck::EmitRetTyWarning(BugReporter& BR, Decl& CodeDecl,
+ bool isNSErrorWarning) {
+
+ std::string msg;
+ llvm::raw_string_ostream os(msg);
+
+ if (isa<ObjCMethodDecl>(CodeDecl))
+ os << "Method";
+ else
+ os << "Function";
+
+ os << " accepting ";
+ os << (isNSErrorWarning ? "NSError**" : "CFErrorRef*");
+ os << " should have a non-void return value to indicate whether or not an "
+ "error occured.";
+
+ BR.EmitBasicReport(isNSErrorWarning
+ ? "Bad return type when passing NSError**"
+ : "Bad return type when passing CFError*",
+ getCategory(), os.str().c_str(), CodeDecl.getLocation());
}
-void NSErrorCheck::CheckSignature(ObjCMethodDecl& M, QualType& ResultTy,
- llvm::SmallVectorImpl<VarDecl*>& Params,
- IdentifierInfo* NSErrorII) {
+void
+NSErrorCheck::CheckSignature(ObjCMethodDecl& M, QualType& ResultTy,
+ llvm::SmallVectorImpl<VarDecl*>& NSErrorParams,
+ llvm::SmallVectorImpl<VarDecl*>& CFErrorParams,
+ IdentifierInfo* NSErrorII,
+ IdentifierInfo* CFErrorII) {
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);
+ E=M.param_end(); I!=E; ++I) {
+
+ QualType T = (*I)->getType();
+
+ if (CheckNSErrorArgument(T, NSErrorII))
+ NSErrorParams.push_back(*I);
+ else if (CheckCFErrorArgument(T, CFErrorII))
+ CFErrorParams.push_back(*I);
+ }
+}
+
+void
+NSErrorCheck::CheckSignature(FunctionDecl& F, QualType& ResultTy,
+ llvm::SmallVectorImpl<VarDecl*>& NSErrorParams,
+ llvm::SmallVectorImpl<VarDecl*>& CFErrorParams,
+ IdentifierInfo* NSErrorII,
+ IdentifierInfo* CFErrorII) {
+
+ ResultTy = F.getResultType();
+
+ for (FunctionDecl::param_iterator I=F.param_begin(),
+ E=F.param_end(); I!=E; ++I) {
+
+ QualType T = (*I)->getType();
+
+ if (CheckNSErrorArgument(T, NSErrorII))
+ NSErrorParams.push_back(*I);
+ else if (CheckCFErrorArgument(T, CFErrorII))
+ CFErrorParams.push_back(*I);
+ }
}
-bool NSErrorCheck::CheckArgument(QualType ArgTy, IdentifierInfo* NSErrorII) {
+
+bool NSErrorCheck::CheckNSErrorArgument(QualType ArgTy,
+ IdentifierInfo* NSErrorII) {
+
const PointerType* PPT = ArgTy->getAsPointerType();
if (!PPT) return false;
@@ -122,8 +200,21 @@
return IT->getDecl()->getIdentifier() == NSErrorII;
}
+bool NSErrorCheck::CheckCFErrorArgument(QualType ArgTy,
+ IdentifierInfo* CFErrorII) {
+
+ const PointerType* PPT = ArgTy->getAsPointerType();
+ if (!PPT) return false;
+
+ const TypedefType* TT = PPT->getPointeeType()->getAsTypedefType();
+ if (!TT) return false;
+
+ return TT->getDecl()->getIdentifier() == CFErrorII;
+}
+
void NSErrorCheck::CheckParamDeref(VarDecl* Param, GRStateRef rootState,
- GRExprEngine& Eng, GRBugReporter& BR) {
+ GRExprEngine& Eng, GRBugReporter& BR,
+ bool isNSErrorWarning) {
RVal ParamRVal = rootState.GetRVal(lval::DeclVal(Param));
@@ -143,15 +234,23 @@
// Emit an error.
BugReport R(*this, *I);
+
+ name = isNSErrorWarning ? "NSError** null dereference"
+ : "CFErrorRef* null dereference";
std::string msg;
llvm::raw_string_ostream os(msg);
- os << "Potential null dereference. According to coding standards in "
- "'Creating and Returning NSError Objects' the parameter '"
- << Param->getName() << "' may be null.";
+ os << "Potential null dereference. According to coding standards ";
+
+ if (isNSErrorWarning)
+ os << "in 'Creating and Returning NSError Objects' the parameter '";
+ else
+ os << "documented in CoreFoundation/CFError.h the parameter '";
+
+ os << Param->getName() << "' may be null.";
desc = os.str().c_str();
BR.addNotableSymbol(SV->getSymbol());
- BR.EmitWarning(R);
+ BR.EmitWarning(R);
}
}
Modified: cfe/trunk/test/Analysis/CheckNSError.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/CheckNSError.m?rev=56938&r1=56937&r2=56938&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/CheckNSError.m (original)
+++ cfe/trunk/test/Analysis/CheckNSError.m Wed Oct 1 18:24:09 2008
@@ -20,7 +20,7 @@
@end
@implementation A
-- (void)myMethodWhichMayFail:(NSError **)error { // expected-warning: {{Method accepting NSError** argument should have non-void return value to indicate that an error occurred.}}
+- (void)myMethodWhichMayFail:(NSError **)error { // expected-warning: {{Method accepting NSError** should have a non-void return value to indicate whether or not an error occured.}}
*error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // expected-warning: {{Potential null dereference.}}
}
@@ -29,3 +29,15 @@
return 0;
}
@end
+
+struct __CFError {};
+typedef struct __CFError* CFErrorRef;
+
+void foo(CFErrorRef* error) { // expected-warning{{Function accepting CFErrorRef* should have a non-void return value to indicate whether or not an error occured.}}
+ *error = 0; // expected-warning{{Potential null dereference.}}
+}
+
+int bar(CFErrorRef* error) {
+ if (error) *error = 0;
+ return 0;
+}
More information about the cfe-commits
mailing list