pgousseau updated this revision to Diff 37765. pgousseau added a comment. Following Gabor's review:
Remove changes to UncheckedReturn checker. Add a test using the NonNullParamChecker checker. Abstract the origin of the attributes. An analyzer option "faux-attributes" is added. This is 'false' by default and meant to prevent unnecessary parsing of model files. http://reviews.llvm.org/D13731 Files: include/clang/Analysis/AnalysisContext.h include/clang/Analysis/CodeInjector.h include/clang/StaticAnalyzer/Core/AnalyzerOptions.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/Core/AnalysisManager.cpp lib/StaticAnalyzer/Core/AnalyzerOptions.cpp lib/StaticAnalyzer/Frontend/AnalysisConsumer.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/modelFileHasAttributes.model test/Analysis/analyzer-config.c test/Analysis/analyzer-config.cpp test/Analysis/model-attributes.cpp
Index: test/Analysis/model-attributes.cpp =================================================================== --- /dev/null +++ test/Analysis/model-attributes.cpp @@ -0,0 +1,23 @@ +// This is testing the 'faux-attributes' analyzer option. +// The declaration of 'modelFileHasAttributes' found in modelFileHasAttributes.model has 'nonnull' attributes on the 2nd and 3rd parameter. +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -analyze -analyzer-checker=core -analyzer-config faux-attributes=true,model-path=%S/Inputs/Models %s -verify + +int modelFileHasAttributes(int * p0, int * p1, int * p2); + +int f0(int * x, int * y) { + int * p = 0; + modelFileHasAttributes(p, x, y); // no-warning + return 0; +} + +int f1(int * x, int * y) { + int * p = 0; + modelFileHasAttributes(x, p, y); // expected-warning{{Null pointer passed as an argument to a 'nonnull' parameter}} + return 0; +} + +int f2(int * x, int * y) { + int * p = 0; + modelFileHasAttributes(x, y, p); // expected-warning{{Null pointer passed as an argument to a 'nonnull' parameter}} + return 0; +} Index: test/Analysis/analyzer-config.cpp =================================================================== --- test/Analysis/analyzer-config.cpp +++ test/Analysis/analyzer-config.cpp @@ -24,6 +24,7 @@ // CHECK-NEXT: c++-template-inlining = true // CHECK-NEXT: cfg-conditional-static-initializers = true // CHECK-NEXT: cfg-temporary-dtors = false +// CHECK-NEXT: faux-attributes = false // CHECK-NEXT: faux-bodies = true // CHECK-NEXT: graph-trim-interval = 1000 // CHECK-NEXT: inline-lambdas = true @@ -37,4 +38,4 @@ // CHECK-NEXT: mode = deep // CHECK-NEXT: region-store-small-struct-limit = 2 // CHECK-NEXT: [stats] -// CHECK-NEXT: num-entries = 19 +// CHECK-NEXT: num-entries = 20 Index: test/Analysis/analyzer-config.c =================================================================== --- test/Analysis/analyzer-config.c +++ test/Analysis/analyzer-config.c @@ -13,6 +13,7 @@ // CHECK: [config] // CHECK-NEXT: cfg-conditional-static-initializers = true // CHECK-NEXT: cfg-temporary-dtors = false +// CHECK-NEXT: faux-attributes = false // CHECK-NEXT: faux-bodies = true // CHECK-NEXT: graph-trim-interval = 1000 // CHECK-NEXT: inline-lambdas = true @@ -26,5 +27,5 @@ // CHECK-NEXT: mode = deep // CHECK-NEXT: region-store-small-struct-limit = 2 // CHECK-NEXT: [stats] -// CHECK-NEXT: num-entries = 14 +// CHECK-NEXT: num-entries = 15 Index: test/Analysis/Inputs/Models/modelFileHasAttributes.model =================================================================== --- /dev/null +++ test/Analysis/Inputs/Models/modelFileHasAttributes.model @@ -0,0 +1 @@ +int modelFileHasAttributes(int * p0, int * p1, int * p2 __attribute__((nonnull))) __attribute__((nonnull(2))); \ No newline at end of file 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/Frontend/AnalysisConsumer.cpp =================================================================== --- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -500,6 +500,25 @@ } } +namespace { +// AST visitor used to merge model attributes. +class WalkAST : public DataRecursiveASTVisitor<WalkAST> { + AnalysisDeclContextManager &Mgr; + +public: + WalkAST(AnalysisDeclContextManager &Mgr) : Mgr(Mgr) {} + + // Merge model attributes for all call expressions. + bool VisitCallExpr(CallExpr *CE) { + if (FunctionDecl *FD = CE->getDirectCallee()) { + AnalysisDeclContext *ADC = Mgr.getContext(FD); + ADC->MergeModelAttributes(); + } + return true; + } +}; +} // end anonymous namespace + void AnalysisConsumer::HandleTranslationUnit(ASTContext &C) { // Don't run the actions if an error has occurred with parsing the file. DiagnosticsEngine &Diags = PP.getDiagnostics(); @@ -517,6 +536,14 @@ // Introduce a scope to destroy BR before Mgr. BugReporter BR(*Mgr); TranslationUnitDecl *TU = C.getTranslationUnitDecl(); + + // If "faux-attributes=true" is given, merge model's attributes. + AnalysisDeclContextManager &ADCMgr = Mgr->getAnalysisDeclContextManager(); + if (ADCMgr.mergeModelAttributes()) { + WalkAST W(ADCMgr); + W.TraverseDecl(TU); + } + checkerMgr->runCheckersOnASTDecl(TU, *Mgr, BR); // Run the AST-only checks using the order in which functions are defined. Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp =================================================================== --- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp +++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp @@ -325,6 +325,10 @@ return getBooleanOption("faux-bodies", true); } +bool AnalyzerOptions::shouldMergeModelAttributes() { + return getBooleanOption("faux-attributes", false); +} + bool AnalyzerOptions::shouldPrunePaths() { return getBooleanOption("prune-paths", true); } Index: lib/StaticAnalyzer/Core/AnalysisManager.cpp =================================================================== --- lib/StaticAnalyzer/Core/AnalysisManager.cpp +++ lib/StaticAnalyzer/Core/AnalysisManager.cpp @@ -27,6 +27,7 @@ /*AddInitializers=*/true, Options.includeTemporaryDtorsInCFG(), Options.shouldSynthesizeBodies(), + Options.shouldMergeModelAttributes(), Options.shouldConditionalizeStaticInitializers(), /*addCXXNewAllocator=*/true, injector), Index: lib/Analysis/BodyFarm.h =================================================================== --- lib/Analysis/BodyFarm.h +++ lib/Analysis/BodyFarm.h @@ -39,12 +39,22 @@ /// Factory method for creating bodies for Objective-C properties. Stmt *getBody(const ObjCMethodDecl *D); + /// \brief Merge the attributes found in model files. + /// Note, this adds all attributes found in the model file without any sanity + /// checks. + void MergeModelAttr(const FunctionDecl *FD); + 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; + + /// Returns the model's declaration. + FunctionDecl *getModelDecl(const FunctionDecl *D); }; } Index: lib/Analysis/BodyFarm.cpp =================================================================== --- lib/Analysis/BodyFarm.cpp +++ lib/Analysis/BodyFarm.cpp @@ -386,6 +386,64 @@ return Val.getValue(); } +void BodyFarm::MergeModelAttr(const FunctionDecl *FD) { + // Merge attributes attached to the function declaration. + Decl *ModelDecl = getModelDecl(FD); + if (!ModelDecl) + return; + FunctionDecl * ModelFD = cast<FunctionDecl>(ModelDecl); + Optional<AttrVec> ModelFDAttrs; + if (ModelFD->hasAttrs()) + ModelFDAttrs = ModelFD->getAttrs(); + FunctionDecl *CastFD = const_cast<FunctionDecl*>(FD); + if (ModelFDAttrs.hasValue()) { + AttrVec& AV = *ModelFDAttrs; + AttrVec::iterator i = AV.begin(), e = AV.end(); + for (; i != e; ++i) { + CastFD->addAttr(*i); + } + } + // Merge attributes attached to the function parameters. + if (!ModelFD) + return; + unsigned int NumParam = CastFD->getNumParams(); + if (NumParam != ModelFD->getNumParams()) + return; + for (unsigned k = 0; k < NumParam; k++) { + Decl * ModelParamDecl = ModelFD->getParamDecl(k); + if (!ModelParamDecl || (ModelParamDecl && !ModelParamDecl->hasAttrs())) + continue; + Decl * ParamDecl = CastFD->getParamDecl(k); + if (!ParamDecl) + continue; + AttrVec& AV = ModelParamDecl->getAttrs(); + AttrVec::iterator i = AV.begin(), e = AV.end(); + for (; i != e; ++i) { + ParamDecl->addAttr(*i); + } + } +} + +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 @@ -68,10 +68,13 @@ bool addInitializers, bool addTemporaryDtors, bool synthesizeBodies, + bool mergeModelAttrs, bool addStaticInitBranch, bool addCXXNewAllocator, CodeInjector *injector) - : Injector(injector), SynthesizeBodies(synthesizeBodies) + : Injector(injector), + SynthesizeBodies(synthesizeBodies), + MergeModelAttrs(mergeModelAttrs) { cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG; cfgBuildOptions.AddImplicitDtors = addImplicitDtors; @@ -135,6 +138,13 @@ return Tmp && Body->getLocStart().isValid(); } +void AnalysisDeclContext::MergeModelAttributes() { + if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) { + if (Manager) { + getBodyFarm(getASTContext(), Manager->Injector.get()).MergeModelAttr(FD); + } + } +} const ImplicitParamDecl *AnalysisDeclContext::getSelfDecl() const { if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) 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/StaticAnalyzer/Core/AnalyzerOptions.h =================================================================== --- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -496,6 +496,10 @@ /// for well-known functions. bool shouldSynthesizeBodies(); + /// Return true if attributes of functions' declarations taken from model + /// files should be merged with existing ones. + bool shouldMergeModelAttributes(); + /// Returns how often nodes in the ExplodedGraph should be recycled to save /// memory. /// 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 @@ -152,6 +152,11 @@ /// \sa getBody bool isBodyAutosynthesizedFromModelFile() const; + /// \brief Merge the attributes found in model files. + /// Note, this adds all attributes found in the model file without any sanity + /// checks. + void MergeModelAttributes(); + CFG *getCFG(); CFGStmtMap *getCFGStmtMap(); @@ -416,12 +421,16 @@ /// for well-known functions. bool SynthesizeBodies; + /// Flag to indicate whether or not model attributes should be merged. + bool MergeModelAttrs; + public: AnalysisDeclContextManager(bool useUnoptimizedCFG = false, bool addImplicitDtors = false, bool addInitializers = false, bool addTemporaryDtors = false, bool synthesizeBodies = false, + bool mergeModelAttrs = false, bool addStaticInitBranches = false, bool addCXXNewAllocator = true, CodeInjector* injector = nullptr); @@ -442,6 +451,10 @@ /// functions. bool synthesizeBodies() const { return SynthesizeBodies; } + /// Return true if attributes of functions' declarations taken from model + /// files should be merged with existing ones. + bool mergeModelAttributes() const { return MergeModelAttrs; } + const StackFrameContext *getStackFrame(AnalysisDeclContext *Ctx, LocationContext const *Parent, const Stmt *S,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits