| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
| |
That I really should've done in rC348031.
llvm-svn: 348044
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In earlier patches regarding AnalyzerOptions, a lot of effort went into
gathering all config options, and changing the interface so that potential
misuse can be eliminited.
Up until this point, AnalyzerOptions only evaluated an option when it was
querried. For example, if we had a "-no-false-positives" flag, AnalyzerOptions
would store an Optional field for it that would be None up until somewhere in
the code until the flag's getter function is called.
However, now that we're confident that we've gathered all configs, we can
evaluate off of them before analysis, so we can emit a error on invalid input
even if that prticular flag will not matter in that particular run of the
analyzer. Another very big benefit of this is that debug.ConfigDumper will now
show the value of all configs every single time.
Also, almost all options related class have a similar interface, so uniformity
is also a benefit.
The implementation for errors on invalid input will be commited shorty.
Differential Revision: https://reviews.llvm.org/D53692
llvm-svn: 348031
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
I'm in the process of refactoring AnalyzerOptions. The main motivation behind
here is to emit warnings if an invalid -analyzer-config option is given from
the command line, and be able to list them all.
In this patch, I found some flags that should've been used as checker options,
or have absolutely no mention of in AnalyzerOptions, or are nonexistent.
- NonLocalizedStringChecker now uses its "AggressiveReport" flag as a checker
option
- lib/StaticAnalyzer/Frontend/ModelInjector.cpp now accesses the "model-path"
option through a getter in AnalyzerOptions
- -analyzer-config path-diagnostics-alternate=false is not a thing, I removed it,
- lib/StaticAnalyzer/Checkers/AllocationDiagnostics.cpp and
lib/StaticAnalyzer/Checkers/AllocationDiagnostics.h are weird, they actually
only contain an option getter. I deleted them, and fixed RetainCountChecker
to get it's "leak-diagnostics-reference-allocation" option as a checker option,
- "region-store-small-struct-limit" has a proper getter now.
Differential Revision: https://reviews.llvm.org/D53276
llvm-svn: 345985
|
|
|
|
|
|
| |
Differential Revision: https://reviews.llvm.org/D51251
llvm-svn: 340963
|
|
|
|
| |
llvm-svn: 338756
|
|
|
|
|
|
| |
To be investigated.
llvm-svn: 336756
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before C++17 copy elision was optional, even if the elidable copy/move
constructor had arbitrary side effects. The elidable constructor is present
in the AST, but marked as elidable.
In these cases CFG now contains additional information that allows its clients
to figure out if a temporary object is only being constructed so that to pass
it to an elidable constructor. If so, it includes a reference to the elidable
constructor's construction context, so that the client could elide the
elidable constructor and construct the object directly at its final destination.
Differential Revision: https://reviews.llvm.org/D47616
llvm-svn: 335795
|
|
|
|
|
|
|
|
|
|
|
|
| |
When a temporary is constructed with a proper construction context, it should
be safe to inline the destructor. We have added suppressions for some of the
common false positives caused by such inlining, so there should be - and from my
observations there indeed is - more benefit than harm from enabling destructor
inlining.
Differential Revision: https://reviews.llvm.org/D44721
llvm-svn: 328258
|
|
|
|
|
|
|
|
|
|
|
| |
This patch adds two new CFG elements CFGScopeBegin and CFGScopeEnd that indicate
when a local scope begins and ends respectively. We use first VarDecl declared
in a scope to uniquely identify it and add CFGScopeBegin and CFGScopeEnd elements
into corresponding basic blocks.
Differential Revision: https://reviews.llvm.org/D16403
llvm-svn: 327258
|
|
|
|
|
|
|
|
|
|
|
|
| |
Don't enable c++-temp-dtor-inlining by default yet, due to this reference
counting pointe problem.
Otherwise the new mode seems stable and allows us to incrementally fix C++
problems in much less hacky ways.
Differential Revision: https://reviews.llvm.org/D43804
llvm-svn: 326461
|
|
|
|
|
|
|
|
|
|
|
| |
Originally submitted as r326323 and r326324.
Reverted in r326432.
Reverting the commit was a mistake.
The breakage was due to invalid build files in our internal buildsystem,
CMakeLists did not have any cyclic dependencies.
llvm-svn: 326439
|
|
|
|
|
|
|
|
|
|
| |
Also revert "[analyzer] Fix a compiler warning"
This reverts commits r326323 and r326324.
Reason: the commits introduced a cyclic dependency in the build graph.
This happens to work with cmake, but breaks out internal integrate.
llvm-svn: 326432
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The aim of this patch is to be minimal to enable incremental development of
the feature on the top of the tree. This patch should be an NFC when the
feature is turned off. It is turned off by default and still considered as
experimental.
Technical details are available in the EuroLLVM Talk:
http://llvm.org/devmtg/2017-03//2017/02/20/accepted-sessions.html#7
Note that the initial prototype was done by A. Sidorin et al.: http://lists.llvm.org/pipermail/cfe-dev/2015-October/045730.html
Contributions to the measurements and the new version of the code: Peter Szecsi, Zoltan Gera, Daniel Krupp, Kareem Khazem.
Differential Revision: https://reviews.llvm.org/D30691
llvm-svn: 326323
|
|
|
|
|
|
|
|
|
|
| |
on coverage
After the investigation it seems safe to flip the switch.
Differential Revision: https://reviews.llvm.org/D43782
llvm-svn: 326157
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Inline them if possible - a separate flag is added to control this.
The whole thing is under the cfg-temporary-dtors flag, off by default so far.
Temporary destructors are called at the end of full-expression. If the
temporary is lifetime-extended, automatic destructors kick in instead,
which are not addressed in this patch, and normally already work well
modulo the overally broken support for lifetime extension.
The patch operates by attaching the this-region to the CXXBindTemporaryExpr in
the program state, and then recalling it during destruction that was triggered
by that CXXBindTemporaryExpr. It has become possible because
CXXBindTemporaryExpr is part of the construction context since r325210.
Differential revision: https://reviews.llvm.org/D43104
llvm-svn: 325282
|
|
|
|
|
|
|
|
|
|
| |
It was introduced when two -analyzer-config options were added almost
simultaneously in r324793 and r324668 and the option count was not
rebased correctly in the tests.
Fixes the buildbots.
llvm-svn: 324801
|
|
|
|
|
|
| |
Differential Revision: https://reviews.llvm.org/D43131
llvm-svn: 324793
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch adds a new CFGStmt sub-class, CFGConstructor, which replaces
the regular CFGStmt with CXXConstructExpr in it whenever the CFG has additional
information to provide regarding what sort of object is being constructed.
It is useful for figuring out what memory is initialized in client of the
CFG such as the Static Analyzer, which do not operate by recursive AST
traversal, but instead rely on the CFG to provide all the information when they
need it. Otherwise, the statement that triggers the construction and defines
what memory is being initialized would normally occur after the
construct-expression, and the client would need to peek to the next CFG element
or use statement parent map to understand the necessary facts about
the construct-expression.
As a proof of concept, CFGConstructors are added for new-expressions
and the respective test cases are provided to demonstrate how it works.
For now, the only additional data contained in the CFGConstructor element is
the "trigger statement", such as new-expression, which is the parent of the
constructor. It will be significantly expanded in later commits. The additional
data is organized as an auxiliary structure - the "construction context",
which is allocated separately from the CFGElement.
Differential Revision: https://reviews.llvm.org/D42672
llvm-svn: 324668
|
|
|
|
|
|
| |
Differential Revision: https://reviews.llvm.org/D42774
llvm-svn: 324049
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch introduces a new CFG element CFGLoopExit that indicate when a loop
ends. It does not deal with returnStmts yet (left it as a TODO).
It hidden behind a new analyzer-config flag called cfg-loopexit (false by
default).
Test cases added.
The main purpose of this patch right know is to make loop unrolling and loop
widening easier and more efficient. However, this information can be useful for
future improvements in the StaticAnalyzer core too.
Differential Revision: https://reviews.llvm.org/D35668
llvm-svn: 311235
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This feature allows the analyzer to consider loops to completely unroll.
New requirements/rules (for unrolling) can be added easily via ASTMatchers.
Right now it is hidden behind a flag, the aim is to find the correct heuristic
and create a solution which results higher coverage % and more precise
analysis, thus can be enabled by default.
Right now the blocks which belong to an unrolled loop are marked by the
LoopVisitor which adds them to the ProgramState.
Then whenever we encounter a CFGBlock in the processCFGBlockEntrance which is
marked then we skip its investigating. That means, it won't be considered to
be visited more than the maximal bound for visiting since it won't be checked.
llvm-svn: 309006
|
|
|
|
|
|
|
|
|
|
| |
bound option"
Revert r308561 and r308558.
Clang-ppc64be-linux seems to crash while running the test cases.
llvm-svn: 308592
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
requirements/rules (for unrolling) can be added easily via ASTMatchers.
The current implementation is hidden behind a flag.
Right now the blocks which belong to an unrolled loop are marked by the
LoopVisitor which adds them to the ProgramState. Then whenever we encounter a
CFGBlock in the processCFGBlockEntrance which is marked then we skip its
investigating. That means, it won't be considered to be visited more than the
maximal bound for visiting since it won't be checked.
Differential Revision: https://reviews.llvm.org/D34260
llvm-svn: 308558
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Summary:
This mimics the implementation for the implicit destructors. The
generation of this scope leaving elements is hidden behind
a flag to the CFGBuilder, thus it should not affect existing code.
Currently, I'm missing a test (it's implicitly tested by the clang-tidy
lifetime checker that I'm proposing).
I though about a test using debug.DumpCFG, but then I would
have to add an option to StaticAnalyzer/Core/AnalyzerOptions
to enable the scope leaving CFGElement,
which would only be useful to that particular test.
Any other ideas how I could make a test for this feature?
Reviewers: krememek, jordan_rose
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D15031
llvm-svn: 307759
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This makes the analyzer around 10% slower by default,
allowing it to find deeper bugs.
Default values for the following -analyzer-config change:
max-nodes: 150000 -> 225000;
max-inlinable-size: 50 -> 100.
rdar://problem/32539666
Differential Revision: https://reviews.llvm.org/D34277
llvm-svn: 305900
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
constraint managers
Summary: Replace calls to %clang/%clang_cc1 with %clang_analyze_cc1 when invoking static analyzer, and perform runtime substitution to select the appropriate constraint manager, per D28952.
Reviewers: xazax.hun, NoQ, zaks.anna, dcoughlin
Subscribers: mgorny, rgov, mikhail.ramalho, a.sidorin, cfe-commits
Differential Revision: https://reviews.llvm.org/D30373
llvm-svn: 296895
|
|
|
|
|
|
|
|
| |
multiple constraint managers"
This reverts commit ea36f1406e1f36bf456c3f3929839b024128e468.
llvm-svn: 296841
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
constraint managers
Summary: Replace calls to %clang/%clang_cc1 with %clang_analyze_cc1 when invoking static analyzer, and perform runtime substitution to select the appropriate constraint manager, per D28952.
Reviewers: xazax.hun, NoQ, zaks.anna, dcoughlin
Subscribers: mgorny, rgov, mikhail.ramalho, a.sidorin, cfe-commits
Differential Revision: https://reviews.llvm.org/D30373
llvm-svn: 296837
|
|
|
|
|
|
|
|
| |
multiple constraint managers"
This reverts commit f93343c099fff646a2314cc7f4925833708298b1.
llvm-svn: 296836
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
constraint managers
Summary: Replace calls to %clang/%clang_cc1 with %clang_analyze_cc1 when invoking static analyzer, and perform runtime substitution to select the appropriate constraint manager, per D28952.
Reviewers: xazax.hun, NoQ, zaks.anna, dcoughlin
Subscribers: mgorny, rgov, mikhail.ramalho, a.sidorin, cfe-commits
Differential Revision: https://reviews.llvm.org/D30373
llvm-svn: 296835
|
|
|
|
|
|
|
|
| |
multiple constraint managers"
This reverts commit 1b28d0b10e1c8feccb971abb6ef7a18bee589830.
llvm-svn: 296422
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
constraint managers
Summary: Replace calls to %clang/%clang_cc1 with %clang_analyze_cc1 when invoking static analyzer, and perform runtime substitution to select the appropriate constraint manager, per D28952.
Reviewers: xazax.hun, NoQ, zaks.anna, dcoughlin
Subscribers: mgorny, rgov, mikhail.ramalho, a.sidorin, cfe-commits
Differential Revision: https://reviews.llvm.org/D30373
llvm-svn: 296414
|
|
|
|
|
|
|
|
| |
constraint managers"
This reverts commit 8e7780b9e59ddaad1800baf533058d2c064d4787.
llvm-svn: 296317
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
managers
Summary: Replace calls to %clang/%clang_cc1 with %clang_analyze_cc1 when invoking static analyzer, and perform runtime substitution to select the appropriate constraint manager, per D28952.
Reviewers: xazax.hun, NoQ, zaks.anna, dcoughlin
Subscribers: mgorny, rgov, mikhail.ramalho, a.sidorin, cfe-commits
Differential Revision: https://reviews.llvm.org/D30373
llvm-svn: 296312
|
|
|
|
|
|
| |
It was not the cause of the build bot failure.
llvm-svn: 251702
|
|
|
|
|
|
|
|
| |
Seems to be causing clang-cmake-mips build bot to fail (timeout)
http://lab.llvm.org:8011/builders/clang-cmake-mips/builds/10299
llvm-svn: 251697
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Summary:
Dear All,
We have been looking at the following problem, where any code after the constant bound loop is not analyzed because of the limit on how many times the same block is visited, as described in bugzillas #7638 and #23438. This problem is of interest to us because we have identified significant bugs that the checkers are not locating. We have been discussing a solution involving ranges as a longer term project, but I would like to propose a patch to improve the current implementation.
Example issue:
```
for (int i = 0; i < 1000; ++i) {...something...}
int *p = 0;
*p = 0xDEADBEEF;
```
The proposal is to go through the first and last iterations of the loop. The patch creates an exploded node for the approximate last iteration of constant bound loops, before the max loop limit / block visit limit is reached. It does this by identifying the variable in the loop condition and finding the value which is “one away” from the loop being false. For example, if the condition is (x < 10), then an exploded node is created where the value of x is 9. Evaluating the loop body with x = 9 will then result in the analysis continuing after the loop, providing x is incremented.
The patch passes all the tests, with some modifications to coverage.c, in order to make the ‘function_which_gives_up’ continue to give up, since the changes allowed the analysis to progress past the loop.
This patch does introduce possible false positives, as a result of not knowing the state of variables which might be modified in the loop. I believe that, as a user, I would rather have false positives after loops than do no analysis at all. I understand this may not be the common opinion and am interested in hearing your views. There are also issues regarding break statements, which are not considered. A more advanced implementation of this approach might be able to consider other conditions in the loop, which would allow paths leading to breaks to be analyzed.
Lastly, I have performed a study on large code bases and I think there is little benefit in having “max-loop” default to 4 with the patch. For variable bound loops this tends to result in duplicated analysis after the loop, and it makes little difference to any constant bound loop which will do more than a few iterations. It might be beneficial to lower the default to 2, especially for the shallow analysis setting.
Please let me know your opinions on this approach to processing constant bound loops and the patch itself.
Regards,
Sean Eveson
SN Systems - Sony Computer Entertainment Group
Reviewers: jordan_rose, krememek, xazax.hun, zaks.anna, dcoughlin
Subscribers: krememek, xazax.hun, cfe-commits
Differential Revision: http://reviews.llvm.org/D12358
llvm-svn: 251621
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
considers as large
Add an option (-analyzer-config min-blocks-for-inline-large=14) to control the function
size the inliner considers as large, in relation to "max-times-inline-large". The option
defaults to the original hard coded behaviour, which I believe should be adjustable with
the other inlining settings.
The analyzer-config test has been modified so that the analyzer will reach the
getMinBlocksForInlineLarge() method and store the result in the ConfigTable, to ensure it
is dumped by the debug checker.
A patch by Sean Eveson!
Differential Revision: http://reviews.llvm.org/D12406
llvm-svn: 247463
|
|
|
|
|
|
|
|
|
|
|
|
| |
The analyzer doesn't currently expect CFG blocks with terminators to be
empty, but this can happen when generating conditional destructors for
a complex logical expression, such as (a && (b || Temp{})). Moreover,
the branch conditions for these expressions are not persisted in the
state. Even for handling noreturn destructors this needs more work.
This reverts r186498.
llvm-svn: 186925
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Summary:
This patch enables ExprEndgine to reason about temporary object destructors.
However, these destructor calls are never inlined, since this feature is still
broken. Still, this is sufficient to properly handle noreturn temporary
destructors and close bug #15599. I have also enabled the cfg-temporary-dtors
analyzer option by default.
Reviewers: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1131
llvm-svn: 186498
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The analyzer can't see the reference count for shared_ptr, so it doesn't
know whether a given destruction is going to delete the referenced object.
This leads to spurious leak and use-after-free warnings.
For now, just ban destructors named '~shared_ptr', which catches
std::shared_ptr, std::tr1::shared_ptr, and boost::shared_ptr.
PR15987
llvm-svn: 182071
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The analyzer uses LazyCompoundVals to represent rvalues of aggregate types,
most importantly structs and arrays. This allows us to efficiently copy
around an entire struct, rather than doing a memberwise load every time a
struct rvalue is encountered. This can also keep memory usage down by
allowing several structs to "share" the same snapshotted bindings.
However, /lookup/ through LazyCompoundVals can be expensive, especially
since they can end up chaining back to the original value. While we try
to reuse LazyCompoundVals whenever it's safe, and cache information about
this transitivity, the fact is it's sometimes just not a good idea to
perpetuate LazyCompoundVals -- the tradeoffs just aren't worth it.
This commit changes RegionStore so that binding a LazyCompoundVal to struct
will do a memberwise copy if the struct is simple enough. Today's definition
of "simple enough" is "up to N scalar members" (see below), but that could
easily be changed in the future. This is enough to bring the test case in
PR15697 back down to a manageable analysis time (within 20% of its original
time, in an unfair test where the new analyzer is not compiled with LTO).
The actual value of "N" is controlled by a new -analyzer-config option,
'region-store-small-struct-limit'. It defaults to "2", meaning structs with
zero, one, or two scalar members will be considered "simple enough" for
this code path.
It's worth noting that a more straightforward implementation would do this
on load, not on bind, and make use of the structure we already have for this:
CompoundVal. A long time ago, this was actually how RegionStore modeled
aggregate-to-aggregate copies, but today it's only used for compound literals.
Unfortunately, it seems that we've special-cased LazyCompoundVal in certain
places (such as liveness checks) but failed to similarly special-case
CompoundVal in all of them. Until we're confident that CompoundVal is
handled properly everywhere, this solution is safer, since the entire
optimization is just an implementation detail of RegionStore.
<rdar://problem/13599304>
llvm-svn: 179767
|
|
|
|
| |
llvm-svn: 179635
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is an opt-in tweak for leak diagnostics to reference the allocation
site if the diagnostic consumer only wants a pithy amount of information,
and not the entire path.
This is a strawman enhancement that I expect to see some experimentation
with over the next week, and can go away if we don't want it.
Currently it is only used by RetainCountChecker, but could be used
by MallocChecker if and when we decide this should stay in.
llvm-svn: 179634
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This turns on not only destructor inlining, but inlining of constructors
for types with non-trivial destructors. Per r178516, we will still not
inline the constructor or destructor of anything that looks like a
container unless the analyzer-config option 'c++-container-inlining' is
set to 'true'.
In addition to the more precise path-sensitive model, this allows us to
catch simple smart pointer issues:
#include <memory>
void test() {
std::auto_ptr<int> releaser(new int[4]);
} // memory allocated with 'new[]' should not be deleted with 'delete'
<rdar://problem/12295363>
llvm-svn: 178805
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is a heuristic to make up for the fact that the analyzer doesn't
model C++ containers very well. One example is modeling that
'std::distance(I, E) == 0' implies 'I == E'. In the future, it would be
nice to model this explicitly, but for now it just results in a lot of
false positives.
The actual heuristic checks if the base type has a member named 'begin' or
'iterator'. If so, we treat the constructors and destructors of that type
as opaque, rather than inlining them.
This is intended to drastically reduce the number of false positives
reported with experimental destructor support turned on. We can tweak the
heuristic in the future, but we'd rather err on the side of false negatives
for now.
<rdar://problem/13497258>
llvm-svn: 178516
|
|
|
|
| |
llvm-svn: 178318
|
|
|
|
|
|
|
|
|
|
| |
This enables constructor inlining for types with non-trivial destructors.
The plan is to enable destructor inlining within the next month, but that
needs further verification.
<rdar://problem/12295329>
llvm-svn: 176200
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Redefine the shallow mode to inline all functions for which we have a
definite definition (ipa=inlining). However, only inline functions that
are up to 4 basic blocks large and cut the max exploded nodes generated
per top level function in half.
This makes shallow faster and allows us to keep inlining small
functions. For example, we would keep inlining wrapper functions and
constructors/destructors.
With the new shallow, it takes 104s to analyze sqlite3, whereas
the deep mode is 658s and previous shallow is 209s.
llvm-svn: 173958
|
|
|
|
| |
llvm-svn: 173957
|