pgousseau created this revision.
pgousseau added a subscriber: cfe-commits.
Dear All,
I would like to propose a patch for specifying function attributes in .model
files to be used by the analyzer checkers.
There are a number of checkers (such as UncheckedReturn) that contain hardcoded
list of functions, I would like to make these checkers available to a wider
number of functions by tagging the function definition with appropriate
attribute information. I have a situation where we cannot append the function
attributes to library header files; instead I propose a patch that allows us to
publish attribute information via .model files. The patch modifies the .model
file parser to allow for the extraction of function declarations found in the
.model file. The UncheckedReturn checker is modified to handle the
'warn_unused_result' attribute. Attributes obtained from non-model files are
still available to the analyzer, and the interface does not try to hide the
origin of the attribute.
Please can you review this for submission
Regards,
Pierre Gousseau
SN-Systems - Sony Computer Entertainment
http://reviews.llvm.org/D13731
Files:
include/clang/Analysis/AnalysisContext.h
include/clang/Analysis/CodeInjector.h
include/clang/StaticAnalyzer/Frontend/FrontendActions.h
include/clang/StaticAnalyzer/Frontend/ModelConsumer.h
lib/Analysis/AnalysisDeclContext.cpp
lib/Analysis/BodyFarm.cpp
lib/Analysis/BodyFarm.h
lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
lib/StaticAnalyzer/Frontend/FrontendActions.cpp
lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
lib/StaticAnalyzer/Frontend/ModelInjector.cpp
lib/StaticAnalyzer/Frontend/ModelInjector.h
test/Analysis/Inputs/Models/returnNeedsCheckHasModelFile.model
test/Analysis/unchecked-return.cpp
Index: test/Analysis/unchecked-return.cpp
===================================================================
--- /dev/null
+++ test/Analysis/unchecked-return.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -analyze -analyzer-checker=core,security.insecureAPI.UncheckedReturn -analyzer-config faux-bodies=true,model-path=%S/Inputs/Models %s -verify
+
+int returnNeedsCheckHasModelFile();
+
+int f0() {
+ returnNeedsCheckHasModelFile(); // expected-warning{{The return value from the call to 'returnNeedsCheckHasModelFile' is not checked.}}
+ return 0;
+}
Index: test/Analysis/Inputs/Models/returnNeedsCheckHasModelFile.model
===================================================================
--- /dev/null
+++ test/Analysis/Inputs/Models/returnNeedsCheckHasModelFile.model
@@ -0,0 +1 @@
+int returnNeedsCheckHasModelFile() __attribute__((warn_unused_result));
Index: lib/StaticAnalyzer/Frontend/ModelInjector.h
===================================================================
--- lib/StaticAnalyzer/Frontend/ModelInjector.h
+++ lib/StaticAnalyzer/Frontend/ModelInjector.h
@@ -45,6 +45,7 @@
ModelInjector(CompilerInstance &CI);
Stmt *getBody(const FunctionDecl *D) override;
Stmt *getBody(const ObjCMethodDecl *D) override;
+ FunctionDecl *getModelDecl(const FunctionDecl *D) override;
private:
/// \brief Synthesize a body for a declaration
@@ -67,6 +68,9 @@
// FIXME: double memoization is redundant, with memoization both here and in
// BodyFarm.
llvm::StringMap<Stmt *> Bodies;
+
+ // Store the model's function declaration if provided.
+ llvm::StringMap<FunctionDecl *> Decls;
};
}
}
Index: lib/StaticAnalyzer/Frontend/ModelInjector.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/ModelInjector.cpp
+++ lib/StaticAnalyzer/Frontend/ModelInjector.cpp
@@ -37,6 +37,11 @@
return Bodies[D->getName()];
}
+FunctionDecl *ModelInjector::getModelDecl(const FunctionDecl *D) {
+ onBodySynthesis(D);
+ return Decls[D->getName()];
+}
+
void ModelInjector::onBodySynthesis(const NamedDecl *D) {
// FIXME: what about overloads? Declarations can be used as keys but what
@@ -60,6 +65,7 @@
if (!llvm::sys::fs::exists(fileName.str())) {
Bodies[D->getName()] = nullptr;
+ Decls[D->getName()] = nullptr;
return;
}
@@ -95,7 +101,7 @@
Instance.getPreprocessor().InitializeForModelFile();
- ParseModelFileAction parseModelFile(Bodies);
+ ParseModelFileAction parseModelFile(Bodies, Decls);
const unsigned ThreadStackSize = 8 << 20;
llvm::CrashRecoveryContext CRC;
Index: lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
@@ -26,16 +26,20 @@
using namespace clang;
using namespace ento;
-ModelConsumer::ModelConsumer(llvm::StringMap<Stmt *> &Bodies)
- : Bodies(Bodies) {}
+ModelConsumer::ModelConsumer(llvm::StringMap<Stmt *> &Bodies,
+ llvm::StringMap<FunctionDecl *> &Decls)
+ : Bodies(Bodies), Decls(Decls) {}
bool ModelConsumer::HandleTopLevelDecl(DeclGroupRef D) {
for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I) {
- // Only interested in definitions.
- const FunctionDecl *func = llvm::dyn_cast<FunctionDecl>(*I);
- if (func && func->hasBody()) {
- Bodies.insert(std::make_pair(func->getName(), func->getBody()));
+ // We are interested in definitions and declarations.
+ FunctionDecl *func = llvm::dyn_cast<FunctionDecl>(*I);
+ if (func) {
+ Decls.insert(std::make_pair(func->getName(), func));
+ if (func->hasBody()) {
+ Bodies.insert(std::make_pair(func->getName(), func->getBody()));
+ }
}
}
return true;
Index: lib/StaticAnalyzer/Frontend/FrontendActions.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/FrontendActions.cpp
+++ lib/StaticAnalyzer/Frontend/FrontendActions.cpp
@@ -18,11 +18,12 @@
return CreateAnalysisConsumer(CI);
}
-ParseModelFileAction::ParseModelFileAction(llvm::StringMap<Stmt *> &Bodies)
- : Bodies(Bodies) {}
+ParseModelFileAction::ParseModelFileAction(llvm::StringMap<Stmt *> &Bodies,
+ llvm::StringMap<FunctionDecl *> &Decls)
+ : Bodies(Bodies), Decls(Decls) {}
std::unique_ptr<ASTConsumer>
ParseModelFileAction::CreateASTConsumer(CompilerInstance &CI,
StringRef InFile) {
- return llvm::make_unique<ModelConsumer>(Bodies);
+ return llvm::make_unique<ModelConsumer>(Bodies, Decls);
}
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#include "ClangSACheckers.h"
+#include "clang/AST/Attr.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/Analysis/AnalysisContext.h"
#include "clang/Basic/TargetInfo.h"
@@ -101,6 +102,9 @@
void checkCall_random(const CallExpr *CE, const FunctionDecl *FD);
void checkCall_vfork(const CallExpr *CE, const FunctionDecl *FD);
void checkUncheckedReturnValue(CallExpr *CE);
+
+ bool IsAccessRightsAPI(const FunctionDecl *FD);
+ bool IsShouldCheckReturnAPI(const FunctionDecl *FD);
};
} // end anonymous namespace
@@ -689,6 +693,30 @@
if (!FD)
return;
+ bool isAccessRightsAPI = IsAccessRightsAPI(FD);
+ if (!isAccessRightsAPI && !IsShouldCheckReturnAPI(FD))
+ return;
+
+ // Issue a warning.
+ SmallString<256> buf1;
+ llvm::raw_svector_ostream os1(buf1);
+ os1 << "Return value is not checked in call to '" << *FD << '\'';
+
+ SmallString<256> buf2;
+ llvm::raw_svector_ostream os2(buf2);
+ os2 << "The return value from the call to '" << *FD << "' is not checked. ";
+ if (isAccessRightsAPI)
+ os2 << " If an error occurs in '" << *FD
+ << "', the following code may execute with unexpected privileges";
+
+ PathDiagnosticLocation CELoc =
+ PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+ BR.EmitBasicReport(AC->getDecl(), filter.checkName_UncheckedReturn, os1.str(),
+ "Security", os2.str(), CELoc,
+ CE->getCallee()->getSourceRange());
+}
+
+bool WalkAST::IsAccessRightsAPI(const FunctionDecl *FD) {
if (II_setid[0] == nullptr) {
static const char * const identifiers[num_setids] = {
"setuid", "setgid", "seteuid", "setegid",
@@ -707,38 +735,36 @@
break;
if (identifierid >= num_setids)
- return;
+ return false;
const FunctionProtoType *FTP = FD->getType()->getAs<FunctionProtoType>();
if (!FTP)
- return;
+ return false;
// Verify that the function takes one or two arguments (depending on
// the function).
if (FTP->getNumParams() != (identifierid < 4 ? 1 : 2))
- return;
+ return false;
// The arguments must be integers.
for (unsigned i = 0; i < FTP->getNumParams(); i++)
if (!FTP->getParamType(i)->isIntegralOrUnscopedEnumerationType())
- return;
+ return false;
- // Issue a warning.
- SmallString<256> buf1;
- llvm::raw_svector_ostream os1(buf1);
- os1 << "Return value is not checked in call to '" << *FD << '\'';
+ return true;
+}
- SmallString<256> buf2;
- llvm::raw_svector_ostream os2(buf2);
- os2 << "The return value from the call to '" << *FD
- << "' is not checked. If an error occurs in '" << *FD
- << "', the following code may execute with unexpected privileges";
+bool WalkAST::IsShouldCheckReturnAPI(const FunctionDecl *FD) {
- PathDiagnosticLocation CELoc =
- PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
- BR.EmitBasicReport(AC->getDecl(), filter.checkName_UncheckedReturn, os1.str(),
- "Security", os2.str(), CELoc,
- CE->getCallee()->getSourceRange());
+ // Check if the model has a 'warn_unused_result' attribute.
+ AnalysisDeclContextManager *Mgr = AC->getManager();
+ AnalysisDeclContext *FnAC = Mgr->getContext(FD);
+ Optional<AttrVec> Attrs = FnAC->getModelAttrs();
+
+ if (Attrs.hasValue() && hasSpecificAttr<WarnUnusedResultAttr>(*Attrs)) {
+ return true;
+ }
+ return false;
}
//===----------------------------------------------------------------------===//
Index: lib/Analysis/BodyFarm.h
===================================================================
--- lib/Analysis/BodyFarm.h
+++ lib/Analysis/BodyFarm.h
@@ -39,11 +39,16 @@
/// Factory method for creating bodies for Objective-C properties.
Stmt *getBody(const ObjCMethodDecl *D);
+ /// Returns the model's declaration.
+ FunctionDecl *getModelDecl(const FunctionDecl *D);
+
private:
typedef llvm::DenseMap<const Decl *, Optional<Stmt *> > BodyMap;
+ typedef llvm::DenseMap<const Decl *, Optional<FunctionDecl *> > DeclsMap;
ASTContext &C;
BodyMap Bodies;
+ DeclsMap Decls;
CodeInjector *Injector;
};
}
Index: lib/Analysis/BodyFarm.cpp
===================================================================
--- lib/Analysis/BodyFarm.cpp
+++ lib/Analysis/BodyFarm.cpp
@@ -386,6 +386,26 @@
return Val.getValue();
}
+FunctionDecl *BodyFarm::getModelDecl(const FunctionDecl *D) {
+ D = D->getCanonicalDecl();
+
+ Optional<FunctionDecl *> &Val = Decls[D];
+ if (Val.hasValue())
+ return Val.getValue();
+
+ Val = nullptr;
+
+ if (D->getIdentifier() == nullptr)
+ return nullptr;
+
+ StringRef Name = D->getName();
+ if (Name.empty())
+ return nullptr;
+
+ if (Injector) { Val = Injector->getModelDecl(D); }
+ return Val.getValue();
+}
+
static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
const ObjCPropertyDecl *Prop) {
// First, find the backing ivar.
Index: lib/Analysis/AnalysisDeclContext.cpp
===================================================================
--- lib/Analysis/AnalysisDeclContext.cpp
+++ lib/Analysis/AnalysisDeclContext.cpp
@@ -123,6 +123,20 @@
return getBody(Tmp);
}
+Optional<AttrVec> AnalysisDeclContext::getModelAttrs() const {
+
+ Optional<AttrVec> Val;
+ if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
+ if (Manager && Manager->synthesizeBodies()) {
+ Decl *ModelDecl = getBodyFarm(getASTContext(), Manager->Injector.get())
+ .getModelDecl(FD);
+ if (ModelDecl && ModelDecl->hasAttrs())
+ Val = ModelDecl->getAttrs();
+ }
+ }
+ return Val;
+}
+
bool AnalysisDeclContext::isBodyAutosynthesized() const {
bool Tmp;
getBody(Tmp);
Index: include/clang/StaticAnalyzer/Frontend/ModelConsumer.h
===================================================================
--- include/clang/StaticAnalyzer/Frontend/ModelConsumer.h
+++ include/clang/StaticAnalyzer/Frontend/ModelConsumer.h
@@ -31,12 +31,15 @@
/// from a model file.
class ModelConsumer : public ASTConsumer {
public:
- ModelConsumer(llvm::StringMap<Stmt *> &Bodies);
+ ModelConsumer(llvm::StringMap<Stmt *> &Bodies,
+ llvm::StringMap<FunctionDecl *> &Decls);
bool HandleTopLevelDecl(DeclGroupRef D) override;
private:
llvm::StringMap<Stmt *> &Bodies;
+ /// Store the models' declarations.
+ llvm::StringMap<FunctionDecl *> &Decls;
};
}
}
Index: include/clang/StaticAnalyzer/Frontend/FrontendActions.h
===================================================================
--- include/clang/StaticAnalyzer/Frontend/FrontendActions.h
+++ include/clang/StaticAnalyzer/Frontend/FrontendActions.h
@@ -40,15 +40,18 @@
/// parsed, the function definitions will be collected into a StringMap.
class ParseModelFileAction : public ASTFrontendAction {
public:
- ParseModelFileAction(llvm::StringMap<Stmt *> &Bodies);
+ ParseModelFileAction(llvm::StringMap<Stmt *> &Bodies,
+ llvm::StringMap<FunctionDecl *> &Decls);
bool isModelParsingAction() const override { return true; }
protected:
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
StringRef InFile) override;
private:
llvm::StringMap<Stmt *> &Bodies;
+ /// Stores the models' declarations.
+ llvm::StringMap<FunctionDecl *> &Decls;
};
void printCheckerHelp(raw_ostream &OS, ArrayRef<std::string> plugins);
Index: include/clang/Analysis/CodeInjector.h
===================================================================
--- include/clang/Analysis/CodeInjector.h
+++ include/clang/Analysis/CodeInjector.h
@@ -40,6 +40,8 @@
virtual Stmt *getBody(const FunctionDecl *D) = 0;
virtual Stmt *getBody(const ObjCMethodDecl *D) = 0;
+ /// \brief Returns the model's declaration.
+ virtual FunctionDecl *getModelDecl(const FunctionDecl *D) = 0;
};
}
Index: include/clang/Analysis/AnalysisContext.h
===================================================================
--- include/clang/Analysis/AnalysisContext.h
+++ include/clang/Analysis/AnalysisContext.h
@@ -137,6 +137,9 @@
/// by the BodyFarm.
Stmt *getBody(bool &IsAutosynthesized) const;
+ /// \brief Get the attributes from the model.
+ Optional<AttrVec> getModelAttrs() const;
+
/// \brief Checks if the body of the Decl is generated by the BodyFarm.
///
/// Note, the lookup is not free. We are going to call getBody behind
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
