Commit Graph

58 Commits

Author SHA1 Message Date
Sean Eveson 70eece21c2 Reapply r251621 "[Analyzer] Widening loops which do not exit"
It was not the cause of the build bot failure.

llvm-svn: 251702
2015-10-30 15:23:57 +00:00
Sean Eveson 4c7b3bf6ba Revert r251621 "[Analyzer] Widening loops which do not exit" (bot failure)
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
2015-10-30 11:13:07 +00:00
Sean Eveson 83390e45b3 [Analyzer] Widening loops which do not exit
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  and . 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
2015-10-29 10:04:41 +00:00
Gabor Horvath 7239a32975 [Static Analyzer] Properly cash the configuration option for lambda support.
llvm-svn: 247476
2015-09-11 21:19:39 +00:00
Devin Coughlin 0123af994e [analyzer] Add -analyzer-config option for function size the inliner 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
2015-09-11 20:14:05 +00:00
Gabor Horvath 15843343b6 [Static Analyzer] Lambda support.
Differential Revision: http://reviews.llvm.org/D12652

llvm-svn: 247426
2015-09-11 16:55:01 +00:00
Ted Kremenek 3a0678e33c [analyzer] Apply whitespace cleanups by Honggyu Kim.
llvm-svn: 246978
2015-09-08 03:50:52 +00:00
Gabor Horvath e40c71c10a [analyzer] Individual configuration options can be specified for checkers.
Reviewed by: Anna Zaks

Original patch by: Aleksei Sidorin

Differential Revision: http://reviews.llvm.org/D7905

llvm-svn: 231266
2015-03-04 17:59:34 +00:00
David Blaikie 3c8c46efd7 clang-format a recent commit I made
llvm-svn: 222317
2014-11-19 05:48:40 +00:00
David Blaikie 13156b689e Standardize on StringMap::insert, removing uses of StringMap::GetOrCreateValue.
llvm-svn: 222306
2014-11-19 03:06:06 +00:00
Anna Zaks b8de0c4278 Do not inline methods of C++ containers (coming from headers).
This silences false positives (leaks, use of uninitialized value) in simple
code that uses containers such as std::vector and std::list. The analyzer
cannot reason about the internal invariances of those data structures which
leads to false positives. Until we come up with a better solution to that
problem, let's just not inline the methods of the containers and allow objects
to escape whenever such methods are called.

This just extends an already existing flag "c++-container-inlining" and applies
the heuristic not only to constructors and destructors of the containers, but
to all of their methods.

We have a bunch of distinct user reports all related to this issue
(radar://16058651, radar://16580751, radar://16384286, radar://16795491
[PR19637]).

llvm-svn: 211832
2014-06-27 01:03:05 +00:00
Sylvestre Ledru 9882e1a52c With the option '-analyzer-config stable-report-filename=true',
instead of report-XXXXXX.html, scan-build/clang analyzer generate
report-<filename>-<function, method name>-<function position>-<id>.html.
(id = i++ for several issues found in the same function/method)

llvm-svn: 210970
2014-06-14 08:45:32 +00:00
Craig Topper 0dbb783c7b [C++11] Use 'nullptr'. StaticAnalyzer edition.
llvm-svn: 209642
2014-05-27 02:45:47 +00:00
Jordan Rose 8b808d64af [analyzer] Inline C++ operator new when c++-inline-allocators is turned on.
This will let us stage in the modeling of operator new. The -analyzer-config
opton 'c++-inline-allocators' is currently off by default.

Patch by Karthik Bhat!

llvm-svn: 201122
2014-02-11 02:21:06 +00:00
Jordan Rose 7b982b30c0 Revert "[analyzer] Add very limited support for temporary destructors"
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
2013-07-23 02:15:11 +00:00
Pavel Labath 9ced602cc6 [analyzer] Add very limited support for temporary destructors
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 . 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
2013-07-17 08:33:58 +00:00
Jordan Rose fbe4d85035 [analyzer] Don't inline ~shared_ptr.
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
2013-05-17 02:16:49 +00:00
Anna Zaks c5e2eca042 [analyzer] Add an option to use the last location in the main source file as the report location.
Previously, we’ve used the last location of the analyzer issue path as the location of the
report. This might not provide the best user experience, when one analyzer a source
file and the issue appears in the header. Introduce an option to use the last location
of the path that is in the main source file as the report location.

New option can be enabled with -analyzer-config report-in-main-source-file=true.

llvm-svn: 182058
2013-05-16 22:30:45 +00:00
Jordan Rose 2de3daa0a2 [analyzer] Enable destructor inlining by default (c++-inlining=destructors).
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
2013-04-04 23:10:29 +00:00
Jordan Rose e189b869c5 [analyzer] For now, don't inline [cd]tors of C++ containers.
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
2013-04-02 00:26:35 +00:00
Jordan Rose d11ef1aaf7 [analyzer] Allow suppressing diagnostics reported within the 'std' namespace
This is controlled by the 'suppress-c++-stdlib' analyzer-config flag.
It is currently off by default.

This is more suppression than we'd like to do, since obviously there can
be user-caused issues within 'std', but it gives us the option to wield
a large hammer to suppress false positives the user likely can't work
around.

llvm-svn: 178513
2013-04-02 00:26:15 +00:00
Ted Kremenek 338c3aa8d1 Add static analyzer support for conditionally executing static initializers.
llvm-svn: 178318
2013-03-29 00:09:28 +00:00
Ted Kremenek 233c1b0c77 Add configuration plumbing to enable static initializer branching in the CFG for the analyzer.
This setting still isn't enabled yet in the analyzer.  This is
just prep work.

llvm-svn: 178317
2013-03-29 00:09:22 +00:00
Anna Zaks 6fe2fc633a [analyzer] IDC: Add config option; perform the idc check on first “null node” rather than last “non-null”.
The second modification does not lead to any visible result, but, theoretically, is what we should
have been looking at to begin with since we are checking if the node was assumed to be null in
an inlined function.

llvm-svn: 176576
2013-03-06 20:25:59 +00:00
Jordan Rose 4587b28758 [analyzer] Turn on C++ constructor inlining by default.
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
2013-02-27 18:49:43 +00:00
David Blaikie 05785d1622 Include llvm::Optional in clang/Basic/LLVM.h
Post-commit CR feedback from Jordan Rose regarding r175594.

llvm-svn: 175679
2013-02-20 22:23:23 +00:00
Anna Zaks c84d151892 [analyzer] Make shallow mode more shallow.
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
2013-01-30 19:12:39 +00:00
Anna Zaks 66b9f1660e [analyzer] Use analyzer config for max-inlinable-size option.
llvm-svn: 173957
2013-01-30 19:12:36 +00:00
Anna Zaks 70aa53180d [analyzer] Remove further references to analyzer-ipa.
Thanks Jordan!

llvm-svn: 173955
2013-01-30 19:12:26 +00:00
Jordan Rose 329bbe8e11 [analyzer] Add 'prune-paths' config option to disable path pruning.
This should be used for testing only. Path pruning is still on by default.

llvm-svn: 173545
2013-01-26 01:28:15 +00:00
Jordan Rose 8de30305f6 [analyzer] Rename PruneNullReturnPaths to SuppressNullReturnPaths.
"Prune" is the term for eliminating pieces of a path that are not
relevant to the user. "Suppress" means don't show that path at all.

llvm-svn: 173544
2013-01-26 01:28:09 +00:00
Anna Zaks 36d988f023 [analyzer] Add "-analyzer-config mode=[deep|shallow] ".
The idea is to introduce a higher level "user mode" option for
different use scenarios. For example, if one wants to run the analyzer
for a small project each time the code is built, they would use
the "shallow" mode. 

The user mode option will influence the default settings for the
lower-level analyzer options. For now, this just influences the ipa
modes, but we plan to find more optimal settings for them.

llvm-svn: 173386
2013-01-24 23:15:34 +00:00
Anna Zaks 6bab4ef4e8 [analyzer] Replace "-analyzer-ipa" with "-analyzer-config ipa".
The idea is to eventually place all analyzer options under
"analyzer-config". In addition, this lays the ground for introduction of
a high-level analyzer mode option, which will influence the
default setting for IPAMode.

llvm-svn: 173385
2013-01-24 23:15:30 +00:00
Anna Zaks c7f5e69e50 [analyzer] refactor: access IPAMode through the accessor.
llvm-svn: 173384
2013-01-24 23:15:25 +00:00
Dmitri Gribenko f857950d39 Remove useless 'llvm::' qualifier from names like StringRef and others that are
brought into 'clang' namespace by clang/Basic/LLVM.h

llvm-svn: 172323
2013-01-12 19:30:44 +00:00
Anna Zaks d53182b0df [analyzer] Implement "do not inline large functions many times"
performance heuristic

After inlining a function with more than 13 basic blocks 32 times, we
are not going to inline it anymore. The idea is that inlining large
functions leads to drastic performance implications. Since the function
has already been inlined, we know that we've analyzed it in many
contexts. 

The following metrics are used:
 - Large function is a function with more than 13 basic blocks (we
should switch to another metric, like cyclomatic complexity)
 - We consider that we've inlined a function many times if it's been
inlined 32 times. This number is configurable with -analyzer-config
max-times-inline-large=xx

This heuristic addresses a performance regression introduced with
inlining on one benchmark. The analyzer on this benchmark became 60
times slower with inlining turned on. The heuristic allows us to analyze
it in 24% of the time. The performance improvements on the other
benchmarks I've tested with are much lower - under 10%, which is
expected.

llvm-svn: 170361
2012-12-17 20:08:51 +00:00
Chandler Carruth 3a02247dc9 Sort all of Clang's files under 'lib', and fix up the broken headers
uncovered.

This required manually correcting all of the incorrect main-module
headers I could find, and running the new llvm/utils/sort_includes.py
script over the files.

I also manually added quite a few missing headers that were uncovered by
shuffling the order or moving headers up to be main-module-headers.

llvm-svn: 169237
2012-12-04 09:13:33 +00:00
Jordan Rose ec44ac6a59 [analyzer] New option to not suppress null return paths if an argument is null.
Our one basic suppression heuristic is to assume that functions do not
usually return NULL. However, when one of the arguments is NULL it is
suddenly much more likely that NULL is a valid return value. In this case,
we don't suppress the report here, but we do attach /another/ visitor to
go find out if this NULL argument also comes from an inlined function's
error path.

This new behavior, controlled by the 'avoid-suppressing-null-argument-paths'
analyzer-config option, is turned off by default. Turning it on produced
two false positives and no new true positives when running over LLVM/Clang.

This is one of the possible refinements to our suppression heuristics.
<rdar://problem/12350829>

llvm-svn: 166941
2012-10-29 17:31:59 +00:00
Jordan Rose 746c06d0bc [analyzer] Replace -analyzer-no-eagerly-trim-egraph with graph-trim-interval.
After every 1000 CFGElements processed, the ExplodedGraph trims out nodes
that satisfy a number of criteria for being "boring" (single predecessor,
single successor, and more). Rather than controlling this with a cc1 option,
which can only disable this behavior, we now have an analyzer-config option,
'graph-trim-interval', which can change this interval from 1000 to something
else. Setting the value to 0 disables reclamation.

The next commit relies on this behavior to actually test anything.

llvm-svn: 166528
2012-10-23 23:59:05 +00:00
Ted Kremenek f1245ddc78 Silence -Wunused-value warning.
llvm-svn: 165059
2012-10-02 21:50:18 +00:00
Ted Kremenek 4924a0161b Refactor clients of AnalyzerOptions::getBooleanOption() to have
an intermediate helper method to query and populate the Optional value.

llvm-svn: 165043
2012-10-02 20:42:16 +00:00
Ted Kremenek 3c6932922e Tweak AnalyzerOptions::getOptionAsInteger() to populate the string
table, making it printable with the ConfigDump checker.  Along the
way, fix a really serious bug where the value was getting parsed
from the string in code that was in an assert() call.  This means
in a Release-Asserts build this code wouldn't work as expected.

llvm-svn: 165041
2012-10-02 20:31:56 +00:00
Ted Kremenek 5faa5e04a3 Change AnalyzerOptions::mayInlineCXXMemberFunction to default populate
the config string table.  Also setup a test for dumping the analyzer
configuration for C++.

llvm-svn: 165040
2012-10-02 20:31:52 +00:00
Ted Kremenek 4a5b35eeec Have AnalyzerOptions::getBooleanOption() stick the matching config
string in the config table so that it can be dumped as part of the 
config dumper.  Add a test to show that these options are sticking
and can be cross-checked using FileCheck.

llvm-svn: 164954
2012-10-01 18:28:19 +00:00
Jordan Rose 52de8eec01 [analyzer] Suppress bugs whose paths go through the return of a null pointer.
This is a heuristic intended to greatly reduce the number of false
positives resulting from inlining, particularly inlining of generic,
defensive C++ methods that live in header files. The suppression is
triggered in the cases where we ask to track where a null pointer came
from, and it turns out that the source of the null pointer was an inlined
function call.

This change brings the number of bug reports in LLVM from ~1500 down to
around ~300, a much more manageable number. Yes, some true positives may
be hidden as well, but from what I looked at the vast majority of silenced
reports are false positives, and many of the true issues found by the
analyzer are still reported.

I'm hoping to improve this heuristic further by adding some exceptions
next week (cases in which a bug should still be reported).

llvm-svn: 164449
2012-09-22 01:25:06 +00:00
Ted Kremenek 61e2f2d6ec Re-enable faux-bodies by default.
Try this again, now that r164392 is in place.

llvm-svn: 164393
2012-09-21 17:55:34 +00:00
NAKAMURA Takumi 443eef47ef Revert r164364, "Flip "faux-bodies" in the analyzer on by default to flush out bugs."
It crashed test/Analysis/Output/blocks.m on some hosts.

llvm-svn: 164368
2012-09-21 12:00:42 +00:00
Ted Kremenek e460a4ea2d Flip "faux-bodies" in the analyzer on by default to flush out bugs.
llvm-svn: 164364
2012-09-21 06:14:37 +00:00
Ted Kremenek 14f779c4d6 Implement faux-body-synthesis of well-known functions in the static analyzer when
their implementations are unavailable.  Start by simulating dispatch_sync().

This change is largely a bunch of plumbing around something very simple.  We
use AnalysisDeclContext to conjure up a fake function body (using the
current ASTContext) when one does not exist.  This is controlled
under the analyzer-config option "faux-bodies", which is off by default.

The plumbing in this patch is largely to pass the necessary machinery
around.  CallEvent needs the AnalysisDeclContextManager to get
the function definition, as one may get conjured up lazily.

BugReporter and PathDiagnosticLocation needed to be relaxed to handle
invalid locations, as the conjured body has no real source locations.
We do some primitive recovery in diagnostic generation to generate
some reasonable locations (for arrows and events), but it can be
improved.

llvm-svn: 164339
2012-09-21 00:09:11 +00:00
Jordan Rose a522f1cf8b Revert "[analyzer] Disable STL inlining. Blocked by PR13724."
While PR13724 is still an issue, it's not actually an issue in the STL.
We can keep this option around in case there turn out to be widespread
false positives due to poor modeling of the C++ standard library functions,
but for now we'd like to get more data.

This reverts r163633 / c6baadceec1d5148c20ee6c902a102233c547f62.

llvm-svn: 163647
2012-09-11 20:26:49 +00:00