Hi Ted,
Thank you for the review, I have altered the patch accordingly, and also
added the patch to follow up the API change in clang tidy.
On 12 August 2014 08:44, Ted Kremenek <[email protected]> wrote:
>
> On Aug 1, 2014, at 1:44 AM, Gábor Horváth <[email protected]> wrote:
>
> Hi Ted,
>
> Thank you for the review.
>
> On 1 August 2014 07:25, Ted Kremenek <[email protected]> wrote:
>
>> Hi Gábor,
>>
>> This is looking good to me. Some minor nits/comments:
>>
>> - Please add doxygen comments for the CodeInjector class.
>>
>
> Done.
>
>
>> - For the BugReporter patch, are there tests for that functionality
>> change? I saw tests in the other patch, but not that one. It's fine to
>> separate the review of that change before the primary change goes in, but I
>> was curious.
>>
>
> Well, it may be a bit complicated. I deleted some code in BugReporter to
> not to discard bug reports that are in a model file, and the plist part of
> the test case only pass if that patch is applied (if the patch is not
> applied the nullpointer dereference warning that has the position in the
> modelfile will be discarded. In the long term it would be better to report
> these errors elsewhere but it is not supported yet by the bugreporter
> patch). I can move the plist check into a separate testcase and add that
> case to the BugReporter patch instead. The division by zero test should
> work without the BugReporter patch.
>
>
> Ok, this make sense. Can you clarify what you mean by "better to report
> these errors elsewhere"?
>
>
It might be confusing for the user, if the execution path of the bug report
contains a location that is inside a model file so the report contains
codes and files that are not present in the analysed project. So it might
be more user friendly if the locations that are inside a model file would
be excluded from the reported execution path, however if this is not an
issue I am ok with reporting those locations as well.
>
>> - As for breaking code in the 'extra' repository, LLVM-internal API is
>> not sacrosanct. If we break the 'extra' projects we just need to update
>> them, but I'm not certain if that is possible in this case.
>>
>
> As far as I can remember it would be a straightforward fix in the extra
> repository. Clang-tidy calls CreateAnalysisConsumer.
>
>
> Sounds good. Let's get the right API and just fix up clang-tidy.
>
>
>
>> - For comments, please consistently use sentence casing and end with
>> periods, and for type names use the appropriate casing. For example:
>>
>> + // modules create a separate compilerinstance for parsing modules,
>> maybe it is
>> + // for reason so I mimic this behavior
>> + CompilerInstance Instance;
>> ...
>>
>> This comment looks a bit suspect, since it seems like a question to
>> yourself. Here you use the word "I"; who is "I" in the context of this
>> code? The comment also seems like an unanswered question. Is this a stale
>> comment?
>>
>>
> Done, the comment was improved.
>
>
>> Another example is this comment:
>>
>> + // FIXME: double memoization is redundant. Here and in bodyfarm.
>> + llvm::StringMap<Stmt *> Bodies;
>>
>> This can be made slightly cleaner. For example:
>>
>> + // FIXME: Double memorization is redundant, with
>> + /// memoization both here and in BodyFarm.
>> + llvm::StringMap<Stmt *> Bodies;
>>
>
> Done.
>
> - Only use doxygen comments for documentation. For example:
>>
>> + if (notzero_notmodeled(p)) {
>> + /// There is no information about the value of p, because
>> + /// notzero_notmodeled is not modeled and the function definition
>> + /// is not available.
>> + int j = 5 / p; // expected-warning {{Division by zero}}
>> + }
>>
>> In this case we should use '//', not '///'. The former are true
>> comments, and the latter are candidates to be extracted for documentation.
>>
>>
> Done.
>
>
>> Overall, however, this is getting really close.
>>
>>
> It is great.
>
> Thanks,
> Gábor
>
>
> Wonderful. The rest of my comments are minor:
>
> +/// \brief CodeInjector is an interface which is responsible forinjecting
> AST of
> +/// function definitions that may not be available in the original source.
> +///
> +/// The getBody function will be called each time the static analyzer
> examines a
> +/// function call that has no definition available in the current
> translation
> +/// unit. If the returned statement is not a nullpointer, it is assumed
> to be
> +/// the body of a function which will be used for the analysis. The
> source of
> +/// the body can be arbitrary, but it is advised to use memoization to
> avoid
> +/// unnecessary reparsing of the external source that provides the body
> of the
> +/// functions.
>
>
> "forinjecting" -> "for injecting"
> "nullpointer" -> "null pointer"
>
> +++ include/clang/StaticAnalyzer/Frontend/FrontendActions.h (working copy)
> @@ -10,10 +10,16 @@
> #ifndef LLVM_CLANG_GR_FRONTENDACTIONS_H
> #define LLVM_CLANG_GR_FRONTENDACTIONS_H
>
> +#include <map>
> +
>
>
> This "#include" of <map> doesn't seem needed. Neither is the one in
> ModelConsumer.h
>
> +++ lib/StaticAnalyzer/Frontend/ModelConsumer.cpp (working copy)
> @@ -0,0 +1,42 @@
> +//===--- ModelConsumer.cpp - ASTConsumer for consuming model files
> --------===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
>
> +//===----------------------------------------------------------------------===//
> +///
> +/// \file
> +/// \brief This file implements an ASTConsumer for consuming model files.
> +///
> +/// This ASTConsumer handles the AST of a parsed model file. All top level
> +/// function definitions will be collected from that model file for later
> +/// retrieval during the static analyzis. The body of these functions
> will not
> +/// be injected into the ASTUnit of the analyzed translation unit. It
> will be
> +/// available through the BodyFarm which is utilized by the
> AnalysisDeclContext
> +/// class.
> +///
>
>
> "analyzis" -> "analysis"
>
> + // The instance wants to take ownership, however disablefree frontend
> option
> + // is set to true to avoid double free issues
>
>
> Use the actual casing for the option for technical precision:
>
> DisableFree
>
> + /// \brief Synthetize 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 synthetized 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.
>
>
> Synthetize -> Synthesize
> synthetized -> synthesized
>
> + std::vector<std::unique_ptr<ASTUnit> > ModelAsts;
>
>
> I'd prefer this to be "ModelASTs", as 'AST' is an acronym.
>
>
It was an unused member (from an earlier implementation) that I forgot to
remove, but done now.
> Otherwise, this all looks great to me.
>
>
>
>> Cheers,
>> Ted
>>
>> On Jul 30, 2014, at 3:29 AM, Gábor Horváth <[email protected]> wrote:
>>
>> Hi Ted,
>>
>> Thank you for the review.
>>
>>
>> On 30 July 2014 08:18, Ted Kremenek <[email protected]> wrote:
>>
>>> Hi Gábor,
>>>
>>> Thanks for making progress on this very promising enhancement to the
>>> analyzer. I have an assortment of comments, in no particular order:
>>>
>>> - ModelInjector.h and ModelConsumer.h
>>>
>>> There is a comment at the top of these files, but I think a bit more
>>> explanation is needed. For example:
>>>
>>> MetaConsumer.cpp:
>>>
>>> +// "Meta" ASTConsumer for consuming model files.
>>>
>>> That doesn't really explain anything. What does "Meta" in quotes mean?
>>> I think an explanation here on what this does is helpful when someone
>>> discovers this code for the first time.
>>>
>>> Similarly, we should add some high-level comments for CodeInjector.h and
>>> ModelInjector.h. We have a good start in ModelInjector.h:
>>>
>>> +/// \file
>>> +/// \brief Defines the clang::ento::ModelInjector class which
>>> implements the
>>> +/// clang::CodeInjector interface. This class is responsible for
>>> injecting
>>> +/// function definitions that were synthetized from model files.
>>> +///
>>>
>>> Let's consider expanding it:
>>>
>>> /// \brief This file defines the clang::ento::ModelInjector class which
>>> implements the
>>> /// clang::CodeInjector interface. This class is responsible for
>>> injecting
>>> /// function definitions that were synthesized from model files.
>>>
>>> /// Model files allow definitions of functions to be lazily constituted
>>> for functions
>>> /// which lack bodies in the original source code. This allows the
>>> analyzer
>>> /// to more precisely analyze code that calls such functions, analyzing
>>> the
>>> /// artificial definitions (which typically approximate the semantics
>>> of the
>>> /// called function) when called by client code. These definitions are
>>> /// reconstituted lazily, on-demand, by the static analyzer engine.
>>>
>>> CodeInjector.h provides some information, but it is a bit vague:
>>>
>>> +///
>>> +/// \file
>>> +/// \brief Defines the clang::CodeInjector interface which is
>>> responsible for
>>> +/// injecting AST of function definitions from external source.
>>> +///
>>>
>>> It's a bit unclear how this gets used. I think a bit of prose here
>>> would help clarify its role in the static analyzer. I also think the
>>> CodeInjector interface is also more abstract than the prose describes.
>>> There's nothing about CodeInjector's interface that requires the injected
>>> definitions to come from an external source. That's an implementation
>>> detail of a concrete subclass. Instead, all CodeInjector does is provide
>>> an interface that lazily provides definitions for functions and methods
>>> that may not be present in the original source.
>>>
>>
>> I have added some further documentation to address these issues.
>>
>>
>>
>>>
>>> I'm also looking at the change to
>>> StaticAnalyzer/Frontend/FrontendActions.cpp, and wonder if we can simplify
>>> things:
>>>
>>> +++ lib/StaticAnalyzer/Frontend/FrontendActions.cpp (working copy)
>>> @@ -7,9 +7,11 @@
>>> //
>>>
>>>
>>> //===----------------------------------------------------------------------===//
>>>
>>> +#include "clang/Frontend/CompilerInstance.h"
>>> #include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
>>> -#include "clang/Frontend/CompilerInstance.h"
>>> #include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
>>> +#include "clang/StaticAnalyzer/Frontend/ModelConsumer.h"
>>> +#include "ModelInjector.h"
>>> using namespace clang;
>>> using namespace ento;
>>>
>>> @@ -18,6 +20,14 @@
>>> return CreateAnalysisConsumer(CI.getPreprocessor(),
>>> CI.getFrontendOpts().OutputFile,
>>> CI.getAnalyzerOpts(),
>>> - CI.getFrontendOpts().Plugins);
>>> + CI.getFrontendOpts().Plugins,
>>> + new ModelInjector(CI));
>>> }
>>>
>>>
>>>
>>> It looks like CreateAnalysisConsumer just continues to grow more
>>> arguments, all which derive from using 'CI'. This seems silly, since this
>>> function is called in one place. Instead of intro ducting a dependency on
>>> ModelInjector.h in this file, we can just sink these arguments into
>>> CreateAnalysisConsumer() itself, resulting in:
>>>
>>> return CreateAnalysisConsumer(CI);
>>>
>>> and let CreateAnalysisConsumer() do all that boilerplate.
>>>
>>
>> That was my original idea as well but it broke the compilation of some
>> code in extra repository and I wasn't sure if it is ok to break the API
>> with this patch. But I find it cleaner this way so I modified it in this
>> iteration.
>>
>>
>>> Next, let's look at the change to FrontendAction:
>>>
>>> class FrontendAction {
>>> + /// Is this action invoked on a model file? Model files are incomplete
>>> + /// translation units that relies on type information from another
>>> translation
>>> + /// unit. Check ParseModelFileAction for details.
>>> + bool ModelFile;
>>>
>>>
>>> Perhaps "IsModelFile"? "ModelFile" sounds like it should be a reference
>>> to the file itself.
>>>
>>> FrontendInputFile CurrentInput;
>>> std::unique_ptr<ASTUnit> CurrentASTUnit;
>>> CompilerInstance *Instance;
>>> @@ -105,7 +109,11 @@
>>> /// @}
>>>
>>> public:
>>> - FrontendAction();
>>> + /// \brief Constructor
>>> + ///
>>> + /// \param modelFile determines whether the source files this action
>>> invoked
>>> + /// on should be treated as a model file. Defaults to false.
>>> + FrontendAction(bool modelFile = false);
>>>
>>>
>>> It seems suboptimal to modify the interface of FrontendAction just for
>>> this one edge case. Instead of modifying the constructor arguments, we
>>> could default initialize "IsModelFile" to false, and have a setter to
>>> change it. For example:
>>>
>>> ParseModelFileAction::ParseModelFileAction(llvm::StringMap<Stmt *>
>>> &Bodies)
>>> : ASTFrontendAction(/*ModelFile=*/true), Bodies(Bodies) {}
>>>
>>> becomes:
>>>
>>> ParseModelFileAction::ParseModelFileAction(llvm::StringMap<Stmt *>
>>> &Bodies)
>>> : Bodies(Bodies) {
>>> IsModelFile = true;
>>> }
>>>
>>> Looking at this more, I wonder if we should modify FrontendAction at
>>> all. The only place where isModelParsingAction() is called is in one spot
>>> in CompilerInstance.cpp:
>>>
>>> if (hasSourceManager() && !Act.isModelParsingAction())
>>>
>>> It *might* be cleaner to just have a virtual member function in
>>> FrontendAction, which defaults to returning false, but is generic for all
>>> subclasses to override. Then we don't need the "IsModelFile" field in
>>> FrontendAction at all, and we just have ParseModelFileAction override that
>>> single member function. We could then name that method to be something a
>>> bit more generic. That would allow us to not touch FrontendAction at all
>>> except for providing that single virtual method that can be overridden in
>>> subclasses. I somewhat prefer this approach because it provides a cleaner
>>> separation of concerns between FrontendAction (which is defined
>>> libFrontend) and the static analyzer. That would also allow you to get rid
>>> of isModelParsingAction() entirely (replacing it with something more
>>> generic).
>>>
>>>
>> You are right, it is much cleaner to use a virtual function, so I
>> modified the patch to use that approach. The new virtual function has the
>> same name because I have yet to find any better and more general name yet.
>> Do you have an idea for a better name?
>>
>>
>>> As for the test case:
>>>
>>> +typedef int* intptr;
>>> +
>>> +void modelled(intptr p);
>>> +
>>> +int main() {
>>> + modelled(0);
>>> + return 0;
>>> +}
>>>
>>>
>>> Please add some comments in this test file explaining what is happening.
>>> Also, it would be great if this both used FileCheck (which it does now)
>>> but also verified the diagnostics so we get cross-checking of the output
>>> (we see this in some analyzer tests). It also makes it easier to
>>> understand the test.
>>>
>>> Also, is there a reason to break up the tests
>>> between model-suppress-falsepos.cpp and model-file.cpp? It seems like one
>>> test file will do fine; just clearly comment on what is happening for each
>>> test. I also recommend called the modeled function "modeledFunction"
>>> instead of "modelled" (which according to my spell checker has an
>>> additional 'l').
>>>
>>
>> I have merged the test files and also added some commets to explain what
>> is going on. I have fixed the misspelling as well. The nullpointer
>> dereference is only checked through plist because the point where the
>> comment with the expected warning should be added is inside the model file
>> and it did not work for me if the comment was in a separate file. If there
>> is a different way to verify the warnings that are in a separate file and I
>> did not find it, please let me know.
>>
>>
>>>
>>> As for the model files themselves:
>>>
>>> Index: test/Analysis/modelled.model
>>> ===================================================================
>>> --- test/Analysis/modelled.model (revision 0)
>>> +++ test/Analysis/modelled.model (working copy)
>>> @@ -0,0 +1,3 @@
>>> +void modelled(intptr p) {
>>> + ++*p;
>>> +}
>>> \ No newline at end of file
>>> Index: test/Analysis/notzero.model
>>> ===================================================================
>>> --- test/Analysis/notzero.model (revision 0)
>>> +++ test/Analysis/notzero.model (working copy)
>>>
>>>
>>> Let's put these in a separate subdirectory, for example, "models",
>>> instead of mixing them with the tests. This way they really serve as
>>> "inputs" to the analyzer.
>>>
>>
>> I have moved the model files to tests/Inputs/Models.
>>
>>
>>>
>>> Overall this is looking good. I think the explanatory comments will
>>> really help people understand what this is doing, and I think changing how
>>> we thread the information through FrontendAction will help not introduce an
>>> artificial tainting of FrontendAction with concepts specific to the static
>>> analyzer.
>>>
>>> Cheers,
>>> Ted
>>>
>>>
>> Thanks,
>> Gábor
>>
>>
>>>
>>> On Jul 16, 2014, at 2:45 AM, Gábor Horváth <[email protected]> wrote:
>>>
>>>
>>>
>>>
>>> On 14 July 2014 19:32, Anna Zaks <[email protected]> wrote:
>>>
>>>>
>>>> On Jul 13, 2014, at 6:11 AM, Gábor Horváth <[email protected]> wrote:
>>>>
>>>> Hi Anna,
>>>>
>>>> Thank you for the review. I have tweaked the test, so it no longer
>>>> requires the error reporting tweak that is not done yet to pass. I have
>>>> also added some high level comments to some files, if you think some
>>>> information is lacking I will add them in the next iteration as well. The
>>>> BugReporter patch is now separated into a different patch.
>>>>
>>>>
>>>> On 11 July 2014 18:02, Anna Zaks <[email protected]> wrote:
>>>>
>>>>>
>>>>> For example, modeling functions should allow you to find bugs and
>>>>> suppress false positives outside of those functions. I would suggest
>>>>> adding
>>>>> a few of those tests first.
>>>>>
>>>>>
>>>> How are the false positives suppressed? I did not find any resource on
>>>> that. Found some analyzer attributes but I did not find them suitable for
>>>> this purpuse at the first glance. But I think once the locations that are
>>>> in a model file are omitted from the report path, the regular methods for
>>>> suppressing false positives should work (and I will definitely add test
>>>> case to ensure this once it is done).
>>>>
>>>>
>>>> What I meant is that it is possible to construct a test where ability
>>>> to model a function would eliminate a false positive. This would be another
>>>> way to test your patch without worrying about BugReporter.
>>>>
>>>
>>> I got it now, thansk. I have updated the patch with a test case where a
>>> false positive case is eliminated by a model file.
>>>
>>> Thanks,
>>> Gábor
>>>
>>>
>>>> Thanks,
>>>> Gábor
>>>> <api_modeling.patch><bugreporter.patch>
>>>>
>>>>
>>>>
>>> <api_modeling.patch><bugreporter.patch>
>>>
>>>
>>>
>> <api_modeling.patch><bugreporter.patch>
>>
>>
>>
> <api_modeling.patch><bugreporter.patch>
>
>
>
Thanks,
Gábor
Index: include/clang/Analysis/AnalysisContext.h
===================================================================
--- include/clang/Analysis/AnalysisContext.h (revision 215430)
+++ include/clang/Analysis/AnalysisContext.h (working copy)
@@ -17,8 +17,10 @@
#include "clang/AST/Decl.h"
#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/CodeInjector.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/OwningPtr.h"
#include "llvm/Support/Allocator.h"
#include <memory>
@@ -398,6 +400,10 @@
ContextMap Contexts;
LocationContextManager LocContexts;
CFG::BuildOptions cfgBuildOptions;
+
+ /// Pointer to an interface that can provide function bodies for
+ /// declarations from external source.
+ llvm::OwningPtr<CodeInjector> Injector;
/// Flag to indicate whether or not bodies should be synthesized
/// for well-known functions.
@@ -410,7 +416,8 @@
bool addTemporaryDtors = false,
bool synthesizeBodies = false,
bool addStaticInitBranches = false,
- bool addCXXNewAllocator = true);
+ bool addCXXNewAllocator = true,
+ CodeInjector* injector = nullptr);
~AnalysisDeclContextManager();
Index: include/clang/Analysis/CodeInjector.h
===================================================================
--- include/clang/Analysis/CodeInjector.h (revision 0)
+++ include/clang/Analysis/CodeInjector.h (working copy)
@@ -0,0 +1,46 @@
+//===-- CodeInjector.h ------------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief Defines the clang::CodeInjector interface which is responsible for
+/// injecting AST of function definitions that may not be available in the
+/// original source.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_ANALYSIS_CODEINJECTOR_H
+#define LLVM_CLANG_ANALYSIS_CODEINJECTOR_H
+
+namespace clang {
+
+class Stmt;
+class FunctionDecl;
+class ObjCMethodDecl;
+
+/// \brief CodeInjector is an interface which is responsible for injecting AST
+/// of function definitions that may not be available in the original source.
+///
+/// The getBody function will be called each time the static analyzer examines a
+/// function call that has no definition available in the current translation
+/// unit. If the returned statement is not a null pointer, it is assumed to be
+/// the body of a function which will be used for the analysis. The source of
+/// the body can be arbitrary, but it is advised to use memoization to avoid
+/// unnecessary reparsing of the external source that provides the body of the
+/// functions.
+class CodeInjector {
+public:
+ CodeInjector();
+ virtual ~CodeInjector();
+
+ virtual Stmt *getBody(const FunctionDecl *D) = 0;
+ virtual Stmt *getBody(const ObjCMethodDecl *D) = 0;
+};
+}
+
+#endif
\ No newline at end of file
Index: include/clang/Frontend/FrontendAction.h
===================================================================
--- include/clang/Frontend/FrontendAction.h (revision 215430)
+++ include/clang/Frontend/FrontendAction.h (working copy)
@@ -157,6 +157,13 @@
/// @name Supported Modes
/// @{
+ /// \brief Is this action invoked on a model file?
+ ///
+ /// Model files are incomplete translation units that relies on type
+ /// information from another translation unit. Check ParseModelFileAction for
+ /// details.
+ virtual bool isModelParsingAction() const { return false; }
+
/// \brief Does this action only use the preprocessor?
///
/// If so no AST context will be created and this action will be invalid
@@ -224,6 +231,7 @@
void ExecuteAction() override;
public:
+ ASTFrontendAction() {}
bool usesPreprocessorOnly() const override { return false; }
};
Index: include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h (revision 215430)
+++ include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h (working copy)
@@ -23,6 +23,8 @@
namespace clang {
+class CodeInjector;
+
namespace ento {
class CheckerManager;
@@ -50,7 +52,8 @@
StoreManagerCreator storemgr,
ConstraintManagerCreator constraintmgr,
CheckerManager *checkerMgr,
- AnalyzerOptions &Options);
+ AnalyzerOptions &Options,
+ CodeInjector* injector = nullptr);
~AnalysisManager();
Index: include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
===================================================================
--- include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h (revision 215430)
+++ include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h (working copy)
@@ -25,6 +25,8 @@
class Preprocessor;
class DiagnosticsEngine;
+class CodeInjector;
+class CompilerInstance;
namespace ento {
class CheckerManager;
@@ -38,8 +40,7 @@
/// analysis passes. (The set of analyses run is controlled by command-line
/// options.)
std::unique_ptr<AnalysisASTConsumer>
-CreateAnalysisConsumer(const Preprocessor &pp, const std::string &output,
- AnalyzerOptionsRef opts, ArrayRef<std::string> plugins);
+CreateAnalysisConsumer(CompilerInstance &CI);
} // end GR namespace
Index: include/clang/StaticAnalyzer/Frontend/FrontendActions.h
===================================================================
--- include/clang/StaticAnalyzer/Frontend/FrontendActions.h (revision 215430)
+++ include/clang/StaticAnalyzer/Frontend/FrontendActions.h (working copy)
@@ -11,9 +11,13 @@
#define LLVM_CLANG_GR_FRONTENDACTIONS_H
#include "clang/Frontend/FrontendAction.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringMap.h"
namespace clang {
+class Stmt;
+
namespace ento {
//===----------------------------------------------------------------------===//
@@ -26,6 +30,27 @@
StringRef InFile) override;
};
+/// \brief Frontend action to parse model files.
+///
+/// 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);
+ bool isModelParsingAction() const override { return true; }
+
+protected:
+ std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+ StringRef InFile) override;
+
+private:
+ llvm::StringMap<Stmt *> &Bodies;
+};
+
void printCheckerHelp(raw_ostream &OS, ArrayRef<std::string> plugins);
} // end GR namespace
Index: include/clang/StaticAnalyzer/Frontend/ModelConsumer.h
===================================================================
--- include/clang/StaticAnalyzer/Frontend/ModelConsumer.h (revision 0)
+++ include/clang/StaticAnalyzer/Frontend/ModelConsumer.h (working copy)
@@ -0,0 +1,44 @@
+//===-- ModelConsumer.h -----------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief This file implements clang::ento::ModelConsumer which is an
+/// ASTConsumer for model files.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_GR_MODELCONSUMER_H
+#define LLVM_CLANG_GR_MODELCONSUMER_H
+
+#include "clang/AST/ASTConsumer.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+
+class Stmt;
+
+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);
+
+ bool HandleTopLevelDecl(DeclGroupRef D) override;
+
+private:
+ llvm::StringMap<Stmt *> &Bodies;
+};
+}
+}
+
+#endif
Index: lib/Analysis/AnalysisDeclContext.cpp
===================================================================
--- lib/Analysis/AnalysisDeclContext.cpp (revision 215430)
+++ lib/Analysis/AnalysisDeclContext.cpp (working copy)
@@ -69,8 +69,9 @@
bool addTemporaryDtors,
bool synthesizeBodies,
bool addStaticInitBranch,
- bool addCXXNewAllocator)
- : SynthesizeBodies(synthesizeBodies)
+ bool addCXXNewAllocator,
+ CodeInjector *injector)
+ : Injector(injector), SynthesizeBodies(synthesizeBodies)
{
cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG;
cfgBuildOptions.AddImplicitDtors = addImplicitDtors;
@@ -84,8 +85,8 @@
llvm::DeleteContainerSeconds(Contexts);
}
-static BodyFarm &getBodyFarm(ASTContext &C) {
- static BodyFarm *BF = new BodyFarm(C);
+static BodyFarm &getBodyFarm(ASTContext &C, CodeInjector *injector = nullptr) {
+ static BodyFarm *BF = new BodyFarm(C, injector);
return *BF;
}
@@ -94,7 +95,7 @@
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
Stmt *Body = FD->getBody();
if (!Body && Manager && Manager->synthesizeBodies()) {
- Body = getBodyFarm(getASTContext()).getBody(FD);
+ Body = getBodyFarm(getASTContext(), Manager->Injector.get()).getBody(FD);
if (Body)
IsAutosynthesized = true;
}
@@ -103,7 +104,7 @@
else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
Stmt *Body = MD->getBody();
if (!Body && Manager && Manager->synthesizeBodies()) {
- Body = getBodyFarm(getASTContext()).getBody(MD);
+ Body = getBodyFarm(getASTContext(), Manager->Injector.get()).getBody(MD);
if (Body)
IsAutosynthesized = true;
}
Index: lib/Analysis/BodyFarm.cpp
===================================================================
--- lib/Analysis/BodyFarm.cpp (revision 215430)
+++ lib/Analysis/BodyFarm.cpp (working copy)
@@ -13,6 +13,7 @@
//===----------------------------------------------------------------------===//
#include "BodyFarm.h"
+#include "clang/Analysis/CodeInjector.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
@@ -383,6 +384,7 @@
}
if (FF) { Val = FF(C, D); }
+ else if (Injector) { Val = Injector->getBody(D); }
return Val.getValue();
}
Index: lib/Analysis/BodyFarm.h
===================================================================
--- lib/Analysis/BodyFarm.h (revision 215430)
+++ lib/Analysis/BodyFarm.h (working copy)
@@ -27,10 +27,11 @@
class ObjCMethodDecl;
class ObjCPropertyDecl;
class Stmt;
+class CodeInjector;
class BodyFarm {
public:
- BodyFarm(ASTContext &C) : C(C) {}
+ BodyFarm(ASTContext &C, CodeInjector *injector) : C(C), Injector(injector) {}
/// Factory method for creating bodies for ordinary functions.
Stmt *getBody(const FunctionDecl *D);
@@ -43,6 +44,7 @@
ASTContext &C;
BodyMap Bodies;
+ CodeInjector *Injector;
};
}
Index: lib/Analysis/CMakeLists.txt
===================================================================
--- lib/Analysis/CMakeLists.txt (revision 215430)
+++ lib/Analysis/CMakeLists.txt (working copy)
@@ -11,6 +11,7 @@
CallGraph.cpp
CocoaConventions.cpp
Consumed.cpp
+ CodeInjector.cpp
Dominators.cpp
DataflowWorklist.cpp
FormatString.cpp
Index: lib/Analysis/CodeInjector.cpp
===================================================================
--- lib/Analysis/CodeInjector.cpp (revision 0)
+++ lib/Analysis/CodeInjector.cpp (working copy)
@@ -0,0 +1,15 @@
+//===-- CodeInjector.cpp ----------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/CodeInjector.h"
+
+using namespace clang;
+
+CodeInjector::CodeInjector() {}
+CodeInjector::~CodeInjector() {}
\ No newline at end of file
Index: lib/Frontend/CompilerInstance.cpp
===================================================================
--- lib/Frontend/CompilerInstance.cpp (revision 215430)
+++ lib/Frontend/CompilerInstance.cpp (working copy)
@@ -806,8 +806,9 @@
llvm::EnableStatistics();
for (unsigned i = 0, e = getFrontendOpts().Inputs.size(); i != e; ++i) {
- // Reset the ID tables if we are reusing the SourceManager.
- if (hasSourceManager())
+ // Reset the ID tables if we are reusing the SourceManager and parsing
+ // regular files.
+ if (hasSourceManager() && !Act.isModelParsingAction())
getSourceManager().clearIDTables();
if (Act.BeginSourceFile(*this, getFrontendOpts().Inputs[i])) {
Index: lib/Frontend/FrontendAction.cpp
===================================================================
--- lib/Frontend/FrontendAction.cpp (revision 215430)
+++ lib/Frontend/FrontendAction.cpp (working copy)
@@ -287,8 +287,9 @@
}
}
- // Set up the preprocessor.
- CI.createPreprocessor(getTranslationUnitKind());
+ // Set up the preprocessor if needed.
+ if (!CI.hasPreprocessor())
+ CI.createPreprocessor(getTranslationUnitKind());
// Inform the diagnostic client we are processing a source file.
CI.getDiagnosticClient().BeginSourceFile(CI.getLangOpts(),
@@ -307,7 +308,9 @@
// Create the AST context and consumer unless this is a preprocessor only
// action.
if (!usesPreprocessorOnly()) {
- CI.createASTContext();
+ // Parsing a model file should reuse the existing ASTContext.
+ if (!isModelParsingAction())
+ CI.createASTContext();
std::unique_ptr<ASTConsumer> Consumer =
CreateWrappedASTConsumer(CI, InputFile);
@@ -314,7 +317,9 @@
if (!Consumer)
goto failure;
- CI.getASTContext().setASTMutationListener(Consumer->GetASTMutationListener());
+ // FIXME: should not overwrite ASTMutationListener when parsing model files?
+ if (!isModelParsingAction())
+ CI.getASTContext().setASTMutationListener(Consumer->GetASTMutationListener());
if (!CI.getPreprocessorOpts().ChainedIncludes.empty()) {
// Convert headers to PCH and chain them.
Index: lib/StaticAnalyzer/Core/AnalysisManager.cpp
===================================================================
--- lib/StaticAnalyzer/Core/AnalysisManager.cpp (revision 215430)
+++ lib/StaticAnalyzer/Core/AnalysisManager.cpp (working copy)
@@ -20,13 +20,16 @@
StoreManagerCreator storemgr,
ConstraintManagerCreator constraintmgr,
CheckerManager *checkerMgr,
- AnalyzerOptions &Options)
+ AnalyzerOptions &Options,
+ CodeInjector *injector)
: AnaCtxMgr(Options.UnoptimizedCFG,
/*AddImplicitDtors=*/true,
/*AddInitializers=*/true,
Options.includeTemporaryDtorsInCFG(),
Options.shouldSynthesizeBodies(),
- Options.shouldConditionalizeStaticInitializers()),
+ Options.shouldConditionalizeStaticInitializers(),
+ /*addCXXNewAllocator=*/true,
+ injector),
Ctx(ctx),
Diags(diags),
LangOpts(lang),
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (revision 215430)
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (working copy)
@@ -18,6 +18,7 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/ParentMap.h"
+#include "clang/Analysis/CodeInjector.h"
#include "clang/Analysis/Analyses/LiveVariables.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/CallGraph.h"
@@ -33,6 +34,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
#include "clang/StaticAnalyzer/Frontend/CheckerRegistration.h"
+#include "clang/Frontend/CompilerInstance.h"
#include "llvm/ADT/DepthFirstIterator.h"
#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/ADT/SmallPtrSet.h"
@@ -42,6 +44,7 @@
#include "llvm/Support/Program.h"
#include "llvm/Support/Timer.h"
#include "llvm/Support/raw_ostream.h"
+#include "ModelInjector.h"
#include <memory>
#include <queue>
@@ -157,6 +160,7 @@
const std::string OutDir;
AnalyzerOptionsRef Opts;
ArrayRef<std::string> Plugins;
+ CodeInjector *Injector;
/// \brief Stores the declarations from the local translation unit.
/// Note, we pre-compute the local declarations at parse time as an
@@ -184,9 +188,10 @@
AnalysisConsumer(const Preprocessor& pp,
const std::string& outdir,
AnalyzerOptionsRef opts,
- ArrayRef<std::string> plugins)
- : RecVisitorMode(0), RecVisitorBR(nullptr),
- Ctx(nullptr), PP(pp), OutDir(outdir), Opts(opts), Plugins(plugins) {
+ ArrayRef<std::string> plugins,
+ CodeInjector *injector)
+ : RecVisitorMode(0), RecVisitorBR(nullptr), Ctx(nullptr), PP(pp),
+ OutDir(outdir), Opts(opts), Plugins(plugins), Injector(injector) {
DigestAnalyzerOptions();
if (Opts->PrintStats) {
llvm::EnableStatistics();
@@ -286,6 +291,7 @@
Ctx = &Context;
checkerMgr.reset(createCheckerManager(*Opts, PP.getLangOpts(), Plugins,
PP.getDiagnostics()));
+
Mgr.reset(new AnalysisManager(*Ctx,
PP.getDiagnostics(),
PP.getLangOpts(),
@@ -293,7 +299,8 @@
CreateStoreMgr,
CreateConstraintMgr,
checkerMgr.get(),
- *Opts));
+ *Opts,
+ Injector));
}
/// \brief Store the top level decls in the set to be processed later on.
@@ -688,13 +695,14 @@
//===----------------------------------------------------------------------===//
std::unique_ptr<AnalysisASTConsumer>
-ento::CreateAnalysisConsumer(const Preprocessor &pp, const std::string &outDir,
- AnalyzerOptionsRef opts,
- ArrayRef<std::string> plugins) {
+ento::CreateAnalysisConsumer(CompilerInstance &CI) {
// Disable the effects of '-Werror' when using the AnalysisConsumer.
- pp.getDiagnostics().setWarningsAsErrors(false);
+ CI.getPreprocessor().getDiagnostics().setWarningsAsErrors(false);
- return llvm::make_unique<AnalysisConsumer>(pp, outDir, opts, plugins);
+ return llvm::make_unique<AnalysisConsumer>(
+ CI.getPreprocessor(), CI.getFrontendOpts().OutputFile,
+ CI.getAnalyzerOpts(), CI.getFrontendOpts().Plugins,
+ new ModelInjector(CI));
}
//===----------------------------------------------------------------------===//
Index: lib/StaticAnalyzer/Frontend/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Frontend/CMakeLists.txt (revision 215430)
+++ lib/StaticAnalyzer/Frontend/CMakeLists.txt (working copy)
@@ -7,7 +7,9 @@
add_clang_library(clangStaticAnalyzerFrontend
AnalysisConsumer.cpp
CheckerRegistration.cpp
+ ModelConsumer.cpp
FrontendActions.cpp
+ ModelInjector.cpp
LINK_LIBS
clangAST
Index: lib/StaticAnalyzer/Frontend/FrontendActions.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/FrontendActions.cpp (revision 215430)
+++ lib/StaticAnalyzer/Frontend/FrontendActions.cpp (working copy)
@@ -8,16 +8,21 @@
//===----------------------------------------------------------------------===//
#include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
-#include "clang/Frontend/CompilerInstance.h"
#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/StaticAnalyzer/Frontend/ModelConsumer.h"
using namespace clang;
using namespace ento;
std::unique_ptr<ASTConsumer>
AnalysisAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
- return CreateAnalysisConsumer(CI.getPreprocessor(),
- CI.getFrontendOpts().OutputFile,
- CI.getAnalyzerOpts(),
- CI.getFrontendOpts().Plugins);
+ return CreateAnalysisConsumer(CI);
}
+ParseModelFileAction::ParseModelFileAction(llvm::StringMap<Stmt *> &Bodies)
+ : Bodies(Bodies) {}
+
+std::unique_ptr<ASTConsumer>
+ParseModelFileAction::CreateASTConsumer(CompilerInstance &CI,
+ StringRef InFile) {
+ return llvm::make_unique<ModelConsumer>(Bodies);
+}
Index: lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/ModelConsumer.cpp (revision 0)
+++ lib/StaticAnalyzer/Frontend/ModelConsumer.cpp (working copy)
@@ -0,0 +1,42 @@
+//===--- ModelConsumer.cpp - ASTConsumer for consuming model files --------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief This file implements an ASTConsumer for consuming model files.
+///
+/// This ASTConsumer handles the AST of a parsed model file. All top level
+/// function definitions will be collected from that model file for later
+/// retrieval during the static analysis. The body of these functions will not
+/// be injected into the ASTUnit of the analyzed translation unit. It will be
+/// available through the BodyFarm which is utilized by the AnalysisDeclContext
+/// class.
+///
+//===----------------------------------------------------------------------===//
+
+#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) {}
+
+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()));
+ }
+ }
+ return true;
+}
\ No newline at end of file
Index: lib/StaticAnalyzer/Frontend/ModelInjector.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/ModelInjector.cpp (revision 0)
+++ lib/StaticAnalyzer/Frontend/ModelInjector.cpp (working copy)
@@ -0,0 +1,114 @@
+//===-- ModelInjector.cpp ---------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ModelInjector.h"
+
+#include <string>
+#include <utility>
+
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
+#include "clang/Serialization/ASTReader.h"
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/AST/Decl.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/Support/CrashRecoveryContext.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/ADT/STLExtras.h"
+
+using namespace clang;
+using namespace ento;
+
+ModelInjector::ModelInjector(CompilerInstance &CI) : CI(CI) {}
+
+Stmt *ModelInjector::getBody(const FunctionDecl *D) {
+ onBodySynthesis(D);
+ return Bodies[D->getName()];
+}
+
+Stmt *ModelInjector::getBody(const ObjCMethodDecl *D) {
+ onBodySynthesis(D);
+ return Bodies[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)
+ 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;
+ return;
+ }
+
+ IntrusiveRefCntPtr<CompilerInvocation> Invocation(
+ new CompilerInvocation(CI.getInvocation()));
+
+ FrontendOptions &FrontendOpts = Invocation->getFrontendOpts();
+ InputKind IK = IK_CXX; // FIXME
+ FrontendOpts.Inputs.clear();
+ FrontendOpts.Inputs.push_back(FrontendInputFile(fileName, IK));
+ FrontendOpts.DisableFree = true;
+
+ Invocation->getDiagnosticOpts().VerifyDiagnostics = 0;
+
+ // Modules are parsed by a separate CompilerInstance, so this code mimics that
+ // behavior for models
+ CompilerInstance Instance;
+ Instance.setInvocation(&*Invocation);
+ Instance.createDiagnostics(
+ new ForwardingDiagnosticConsumer(CI.getDiagnosticClient()),
+ /*ShouldOwnClient=*/true);
+
+ Instance.setVirtualFileSystem(&CI.getVirtualFileSystem());
+
+ // The instance wants to take ownership, however DisableFree frontend option
+ // 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.setSema(&CI.getSema());
+
+ ParseModelFileAction parseModelFile(Bodies);
+
+ const unsigned ThreadStackSize = 8 << 20;
+ llvm::CrashRecoveryContext CRC;
+
+ CRC.RunSafelyOnThread([&]() { Instance.ExecuteAction(parseModelFile); },
+ ThreadStackSize);
+
+ 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);
+}
Index: lib/StaticAnalyzer/Frontend/ModelInjector.h
===================================================================
--- lib/StaticAnalyzer/Frontend/ModelInjector.h (revision 0)
+++ lib/StaticAnalyzer/Frontend/ModelInjector.h (working copy)
@@ -0,0 +1,75 @@
+//===-- ModelInjector.h -----------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief This file defines the clang::ento::ModelInjector class which implements the
+/// clang::CodeInjector interface. This class is responsible for injecting
+/// function definitions that were synthesized from model files.
+///
+/// Model files allow definitions of functions to be lazily constituted for functions
+/// which lack bodies in the original source code. This allows the analyzer
+/// to more precisely analyze code that calls such functions, analyzing the
+/// artificial definitions (which typically approximate the semantics of the
+/// called function) when called by client code. These definitions are
+/// reconstituted lazily, on-demand, by the static analyzer engine.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_SA_FRONTEND_MODELINJECTOR_H
+#define LLVM_CLANG_SA_FRONTEND_MODELINJECTOR_H
+
+#include <map>
+#include <vector>
+#include <memory>
+
+#include "clang/Analysis/CodeInjector.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+
+class CompilerInstance;
+class ASTUnit;
+class ASTReader;
+class NamedDecl;
+class Module;
+
+namespace ento {
+class ModelInjector : public CodeInjector {
+public:
+ ModelInjector(CompilerInstance &CI);
+ Stmt *getBody(const FunctionDecl *D);
+ Stmt *getBody(const ObjCMethodDecl *D);
+
+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;
+};
+}
+}
+
+#endif
\ No newline at end of file
Index: test/Analysis/Inputs/Models/modeledFunction.model
===================================================================
--- test/Analysis/Inputs/Models/modeledFunction.model (revision 0)
+++ test/Analysis/Inputs/Models/modeledFunction.model (working copy)
@@ -0,0 +1,3 @@
+void modelled(intptr p) {
+ ++*p;
+}
\ No newline at end of file
Index: test/Analysis/Inputs/Models/notzero.model
===================================================================
--- test/Analysis/Inputs/Models/notzero.model (revision 0)
+++ test/Analysis/Inputs/Models/notzero.model (working copy)
@@ -0,0 +1,3 @@
+bool notzero(int i) {
+ return i != 0;
+}
\ No newline at end of file
Index: test/Analysis/model-file.cpp
===================================================================
--- test/Analysis/model-file.cpp (revision 0)
+++ test/Analysis/model-file.cpp (working copy)
@@ -0,0 +1,288 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-config faux-bodies=true,model-path=%S/Inputs/Models -analyzer-output=plist-multi-file -verify %s -o %t
+// RUN: FileCheck --input-file=%t %s
+
+typedef int* intptr;
+
+// This function is modeled and the p pointer is dereferenced in the model
+// function and there is no function definition available. The modeled
+// function can use any types that are available in the original translation
+// unit, for example intptr in this case.
+void modeledFunction(intptr p);
+
+// This function is modeled and returns true if the parameter is not zero
+// and there is no function definition available.
+bool notzero(int i);
+
+// This functions is not modeled and there is no function definition.
+// available
+bool notzero_notmodeled(int i);
+
+int main() {
+ // There is a nullpointer dereference inside this function.
+ modeledFunction(0);
+
+ int p = 0;
+ if (notzero(p)) {
+ // It is known that p != 0 because of the information provided by the
+ // model of the notzero function.
+ int j = 5 / p;
+ }
+
+ if (notzero_notmodeled(p)) {
+ // There is no information about the value of p, because
+ // notzero_notmodeled is not modeled and the function definition
+ // is not available.
+ int j = 5 / p; // expected-warning {{Division by zero}}
+ }
+
+ return 0;
+}
+
+// CHECK: <key>diagnostics</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>path</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>control</string>
+// CHECK-NEXT: <key>edges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>start</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>22</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>22</integer>
+// CHECK-NEXT: <key>col</key><integer>17</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>end</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>24</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>24</integer>
+// CHECK-NEXT: <key>col</key><integer>5</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>event</string>
+// CHECK-NEXT: <key>location</key>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>24</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <key>ranges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>24</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>24</integer>
+// CHECK-NEXT: <key>col</key><integer>7</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>depth</key><integer>0</integer>
+// CHECK-NEXT: <key>extended_message</key>
+// CHECK-NEXT: <string>'p' initialized to 0</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>'p' initialized to 0</string>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>control</string>
+// CHECK-NEXT: <key>edges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>start</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>24</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>24</integer>
+// CHECK-NEXT: <key>col</key><integer>5</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>end</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>25</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>25</integer>
+// CHECK-NEXT: <key>col</key><integer>4</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>control</string>
+// CHECK-NEXT: <key>edges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>start</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>25</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>25</integer>
+// CHECK-NEXT: <key>col</key><integer>4</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>end</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>31</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>31</integer>
+// CHECK-NEXT: <key>col</key><integer>4</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>control</string>
+// CHECK-NEXT: <key>edges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>start</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>31</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>31</integer>
+// CHECK-NEXT: <key>col</key><integer>4</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>end</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>31</integer>
+// CHECK-NEXT: <key>col</key><integer>7</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>31</integer>
+// CHECK-NEXT: <key>col</key><integer>24</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>control</string>
+// CHECK-NEXT: <key>edges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>start</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>31</integer>
+// CHECK-NEXT: <key>col</key><integer>7</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>31</integer>
+// CHECK-NEXT: <key>col</key><integer>24</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>end</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>35</integer>
+// CHECK-NEXT: <key>col</key><integer>15</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>35</integer>
+// CHECK-NEXT: <key>col</key><integer>15</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>event</string>
+// CHECK-NEXT: <key>location</key>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>35</integer>
+// CHECK-NEXT: <key>col</key><integer>15</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <key>ranges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>35</integer>
+// CHECK-NEXT: <key>col</key><integer>13</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>35</integer>
+// CHECK-NEXT: <key>col</key><integer>17</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>depth</key><integer>0</integer>
+// CHECK-NEXT: <key>extended_message</key>
+// CHECK-NEXT: <string>Division by zero</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Division by zero</string>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>description</key><string>Division by zero</string>
+// CHECK-NEXT: <key>category</key><string>Logic error</string>
+// CHECK-NEXT: <key>type</key><string>Division by zero</string>
+// CHECK-NEXT: <key>issue_context_kind</key><string>function</string>
+// CHECK-NEXT: <key>issue_context</key><string>main</string>
+// CHECK-NEXT: <key>issue_hash</key><string>15</string>
+// CHECK-NEXT: <key>location</key>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>35</integer>
+// CHECK-NEXT: <key>col</key><integer>15</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
\ No newline at end of file
Index: lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- lib/StaticAnalyzer/Core/BugReporter.cpp (revision 215430)
+++ lib/StaticAnalyzer/Core/BugReporter.cpp (working copy)
@@ -3087,8 +3087,15 @@
for (ArrayRef<BugReport*>::iterator I = bugReports.begin(),
E = bugReports.end(); I != E; ++I) {
if ((*I)->isValid()) {
+ const ExplodedNode *E = (*I)->getErrorNode();
+ const LocationContext *LCtx = E->getLocationContext();
+
+ // Truncate path that ends up in a modelled function.
+ // if (LCtx->getAnalysisDeclContext()->isBodyAutosynthesized())
+ // continue;
+
HasValid = true;
- errorNodes.push_back((*I)->getErrorNode());
+ errorNodes.push_back(E);
} else {
// Keep the errorNodes list in sync with the bugReports list.
HasInvalid = true;
@@ -3246,18 +3253,6 @@
void BugReporter::emitReport(BugReport* R) {
// To guarantee memory release.
std::unique_ptr<BugReport> UniqueR(R);
-
- // Defensive checking: throw the bug away if it comes from a BodyFarm-
- // generated body. We do this very early because report processing relies
- // on the report's location being valid.
- // FIXME: Valid bugs can occur in BodyFarm-generated bodies, so really we
- // need to just find a reasonable location like we do later on with the path
- // pieces.
- if (const ExplodedNode *E = R->getErrorNode()) {
- const LocationContext *LCtx = E->getLocationContext();
- if (LCtx->getAnalysisDeclContext()->isBodyAutosynthesized())
- return;
- }
bool ValidSourceLoc = R->getLocation(getSourceManager()).isValid();
assert(ValidSourceLoc);
Index: clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tidy/ClangTidy.cpp (revision 215430)
+++ clang-tidy/ClangTidy.cpp (working copy)
@@ -241,9 +241,7 @@
AnalyzerOptions->AnalyzeNestedBlocks = true;
AnalyzerOptions->eagerlyAssumeBinOpBifurcation = true;
std::unique_ptr<ento::AnalysisASTConsumer> AnalysisConsumer =
- ento::CreateAnalysisConsumer(
- Compiler.getPreprocessor(), Compiler.getFrontendOpts().OutputFile,
- AnalyzerOptions, Compiler.getFrontendOpts().Plugins);
+ ento::CreateAnalysisConsumer(Compiler);
AnalysisConsumer->AddDiagnosticConsumer(
new AnalyzerDiagnosticConsumer(Context));
Consumers.push_back(std::move(AnalysisConsumer));
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
