Hi Ted,
It looks like I did a fatal mistake. To reduce compile and link times and
make the test suite faster to be able to iterate faster during the
development I used the RelWithDebInfo build type. Unfortunately it looks
like the asserts are turned off in this build type. However I did my best
to workaround the problems.
On 14 August 2014 09:13, Ted Kremenek <[email protected]> wrote:
> Hi Gábor,
>
> Great! I'm seeing crashes in diagnostic emission when running the tests:
>
> test: clang/test/Analysis/inlining/path-notes.m
> Assertion failed: (Loc.isValid()), function PathDiagnosticLocation, file
> clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h, line
> 186.
>
>
I did not notice my patch was the reason that made this patch fail. The
reason is, when a function body is synthesised from a hand built AST it has
no valid source locations and it asserts when such locations is reported. I
worked around this issue, so one now able to decide whether the code that
is synthesised is from a model file or hand written AST.
> I'm also seeing assertions when testing the model file:
>
> test: clang/test/Analysis/model-file.cpp
> Assertion failed: (MainFileID.isInvalid() && "MainFileID already set!"),
> function setMainFileID, file clang/include/clang/Basic/SourceManager.h,
> line 757.
>
>
The parsing of a file start with the preprocessor entering to the main
file. To be able to start parsing it is important to temporarily modify the
the main file ID, so I had no better idea than removing that assert.
However there were several other ones that were the result of reusing the
preprocessor of the main source. It was important to reuse that
preprocessor, because the identifier table from that preprocessor is
necessary to parse the model file and I did not found any good method to
merge those information into a new preprocessor instance. So I created two
new methods, one to prepare a preprocessor to parse a model file and one to
clean it up after the parsing of a such file.
> Are you not seeing these?
>
>
Sorry for my oversight about the turned off asserts, I guess we need to
iterate a bit more on the patch.
> Cheers,
> Ted
>
>
Thanks,
Gábor
> On Aug 13, 2014, at 11:35 PM, Gábor Horváth <[email protected]> wrote:
>
> 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>
>>
>>
>>
> <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>
@@ -143,6 +145,14 @@
/// \sa getBody
bool isBodyAutosynthesized() const;
+ /// \brief Checks if the body of the Decl is generated by the BodyFarm from a
+ /// model file.
+ ///
+ /// Note, the lookup is not free. We are going to call getBody behind
+ /// the scenes.
+ /// \sa getBody
+ bool isBodyAutosynthesizedFromModelFile() const;
+
CFG *getCFG();
CFGStmtMap *getCFGStmtMap();
@@ -398,6 +408,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 +424,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/Basic/SourceManager.h
===================================================================
--- include/clang/Basic/SourceManager.h (revision 215558)
+++ include/clang/Basic/SourceManager.h (working copy)
@@ -754,7 +754,6 @@
/// \brief Set the file ID for the main source file.
void setMainFileID(FileID FID) {
- assert(MainFileID.isInvalid() && "MainFileID already set!");
MainFileID = FID;
}
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/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h (revision 215558)
+++ include/clang/Lex/Preprocessor.h (working copy)
@@ -194,6 +194,10 @@
/// with this preprocessor.
PragmaNamespace *PragmaHandlers;
+ /// \brief Pragma handlers of the original source is stored here during the
+ /// parsing of a model file.
+ PragmaNamespace *PragmaHandlersBackup;
+
/// \brief Tracks all of the comment handlers that the client registered
/// with this preprocessor.
std::vector<CommentHandler *> CommentHandlers;
@@ -464,6 +468,17 @@
/// lifetime of the preprocessor.
void Initialize(const TargetInfo &Target);
+ /// \brief Initialize the preprocessor to parse a model file
+ ///
+ /// To parse model files the preprocessor of the original source is reused to
+ /// preserver the identifier table. However to avoid some duplicate
+ /// information in the preprocessor some cleanup is needed before it is used
+ /// to parse model files. This method does that cleanup.
+ void InitializeForModelFile();
+
+ /// \brief Cleanup after model file parsing
+ void FinalizeForModelFile();
+
/// \brief Retrieve the preprocessor options used to initialize this
/// preprocessor.
PreprocessorOptions &getPreprocessorOpts() const { return *PPOpts; }
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;
}
@@ -128,6 +129,13 @@
return Tmp;
}
+bool AnalysisDeclContext::isBodyAutosynthesizedFromModelFile() const {
+ bool Tmp;
+ Stmt *Body = getBody(Tmp);
+ return Tmp && Body->getLocStart().isValid();
+}
+
+
const ImplicitParamDecl *AnalysisDeclContext::getSelfDecl() const {
if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D))
return MD->getSelfDecl();
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/Lex/Preprocessor.cpp
===================================================================
--- lib/Lex/Preprocessor.cpp (revision 215558)
+++ lib/Lex/Preprocessor.cpp (working copy)
@@ -187,6 +187,25 @@
HeaderInfo.setTarget(Target);
}
+void Preprocessor::InitializeForModelFile() {
+ NumEnteredSourceFiles = 0;
+
+ // Reset pragmas
+ PragmaHandlersBackup = PragmaHandlers;
+ PragmaHandlers = new PragmaNamespace(StringRef());
+ RegisterBuiltinPragmas();
+
+ // Reset PredefinesFileID
+ PredefinesFileID = FileID();
+}
+
+void Preprocessor::FinalizeForModelFile() {
+ NumEnteredSourceFiles = 1;
+
+ delete PragmaHandlers;
+ PragmaHandlers = PragmaHandlersBackup;
+}
+
void Preprocessor::setPTHManager(PTHManager* pm) {
PTH.reset(pm);
FileMgr.addStatCache(PTH->createStatCache());
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,119 @@
+//===-- 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.getDiagnostics().setSourceManager(&SM);
+
+ 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.getPreprocessor().InitializeForModelFile();
+
+ ParseModelFileAction parseModelFile(Bodies);
+
+ const unsigned ThreadStackSize = 8 << 20;
+ llvm::CrashRecoveryContext CRC;
+
+ CRC.RunSafelyOnThread([&]() { Instance.ExecuteAction(parseModelFile); },
+ ThreadStackSize);
+
+ Instance.getPreprocessor().FinalizeForModelFile();
+
+ Instance.resetAndLeakSourceManager();
+ Instance.resetAndLeakFileManager();
+ Instance.resetAndLeakPreprocessor();
+
+ // The preprocessor enters to the main file id when parsing is started, so
+ // the main file id is changed to the model file during parsing and it needs
+ // to be reseted to the former main file id after parsing of the model file
+ // is done.
+ SM.setMainFileID(mainFileID);
+}
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;
@@ -3247,15 +3254,15 @@
// 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())
+ const AnalysisDeclContext *DeclCtx =
+ E->getLocationContext()->getAnalysisDeclContext();
+ // The source of autosynthesized body can be handcrafted AST or a model
+ // file. The locations from handcrafted ASTs have no valid source locations
+ // and have to be discarded. Locations from model files should be preserved
+ // for processing and reporting.
+ if (DeclCtx->isBodyAutosynthesized() &&
+ !DeclCtx->isBodyAutosynthesizedFromModelFile())
return;
}
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
