Hi Ted,
The ModelInjector is no longer instantiated when the modelpath is not set.
Thanks,
Gábor
On 13 August 2014 07:11, Ted Kremenek <[email protected]> wrote:
> Thanks Gábor. This looks great.
>
> One last thing that occurred to me is that we should not create a
> ModelInjector at all unless the model-path is specified. This allows us to
> preserve the existing behavior in the analyzer while we continue to evolve
> this new functionality.
>
> Specifically:
>
> 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));
> }
>
>
> We can query 'opts' to see if model-path is empty; if it is we can pass
> nullptr instead of 'new ModelInjector(CI)'.
>
> On Aug 12, 2014, at 1:22 AM, Gábor Horváth <[email protected]> wrote:
>
> 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
> <api_modeling.patch><bugreporter.patch><clangTidy.patch>
>
>
>
Index: include/clang/Analysis/AnalysisContext.h
===================================================================
--- include/clang/Analysis/AnalysisContext.h (revision 215558)
+++ 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 215558)
+++ 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 215558)
+++ 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 215558)
+++ 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 215558)
+++ include/clang/StaticAnalyzer/Frontend/FrontendActions.h (working copy)
@@ -11,9 +11,13 @@
#define LLVM_CLANG_STATICANALYZER_FRONTEND_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 215558)
+++ 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 215558)
+++ 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 215558)
+++ 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 215558)
+++ 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 215558)
+++ 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 215558)
+++ 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 215558)
+++ 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 215558)
+++ 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,17 @@
//===----------------------------------------------------------------------===//
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);
+ 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);
}
//===----------------------------------------------------------------------===//
Index: lib/StaticAnalyzer/Frontend/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Frontend/CMakeLists.txt (revision 215558)
+++ 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 215558)
+++ 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 215558)
+++ 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 215559)
+++ 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
