[Analyzer] Supporting function attributes in .model files.

[pgousseau]

Summary

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 nullability checkers) that rely on attributes, 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. Attributes are merged by reusing Sema's merging capabilities.
A new option 'faux-attributes' is added to enable this feature. 'faux-attributes' is disabled by default.

Please can you review this for submission

Regards,

Pierre Gousseau
SN-Systems - Sony Computer Entertainment

[pgousseau]

Fix incorrect caching of model files declarations.

[xazax.hun]

Hi!

I have some high level questions and notes about this patch.

I implemented the function modelling as a Google Summer of Code project and Ted Kremenek was my mentor. I am happy that you found an useful application of the function modeling system, and I think, in general, it is a good idea to be able to store attributes for the modelled functions. However I am a bit surprised, when I saw this patch. The main reason is that, models lack a lot of functionality right now.

Main missing features:

• Ability to specify multiple model paths similar to how header include paths are specified.
• Support for methods, namespaces, overloaded functions.

Did you find the feature useful despite those limitations? I am interested to improve the situation in the future, unfortunately I find very little time to work on this area recently, but I do welcome every changes.

I have two comments before I start to review the patch itself. Right now this patch contains two modifications. One for the .model files and one for a checker. I think it would be better to separate these two changes into two separate patches. If the motivation behind the merge of those patches is that, you want to test a feature you implemented in .model files, than I think there are other checker that are using attributes, so I think you should be able to provide a test case with the two changes separated. (For example using the nonnull parameter checker.)

The other comment is that, the .model files are intended to work in a way, that checkers should not be able utilize the information whether some data is coming from a model file or the analyzed translation unit. In fact, this is an abstraction, that makes it possible to develop the checkers and the modelling mechanism independently. With you patch, the checker should observe the model declaration explicitly. I think, a better design would somehow merge the attributes that are coming from the original translation unit with the attributes coming from the model files, and expore that set of attributes. This way the checker could not tell what the source of the information is. Unfortunately I do not know what would be the best way to enfore this, since the checkers can just observe the AST of the original translation unit and bypassing the models.

[pgousseau]

In D13731#267905, @xazax.hun wrote:

Hi!

Hi! Thank you for reviewing.

I have some high level questions and notes about this patch.

I implemented the function modelling as a Google Summer of Code project and Ted Kremenek was my mentor. I am happy that you found an useful application of the function modeling system, and I think, in general, it is a good idea to be able to store attributes for the modelled functions. However I am a bit surprised, when I saw this patch. The main reason is that, models lack a lot of functionality right now.

Main missing features:

• Ability to specify multiple model paths similar to how header include paths are specified.
• Support for methods, namespaces, overloaded functions.

Did you find the feature useful despite those limitations? I am interested to improve the situation in the future, unfortunately I find very little time to work on this area recently, but I do welcome every changes.

Yes I think this is useful despite the missing features. With the current model file design I am confident those features could be easily added at a later stage?

I have two comments before I start to review the patch itself. Right now this patch contains two modifications. One for the .model files and one for a checker. I think it would be better to separate these two changes into two separate patches. If the motivation behind the merge of those patches is that, you want to test a feature you implemented in .model files, than I think there are other checker that are using attributes, so I think you should be able to provide a test case with the two changes separated. (For example using the nonnull parameter checker.)

I see, yes using the nonnull param checker for testing seems a better fit, I will update the patch.

The other comment is that, the .model files are intended to work in a way, that checkers should not be able utilize the information whether some data is coming from a model file or the analyzed translation unit. In fact, this is an abstraction, that makes it possible to develop the checkers and the modelling mechanism independently. With you patch, the checker should observe the model declaration explicitly. I think, a better design would somehow merge the attributes that are coming from the original translation unit with the attributes coming from the model files, and expore that set of attributes. This way the checker could not tell what the source of the information is. Unfortunately I do not know what would be the best way to enfore this, since the checkers can just observe the AST of the original translation unit and bypassing the models.

I agree, abstracting the attributes' origin is nicer, hopefully we can find a solution that is elegant enough. I will have a look.
Thanks!

Pierre

[pgousseau]

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.

[zaks.anna]

lib/Analysis/BodyFarm.h

43

Why do we not have sanity checks? What happens when there is a conflict?

lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

512

This name is too generic.

571

Should we walk the entire AST here only to get the info from the few functions in the farm?

The AnalysisConsumer visitor tries to make sure the whole AST is not serialized, which is very expensive when dealing with PCH files. (You an find comments about it if you search for "PCH".)

lib/StaticAnalyzer/Frontend/ModelConsumer.cpp

37

Why func lost constness here?

lib/StaticAnalyzer/Frontend/ModelInjector.cpp

49

Does ModelInjector::onBodySynthesis return immediately if the model has been parsed but the Decl is not in the map?

If that is not the case, wouldn't it be very expensive to call onBodySynthesis on every decl, most of which are not modeled? If I understand correctly, we would try to parse the model file for every such decl.

test/Analysis/model-attributes.cpp

3

80 col?

[pgousseau]

lib/Analysis/BodyFarm.h

43

I am worried that any custom sanity check I put would end up confusing the "faux-attributes" user ... I have just noticed that Sema has some merging capabilities, I will have a look, see if it can be reused. Thanks!

lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

512

Will rename it thanks!

571

I see, yes this can be optimized thanks!

lib/StaticAnalyzer/Frontend/ModelConsumer.cpp

37

No reason, will fix thanks!

lib/StaticAnalyzer/Frontend/ModelInjector.cpp

49

Yes, there might be cases where the model file's Decl might not match the model file filename, causing re-parsing. Will fix thanks!

test/Analysis/model-attributes.cpp

3

Will fix!

[pgousseau]

Following Anna's review:

Remove unnecessary AST walk over declarations by reusing already captured declarations.
Add handling of merge conflicts using Sema merge methods.
Add condition at the end of ModelInjector::onBodySynthesis to prevent unnecessary model file parsing.
Fix 80 col in regression tests.
Add regression tests to exercise merge conflicts.

The const qualifier on model's declarations has to be removed as Sema merge methods take non const parameters.

[pgousseau]

Ping!

[pgousseau]

Ping!

[zaks.anna]

Pierre,

Have you seen the post about API Notes?
http://llvm.cc/t/cfe-dev-clang-and-swift/331

I believe using API notes would be a better approach for adding annotations. By the time the static analyzer sees the AST, the annotations would already be there. The API Notes should already seamlessly work for nullability annotations. Does that work for you? (Do we still need some parts of this patch? I am not sure.)

A related topic is deciding what type of attributes should be used by the static analyzer checkers. Currently, there are 2 options, neither of which is ideal:

1. Extend clang with new attributes for static analyzes. This is not highly desirable because the clang attributes namespace will get polluted and compiler users might start using them. This approach is also not convenient for proprietary checkers.
2. Annotate attribute (AnnotateAttr) is used in some places; however, this attribute creeps into LLVMIR, which caused problems in the past.

The best approach would be to put all analyzer attributes behind "a fence" so that we could add/remove them without worrying that compiler(not analyzer) users depend on them. The attribute would not be CodeGened.

Here what it might look like:
analyzer_annotate(family, arg…)
family: id
arg: [id=] value
value: number | string | id

Is anyone interested in implementing this?

[pgousseau]

In D13731#302989, @zaks.anna wrote:

Pierre,

Have you seen the post about API Notes?
http://llvm.cc/t/cfe-dev-clang-and-swift/331

I believe using API notes would be a better approach for adding annotations. By the time the static analyzer sees the AST, the annotations would already be there. The API Notes should already seamlessly work for nullability annotations. Does that work for you? (Do we still need some parts of this patch? I am not sure.)

Thanks for pointing it out! It is not clear yet if APINotes is suited for our problematic, will need to ask some questions first on the "Clang and Swift" cfe-dev thread.

A related topic is deciding what type of attributes should be used by the static analyzer checkers. Currently, there are 2 options, neither of which is ideal:

1. Extend clang with new attributes for static analyzes. This is not highly desirable because the clang attributes namespace will get polluted and compiler users might start using them. This approach is also not convenient for proprietary checkers.
2. Annotate attribute (AnnotateAttr) is used in some places; however, this attribute creeps into LLVMIR, which caused problems in the past.

The best approach would be to put all analyzer attributes behind "a fence" so that we could add/remove them without worrying that compiler(not analyzer) users depend on them. The attribute would not be CodeGened.

Here what it might look like:
analyzer_annotate(family, arg…)
family: id
arg: [id=] value
value: number | string | id

Is anyone interested in implementing this?

The "analyzer_annotate" idea sounds good, I would need to do more evaluation before deciding this is suitable for us though.

[pgousseau]

Abandoning for now while evaluating Swift's APINotes.

include/clang/Analysis/AnalysisContext.h

public:
   /// \brief Checks if the body of the Decl is generated by the BodyFarm from a
   /// model file.
   ///
   /// Note, the lookup is not free. We are going to call getBody behind
   /// the scenes.
   /// \sa getBody
   bool isBodyAutosynthesizedFromModelFile() const;
 
+  /// \brief Retrieve the declaration from the model file.
+  /// \sa getModelDecl
+  FunctionDecl *getModelDecl() const;
+
   CFG *getCFG();
 
   CFGStmtMap *getCFGStmtMap();
 
   CFGReverseBlockReachabilityAnalysis *getCFGReachablityAnalysis();
 
   /// Return a version of the CFG without any edges pruned.
   CFG *getUnoptimizedCFG();
class AnalysisDeclContextManager {
   /// Pointer to an interface that can provide function bodies for
   /// declarations from external source.
   std::unique_ptr<CodeInjector> Injector;
   
   /// Flag to indicate whether or not bodies should be synthesized
   /// 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);
 
   ~AnalysisDeclContextManager();
 
   AnalysisDeclContext *getContext(const Decl *D);
 
   bool getUseUnoptimizedCFG() const {
     return !cfgBuildOptions.PruneTriviallyFalseEdges;
   }
 
   CFG::BuildOptions &getCFGBuildOptions() {
     return cfgBuildOptions;
   }
   
   /// Return true if faux bodies should be synthesized for well-known
   /// 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,
                                          const CFGBlock *Blk,
                                          unsigned Idx) {
     return LocContexts.getStackFrame(Ctx, Parent, S, Blk, Idx);
   }
 

include/clang/Analysis/CodeInjector.h

 /// functions.
 class CodeInjector {
 public:
   CodeInjector();
   virtual ~CodeInjector();
 
   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;
 };
 }
 
 #endif

include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

public:
   //
   // This is controlled by "-analyzer-config max-inlinable-size" option.
   unsigned getMaxInlinableSize();
 
   /// Returns true if the analyzer engine should synthesize fake bodies
   /// 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.
   ///
   /// This is controlled by the 'graph-trim-interval' config option. To disable
   /// node reclamation, set the option to "0".
   unsigned getGraphTrimInterval();
 
   /// Returns the maximum times a large function could be inlined.

include/clang/StaticAnalyzer/Frontend/FrontendActions.h

 /// This frontend action is responsible for parsing model files. Model files can
 /// not be parsed on their own, they rely on type information that is available
 /// in another translation unit. The parsing of model files is done by a
 /// separate compiler instance that reuses the ASTContext and othen information
 /// from the main translation unit that is being compiled. After a model file is
 /// 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);
 
 } // end GR namespace
 
 } // end namespace clang
 
 #endif

include/clang/StaticAnalyzer/Frontend/ModelConsumer.h

 namespace ento {
 
 /// \brief ASTConsumer to consume model files' AST.
 ///
 /// This consumer collects the bodies of function definitions into a StringMap
 /// 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;
 };
 }
 }
 
 #endif

lib/Analysis/AnalysisDeclContext.cpp

: Manager(Mgr),
   cfgBuildOptions.forcedBlkExprs = &forcedBlkExprs;
 }
 
 AnalysisDeclContextManager::AnalysisDeclContextManager(bool useUnoptimizedCFG,
                                                        bool addImplicitDtors,
                                                        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;
   cfgBuildOptions.AddInitializers = addInitializers;
   cfgBuildOptions.AddTemporaryDtors = addTemporaryDtors;
   cfgBuildOptions.AddStaticInitBranches = addStaticInitBranch;
   cfgBuildOptions.AddCXXNewAllocator = addCXXNewAllocator;
 }

 }
 
 bool AnalysisDeclContext::isBodyAutosynthesizedFromModelFile() const {
   bool Tmp;
   Stmt *Body = getBody(Tmp);
   return Tmp && Body->getLocStart().isValid();
 }
 
+FunctionDecl *AnalysisDeclContext::getModelDecl() const {
+  if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
+    if (Manager) {
+      return getBodyFarm(getASTContext(), Manager->Injector.get())
+          .getModelDecl(FD);
+    }
+  }
+  return nullptr;
+}
 
 const ImplicitParamDecl *AnalysisDeclContext::getSelfDecl() const {
   if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D))
     return MD->getSelfDecl();
   if (const BlockDecl *BD = dyn_cast<BlockDecl>(D)) {
     // See if 'self' was captured by the block.
     for (const auto &I : BD->captures()) {
       const VarDecl *VD = I.getVariable();

lib/Analysis/BodyFarm.h

public:
   BodyFarm(ASTContext &C, CodeInjector *injector) : C(C), Injector(injector) {}
   
   /// Factory method for creating bodies for ordinary functions.
   Stmt *getBody(const FunctionDecl *D);
 
   /// Factory method for creating bodies for Objective-C properties.
   Stmt *getBody(const ObjCMethodDecl *D);
 
+  /// \brief Retrieve the declaration from the model file.
+  FunctionDecl *getModelDecl(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;
 };
 }
 
 #endif

lib/Analysis/BodyFarm.cpp

FF = llvm::StringSwitch<FunctionFarmer>(Name)
           .Default(nullptr);
   }
   
   if (FF) { Val = FF(C, D); }
   else if (Injector) { Val = Injector->getBody(D); }
   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.
   const ObjCIvarDecl *IVar = Prop->getPropertyIvarDecl();
   if (!IVar)
     return nullptr;
 
   // Ignore weak variables, which have special behavior.

lib/StaticAnalyzer/Core/AnalysisManager.cpp

AnalysisManager::AnalysisManager(ASTContext &ctx, DiagnosticsEngine &diags,
                                  CheckerManager *checkerMgr,
                                  AnalyzerOptions &Options,
                                  CodeInjector *injector)
   : AnaCtxMgr(Options.UnoptimizedCFG,
               /*AddImplicitDtors=*/true,
               /*AddInitializers=*/true,
               Options.includeTemporaryDtorsInCFG(),
               Options.shouldSynthesizeBodies(),
+              Options.shouldMergeModelAttributes(),
               Options.shouldConditionalizeStaticInitializers(),
               /*addCXXNewAllocator=*/true,
               injector),
     Ctx(ctx),
     Diags(diags),
     LangOpts(lang),
     PathConsumers(PDC),
     CreateStoreMgr(storemgr), CreateConstraintMgr(constraintmgr),

lib/StaticAnalyzer/Core/AnalyzerOptions.cpp

unsigned AnalyzerOptions::getMaxNodesPerTopLevelFunction() {
   }
   return MaxNodesPerTopLevelFunction.getValue();
 }
 
 bool AnalyzerOptions::shouldSynthesizeBodies() {
   return getBooleanOption("faux-bodies", true);
 }
 
+bool AnalyzerOptions::shouldMergeModelAttributes() {
+  return getBooleanOption("faux-attributes", false);
+}
+
 bool AnalyzerOptions::shouldPrunePaths() {
   return getBooleanOption("prune-paths", true);
 }
 
 bool AnalyzerOptions::shouldConditionalizeStaticInitializers() {
   return getBooleanOption("cfg-conditional-static-initializers", true);
 }
 
 bool AnalyzerOptions::shouldInlineLambdas() {
   if (!InlineLambdas.hasValue())
     InlineLambdas = getBooleanOption("inline-lambdas", /*Default=*/true);
   return InlineLambdas.getValue();
 }

lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/CallGraph.h"
 #include "clang/Analysis/CodeInjector.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Sema/Sema.h"
 #include "clang/StaticAnalyzer/Checkers/LocalCheckers.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"

 
 public:
   ASTContext *Ctx;
   const Preprocessor &PP;
   const std::string OutDir;
   AnalyzerOptionsRef Opts;
   ArrayRef<std::string> Plugins;
   CodeInjector *Injector;
+  CompilerInstance &CI;
 
   /// \brief Stores the declarations from the local translation unit.
   /// Note, we pre-compute the local declarations at parse time as an
   /// optimization to make sure we do not deserialize everything from disk.
   /// The local declaration to all declarations ratio might be very small when
   /// working with a PCH file.
   SetOfDecls LocalTUDecls;
 
public:
   /// The information about analyzed functions shared throughout the
   /// translation unit.
   FunctionSummariesTy FunctionSummaries;
 
   AnalysisConsumer(const Preprocessor& pp,
                    const std::string& outdir,
                    AnalyzerOptionsRef opts,
                    ArrayRef<std::string> plugins,
-                   CodeInjector *injector)
+                   CodeInjector *injector,
+                   CompilerInstance &CI)
     : RecVisitorMode(0), RecVisitorBR(nullptr), Ctx(nullptr), PP(pp),
-      OutDir(outdir), Opts(opts), Plugins(plugins), Injector(injector) {
+      OutDir(outdir), Opts(opts), Plugins(plugins), Injector(injector), CI(CI) {
     DigestAnalyzerOptions();
     if (Opts->PrintStats) {
       llvm::EnableStatistics();
       TUTotalTimer = new llvm::Timer("Analyzer Total Time");
     }
   }
 
   ~AnalysisConsumer() override {
#include "clang/StaticAnalyzer/Core/Analyses.def"
   }
 
 private:
   void storeTopLevelDecls(DeclGroupRef DG);
 
   /// \brief Check if we should skip (not analyze) the given function.
   AnalysisMode getModeForDecl(Decl *D, AnalysisMode Mode);
 
+  /// \brief Merge attributes from model files' declarations.
+  /// Iterates over all local TU declarations.
+  void MergeModelAttributes(AnalysisDeclContextManager &ADCMgr);
+
 };
 } // end anonymous namespace
 
 
 //===----------------------------------------------------------------------===//
 // AnalysisConsumer implementation.
 //===----------------------------------------------------------------------===//
 llvm::Timer* AnalysisConsumer::TUTotalTimer = nullptr;
for (llvm::ReversePostOrderTraversal<clang::CallGraph*>::rpo_iterator
     for (SetOfConstDecls::iterator I = VisitedCallees.begin(),
                                    E = VisitedCallees.end(); I != E; ++I) {
         Visited.insert(*I);
     }
     VisitedAsTopLevel.insert(D);
   }
 }
 
+void AnalysisConsumer::MergeModelAttributes(
+    AnalysisDeclContextManager &ADCMgr) {
+  // For all local TU function declarations, check if there is a corresponding
+  // model's declaration. If yes, merge its attributes, using Sema to handle
+  // conflicts.
+  assert(CI.hasSema() && "Compiler instance should have Sema at this point.");
+  Sema &S = CI.getSema();
+  const unsigned LocalTUDeclsSize = LocalTUDecls.size();
+  for (unsigned i = 0; i < LocalTUDeclsSize; ++i) {
+    Decl *D = LocalTUDecls[i];
+    FunctionDecl *NewFD = dyn_cast<FunctionDecl>(D);
+    if (!NewFD)
+      continue;
+    AnalysisDeclContext *ADC = ADCMgr.getContext(NewFD);
+    FunctionDecl *OldFD = ADC->getModelDecl();
+    if (!OldFD) // No matching model declarations found.
+      continue;
+    NamedDecl *OldND = cast<NamedDecl>(OldFD);
+    // Merge the declarations.
+    S.MergeFunctionDecl(NewFD, OldND,
+                        S.getScopeForContext(NewFD->getDeclContext()),
+                        true /*MergeTypeWithOld*/);
+    // Sema wont merge parameters' attributes if the old declaration does not
+    // already have attributes. So we do it manually here.
+    unsigned NewNumParams = NewFD->getNumParams();
+    unsigned OldNewNumParams = OldFD->getNumParams();
+    if (NewNumParams != OldNewNumParams)
+      return;
+    for (unsigned i = 0; i < NewNumParams; i++) {
+      ParmVarDecl *NewP = NewFD->getParamDecl(i);
+      ParmVarDecl *OldP = OldFD->getParamDecl(i);
+      S.mergeDeclAttributes(NewP, OldP);
+    }
+  }
+}
+
 void AnalysisConsumer::HandleTranslationUnit(ASTContext &C) {
   // Don't run the actions if an error has occurred with parsing the file.
   DiagnosticsEngine &Diags = PP.getDiagnostics();
   if (Diags.hasErrorOccurred() || Diags.hasFatalErrorOccurred())
     return;
 
   // Don't analyze if the user explicitly asked for no checks to be performed
   // on this file.
   if (Opts->DisableAllChecks)
     return;
 
   {
     if (TUTotalTimer) TUTotalTimer->startTimer();
 
+    // If "faux-attributes=true" is given, merge model's attributes.
+    AnalysisDeclContextManager &ADCMgr = Mgr->getAnalysisDeclContextManager();
+    if (ADCMgr.mergeModelAttributes()) {
+      MergeModelAttributes(ADCMgr);
+    }
+
     // Introduce a scope to destroy BR before Mgr.
     BugReporter BR(*Mgr);
     TranslationUnitDecl *TU = C.getTranslationUnitDecl();
+
     checkerMgr->runCheckersOnASTDecl(TU, *Mgr, BR);
 
     // Run the AST-only checks using the order in which functions are defined.
     // If inlining is not turned on, use the simplest function order for path
     // sensitive analyzes as well.
     RecVisitorMode = AM_Syntax;
     if (!Mgr->shouldInlineCall())
       RecVisitorMode |= AM_Path;
     RecVisitorBR = &BR;
 
ento::CreateAnalysisConsumer(CompilerInstance &CI) {
   CI.getPreprocessor().getDiagnostics().setWarningsAsErrors(false);
 
   AnalyzerOptionsRef analyzerOpts = CI.getAnalyzerOpts();
   bool hasModelPath = analyzerOpts->Config.count("model-path") > 0;
 
   return llvm::make_unique<AnalysisConsumer>(
       CI.getPreprocessor(), CI.getFrontendOpts().OutputFile, analyzerOpts,
       CI.getFrontendOpts().Plugins,
-      hasModelPath ? new ModelInjector(CI) : nullptr);
+      hasModelPath ? new ModelInjector(CI) : nullptr, CI);
 }
 
 //===----------------------------------------------------------------------===//
 // Ubigraph Visualization.  FIXME: Move to separate file.
 //===----------------------------------------------------------------------===//
 
 namespace {
 

lib/StaticAnalyzer/Frontend/FrontendActions.cpp

 using namespace clang;
 using namespace ento;
 
 std::unique_ptr<ASTConsumer>
 AnalysisAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
   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);
 }

lib/StaticAnalyzer/Frontend/ModelConsumer.cpp

 
 #include "clang/StaticAnalyzer/Frontend/ModelConsumer.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclGroup.h"
 
 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()) {
+    // We are interested in definitions and declarations.
+    // We cant have constness due to Sema's merging methods taking non-const
+    // params.
+    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;
 }

lib/StaticAnalyzer/Frontend/ModelInjector.h

 class Module;
 
 namespace ento {
 class ModelInjector : public CodeInjector {
 public:
   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
   ///
   /// This method first looks up the appropriate model file based on the
   /// model-path configuration option and the name of the declaration that is
   /// looked up. If no model were synthesized yet for a function with that name
   /// it will create a new compiler instance to parse the model file using the
   /// ASTContext, Preprocessor, SourceManager of the original compiler instance.
   /// The former resources are shared between the two compiler instance, so the
   /// newly created instance have to "leak" these objects, since they are owned
   /// by the original instance.
   ///
   /// The model-path should be either an absolute path or relative to the
   /// working directory of the compiler.
   void onBodySynthesis(const NamedDecl *D);
 
   CompilerInstance &CI;
 
   // 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;
 };
 }
 }
 
 #endif

lib/StaticAnalyzer/Frontend/ModelInjector.cpp

Stmt *ModelInjector::getBody(const FunctionDecl *D) {
   return Bodies[D->getName()];
 }
 
 Stmt *ModelInjector::getBody(const ObjCMethodDecl *D) {
   onBodySynthesis(D);
   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();
   FileID mainFileID = SM.getMainFileID();
 
   AnalyzerOptionsRef analyzerOpts = CI.getAnalyzerOpts();
   llvm::StringRef modelPath = analyzerOpts->Config["model-path"];
 
   llvm::SmallString<128> fileName;
 
   if (!modelPath.empty())
     fileName =
         llvm::StringRef(modelPath.str() + "/" + D->getName().str() + ".model");
   else
     fileName = llvm::StringRef(D->getName().str() + ".model");
 
   if (!llvm::sys::fs::exists(fileName.str())) {
     Bodies[D->getName()] = nullptr;
+    Decls[D->getName()] = nullptr;
     return;
   }
 
   IntrusiveRefCntPtr<CompilerInvocation> Invocation(
       new CompilerInvocation(CI.getInvocation()));
 
   FrontendOptions &FrontendOpts = Invocation->getFrontendOpts();
   InputKind IK = IK_CXX; // FIXME
void ModelInjector::onBodySynthesis(const NamedDecl *D) {
   // is set to true to avoid double free issues
   Instance.setFileManager(&CI.getFileManager());
   Instance.setSourceManager(&SM);
   Instance.setPreprocessor(&CI.getPreprocessor());
   Instance.setASTContext(&CI.getASTContext());
 
   Instance.getPreprocessor().InitializeForModelFile();
 
-  ParseModelFileAction parseModelFile(Bodies);
+  ParseModelFileAction parseModelFile(Bodies, Decls);
 
   const unsigned ThreadStackSize = 8 << 20;
   llvm::CrashRecoveryContext CRC;
 
   CRC.RunSafelyOnThread([&]() { Instance.ExecuteAction(parseModelFile); },
                         ThreadStackSize);
 
+  // Prevent the model file from being parsed again.
+  if (Bodies.count(D->getName()) == 0)
+    Bodies[D->getName()] = nullptr;
+  if (Decls.count(D->getName()) == 0)
+    Decls[D->getName()] = nullptr;
+
   Instance.getPreprocessor().FinalizeForModelFile();
 
   Instance.resetAndLeakSourceManager();
   Instance.resetAndLeakFileManager();
   Instance.resetAndLeakPreprocessor();
 
   // The preprocessor enters to the main file id when parsing is started, so
   // the main file id is changed to the model file during parsing and it needs
   // to be reseted to the former main file id after parsing of the model file
   // is done.
   SM.setMainFileID(mainFileID);
 }

test/Analysis/Inputs/Models/modelFileHasAttributes.model

+int modelFileHasAttributes(int *p0, int *p1, int *p2 __attribute__((nonnull)),
+                           int *_Nonnull p3) __attribute__((nonnull(2)));
 No newline at end of file

test/Analysis/Inputs/Models/modelFileHasConflicts.model

+void modelFileHasConflicts(int *_Nullable p)
+    __attribute__((visibility("protected")));
 No newline at end of file

test/Analysis/analyzer-config.c

 // RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper -Xclang -analyzer-max-loop -Xclang 34 > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 
 void bar() {}
 void foo() {
   // Call bar 33 times so max-times-inline-large is met and
   // min-blocks-for-inline-large is checked
   for (int i = 0; i < 34; ++i) {
     bar();
   }
 }
 
 // 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
 // CHECK-NEXT: ipa = dynamic-bifurcate
 // CHECK-NEXT: ipa-always-inline-size = 3
 // CHECK-NEXT: leak-diagnostics-reference-allocation = false
 // CHECK-NEXT: max-inlinable-size = 50
 // CHECK-NEXT: max-nodes = 150000
 // CHECK-NEXT: max-times-inline-large = 32
 // CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // 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
 

test/Analysis/analyzer-config.cpp

 // CHECK: [config]
 // CHECK-NEXT: c++-container-inlining = false
 // CHECK-NEXT: c++-inlining = destructors
 // CHECK-NEXT: c++-shared_ptr-inlining = false
 // CHECK-NEXT: c++-stdlib-inlining = true
 // 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
 // CHECK-NEXT: ipa = dynamic-bifurcate
 // CHECK-NEXT: ipa-always-inline-size = 3
 // CHECK-NEXT: leak-diagnostics-reference-allocation = false
 // CHECK-NEXT: max-inlinable-size = 50
 // CHECK-NEXT: max-nodes = 150000
 // CHECK-NEXT: max-times-inline-large = 32
 // CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // 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

test/Analysis/model-attributes.cpp

+// 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, and a '_Nonnull' attribute on the 4th parameter.
+// The declaration of 'modelFileHasConflicts' has a visibility 'protected'
+// attribute and a '_Nullable' parameter attribute.
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown -analyze -analyzer-checker=core,nullability -analyzer-config faux-attributes=true,model-path=%S/Inputs/Models %s -verify
+
+int modelFileHasAttributes(int *p0, int *p1, int *p2, int *p3);
+void modelFileHasConflicts(int *_Nonnull p) __attribute__((visibility("hidden")));
+// expected-warning@-1 {{nullability specifier '_Nonnull' conflicts with existing specifier '_Nullable'}}
+// expected-note@-2 {{previous attribute is here}}
+// expected-note@-3 {{previous declaration is here}}
+
+int f0(int *x, int *y, int *z) {
+  int *p = 0;
+  modelFileHasAttributes(p, x, y, z); // no-warning
+  return 0;
+}
+
+int f1(int *x, int *y, int *z) {
+  int *p = 0;
+  modelFileHasAttributes(x, p, y, z);
+  // expected-warning@-1{{Null pointer passed as an argument to a 'nonnull' parameter}}
+  return 0;
+}
+
+int f2(int *x, int *y, int *z) {
+  int *p = 0;
+  modelFileHasAttributes(x, y, p, z);
+  // expected-warning@-1{{Null pointer passed as an argument to a 'nonnull' parameter}}
+  return 0;
+}
+
+int f2a(int *x, int *y, int *z) {
+  int *p = 0;
+  modelFileHasAttributes(x, y, z, p);
+  // expected-warning@-1{{Null passed to a callee that requires a non-null argument}}
+  return 0;
+}
+
+int f3() {
+  modelFileHasConflicts(0);
+  // expected-warning@-1 {{null passed to a callee that requires a non-null argument}}
+  // expected-error@./Inputs/Models/modelFileHasConflicts.model:2 {{visibility does not match previous declaration}}
+  // expected-warning@./Inputs/Models/modelFileHasConflicts.model:1 {{nullability specifier '_Nullable' conflicts with existing specifier '_Nonnull'}}
+  // expected-note@./Inputs/Models/modelFileHasConflicts.model:1 {{previous declaration is here}}
+  return 0;
+}