Commit Graph

1153 Commits

Author SHA1 Message Date
Jordan Rose fe856d58a3 [analyzer] Do a better job describing C++ member functions in the call stack.
Examples:
  Calling constructor for 'Foo'
  Entered call from 'Foo::create'

llvm-svn: 172832
2013-01-18 18:27:14 +00:00
David Greene 0d5a34bcad Fix Cast
Properly use const_cast to fix a cast-away-const error.

llvm-svn: 172561
2013-01-15 22:09:45 +00:00
Jordan Rose 269894ca23 [analyzer] Add ProgramStatePartialTrait<const void *>.
This should fix cast-away-const warnings reported by David Greene.

llvm-svn: 172446
2013-01-14 18:58:42 +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
Ted Kremenek 4e9a2dbde5 Refine analyzer's handling of unary '!' and floating types to not assert.
Fixes PR 14634 and <rdar://problem/12903080>.

llvm-svn: 172274
2013-01-11 23:36:25 +00:00
Ted Kremenek 039fac0347 Correctly propagate uninitialized values within logical expressions.
Fixes assertion failure reported in PR 14635 and
<rdar://problem/12902945> respectively.

llvm-svn: 172263
2013-01-11 22:35:39 +00:00
Ted Kremenek 2f2edd3fb1 Do not model loads from complex types, since we don't accurately model the imaginary and real parts yet.
Fixes false positive reported in <rdar://problem/12964481>.

llvm-svn: 171987
2013-01-09 18:46:17 +00:00
Anna Zaks 454a384e59 [analyzer] Only include uniqueling location as issue_hash when available
This makes us more optimistic when matching reports in a changing code
base. Addresses Jordan's feedback for r171825.

llvm-svn: 171884
2013-01-08 19:19:46 +00:00
Anna Zaks a043d0cef2 [analyzer] Include the bug uniqueing location in the issue_hash.
The issue here is that if we have 2 leaks reported at the same line for
which we cannot print the corresponding region info, they will get
treated as the same by issue_hash+description. We need to AUGMENT the
issue_hash with the allocation info to differentiate the two issues.

Add the "hash" (offset from the beginning of a function) representing
allocation site to solve the issue.

We might want to generalize solution in the future when we decide to
track more than just the 2 locations from the diagnostics.

llvm-svn: 171825
2013-01-08 00:25:29 +00:00
Anna Zaks 58b961d176 [analyzer] Plist: change the type of issue_hash from int to string.
This gives more flexibility to what could be stored as issue_hash.

llvm-svn: 171824
2013-01-08 00:25:22 +00:00
Anna Zaks 3fdcc0bda3 [analyzer] Rename callback EndPath -> EndFunction
This better reflects when callback is called and what the checkers
are relying on. (Both names meant the same pre-IPA.)

llvm-svn: 171432
2013-01-03 00:25:29 +00:00
Chandler Carruth 44eb4f66f4 Re-sort #include lines using the llvm/utils/sort_includes.py script.
Removes a duplicate #include as well as cleaning up some sort order
regressions since I last ran the script over Clang.

llvm-svn: 171364
2013-01-02 10:28:36 +00:00
Roman Divacky 241f45118b Remove duplicate includes.
llvm-svn: 170903
2012-12-21 17:07:08 +00:00
Anna Zaks 9747febba9 [analyzer] Address Jordan's nitpicks as per code review of r170625.
llvm-svn: 170832
2012-12-21 01:50:14 +00:00
Anna Zaks dc15415da4 [analyzer] Add the pointer escaped callback.
Instead of using several callbacks to identify the pointer escape event,
checkers now can register for the checkPointerEscape.

Converted the Malloc checker to use the new callback.
SimpleStreamChecker will be converted next.

llvm-svn: 170625
2012-12-20 00:38:25 +00:00
Ted Kremenek 3a081a0339 Pass AnalyzerOptions to PathDiagnosticConsumer to make analyzer options accessible there.
This is plumbing needed for later functionality changes.

llvm-svn: 170488
2012-12-19 01:35:35 +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
Anton Yartsev 20ae1dbfd1 fixed line endings
llvm-svn: 170238
2012-12-14 20:28:48 +00:00
Anton Yartsev 5363bf157f added post-statement callback to CXXNewExpr and pre-statement callback to CXXDeleteExpr
llvm-svn: 170234
2012-12-14 19:48:34 +00:00
Anna Zaks a40bcac0ef [analyzer] Propagate the checker's state from checkBranchCondition
Fixes a bug, where we were dropping the state modifications from the
checkBranchCondition checker callback.

llvm-svn: 170232
2012-12-14 19:08:20 +00:00
Ted Kremenek 45bb8db372 Refactor dump methods to make RegionBindingsRef printable in the debugger.
llvm-svn: 170170
2012-12-14 01:23:13 +00:00
Jordan Rose 4cfdbff3c7 [analyzer] Don't crash running destructors for multidimensional arrays.
We don't handle array destructors correctly yet, but we now apply the same
hack (explicitly destroy the first element, implicitly invalidate the rest)
for multidimensional arrays that we already use for linear arrays.

<rdar://problem/12858542>

llvm-svn: 170000
2012-12-12 19:13:44 +00:00
Anna Zaks 5d484780fb [analyzer] Optimization heuristic: do not reanalyze every ObjC method as
top level.

This heuristic is already turned on for non-ObjC methods
(inlining-mode=noredundancy). If a method has been previously analyzed,
while being inlined inside of another method, do not reanalyze it as top
level.

This commit applies it to ObjCMethods as well. The main caveat here is
that to catch the retain release errors, we are still going to reanalyze
all the ObjC methods but without inlining turned on.

Gives 21% performance increase on one heavy ObjC benchmark, which
suffered large performance regressions due to ObjC inlining.

llvm-svn: 169639
2012-12-07 21:51:47 +00:00
Jordan Rose 9a33913645 [analyzer] Fix r168019 to work with unpruned paths as well.
This is the case where the analyzer tries to print out source locations
for code within a synthesized function body, which of course does not have
a valid source location. The previous fix attempted to do this during
diagnostic path pruning, but some diagnostics have pruning disabled, and
so any diagnostic with a path that goes through a synthesized body will
either hit an assertion or emit invalid output.

<rdar://problem/12657843> (again)

llvm-svn: 169631
2012-12-07 19:56:29 +00:00
Ted Kremenek 54c9a4fad1 Reduce conversions between Store <-> ImmutableMapRef in RegionStore.
This reduces canonicalization of ImmutableMaps.  This reduces analysis time
of one heavy Objective-C file by another 1%.

llvm-svn: 169630
2012-12-07 19:54:25 +00:00
Ted Kremenek 897702e30a Add helper method to convert from a RegionStoreRefBindings to a Store.
llvm-svn: 169622
2012-12-07 18:32:08 +00:00
Ted Kremenek 245e45af7d Cache queries to lookupPrivateMethod() within ObjCMethodCall::getRuntimeDefinition().
The same queries can happen thousands of times.  This reduces the analysis
time on one heavy Objective-C file by 2.4%.

llvm-svn: 169589
2012-12-07 07:30:19 +00:00
Ted Kremenek f19db16b0e Further reduce analysis time by 0.2% on a heavy Objective-C example by avoiding over-eager canonicalization of clusters.
llvm-svn: 169586
2012-12-07 06:49:27 +00:00
David Blaikie b006d38476 Unbreak the GCC (4.4 & other bot) builds from r169571.
llvm-svn: 169581
2012-12-07 03:28:20 +00:00
Ted Kremenek 147784fdf2 Change RegionStore to always use ImmutableMapRef for processing cluster bindings.
This reduces analysis time by 1.2% on one test case (Objective-C), but
also cleans up some of the code conceptually as well.  We can possible
just make RegionBindingsRef -> RegionBindings, but I wanted to stage
things.

After this, we should revisit Jordan's optimization of not canonicalizing
the immutable AVL trees for the cluster bindings as well.

llvm-svn: 169571
2012-12-07 01:55:21 +00:00
Ted Kremenek cb95a8fd20 Revert "[analyzer] Aggressively cut back on the canonicalization in RegionStore."
Jordan and I discussed this, and we are going to do this another way.

llvm-svn: 169538
2012-12-06 19:40:32 +00:00
Jordan Rose b10aae3fec [analyzer] Remove isa<> followed by dyn_cast<>.
llvm-svn: 169530
2012-12-06 18:58:29 +00:00
Jordan Rose 642e063838 [analyzer] Remove unused fields from ExprEngine.
'currStmt', 'CleanedState', and 'EntryNode' were being set, but only ever
used locally.

llvm-svn: 169529
2012-12-06 18:58:26 +00:00
Jordan Rose de606eaf18 [analyzer] Remove checks that predate the linearized CFG.
llvm-svn: 169528
2012-12-06 18:58:22 +00:00
Jordan Rose 5e4e61ddf9 [analyzer] Use a smarter algorithm to find the last block in an inlined call.
Previously we would search for the last statement, then back up to the
entrance of the block that contained that statement. Now, while we're
scanning for the statement, we just keep track of which blocks are being
exited (in reverse order).

llvm-svn: 169526
2012-12-06 18:58:15 +00:00
Jordan Rose 1ecba4cc69 [analyzer] Use optimized assumeDual for branches.
This doesn't seem to make much of a difference in practice, but it does
have the potential to avoid a trip through the constraint manager.

llvm-svn: 169524
2012-12-06 18:58:09 +00:00
Jordan Rose 5f28afc8a1 [analyzer] Aggressively cut back on the canonicalization in RegionStore.
Whenever we touch a single bindings cluster multiple times, we can delay
canonicalizing it until the final access. This has some interesting
implications, in particular that we shouldn't remove an /empty/ cluster
from the top-level map until canonicalization.

This is good for a 2% speedup or so on the test case in
<rdar://problem/12810842>

llvm-svn: 169523
2012-12-06 18:58:06 +00:00
Jordan Rose 047208027a [analyzer] Remove bindExprAndLocation, which does extra work for no gain.
This feature was probably intended to improve diagnostics, but was currently
only used when dumping the Environment. It shows what location a given value
was loaded from, e.g. when evaluating an LValueToRValue cast.

llvm-svn: 169522
2012-12-06 18:58:01 +00:00
Ted Kremenek bcf905326c Only provide explicit getCapturedRegion() and getOriginalRegion() from referenced_vars_iterator.
This is a nice conceptual cleanup.

llvm-svn: 169480
2012-12-06 07:17:20 +00:00
Ted Kremenek ff989016c1 Pull logic to map from VarDecl* to captured region using a helper function. WIP.
llvm-svn: 169479
2012-12-06 07:17:13 +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
Benjamin Kramer 444a1304ad Include pruning and general cleanup.
llvm-svn: 169095
2012-12-01 17:12:56 +00:00
Benjamin Kramer d7d2b1fe45 Don't include Type.h in DeclarationName.h.
Recursively prune some includes.

llvm-svn: 169094
2012-12-01 16:35:25 +00:00
Benjamin Kramer ea70eb30a0 Pull the Attr iteration parts out of Attr.h, so including DeclBase.h doesn't pull in all the generated Attr code.
Required to pull some functions out of line, but this shouldn't have a perf impact.
No functionality change.

llvm-svn: 169092
2012-12-01 15:09:41 +00:00
Ted Kremenek 2317f30f4d Correctly handle IntegralToBool casts in C++ in the static analyzer. Fixes <rdar://problem/12759044>.
llvm-svn: 168843
2012-11-29 00:50:20 +00:00
Ted Kremenek 94c8348859 Remove workaround in RegionStore in r168741 since it is handled more generally by r168757.
llvm-svn: 168774
2012-11-28 05:36:28 +00:00
Ted Kremenek 18035d7125 Fix another false positive due to a CXX temporary object appearing in a C initializer.
The stop-gap here is to just drop such objects when processing the InitListExpr.
We still need a better solution.

Fixes <rdar://problem/12755044>.

llvm-svn: 168757
2012-11-28 01:49:01 +00:00
Ted Kremenek 5092c73187 Provide stop-gap solution to crash reported in PR 14436.
This was also covered by <rdar://problem/12753384>.  The static analyzer
evaluates a CXXConstructExpr within an initializer expression and
RegionStore doesn't know how to handle the resulting CXXTempObjectRegion
that gets created.  We need a better solution than just dropping the
value, but we need to better understand how to implement the right
semantics here.

Thanks to Jordan for his help diagnosing the behavior here.

llvm-svn: 168741
2012-11-27 23:05:37 +00:00
Anna Zaks e3beeaa5e7 [analyzer] Fix a crash reported in PR 14400.
The AllocaRegion did not have the superRegion (based on LocationContext)
as part of it's hash. As a consequence, the AllocaRegions from
different frames were uniqued to be the same region.

llvm-svn: 168599
2012-11-26 19:11:46 +00:00
Jordan Rose 19bc88c3d4 [analyzer] Fix a use-after-free introduced in r168019.
In code like this:

void foo() {
     bar();
     baz();
}

...the location for the call to 'bar()' was being used as a backup location
for the call to 'baz()'. This is fine unless the call to 'bar()' is deemed
uninteresting and that part of the path deleted.

(This looks like a logic error as well, but in practice the only way 'baz()'
could have an invalid location is if the entire body of 'foo()' is
synthesized, meaning the call to 'bar()' will be using the location of the
call to 'foo()' anyway. Nevertheless, the new version better matches the
intent of the code.)

Found by Matt Beaumont-Gay using ASan. Thanks, Matt!

llvm-svn: 168080
2012-11-15 20:10:05 +00:00
Jordan Rose e37ab50a6e [analyzer] Report leaks at the closing brace of a function body.
This fixes a few cases where we'd emit path notes like this:

  +---+
 1|   v
  p = malloc(len);
  ^   |2
  +---+

In general this should make path notes more consistent and more correct,
especially in cases where the leak happens on the false branch of an if
that jumps directly to the end of the function. There are a couple places
where the leak is reported farther away from the cause; these are usually
cases where there are several levels of nested braces before the end of
the function. This still matches our current behavior for when there /is/
a statement after all the braces, though.

llvm-svn: 168070
2012-11-15 19:11:43 +00:00
Jordan Rose b5b0fc196e [analyzer] Mark symbol values as dead in the environment.
This allows us to properly remove dead bindings at the end of the top-level
stack frame, using the ReturnStmt, if there is one, to keep the return value
live. This in turn removes the need for a check::EndPath callback in leak
checkers.

This does cause some changes in the path notes for leak checkers. Previously,
a leak would be reported at the location of the closing brace in a function.
Now, it gets reported at the last statement. This matches the way leaks are
currently reported for inlined functions, but is less than ideal for both.

llvm-svn: 168066
2012-11-15 19:11:27 +00:00
Jordan Rose 2d98b97e10 [analyzer] Make sure calls in synthesized functions have valid path locations.
We do this by using the "most recent" good location: if a synthesized
function 'A' calls another function 'B', the path notes for the call to 'B'
will be placed at the same location as the path note for calling 'A'.

Similarly, the call to 'A' will have a note saying "Entered call from...",
and now we just don't emit that (since the user doesn't have a body to look
at anyway).

Previously, we were doing this for the "Calling..." notes, but not for the
"Entered call from..." or "Returning to caller". This caused a crash when
the path entered and then exiting a call within a synthesized body.

<rdar://problem/12657843>

llvm-svn: 168019
2012-11-15 02:07:23 +00:00
Anna Zaks abdc72d970 [analyzer] Address Jordan's feedback for r167780.
llvm-svn: 167790
2012-11-13 00:13:44 +00:00
Anna Zaks 6ec9c3cbc1 [analyzer] Follow up to r167762 - precisely determine the adjustment
conditions.

The adjustment is needed only in case of dynamic dispatch performed by
the analyzer - when the runtime declaration is different from the static
one.

Document this explicitly in the code (by adding a helper). Also, use
canonical Decls to avoid matching against the case where the definition
is different from found declaration.

This fix suppresses the testcase I added in r167762, so add another
testcase to make sure we do test commit r167762.

llvm-svn: 167780
2012-11-12 23:40:29 +00:00
Anna Zaks 4e255b62f1 [analyzer] Fix a regression (from r 165079): compare canonical types.
Suppresses a leak false positive (radar://12663777).

In addition, we'll need to rewrite the adjustReturnValue() method not to
return UnknownVal by default, but rather assert in cases we cannot
handle. To make it possible, we need to correctly handle some of the
edge cases we already know about.

llvm-svn: 167762
2012-11-12 22:06:24 +00:00
Jordan Rose 9eb409ace9 [analyzer] When invalidating symbolic offset regions, take fields into account.
Previously, RegionStore was being VERY conservative in saying that because
p[i].x and p[i].y have a concrete base region of 'p', they might overlap.
Now, we check the chain of fields back up to the base object and check if
they match.

This only kicks in when dealing with symbolic offset regions because
RegionStore's "base+offset" representation of concrete offset regions loses
all information about fields. In cases where all offsets are concrete
(s.x and s.y), RegionStore will already do the right thing, but mixing
concrete and symbolic offsets can cause bindings to be invalidated that
are known to not overlap (e.g. p[0].x and p[i].y).
This additional refinement is tracked by <rdar://problem/12676180>.

<rdar://problem/12530149>

llvm-svn: 167654
2012-11-10 01:40:08 +00:00
Jordan Rose 520a30fd05 [analyzer] Move convenience REGISTER_*_WITH_PROGRAMSTATE to CheckerContext.h
As Anna pointed out, ProgramStateTrait.h is a relatively obscure header,
and checker writers may not know to look there to add their own custom
state.

The base macro that specializes the template remains in ProgramStateTrait.h
(REGISTER_TRAIT_WITH_PROGRAMSTATE), which allows the analyzer core to keep
using it.

llvm-svn: 167385
2012-11-05 16:58:00 +00:00
NAKAMURA Takumi ba15a7974a StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp: Appease msvc.
0 (as nullptr) is incompatible to pointer in type matching on msvc.

llvm-svn: 167355
2012-11-03 13:59:36 +00:00
Anna Zaks 8d1f6ed9a8 [analyzer] Run remove dead on end of path.
This will simplify checkers that need to register for leaks. Currently,
they have to register for both: check dead and check end of path.

I've modified the SymbolReaper to consider everything on the stack dead
if the input StackLocationContext is 0.

(This is a bit disruptive, so I'd like to flash out all the issues
asap.)

llvm-svn: 167352
2012-11-03 02:54:20 +00:00
Anna Zaks 2510608e81 [analyzer] Refactor: Remove Pred from NodeBuilderContext.
Node builders should manage the nodes, not the context.

llvm-svn: 167350
2012-11-03 02:54:11 +00:00
Jordan Rose 829c383114 [analyzer] Add some convenience accessors to CallEvent, and use them.
These are CallEvent-equivalents of helpers already accessible in
CheckerContext, as part of making it easier for new checkers to be written
using CallEvent rather than raw CallExprs.

llvm-svn: 167338
2012-11-02 23:49:29 +00:00
Jordan Rose 0da6747901 [analyzer] isCLibraryFunction: check that the function is at TU-scope.
Also, Decls already carry a pointer to the ASTContext, so there's no need
to pass an extra argument to the predicate.

llvm-svn: 167337
2012-11-02 23:49:24 +00:00
Jordan Rose 0c153cb277 [analyzer] Use nice macros for the common ProgramStateTraits (map, set, list).
Also, move the REGISTER_*_WITH_PROGRAMSTATE macros to ProgramStateTrait.h.

This doesn't get rid of /all/ explicit uses of ProgramStatePartialTrait,
but it does get a lot of them.

llvm-svn: 167276
2012-11-02 01:54:06 +00:00
Jordan Rose e10d5a7659 [analyzer] Rename 'EmitReport' to 'emitReport'.
No functionality change.

llvm-svn: 167275
2012-11-02 01:53:40 +00:00
Jordan Rose 417591fba7 [analyzer] Let ConstraintManager subclasses provide a more efficient checkNull.
Previously, every call to a ConstraintManager's isNull would do a full
assumeDual to test feasibility. Now, ConstraintManagers can override
checkNull if they have a cheaper way to do the same thing.
RangeConstraintManager can do this in less than half the work.

<rdar://problem/12608209>

llvm-svn: 167138
2012-10-31 16:44:55 +00:00
Anna Zaks 7bd0674dea [analyzer]Don't invalidate const arguments when there is no
IdentifierInfo.

Ee: C++ copy constructors.
llvm-svn: 167092
2012-10-31 01:18:26 +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 199fdd825f [analyzer] Use the CallEnter node to get a value for tracked null arguments.
Additionally, don't collect PostStore nodes -- they are often used in
path diagnostics.

Previously, we tried to track null arguments in the same way as any other
null values, but in many cases the necessary nodes had already been
collected (a memory optimization in ExplodedGraph). Now, we fall back to
using the value of the argument at the time of the call, which may not
always match the actual contents of the region, but often will.

This is a precursor to improving our suppression heuristic.
<rdar://problem/12350829>

llvm-svn: 166940
2012-10-29 17:31:53 +00:00
Ted Kremenek 808102685b Add comments for RemoveRedundantMsgs, rename it to removeRedundantMsgs() per Jordan's feedback.
llvm-svn: 166778
2012-10-26 16:02:36 +00:00
Ted Kremenek a5958869f6 TrackConstraintBRVisitor and ConditionBRVisitor can emit similar
path notes for cases where a value may be assumed to be null, etc.
Instead of having redundant diagnostics, do a pass over the generated
PathDiagnostic pieces and remove notes from TrackConstraintBRVisitor
that are already covered by ConditionBRVisitor, whose notes tend
to be better.

Fixes <rdar://problem/12252783>

llvm-svn: 166728
2012-10-25 22:07:10 +00:00
Jordan Rose 1bbd143945 [analyzer] Handle 'SomeVar.SomeEnumConstant', which is legal in C++.
This caused assertion failures analyzing LLVM.

<rdar://problem/12560282>

llvm-svn: 166529
2012-10-23 23:59:08 +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
Jordan Rose 3957fd5858 [analyzer] Assume 'new' never returns NULL if it could throw an exception.
This is actually required by the C++ standard in
[basic.stc.dynamic.allocation]p3:

  If an allocation function declared with a non-throwing
  exception-specification fails to allocate storage, it shall return a
  null pointer. Any other allocation function that fails to allocate
  storage shall indicate failure only by throwing an exception of a type
  that would match a handler of type std::bad_alloc.

We don't bother checking for the specific exception type, but just go off
the operator new prototype. This should help with a certain class of lazy
initalization false positives.

<rdar://problem/12115221>

llvm-svn: 166363
2012-10-20 02:32:51 +00:00
Jordan Rose 8e785e214b [analyzer] When binding to a ParenExpr, bind to its inner expression instead.
This actually looks through several kinds of expression, such as
OpaqueValueExpr and ExprWithCleanups. The idea is that binding and lookup
should be consistent, and so if the environment needs to be modified later,
the code doing the modification will not have to manually look through these
"transparent" expressions to find the real binding to change.

This is necessary for proper updating of struct rvalues as described in
the previous commit.

llvm-svn: 166121
2012-10-17 19:35:44 +00:00
Jordan Rose 29fc261cd7 [analyzer] Create a temporary region when accessing a struct rvalue.
In C++, rvalues that need to have their address taken (for example, to be
passed to a function by const reference) will be wrapped in a
MaterializeTemporaryExpr, which lets CodeGen know to create a temporary
region to store this value. However, MaterializeTemporaryExprs are /not/
created when a method is called on an rvalue struct, even though the 'this'
pointer needs a valid value. CodeGen works around this by creating a
temporary region anyway; now, so does the analyzer.

The analyzer also does this when accessing a field of a struct rvalue.
This is a little unfortunate, since the rest of the struct will soon be
thrown away, but it does make things consistent with the rest of the
analyzer.

This allows us to bring back the assumption that all known 'this' values
are Locs. This is a revised version of r164828-9, reverted in r164876-7.

<rdar://problem/12137950>

llvm-svn: 166120
2012-10-17 19:35:37 +00:00
Anna Zaks f2546f6726 [analyzer] Embed the analyzer version into the plist output.
llvm-svn: 165994
2012-10-15 22:48:19 +00:00
Jordan Rose 690c063b73 [analyzer] Remove the "direct bindings only" Environment lookup.
This was only used by OSAtomicChecker and makes it more
difficult to update values for expressions that the environment
may look through instead (it's not the same as IgnoreParens).
With this gone, we can have bindExpr bind to the inner
expression that getSVal will find.

Groundwork for <rdar://problem/12137950>

llvm-svn: 165866
2012-10-13 05:05:20 +00:00
Jordan Rose 88b690dd2e [analyzer] Remove unneeded 'inlineCall' checker callback.
I believe the removed assert in CheckerManager says it best:

	InlineCall is a special hacky callback to allow intrusive
	evaluation of the call (which simulates inlining). It is
	currently only used by OSAtomicChecker and should go away
	at some point.

OSAtomicChecker has gone away; inlineCall can now go away as well!

llvm-svn: 165865
2012-10-13 05:05:13 +00:00
Jordan Rose e15fb77df8 Reapply "[analyzer] Treat fields of unions as having symbolic offsets."
This time, actually uncomment the code that's supposed to fix the problem.

This reverts r165671 / 8ceb837585ed973dc36fba8dfc57ef60fc8f2735.

llvm-svn: 165676
2012-10-10 23:23:21 +00:00
Eric Christopher a529f8c9c2 Temporarily Revert "[analyzer] Treat fields of unions as having symbolic offsets."
Author: Jordan Rose <jordan_rose@apple.com>
Date:   Wed Oct 10 21:31:21 2012 +0000

    [analyzer] Treat fields of unions as having symbolic offsets.

    This allows only one field to be active at a time in RegionStore.
    This isn't quite the correct behavior for unions, but it at least
    would handle the case of "value goes in, value comes out" from the
    same field.

    RegionStore currently has a number of places where any access to a union
    results in UnknownVal being returned. However, it is clearly missing
    some cases, or the original issue wouldn't have occurred. It is probably
    now safe to remove those changes, but that's a potentially destabilizing
    change that should wait for more thorough testing.

    Fixes PR14054.

    git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@165660 91177308-0d34-0410-b5e6-96231b3b80d8

This reverts commit cf9030e480f77ab349672f00ad302e216c26c92c.

llvm-svn: 165671
2012-10-10 22:49:05 +00:00
Jordan Rose fb29410c85 [analyzer] Treat fields of unions as having symbolic offsets.
This allows only one field to be active at a time in RegionStore.
This isn't quite the correct behavior for unions, but it at least
would handle the case of "value goes in, value comes out" from the
same field.

RegionStore currently has a number of places where any access to a union
results in UnknownVal being returned. However, it is clearly missing
some cases, or the original issue wouldn't have occurred. It is probably
now safe to remove those changes, but that's a potentially destabilizing
change that should wait for more thorough testing.

Fixes PR14054.

llvm-svn: 165660
2012-10-10 21:31:21 +00:00
Jordan Rose c8a78a37bb [analyzer] Handle implicit statements used for end-of-path nodes' source locs.
Some implicit statements, such as the implicit 'self' inserted for "free"
Objective-C ivar access, have invalid source locations. If one of these
statements is the location where an issue is reported, we'll now look at
the enclosing statements for a valid source location.

<rdar://problem/12446776>

llvm-svn: 165354
2012-10-06 01:19:30 +00:00
Jordan Rose 1dd2afd876 [analyzer] Adjust the return type of an inlined devirtualized method call.
In C++, overriding virtual methods are allowed to specify a covariant
return type -- that is, if the return type of the base method is an
object pointer type (or reference type), the overriding method's return
type can be a pointer to a subclass of the original type. The analyzer
was failing to take this into account when devirtualizing a method call,
and anything that relied on the return value having the proper type later
would crash.

In Objective-C, overriding methods are allowed to specify ANY return type,
meaning we can NEVER be sure that devirtualizing will give us a "safe"
return value. Of course, a program that does this will most likely crash
at runtime, but the analyzer at least shouldn't crash.

The solution is to check and see if the function/method being inlined is
the function that static binding would have picked. If not, check that
the return value has the same type. If the types don't match, see if we
can fix it with a derived-to-base cast (the C++ case). If we can't,
return UnknownVal to avoid crashing later.

<rdar://problem/12409977>

llvm-svn: 165079
2012-10-03 01:08:35 +00:00
Jordan Rose 9aa9980217 [analyzer] Push evalDynamicCast and evalDerivedToBase up to Store.
These functions are store-agnostic, and would benefit from information in
DynamicTypeInfo but gain nothing from the store type.

No intended functionality change.

llvm-svn: 165078
2012-10-03 01:08:32 +00:00
Jordan Rose 7bb2611400 Teach getCXXRecordDeclForPointerType about references.
Then, rename it getPointeeCXXRecordDecl and give it a nice doc comment,
and actually use it.

No intended functionality change.

llvm-svn: 165077
2012-10-03 01:08:28 +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
Jordan Rose 92375adafb [analyzer] Allow ObjC ivar lvalues where the base is nil.
By analogy with C structs, this seems to be legal, if probably discouraged.
It's only if the ivar is read from or written to that there's a problem.
Running a program that gets the "address" of an instance variable does in
fact return the offset when the base "object" is nil.

This isn't a full revert because r164442 includes some diagnostic tweaks
as well; those have been kept.

This partially reverts r164442 / 08965091770c9b276c238bac2f716eaa4da2dca4.

llvm-svn: 164960
2012-10-01 19:07:22 +00:00
Jordan Rose 12024f8776 Revert "[analyzer] Check that a member expr is valid even when the result is an lvalue."
The original intent of this commit was to catch potential null dereferences
early, but it breaks the common "home-grown offsetof" idiom (PR13927):

 (((struct Foo *)0)->member - ((struct foo *)0))

As it turns out, this appears to be legal in C, per a footnote in
C11 6.5.3.2: "Thus, &*E is equivalent to E (even if E is a null pointer)".
In C++ this issue is still open:
  http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#232

We'll just have to make sure we have good path notes in the future.

This reverts r164441 / 9be016dcd1ca3986873a7b66bd4bc027309ceb59.

llvm-svn: 164958
2012-10-01 19:07:15 +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 88dd13fdca Reapply "[analyzer] Handle inlined constructors for rvalue temporaries correctly."
This is related to but not blocked by <rdar://problem/12137950>
("Return-by-value structs do not have associated regions")

This reverts r164875 / 3278d41e17749dbedb204a81ef373499f10251d7.

llvm-svn: 164952
2012-10-01 17:51:35 +00:00
Jordan Rose d63f04d8a7 [analyzer] Make ProgramStateManager's SubEngine parameter optional.
It is possible and valid to have a state manager and associated objects
without having a SubEngine or checkers.

Patch by Olaf Krzikalla!

llvm-svn: 164947
2012-10-01 16:53:40 +00:00
Jordan Rose d60b9168fa Revert "[analyzer] Create a temporary region for rvalue structs when accessing fields"
This reverts commit 6f61df3e7256413dcb99afb9673f4206e3c4992c.

llvm-svn: 164877
2012-09-29 01:36:51 +00:00
Jordan Rose d9b0268401 Revert "[analyzer] Create a temp region when a method is called on a struct rvalue."
This reverts commit 0006ba445962621ed82ec84400a6b978205a3fbc.

llvm-svn: 164876
2012-09-29 01:36:47 +00:00
Jordan Rose cd9000e840 Revert "[analyzer] Handle inlined constructors for rvalue temporaries correctly."
This reverts commit 580cd17f256259f39a382e967173f34d68e73859.

llvm-svn: 164875
2012-09-29 01:36:42 +00:00
Jordan Rose 19ed6748ea [analyzer] Handle inlined constructors for rvalue temporaries correctly.
Previously the analyzer treated all inlined constructors like lvalues,
setting the value of the CXXConstructExpr to the newly-constructed
region. However, some CXXConstructExprs behave like rvalues -- in
particular, the implicit copy constructor into a pass-by-value argument.
In this case, we want only the /contents/ of a temporary object to be
passed, so that we can use the same "copy each argument into the
parameter region" algorithm that we use for scalar arguments.

This may change when we start modeling destructors of temporaries,
but for now this is the last part of <rdar://problem/12137950>.

llvm-svn: 164830
2012-09-28 17:15:25 +00:00
Jordan Rose b559f18584 [analyzer] Create a temp region when a method is called on a struct rvalue.
An rvalue has no address, but calling a C++ member function requires a
'this' pointer. This commit makes the analyzer create a temporary region
in which to store the struct rvalue and use as a 'this' pointer whenever
a member function is called on an rvalue, which is essentially what
CodeGen does.

More of <rdar://problem/12137950>. The last part is tracking down the
C++ FIXME in array-struct-region.cpp.

llvm-svn: 164829
2012-09-28 17:15:21 +00:00
Jordan Rose e7126582a4 [analyzer] Create a temporary region for rvalue structs when accessing fields
Struct rvalues are represented in the analyzer by CompoundVals,
LazyCompoundVals, or plain ConjuredSymbols -- none of which have associated
regions. If the entire structure is going to persist, this is not a
problem -- either the rvalue will be assigned to an existing region, or
a MaterializeTemporaryExpr will be present to create a temporary region.
However, if we just need a field from the struct, we need to create the
temporary region ourselves.

This is inspired by the way CodeGen handles calls to temporaries;
support for that in the analyzer is coming next.

Part of <rdar://problem/12137950>

llvm-svn: 164828
2012-09-28 17:15:12 +00:00
Ted Kremenek 8971b028e7 Revert "Use sep instead of ' '."
This isn't correct, as Jordan correctly points out.

llvm-svn: 164711
2012-09-26 18:06:08 +00:00
Ted Kremenek 2de0a9919b Use sep instead of ' '.
llvm-svn: 164709
2012-09-26 17:23:31 +00:00
Ted Kremenek a808e165b2 Remove unnecessary ASTContext& parameter from SymExpr::getType().
llvm-svn: 164661
2012-09-26 06:00:14 +00:00
Jordan Rose db72e2fc37 Reapply "[analyzer] Remove constraints on dead symbols as part of removeDeadBindings."
Previously, we'd just keep constraints around forever, which means we'd
never be able to merge paths that differed only in constraints on dead
symbols.

Because we now allow constraints on symbolic expressions, not just single
symbols, this requires changing SymExpr::symbol_iterator to include
intermediate symbol nodes in its traversal, not just the SymbolData leaf
nodes.

This depends on the previous commit to be correct. Originally applied in
r163444, reverted in r164275, now being re-applied.

llvm-svn: 164622
2012-09-25 19:03:06 +00:00
Jordan Rose 60d704ab4a [analyzer] Calculate liveness for symbolic exprs as well as atomic symbols.
No tests, but this allows the optimization of removing dead constraints.
We can then add tests that we don't do this prematurely.

<rdar://problem/12333297>

Note: the added FIXME to investigate SymbolRegionValue liveness is
tracked by <rdar://problem/12368183>. This patch does not change the
existing behavior.

llvm-svn: 164621
2012-09-25 19:03:01 +00:00
Anna Zaks 3533a54a97 [analyzer]Prevent infinite recursion(assume->checker:evalAssume->assume)
(Unfortunately, I do not have a good reduced test case for this.)

llvm-svn: 164541
2012-09-24 17:43:41 +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
Jordan Rose 4ac7cba404 [analyzer] Track a null value back through FindLastStoreBRVisitor.
Also, tidy up the other tracking visitors so that they mark the right
things as interesting and don't do extra work.

llvm-svn: 164448
2012-09-22 01:25:00 +00:00
Jordan Rose fa92f0f298 [analyzer] Always allow BugReporterVisitors to see the bug path.
Before, PathDiagnosticConsumers that did not support actual path output
would (sensibly) cause the generation of the full path to be skipped.
However, BugReporterVisitors may want to see the path in order to mark a
BugReport as invalid.

Now, even for a path generation scheme of 'None' we will still create a
trimmed graph and walk backwards through the bug path, doing no work other
than passing the nodes to the BugReporterVisitors. This isn't cheap, but
it's necessary to properly do suppression when the first path consumer does
not support path notes.

In the future, we should try only generating the path and visitor-provided
path notes once, or at least only creating the trimmed graph once.

llvm-svn: 164447
2012-09-22 01:24:56 +00:00
Jordan Rose 5a751b993f [analyzer] Allow a BugReport to be marked "invalid" during path generation.
This is intended to allow visitors to make decisions about whether a
BugReport is likely a false positive. Currently there are no visitors
making use of this feature, so there are no tests.

When a BugReport is marked invalid, the invalidator must provide a key
that identifies the invaliation (intended to be the visitor type and a
context pointer of some kind). This allows us to reverse the decision
later on. Being able to reverse a decision about invalidation gives us more
flexibility, and allows us to formulate conditions like "this report is
invalid UNLESS the original argument is 'foo'". We can use this to
fine-tune our false-positive suppression (coming soon).

llvm-svn: 164446
2012-09-22 01:24:53 +00:00
Jordan Rose 6f3d2f0acd [analyzer] Look through OpaqueValueExprs when tracking a nil value.
This allows us to show /why/ a particular object is nil, even when it is
wrapped in an OpaqueValueExpr.

llvm-svn: 164445
2012-09-22 01:24:49 +00:00
Jordan Rose 106b037a85 [analyzer] Better path notes for null pointers passed as arguments.
Rather than saying "Null pointer value stored to 'foo'", we now say
"Passing null pointer value via Nth parameter 'foo'", which is much better.
The note is also now on the argument expression as well, rather than the
entire call.

This paves the way for continuing to track arguments back to their sources.

<rdar://problem/12211490>

llvm-svn: 164444
2012-09-22 01:24:46 +00:00
Jordan Rose c102b35b44 Use llvm::getOrdinalSuffix to print ordinal numbers in diagnostics.
Just a refactoring of common infrastructure. No intended functionality change.

llvm-svn: 164443
2012-09-22 01:24:42 +00:00
Jordan Rose 1d64a49855 [analyzer] Check that an ObjCIvarRefExpr's base is non-null even as an lvalue.
Like with struct fields, we want to catch cases like this early,
so that we can produce better diagnostics and path notes:

  PointObj *p = nil;
  int *px = &p->_x; // should warn here
  *px = 1;

llvm-svn: 164442
2012-09-22 01:24:38 +00:00
Jordan Rose 04dcb7235f [analyzer] Check that a member expr is valid even when the result is an lvalue.
We want to catch cases like this early, so that we can produce better
diagnostics and path notes:

  Point *p = 0;
  int *px = &p->x; // should warn here
  *px = 1;

llvm-svn: 164441
2012-09-22 01:24:33 +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 089c5510b8 Simplify getRuntimeDefinition() back to taking no arguments.
llvm-svn: 164363
2012-09-21 06:13:13 +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 ae134c6449 Revert "[analyzer] Remove constraints on dead symbols as part of removeDeadBindings."
While we definitely want this optimization in the future, we're not
currently handling constraints on symbolic /expressions/ correctly.
These should stay live even if the SymExpr itself is no longer referenced
because could recreate an identical SymExpr later. Only once the SymExpr
can no longer be recreated -- i.e. a component symbol is dead -- can we
safely remove the constraints on it.

This liveness issue is tracked by <rdar://problem/12333297>.

This reverts r163444 / 24c7f98828e039005cff3bd847e7ab404a6a09f8.

llvm-svn: 164275
2012-09-20 01:54:56 +00:00
Anna Zaks 4278234360 [analyzer] Teach the analyzer about implicit initialization of statics
in ObjCMethods.

Extend FunctionTextRegion to represent ObjC methods as well as
functions. Note, it is not clear what type ObjCMethod region should
return. Since the type of the FunctionText region is not currently used,
defer solving this issue.

llvm-svn: 164046
2012-09-17 19:13:56 +00:00
Anna Zaks f6a5d793d2 [analyzer] Don't reimplement an existing function.
Thanks Jordan.

llvm-svn: 163762
2012-09-13 00:37:12 +00:00
Ted Kremenek 8b3f938697 Refactor logic in ExprEngine for detecting 'noreturn' methods
in NSException to a helper object in libAnalysis that can also
be used by Sema.  Not sure if the predicate name 'isImplicitNoReturn'
is the best one, but we can massage that later.

No functionality change.

llvm-svn: 163759
2012-09-13 00:21:31 +00:00
Anna Zaks 5d2964e770 [analyzer] Do not report use of undef on "return foo();" when the return type is void.
Fixes a false positive found by analyzing LLVM code base.

llvm-svn: 163750
2012-09-12 22:57:40 +00:00
Anna Zaks e663b80975 [analyzer] Teach UndefOrNullArgVisitor to track parent regions.
llvm-svn: 163748
2012-09-12 22:57:30 +00:00
Jordan Rose 5297748e3f [analyzer] Fix another use of the address of a temporary, like r163402.
Again, GCC is more aggressive about reusing temporary space than we are,
leading to Release build crashes for this undefined behavior.

PR13710 (though it may not be the only problem there)

llvm-svn: 163747
2012-09-12 22:48:08 +00:00
Jordan Rose d44977ef64 [analyzer] Handle when the dynamic type is worse than the static type.
Currently we don't update the dynamic type of a C++ object when it is
cast. This can cause the situation above, where the static type of the
region is now known to be a subclass of the dynamic type.

Once we start updating DynamicTypeInfo in response to the various kinds
of casts in C++, we can re-add this assert to make sure we don't miss
any cases. This work is tracked by <rdar://problem/12287087>.

In -Asserts builds, we will simply not return any runtime definition
when our DynamicTypeInfo is known to be incorrect like this.

llvm-svn: 163745
2012-09-12 21:48:17 +00:00
Jordan Rose 99c6c2b4e2 Revert "[analyzer] Use the static type for a virtual call if the dynamic type is worse."
Using the static type may be inconsistent with later calls. We should just
report that there is no inlining definition available if the static type is
better than the dynamic type. See next commit.

This reverts r163644 / 19d5886d1704e24282c86217b09d5c6d35ba604d.

llvm-svn: 163744
2012-09-12 21:48:13 +00:00
Ted Kremenek ba22a035ad Fix regression where "looping back to the head of" PathDiagnosticEvents
were not emitted.

Fixes <rdar://problem/12280665>.

llvm-svn: 163683
2012-09-12 06:22:18 +00:00
Richard Smith b15fe3a5e4 PR13811: Add a FunctionParmPackExpr node to handle references to function
parameter packs where the reference is not being expanded but the pack has
been. Previously, Clang would segfault in such cases.

llvm-svn: 163672
2012-09-12 00:56:43 +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
Jordan Rose e35fdeb330 [analyzer] Use the static type for a virtual call if the dynamic type is worse.
reinterpret_cast does not provide any of the usual type information that
static_cast or dynamic_cast provide -- only the new type. This can get us
in a situation where the dynamic type info for an object is actually a
superclass of the static type, which does not match what CodeGen does at all.
In these cases, just fall back to the static type as the best possible type
for devirtualization.

Should fix the crashes on our internal buildbot.

llvm-svn: 163644
2012-09-11 18:47:13 +00:00
Anna Zaks 464493fbf4 [analyzer] Disable STL inlining. Blocked by PR13724.
llvm-svn: 163633
2012-09-11 17:15:39 +00:00
Jordan Rose 12f669e3cd [analyzer] Member function calls that use qualified names are non-virtual.
C++11 [expr.call]p1: ...If the selected function is non-virtual, or if the
  id-expression in the class member access expression is a qualified-id,
  that function is called. Otherwise, its final overrider in the dynamic type
  of the object expression is called.

<rdar://problem/12255556>

llvm-svn: 163577
2012-09-11 00:31:02 +00:00
Anna Zaks 1ded453e36 [analyzer] Turn stl inlining back on.
The one reported bug, which was exposed by stl inlining, is addressed in
r163558.

llvm-svn: 163574
2012-09-10 23:59:02 +00:00
Anna Zaks 4f9c460874 [analyzer] Do not count calls to small functions when computing stack
depth.

We only want to count how many substantial functions we inlined. This
is an improvement to r163558.

llvm-svn: 163571
2012-09-10 23:35:11 +00:00
Anna Zaks 5446f4dfb1 [analyzer] Add an option to enable/disable objc inlining.
llvm-svn: 163562
2012-09-10 22:56:41 +00:00
Anna Zaks 14ce52492f [analyzer] Add ipa-always-inline-size option (with 3 as the default).
The option allows to always inline very small functions, whose size (in
number of basic blocks) is set using -analyzer-config
ipa-always-inline-size option.

llvm-svn: 163558
2012-09-10 22:37:19 +00:00
Jordan Rose c6fcbf06a6 [analyzer] Make the defaults explicit for each of the new config options.
Also, document both new inlining options in IPA.txt.

llvm-svn: 163551
2012-09-10 21:54:24 +00:00
Jordan Rose 1e0e4001c8 [analyzer] For now, don't inline C++ standard library functions.
This is a (heavy-handed) solution to PR13724 -- until we know we can do
a good job inlining the STL, it's best to be consistent and not generate
more false positives than we did before. We can selectively whitelist
certain parts of the 'std' namespace that are known to be safe.

This is controlled by analyzer config option 'c++-stdlib-inlining', which
can be set to "true" or "false".

This commit also adds control for whether or not to inline any templated
functions (member or non-member), under the config option
'c++-template-inlining'. This option is currently on by default.

llvm-svn: 163548
2012-09-10 21:27:35 +00:00
Ted Kremenek a0fa5d6564 Fix another case where we should be using isBeforeInTranslationUnit().
llvm-svn: 163533
2012-09-10 19:07:56 +00:00
Ted Kremenek 54fd079265 Add a few more cases where we should be using isBeforeInTranslationUnit().
llvm-svn: 163531
2012-09-10 19:02:33 +00:00
Ted Kremenek 6c7a5eae6d Revert "Revert Ted's r163489 and r163490, due to breakage."
I need to see how this breaks on other platforms when I fix the issue
that Benjamin Kramer pointed out.

This includes r163489 and r163490, plus a two line change.

llvm-svn: 163512
2012-09-10 14:50:55 +00:00
NAKAMURA Takumi 6eb1399088 Revert Ted's r163489 and r163490, due to breakage.
r163489, "Take another crack at stabilizing the emission order of analyzer"
r163490, "Use isBeforeInTranslationUnitThan() instead of operator<."

llvm-svn: 163497
2012-09-10 09:17:27 +00:00
Ted Kremenek f1fc8ce65d Use isBeforeInTranslationUnitThan() instead of operator<.
llvm-svn: 163490
2012-09-10 06:56:07 +00:00
Ted Kremenek 3d92699d3c Take another crack at stabilizing the emission order of analyzer
diagnostics without using FoldingSetNodeIDs.  This is done
by doing a complete recursive comparison of the PathDiagnostics.

Note that the previous method of comparing FoldingSetNodeIDs did
not end up relying on unstable things such as pointer addresses, so
I suspect this may still have some issues on various buildbots because
I'm not sure if the true source of non-determinism has been eliminated.
The tests pass for me, so the only way to know is to commit this change
and see what happens.

llvm-svn: 163489
2012-09-10 06:20:06 +00:00
Ted Kremenek 9b9ee2a616 Indent the "message" key in analyzer plist output.
llvm-svn: 163487
2012-09-10 06:19:43 +00:00
Ted Kremenek e9764d8f91 Remove dead method ProgramState::MarshalState().
llvm-svn: 163479
2012-09-09 14:55:59 +00:00
Ted Kremenek e7ec4ef48d Fix bug in BugReporter::RemoveUneededCalls() where "prunable"
PathDiagnosticEventPieces were *always* pruned.  Instead, they
are suppose to only be pruned if the entire call gets pruned.

llvm-svn: 163460
2012-09-08 07:18:18 +00:00
Ted Kremenek b0d1c70258 Attempt (again) to stabilize the order of the emission of diagnostics
of the analyzer by using the FullProfile() of a PathDiagnostic
for ordering them.

llvm-svn: 163455
2012-09-08 04:26:37 +00:00
Jordan Rose 5481cfefa6 [analyzer] ObjCSelfInitChecker should always clean up in postCall checks.
ObjCSelfInitChecker stashes information in the GDM to persist it across
function calls; it is stored in pre-call checks and retrieved post-call.
The post-call check is supposed to clear out the stored state, but was
failing to do so in cases where the call did not have a symbolic return
value.

This was actually causing the inappropriate cache-out from r163361.
Per discussion with Anna, we should never actually cache out when
assuming the receiver of an Objective-C message is non-nil, because
we guarded that node generation by checking that the state has changed.
Therefore, the only states that could reach this exact ExplodedNode are
ones that should have merged /before/ making this assumption.

r163361 has been reverted and the test case removed, since it won't
actually test anything interesting now.

llvm-svn: 163449
2012-09-08 01:47:28 +00:00
Ted Kremenek 1fdcfcdf13 Revert "Attempt to make the PathDiagnostic emission order more deterministic by"
llvm-svn: 163446
2012-09-08 01:25:00 +00:00
Ted Kremenek af4cc7eab1 Revert "Further tweaks to hopefully make the PathDiagnostic emission more deterministic."
llvm-svn: 163445
2012-09-08 01:24:53 +00:00
Jordan Rose 5860e329a4 [analyzer] Remove constraints on dead symbols as part of removeDeadBindings.
Previously, we'd just keep constraints around forever, which means we'd
never be able to merge paths that differed only in constraints on dead
symbols.

Because we now allow constraints on symbolic expressions, not just single
symbols, this requires changing SymExpr::symbol_iterator to include
intermediate symbol nodes in its traversal, not just the SymbolData leaf
nodes.

llvm-svn: 163444
2012-09-08 01:24:53 +00:00
Jordan Rose dd5e8c4975 [analyzer] Symbolic regions are live if any subregions are live.
RegionStoreManager was only treating a SymbolicRegion's symbel as live
if there was a binding referring to the region itself.

No test case because constraints are currently not being cleaned out
of the constraint manager at all (even if the symbol is legitimately dead).

llvm-svn: 163443
2012-09-08 01:24:49 +00:00
Jordan Rose aaf8318480 [analyzer] Cast the result of a placement new-expression to the correct type.
This is necessary because further analysis will assume that the SVal's
type matches the AST type. This caused a crash when trying to perform
a derived-to-base cast on a C++ object that had been new'd to be another
object type.

Yet another crash in PR13763.

llvm-svn: 163442
2012-09-08 01:24:38 +00:00
Ted Kremenek a11a741e2f Further tweaks to hopefully make the PathDiagnostic emission more deterministic.
llvm-svn: 163430
2012-09-07 23:13:11 +00:00
Ted Kremenek 244e1d7d0f Remove ProgramState::getSymVal(). It was being misused by Checkers,
with at least one subtle bug in MacOSXKeyChainAPIChecker where the
calling the method was a substitute for assuming a symbolic value
was null (which is not the case).

We still keep ConstraintManager::getSymVal(), but we use that as
an optimization in SValBuilder and ProgramState::getSVal() to
constant-fold SVals.  This is only if the ConstraintManager can
provide us with that information, which is no longer a requirement.
As part of this, introduce a default implementation of
ConstraintManager::getSymVal() which returns null.

For Checkers, introduce ConstraintManager::isNull(), which queries
the state to see if the symbolic value is constrained to be a null
value.  It does this without assuming it has been implicitly constant
folded.

llvm-svn: 163428
2012-09-07 22:31:01 +00:00
Ted Kremenek 334ad6ac13 Attempt to make the PathDiagnostic emission order more deterministic by
looking at PathPieces.

llvm-svn: 163427
2012-09-07 22:24:24 +00:00
Ted Kremenek 58ec11612c Remove ConstraintManager:isEqual(). It is no longer used.
llvm-svn: 163425
2012-09-07 22:24:18 +00:00
Jordan Rose 8dc77398a1 [analyzer] Use cast<> instead of getAs<> for a CFGElement known to be a CFGStmt.
When adding the next statement to the CoreEngine's work list, we take care
of all the special cases first. We certainly shouldn't be building
PostStmts with null statements (the diagnostics machinery assumes such
StmtPoints do not exist), and we should find out sooner if we're missing
a special case.

A refinement of r163402 that should help prevent further issues like PR13760.

llvm-svn: 163409
2012-09-07 19:48:09 +00:00
Jordan Rose 3c2713accf [analyzer] Don't use the address of a temporary CFGElement.
GCC destroys temporary objects more aggressively than clang, so this
results in incorrect behavior when compiling GCC Release builds.

We could avoid this issue under C++11 by preventing getAs from being
called when 'this' is an rvalue:

  template<class ElemTy> const ElemTy *getAs() const & { ... }
  template<class ElemTy> const ElemTy *getAs() const && = delete;

Unfortunately, we do not have compatibility macros for this behavior yet.

This will hopefully fix PR13760 and PR13762.

llvm-svn: 163402
2012-09-07 18:36:17 +00:00
Anna Zaks 67e0062b7c [analyzer] Explain why we need condition 8.
llvm-svn: 163394
2012-09-07 16:22:09 +00:00
Ted Kremenek 891bcdb644 ExplodedGraph::shouldCollectNode() should not collect nodes for non-Expr Stmts
(as this previously was the case before this was refactored).  We also shouldn't
need to specially handle BinaryOperators since the eagerly-assume heuristic tags
such nodes.

llvm-svn: 163374
2012-09-07 06:56:18 +00:00
Ted Kremenek 7c15040e98 Fix bug in ConditionBRVisitor where for C++ (and not C) we were not ignoring
implicit pointer-to-boolean conversions in condition expressions.  This would
result in inconsistent diagnostic emission between C and C++.

A consequence of this is now ConditionBRVisitor and TrackConstraintBRVisitor may
emit redundant diagnostics, for example:

  "Assuming pointer value is null" (TrackConstraintBRVisitor)
  "Assuming 'p' is null" (ConditionBRVisitor)

We need to reconcile the two, and perhaps prefer one over the other in some
cases.

llvm-svn: 163372
2012-09-07 06:51:37 +00:00
Jordan Rose 81456d9f6d [analyzer] Fail gracefully when the dynamic type is outside the hierarchy.
With some particularly evil casts, we can get an object whose dynamic type
is not actually a subclass of its static type. In this case, we won't even
find the statically-resolved method as a devirtualization candidate.

Rather than assert that this situation cannot occur, we now simply check
that the dynamic type is not an ancestor or descendent of the static type,
and leave it at that.

This error actually occurred analyzing LLVM: CallEventManager uses a
BumpPtrAllocator to allocate a concrete subclass of CallEvent
(FunctionCall), but then casts it to the actual subclass requested
(such as ObjCMethodCall) to perform the constructor.

Yet another crash in PR13763.

llvm-svn: 163367
2012-09-07 01:19:42 +00:00
Jordan Rose 7e97996f4e [analyzer] Don't crash if we cache out while evaluating an ObjC message.
A bizarre series of coincidences led us to generate a previously-seen
node in the middle of processing an Objective-C message, where we assume
the receiver is non-nil. We were assuming that such an assumption would
never "cache out" like this, and blithely went on using a null ExplodedNode
as the predecessor for the next step in evaluation.

Although the test case committed here is complicated, this could in theory
happen in other ways as well, so the correct fix is just to test if the
non-nil assumption results in an ExplodedNode we've seen before.

<rdar://problem/12243648>

llvm-svn: 163361
2012-09-06 23:44:36 +00:00
Jordan Rose 2bc9674b0a [analyzer] Don't attempt to devirtualize calls to base class destructors.
CXXDestructorCall now has a flag for when it is a base destructor call.
Other kinds of destructor calls (locals, fields, temporaries, and 'delete')
all behave as "whole-object" destructors and do not behave differently
from one another (specifically, in these cases we /should/ try to
devirtualize a call to a virtual destructor).

This was causing crashes in both our internal buildbot, the crash still
being tracked in PR13765, and some of the crashes being tracked in PR13763,
due to a assertion failure. (The behavior under -Asserts happened to be
correct anyway.)

Adding this knowledge also allows our DynamicTypePropagation checker to do
a bit less work; the special rules about virtual method calls during a
destructor only require extra handling during base destructors.

llvm-svn: 163348
2012-09-06 20:37:08 +00:00
Roman Divacky e637711ae0 Dont cast away const needlessly. Found by gcc48 -Wcast-qual.
llvm-svn: 163325
2012-09-06 15:59:27 +00:00
Anna Zaks 3245e584db [analyzer] Enhance the member expr tracking to account for references.
As per Jordan's suggestion. (Came out of code review for r163261.)

llvm-svn: 163269
2012-09-05 23:41:54 +00:00
Jordan Rose 6d671cc34a [analyzer] Always include destructors in the analysis CFG.
While destructors will continue to not be inlined (unless the analyzer
config option 'c++-inlining' is set to 'destructors'), leaving them out
of the CFG is an incomplete model of the behavior of an object, and
can cause false positive warnings (like PR13751, now working).

Destructors for temporaries are still not on by default, since
(a) we haven't actually checked this code to be sure it's fully correct
    (in particular, we probably need to be very careful with regard to
    lifetime-extension when a temporary is bound to a reference,
    C++11 [class.temporary]p5), and
(b) ExprEngine doesn't actually do anything when it sees a temporary
    destructor in the CFG -- not even invalidate the object region.

To enable temporary destructors, set the 'cfg-temporary-dtors' analyzer
config option to '1'. The old -cfg-add-implicit-dtors cc1 option, which
controlled all implicit destructors, has been removed.

llvm-svn: 163264
2012-09-05 22:55:23 +00:00
Anna Zaks e5cb4981d0 [analyzer] Fix a crash PR13762.
llvm-svn: 163262
2012-09-05 22:31:58 +00:00
Anna Zaks b4b2b57ee0 [analyzer] NullOrUndef diagnostics: track symbols binded to regions.
If a region is binded to a symbolic value, we should track the symbol.

(The code I changed was not previously exercised by the regression
tests.)

llvm-svn: 163261
2012-09-05 22:31:55 +00:00
Jordan Rose fcdda36149 [analyzer] Be more forgiving about calling methods on struct rvalues.
The problem is that the value of 'this' in a C++ member function call
should always be a region (or NULL). However, if the object is an rvalue,
it has no associated region (only a conjured symbol or LazyCompoundVal).
For now, we handle this in two ways:

1) Actually respect MaterializeTemporaryExpr. Before, it was relying on
   CXXConstructExpr to create temporary regions for all struct values.
   Now it just does the right thing: if the value is not in a temporary
   region, create one.

2) Have CallEvent recognize the case where its 'this' pointer is a
   non-region, and just return UnknownVal to keep from confusing clients.

The long-term problem is being tracked internally in <rdar://problem/12137950>,
but this makes many test cases pass.

llvm-svn: 163220
2012-09-05 17:11:26 +00:00
Jordan Rose d1a08b6e43 [analyzer] Clean up a couple uses of getPointeeType().
No intended functionality change.

llvm-svn: 163219
2012-09-05 17:11:22 +00:00
Jordan Rose bc009d4493 Revert "[analyzer] Treat all struct values as regions (even rvalues)."
This turned out to have many implications, but what eventually seemed to
make it unworkable was the fact that we can get struct values (as
LazyCompoundVals) from other places besides return-by-value function calls;
that is, we weren't actually able to "treat all struct values as regions"
consistently across the entire analyzer core.

Hopefully we'll be able to come up with an alternate solution soon.

This reverts r163066 / 02df4f0aef142f00d4637cd851e54da2a123ca8e.

llvm-svn: 163218
2012-09-05 17:11:15 +00:00
Jordan Rose 7523d1a847 [analyzer] Don't use makeIntVal to create a floating-point value.
SimpleSValBuilder processes a couple trivial identities, including 'x - x'
and 'x ^ x' (both 0). However, the former could appear with arguments of
floating-point type, and we weren't checking for that. This started
triggering an assert with r163069, which checks that a constant value is
actually going to be used as an integer or pointer.

llvm-svn: 163159
2012-09-04 19:34:58 +00:00
Joao Matos 566359c0bf Revert r163083 per chandlerc's request.
llvm-svn: 163149
2012-09-04 17:49:35 +00:00
Joao Matos c32a7e4d8e Implemented parsing and AST support for the MS __leave exception statement. Also a minor fix to __except printing in StmtPrinter.cpp. Thanks to Aaron Ballman for review.
llvm-svn: 163083
2012-09-02 03:45:41 +00:00
Jordan Rose d229e39a9a [analyzer] Silence unused variable warnings in NDEBUG builds.
No functionality change.

llvm-svn: 163073
2012-09-01 19:15:13 +00:00
Jordan Rose 21580c2f92 [analyzer] Disallow creation of int vals with explicit bit width / signedness.
All clients of BasicValueFactory should be using QualTypes instead, and
indeed it seems they are. This caught the (fortunately harmless) bug
fixed in the previous commit.

No intended functionality change.

llvm-svn: 163069
2012-09-01 17:39:24 +00:00
Jordan Rose a44ad1b35c [analyzer] Don't attempt to create a floating-point value of "1" for ++/--.
The current logic would actually create a float- or double-sized signed
integer value of 1, which is not at all the same.

No test because the value would be swallowed by an Unknown as soon as it
gets added or subtracted to the original value, but it enables the cleanup
in the next patch.

llvm-svn: 163068
2012-09-01 17:39:17 +00:00
Jordan Rose 82ae9898ef [analyzer] Treat all struct values as regions (even rvalues).
This allows us to correctly symbolicate the fields of structs returned by
value, as well as get the proper 'this' value for when methods are called
on structs returned by value.

This does require a moderately ugly hack in the StoreManager: if we assign
a "struct value" to a struct region, that now appears as a Loc value being
bound to a region of struct type. We handle this by simply "dereferencing"
the struct value region, which should create a LazyCompoundVal.

This should fix recent crashes analyzing LLVM and on our internal buildbot.

<rdar://problem/12137950>

llvm-svn: 163066
2012-09-01 17:39:09 +00:00
Jordan Rose 2da564380a [analyzer] Always derive a CallEvent's return type from its origin expr.
Previously, we preferred to get a result type by looking at the callee's
declared result type. This allowed us to handlereferences, which are
represented in the AST as lvalues of their pointee type. (That is, a call
to a function returning 'int &' has type 'int' and value kind 'lvalue'.)

However, this results in us preferring the original type of a function
over a casted type. This is a problem when a function  pointer is casted
to another type, because the conjured result value will have the wrong
type. AdjustedReturnValueChecker is supposed to handle this, but still
doesn't handle the case where there is no "original function" at all,
i.e. where the callee is unknown.

Now, we instead look at the call expression's value kind (lvalue, xvalue,
or prvalue), and adjust the expr's type accordingly. This will have no
effect when the function is inlined, and will conjure the value that will
actually be used when it is not.

This makes AdjustedReturnValueChecker /nearly/ unnecessary; unfortunately,
the cases where it would still be useful are where we need to cast the
result of an inlined function or a checker-evaluated function, and in these
cases we don't know what we're casting /from/ by the time we can do post-
call checks. In light of that, remove AdjustedReturnValueChecker, which
was already not checking quite a few calls.

llvm-svn: 163065
2012-09-01 17:39:00 +00:00
Ted Kremenek cdf814900d Split library clangRewrite into clangRewriteCore and clangRewriteFrontend.
This is similar to how we divide up the StaticAnalyzer libraries to separate
core functionality to what is clearly associated with Frontend actions.

llvm-svn: 163050
2012-09-01 05:09:24 +00:00
Jordan Rose 219c9d0dd3 [analyzer] Though C++ inlining is enabled, don't inline ctors and dtors.
More generally, this adds a new configuration option 'c++-inlining', which
controls which C++ member functions can be considered for inlining. This
uses the new -analyzer-config table, so the cc1 arguments will look like this:

... -analyzer-config c++-inlining=[none|methods|constructors|destructors]

Note that each mode implies that all the previous member function kinds
will be inlined as well; it doesn't make sense to inline destructors
without inlining constructors, for example.

The default mode is 'methods'.

llvm-svn: 163004
2012-08-31 17:06:49 +00:00
Jordan Rose cc0b1bfa56 [analyzer] Ensure that PathDiagnostics profile the same regardless of path.
PathDiagnostics are actually profiled and uniqued independently of the
path on which the bug occurred. This is used to merge diagnostics that
refer to the same issue along different paths, as well as by the plist
diagnostics to reference files created by the HTML diagnostics.

However, there are two problems with the current implementation:

1) The bug description is included in the profile, but some
   PathDiagnosticConsumers prefer abbreviated descriptions and some
   prefer verbose descriptions. Fixed by including both descriptions in
   the PathDiagnostic objects and always using the verbose one in the profile.

2) The "minimal" path generation scheme provides extra information about
   which events came from macros that the "extensive" scheme does not.
   This resulted not only in different locations for the plist and HTML
   diagnostics, but also in diagnostics being uniqued in the plist output
   but not in the HTML output. Fixed by storing the "end path" location
   explicitly in the PathDiagnostic object, rather than trying to find the
   last piece of the path when the diagnostic is requested.

This should hopefully finish unsticking our internal buildbot.

llvm-svn: 162965
2012-08-31 00:36:26 +00:00
Jordan Rose 7444f5d826 [analyzer] Fix a crash in plist-html generation introduced in r162939.
Basically, do the correct thing to fix the XML generation error, rather
than making it even worse by unilaterally dereferencing a null pointer.

llvm-svn: 162964
2012-08-31 00:36:20 +00:00
Eli Friedman 34866c7719 Change the representation of builtin functions in the AST
(__builtin_* etc.) so that it isn't possible to take their address.
Specifically, introduce a new type to represent a reference to a builtin
function, and a new cast kind to convert it to a function pointer in the
operand of a call.  Fixes PR13195.

llvm-svn: 162962
2012-08-31 00:14:07 +00:00
Anna Zaks a8017eca1a [analyzer] Refactor the logic that determines if a functions should be
reanalyzed.

The policy on what to reanalyze should be in AnalysisConsumer with the
rest of visitation order logic.

There is no reason why ExprEngine needs to pass the Visited set to
CoreEngine, it can populate it itself.

llvm-svn: 162957
2012-08-30 23:42:02 +00:00
Jordan Rose 03fac27bab [analyzer] Plist diagnostics: Fix a case where we fail to close an XML tag.
If the current path diagnostic does /not/ have files associated with it, we
were simply skipping on to the next diagnostic with 'continue'. But that
also skipped the close tag for the diagnostic's <dict> node.

Part of fixing our internal analyzer buildbot.

llvm-svn: 162939
2012-08-30 20:43:09 +00:00
Ted Kremenek efca7a7e1b Rename 'MaxLoop' to 'maxBlockVisitOnPath' to reflect reality. We
should consider renaming the command line option as well.

llvm-svn: 162932
2012-08-30 19:26:56 +00:00
Ted Kremenek 6f5131f149 Rename AnalyzerOptions 'EagerlyAssume' to 'eagerlyAssumeBinOpBifurcation'.
llvm-svn: 162930
2012-08-30 19:26:48 +00:00
Ted Kremenek 8756c4a1a9 Store const& to AnalyzerOptions in AnalysisManager instead of copying
individual flags.

llvm-svn: 162929
2012-08-30 19:26:43 +00:00
Anna Zaks 07a821fb17 [analyzer] Fixup 162863.
Thanks Jordan.

llvm-svn: 162875
2012-08-29 23:23:39 +00:00
Anna Zaks 5d4ec36323 [analyzer] Improved diagnostic pruning for calls initializing values.
This heuristic addresses the case when a pointer (or ref) is passed
to a function, which initializes the variable (or sets it to something
other than '0'). On the branch where the inlined function does not
set the value, we report use of undefined value (or NULL pointer
dereference). The access happens in the caller and the path
through the callee would get pruned away with regular path pruning. To
solve this issue, we previously disabled diagnostic pruning completely
on undefined and null pointer dereference checks, which entailed very
verbose diagnostics in most cases. Furthermore, not all of the
undef value checks had the diagnostic pruning disabled.

This patch implements the following heuristic: if we pass a pointer (or
ref) to the region (on which the error is reported) into a function and
it's value is either undef or 'NULL' (and is a pointer), do not prune
the function.

llvm-svn: 162863
2012-08-29 21:22:37 +00:00
Ted Kremenek fb5351eed3 Add new -cc1 driver option -analyzer-config, which allows one to specify
a comma separated collection of key:value pairs (which are strings).  This
allows a general way to provide analyzer configuration data from the command line.

No clients yet.

llvm-svn: 162827
2012-08-29 05:55:00 +00:00
Jordan Rose 8d48938bf3 [analyzer] Teach CallEventManager that CXXTemporaryObjectExpr is also a ctor.
Specifically, CallEventManager::getCaller was looking at the call site for
an inlined call and trying to see what kind of call it was, but it only
checked for CXXConstructExprClass. (It's not using an isa<> here to avoid
doing three more checks on the the statement class.)

This caused an unreachable when we actually did inline the constructor of a
temporary object.

PR13717

llvm-svn: 162792
2012-08-28 20:52:21 +00:00
Jordan Rose 2be6e30d96 [analyzer] When we look for the last stmt in a function, skip implicit dtors.
When exiting a function, the analyzer looks for the last statement in the
function to see if it's a return statement (and thus bind the return value).
However, the search for "the last statement" was accepting statements that
were in implicitly-generated inlined functions (i.e. destructors). So we'd
go and get the statement from the destructor, and then say "oh look, this
function had no explicit return...guess there's no return value". And /that/
led to the value being returned being declared dead, and all our leak
checkers complaining.

llvm-svn: 162791
2012-08-28 20:52:13 +00:00
Jordan Rose 595c131460 [analyzer] Don't purge dead symbols at the end of calls if -analyzer-purge=none.
No test case since this is a debug option that we will never turn on by
default since it makes the leak checkers much less useful. (We'll only report
leaks at the end of analysis if -analyzer-purge=none.)

llvm-svn: 162772
2012-08-28 18:16:45 +00:00
Jordan Rose a0f7d35afe [analyzer] Rename addTrackNullOrUndefValueVisitor to trackNullOrUndefValue.
This helper function (in the clang::ento::bugreporter namespace) may add more
than one visitor, but conceptually it's tracking a single use of a null or
undefined value and should do so as best it can.

Also, the BugReport parameter has been made a reference to underscore that
it is non-optional.

llvm-svn: 162720
2012-08-28 00:50:51 +00:00
Jordan Rose 72c5515bab [analyzer] Refactor FindLastStoreBRVisitor to not find the store ahead of time.
As Anna pointed out to me offline, it's a little silly to walk backwards through
the graph to find the store site when BugReporter will do the exact same walk
as part of path diagnostic generation.

llvm-svn: 162719
2012-08-28 00:50:45 +00:00
Jordan Rose 5090904d6c [analyzer] If the last store into a region came from a function, step into it.
Previously, if we were tracking stores to a variable 'x', and came across this:

x = foo();

...we would simply emit a note here and stop. Now, we'll step into 'foo' and
continue tracking the returned value from there.

<rdar://problem/12114689>

llvm-svn: 162718
2012-08-28 00:50:42 +00:00
Jordan Rose e537cc05f5 [analyzer] Rename CallEvent::mayBeInlined to CallEvent::isCallStmt.
The two callers are using this in order to be conservative, so let's just
clarify the information that's actually being provided here. This is not
related to inlining decisions in any way.

No functionality change.

llvm-svn: 162717
2012-08-28 00:50:38 +00:00
Jordan Rose 1a61674f5a [analyzer] Look through casts when trying to track a null pointer dereference.
Also, add comments to addTrackNullOrUndefValueVisitor.

Thanks for the review, Anna!

llvm-svn: 162695
2012-08-27 20:18:30 +00:00
Jordan Rose 561919e5bd [analyzer] Don't inline constructors for objects allocated with operator new.
Because the CXXNewExpr appears after the CXXConstructExpr in the CFG, we don't
actually have the correct region to construct into at the time we decide
whether or not to inline. The long-term fix (discussed in PR12014) might be to
introduce a new CFG node (CFGAllocator) that appears before the constructor.

Tracking the short-term fix in <rdar://problem/12180598>.

llvm-svn: 162689
2012-08-27 18:39:22 +00:00
Anna Zaks 7d2babc046 [analyzer] More internal stats collection.
llvm-svn: 162687
2012-08-27 18:38:32 +00:00
Jordan Rose c93183042f [analyzer] Inline constructors for any object with a trivial destructor.
This allows us to better reason about status objects, like Clang's own
llvm::Optional (when its contents are trivially destructible), which are
often intended to be passed around by value.

We still don't inline constructors for temporaries in the general case.

<rdar://problem/11986434>

llvm-svn: 162681
2012-08-27 17:50:07 +00:00
Jordan Rose 0a0aa84da3 [analyzer] Use the common evalBind infrastructure for initializers.
This allows checkers (like the MallocChecker) to process the effects of the
bind. Previously, using a memory-allocating function (like strdup()) in an
initializer would result in a leak warning.

This does bend the expectations of checkBind a bit; since there is no
assignment expression, the statement being used is the initializer value.
In most cases this shouldn't matter because we'll use a PostInitializer
program point (rather than PostStmt) for any checker-generated nodes, though
we /will/ generate a PostStore node referencing the internal statement.
(In theory this could have funny effects if someone actually does an
assignment within an initializer; in practice, that seems like it would be
very rare.)

<rdar://problem/12171711>

llvm-svn: 162637
2012-08-25 01:06:23 +00:00
Chad Rosier de70e0ef45 [ms-inline asm] As part of a larger refactoring, rename AsmStmt to GCCAsmStmt.
No functional change intended.

llvm-svn: 162632
2012-08-25 00:11:56 +00:00
Ted Kremenek 5bc38bad73 Rework how PathDiagnosticConsumers pass knowledge of what files they
generated for a given diagnostic to another.  Because PathDiagnostics
are specific to a give PathDiagnosticConsumer, store in
a FoldingSet a unique hash for a PathDiagnostic (that will be the same
for the same bug for different PathDiagnosticConsumers) that
stores a list of files generated.  This can then be read by the
other PathDiagnosticConsumers.

This fixes breakage in the PLIST-HTML output.

llvm-svn: 162580
2012-08-24 19:35:19 +00:00
Jordan Rose 51c27163c0 [analyzer] If we dereference a NULL that came from a function, show the return.
More generally, any time we try to track where a null value came from, we
should show if it came from a function. This usually isn't necessary if
the value is symbolic, but if the value is just a constant we previously
just ignored its origin entirely. Now, we'll step into the function and
recursively add a visitor to the returned expression.

<rdar://problem/12114609>

llvm-svn: 162563
2012-08-24 16:34:31 +00:00
Anna Zaks 3d5d3d3e2c [analyzer] Make analyzer less aggressive when dealing with [self init].
With inlining, retain count checker starts tracking 'self' through the
init methods. The analyser results were too noisy if the developer
did not follow 'self = [super init]' pattern (which is common
especially in older code bases) - we reported self init anti-pattern AND
possible use-after-free. This patch teaches the retain count
checker to assume that [super init] does not fail when it's not consumed
by another expression. This silences the retain count warning that warns
about possibility of use-after-free when init fails, while preserving
all the other checking on 'self'.

llvm-svn: 162508
2012-08-24 00:06:12 +00:00
Jordan Rose 434f132060 [analyzer] For now, treat pointers-to-members as non-null void * symbols.
Until we have full support for pointers-to-members, we can at least
approximate some of their use by tracking null and non-null values.
We thus treat &A::m_ptr as a non-null void * symbol, and MemberPointer(0)
as a pointer-sized null constant.

This enables support for what is sometimes called the "safe bool" idiom,
demonstrated in the test case.

llvm-svn: 162495
2012-08-23 23:01:43 +00:00
Jordan Rose 081af085eb [analyzer] Handle UserDefinedConversion casts in C++.
This is trivial; the UserDefinedConversion always wraps a CXXMemberCallExpr
for the appropriate conversion function, so it's just a matter of
propagating that value to the CastExpr itself.

llvm-svn: 162494
2012-08-23 23:01:39 +00:00
Jordan Rose e5d5393efc [analyzer] Support C++ default arguments if they are literal values.
A CXXDefaultArgExpr wraps an Expr owned by a ParmVarDecl belonging to the
called function. In general, ExprEngine and Environment ought to treat this
like a ParenExpr or other transparent wrapper expression, with the inside
expression evaluated first.

However, if we call the same function twice, we'd produce a CFG that contains
the same wrapped expression twice, and we're not set up to handle that. I've
added a FIXME to the CFG builder to come back to that, but meanwhile we can
at least handle expressions that don't need to be explicitly evaluated:
literals. This probably handles many common uses of default parameters:
true/false, null, etc.

Part of PR13385 / <rdar://problem/12156507>

llvm-svn: 162453
2012-08-23 18:10:53 +00:00
Richard Smith 802c4b7015 Fix undefined behavior: member function calls where 'this' is a null pointer.
llvm-svn: 162430
2012-08-23 06:16:52 +00:00
Ted Kremenek 78094caa56 Fix an assortment of doxygen comment issues found by -Wdocumentation.
llvm-svn: 162412
2012-08-22 23:50:41 +00:00
Ted Kremenek 326702f1a1 Despite me asking Jordan to do r162313, revert it. We can provide
another way to whitelist these special cases.  This is an intermediate patch.

llvm-svn: 162386
2012-08-22 19:58:20 +00:00
Ted Kremenek a056d62961 Remove BasicConstraintManager. It hasn't been in active service for a while.
As part of this change, I discovered that a few of our tests were not testing
the RangeConstraintManager.  Luckily all of those passed when I moved them
over to use that constraint manager.

llvm-svn: 162384
2012-08-22 19:47:13 +00:00
Ted Kremenek 6269888166 Rename 'unbindLoc()' (in ProgramState) and 'Remove()' to
'killBinding()'.  The name is more specific, and one just forwarded
to the other.

Add some doxygen comments along the way.

llvm-svn: 162350
2012-08-22 06:37:46 +00:00
Ted Kremenek d94854a42e Rename 'currentX' to 'currX' throughout analyzer and libAnalysis.
Also rename 'getCurrentBlockCounter()' to 'blockCount()'.

This ripples a bunch of code simplifications; mostly aesthetic,
but makes the code a bit tighter.

llvm-svn: 162349
2012-08-22 06:26:15 +00:00
Ted Kremenek d227833cba Rename 'getConjuredSymbol*' to 'conjureSymbol*'.
No need to have the "get", the word "conjure" is a verb too!
Getting a conjured symbol is the same as conjuring one up.

This shortening is largely cosmetic, but just this simple changed
cleaned up a handful of lines, making them less verbose.

llvm-svn: 162348
2012-08-22 06:26:06 +00:00
Ted Kremenek 1afcb7442f Remove Store::bindDecl() and Store::bindDeclWithNoInit(), and
all forwarding methods.

This functionality is already covered by bindLoc().

llvm-svn: 162346
2012-08-22 06:00:18 +00:00
Ted Kremenek 2cd56c4c6e Rename 'BindCompoundLiteral' to 'bindCompoundLiteral' and
add doxygen comments.

llvm-svn: 162345
2012-08-22 06:00:12 +00:00
Ted Kremenek 34d39287b5 Consilidate SmallPtrSet count() followed by insert() into a single insert().
llvm-svn: 162330
2012-08-22 00:02:08 +00:00
Matt Beaumont-Gay 64621ea530 Add an llvm_unreachable to pacify GCC's -Wreturn-type.
llvm-svn: 162325
2012-08-21 22:27:18 +00:00
Jordan Rose e3e95cdf27 [analyzer] Set the default IPA mode to 'basic-inlining', which excludes C++.
Under -analyzer-ipa=basic-inlining, only C functions, blocks, and C++ static
member functions are inlined -- essentially, the calls that behave like simple
C function calls. This is essentially the behavior in Xcode 4.4.

C++ support still has some rough edges, and we don't want users to be worried
about them if they download and run their own checker. (In particular, the
massive number of false positives for analyzing LLVM comes from inlining
defensively-written code in contexts where more aggressive assumptions are
implicitly made. This problem is not unique to C++, but it is exacerbated by
the higher proportion of code that lives in header files in C++.)

The eventual goal is to be comfortable enough with C++ support (and simple
Objective-C support) to advance to -analyzer-ipa=inlining as the default
behavior. See the IPA design notes for more details.

llvm-svn: 162318
2012-08-21 21:44:21 +00:00
Jordan Rose 81125c4497 [analyzer] Push "references are non-null" knowledge up to the common parent.
This reduces duplication across the Basic and Range constraint managers, and
keeps their internals free of dealing with the semantics of C++. It's still
a little unfortunate that the constraint manager is dealing with this at all,
but this is pretty much the only place to put it so that it will apply to all
symbolic values, even when embedded in larger expressions.

llvm-svn: 162313
2012-08-21 20:52:19 +00:00
Jordan Rose 075d5d2e99 [analyzer] Assume that reference symbols are non-null.
By doing this in the constraint managers, we can ensure that ANY reference
whose value we don't know gets the effect, even if it's not a top-level
parameter.

llvm-svn: 162246
2012-08-21 00:27:33 +00:00
Jordan Rose 2b10f3f8a9 [analyzer] Add comments to ExplodedNode::NodeGroup.
No functionality change.

llvm-svn: 162216
2012-08-20 18:59:46 +00:00
Jordan Rose 4b4613cbec [analyzer] Replace boolean IsSink parameters with 'generateSink' methods.
Generating a sink is significantly different behavior from generating a
normal node, and a simple boolean parameter can be rather opaque. Per
offline discussion with Anna, adding new generation methods is the
clearest way to communicate intent.

No functionality change.

llvm-svn: 162215
2012-08-20 18:43:42 +00:00
Jordan Rose 0a9ea7c70d [analyzer] The result of && or || is always a 1 or 0.
Forgetting to at least cast the result was giving us Loc/NonLoc problems
in SValBuilder (hitting an assertion). But the standard (both C and C++)
does actually guarantee that && and || will result in the actual values
1 and 0, typed as 'int' in C and 'bool' in C++, and we can easily model that.

PR13461

llvm-svn: 162209
2012-08-20 17:04:45 +00:00
Jordan Rose a4309c941c [analyzer] Treat C++ 'throw' as a sink.
Our current handling of 'throw' is all CFG-based: it jumps to a 'catch' block
if there is one and the function exit block if not. But this doesn't really
get the right behavior when a function is inlined: execution will continue on
the caller's side, which is always the wrong thing to do.

Even within a single function, 'throw' completely skips any destructors that
are to be run. This is essentially the same problem as @finally -- a CFGBlock
that can have multiple entry points, whose exit points depend on whether it
was entered normally or exceptionally.

Representing 'throw' as a sink matches our current (non-)handling of @throw.
It's not a perfect solution, but it's better than continuing analysis in an
inconsistent or even impossible state.

<rdar://problem/12113713>

llvm-svn: 162157
2012-08-18 00:30:23 +00:00
Jordan Rose a97a99736e [analyzer] Treat @throw as a sink (stop processing).
The CFG approximates @throw as a return statement, but that's not good
enough in inlined functions. Moreover, since Objective-C exceptions are
usually considered fatal, we should be suppressing leak warnings like we
do for calls to noreturn functions (like abort()).

The comments indicate that we were probably intending to do this all along;
it may have been inadvertantly changed during a refactor at one point.

llvm-svn: 162156
2012-08-18 00:30:20 +00:00
Jordan Rose 80547386b8 [analyzer] Use PointerUnion to implement ExplodedNode::NodeGroup.
We shouldn't be reinventing our own wheels. This also paves the way for
marking different kinds of sinks.

No functionality change.

llvm-svn: 162154
2012-08-18 00:30:10 +00:00
Ted Kremenek 9dcf671d13 Remove #if 0 that has been around for a long time.
llvm-svn: 162030
2012-08-16 17:45:32 +00:00
Ted Kremenek 1e60273eed Remove "range_iterator" from PathDiagnosticPiece and just use ArrayRef<SourceRange> for ranges. This
removes conceptual clutter, and can allow us to easy migrate to C++11 style for-range loops if we
ever move to using C++11 in Clang.

llvm-svn: 162029
2012-08-16 17:45:29 +00:00
Ted Kremenek 9bf9af92a4 Allow multiple PathDiagnosticConsumers to be used with a BugReporter at the same time.
This fixes several issues:

- removes egregious hack where PlistDiagnosticConsumer would forward to HTMLDiagnosticConsumer,
but diagnostics wouldn't be generated consistently in the same way if PlistDiagnosticConsumer
was used by itself.

- emitting diagnostics to the terminal (using clang's diagnostic machinery) is no longer a special
case, just another PathDiagnosticConsumer.  This also magically resolved some duplicate warnings,
as we now use PathDiagnosticConsumer's diagnostic pruning, which has scope for the entire translation
unit, not just the scope of a BugReporter (which is limited to a particular ExprEngine).

As an interesting side-effect, diagnostics emitted to the terminal also have their trailing "." stripped,
just like with diagnostics emitted to plists and HTML.  This required some tests to be updated, but now
the tests have higher fidelity with what users will see.

There are some inefficiencies in this patch.  We currently generate the report graph (from the ExplodedGraph)
once per PathDiagnosticConsumer, which is a bit wasteful, but that could be pulled up higher in the
logic stack.  There is some intended duplication, however, as we now generate different PathDiagnostics (for the same issue)
for different PathDiagnosticConsumers.  This is necessary to produce the diagnostics that a particular
consumer expects.

llvm-svn: 162028
2012-08-16 17:45:23 +00:00
Richard Smith 235341bc88 Store SourceManager pointer on PrintingPolicy in the case where we're dumping,
and remove ASTContext reference (which was frequently bound to a dereferenced
null pointer) from the recursive lump of printPretty functions. In so doing,
fix (at least) one case where we intended to use the 'dump' mode, but that
failed because a null ASTContext reference had been passed in.

llvm-svn: 162011
2012-08-16 03:56:14 +00:00
Jordan Rose 6ee44e1f03 [analyzer] Look through all casts when trying to track constraints.
Previously, we were losing path notes (in both text and plist form)
because the interesting DeclRefExpr was buried in a cast.

llvm-svn: 161999
2012-08-16 00:03:33 +00:00
Jordan Rose e9753b0640 [analyzer] Even if we are not inlining a virtual call, still invalidate!
Fixes a mistake introduced in r161916.

llvm-svn: 161987
2012-08-15 21:05:15 +00:00
Jordan Rose 5fc5da0578 [analyzer] Correctly devirtualize virtual method calls in constructors.
This is the other half of C++11 [class.cdtor]p4 (the destructor side
was added in r161915). This also fixes an issue with post-call checks
where the 'this' value was already being cleaned out of the state, thus
being omitted from a reconstructed CXXConstructorCall.

llvm-svn: 161981
2012-08-15 20:07:17 +00:00
Jordan Rose 9910720851 [analyzer] Don't try to devirtualize if the class is incomplete.
A similar issue to the previous commit, introduced by r161915.

llvm-svn: 161961
2012-08-15 17:33:37 +00:00
Jordan Rose 31c3fa9c24 [analyzer] Only adjust the type of 'this' when we devirtualize a method call.
With reinterpret_cast, we can get completely unrelated types in a region
hierarchy together; this was resulting in CXXBaseObjectRegions being layered
directly on an (untyped) SymbolicRegion, whose symbol was from a completely
different type hierarchy. This was what was causing the internal buildbot to
fail.

Reverts r161911, which merely masked the problem.

llvm-svn: 161960
2012-08-15 17:33:34 +00:00
Jordan Rose 5132aaeb04 [analyzer] Don't inline dynamic-dispatch methods unless -analyzer-ipa=dynamic.
Previously we were checking -analyzer-ipa=dynamic-bifurcate only, and
unconditionally inlining everything else that had an available definition,
even under -analyzer-ipa=inlining (but not under -analyzer-ipa=none).

llvm-svn: 161916
2012-08-15 00:52:00 +00:00
Jordan Rose 0f6d63be06 [analyzer] Correctly devirtualize virtual method calls in destructors.
C++11 [class.cdtor]p4: When a virtual function is called directly or
  indirectly from a constructor or from a destructor, including during
  the construction or destruction of the class’s non-static data members,
  and the object to which the call applies is the object under
  construction or destruction, the function called is the final overrider
  in the constructor's or destructor's class and not one overriding it in
  a more-derived class.

llvm-svn: 161915
2012-08-15 00:51:56 +00:00
Jordan Rose 95c841eaa0 [analyzer] A base class needs a complete definition to provide offsets.
No test case yet; trying to reduce one from a failing internal buildbot.

llvm-svn: 161911
2012-08-15 00:36:44 +00:00