pgousseau updated this revision to Diff 37466.
pgousseau added a comment.
Fix incorrect caching of model files declarations.
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,11 +37,16 @@
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
// about file name index? Mangled names may not be suitable for that either.
- if (Bodies.count(D->getName()) != 0)
+ if (Bodies.count(D->getName()) != 0 || Decls.count(D->getName()) != 0)
return;
SourceManager &SM = CI.getSourceManager();
@@ -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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
