Commit Graph

1466 Commits

Author SHA1 Message Date
Jordan Rose 5fded08403 [analyzer] Handle C string default values for const char * arguments.
Previously, SValBuilder knew how to evaluate StringLiterals, but couldn't
handle an array-to-pointer decay for constant values. Additionally,
RegionStore was being too strict about loading from an array, refusing to
return a 'char' value from a 'const char' array. Both of these have been
fixed.

llvm-svn: 186520
2013-07-17 17:16:38 +00:00
Jordan Rose 05b2f98d89 [analyzer] Treat std::initializer_list as opaque rather than aborting.
Previously, the use of a std::initializer_list (actually, a
CXXStdInitializerListExpr) would cause the analyzer to give up on the rest
of the path. Now, it just uses an opaque symbolic value for the
initializer_list and continues on.

At some point in the future we can add proper support for initializer_list,
with access to the elements in the InitListExpr.

<rdar://problem/14340207>

llvm-svn: 186519
2013-07-17 17:16:33 +00:00
Pavel Labath 9ced602cc6 [analyzer] Add very limited support for temporary destructors
Summary:
This patch enables ExprEndgine to reason about temporary object destructors.
However, these destructor calls are never inlined, since this feature is still
broken. Still, this is sufficient to properly handle noreturn temporary
destructors and close bug #15599. I have also enabled the cfg-temporary-dtors
analyzer option by default.

Reviewers: jordan_rose

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D1131

llvm-svn: 186498
2013-07-17 08:33:58 +00:00
Craig Topper f59ba9f545 Fix formatting. No functional change.
llvm-svn: 186437
2013-07-16 18:27:27 +00:00
Craig Topper 694ddc73ea Add 'const' qualifiers to static const char* variables.
llvm-svn: 186383
2013-07-16 05:03:10 +00:00
Jordan Rose 6444653a06 [analyzer] Remove bogus assert: in C++11, 'new' can do list-initialization.
Previously, we asserted that whenever 'new' did not include a constructor
call, the type must be a non-record type. In C++11, however, uniform
initialization syntax (braces) allow 'new' to construct records with
list-initialization: "new Point{1, 2}".

Removing this assertion should be perfectly safe; the code here matches
what VisitDeclStmt does for regions allocated on the stack.

<rdar://problem/14403437>

llvm-svn: 186028
2013-07-10 19:14:10 +00:00
Anna Zaks e0ad10404d [analyzer] Fixup for r185609: actually do suppress warnings coming out of std::list.
list is the name of a class, not a namespace. Change the test as well - the previous
version did not test properly.

Fixes radar://14317928.

llvm-svn: 185898
2013-07-09 01:55:00 +00:00
Rafael Espindola 18627115f4 Use llvm::sys::fs::createUniqueFile.
Include a test that clang now produces output files with permissions matching
the umask.

llvm-svn: 185727
2013-07-05 21:13:58 +00:00
Rafael Espindola e4ffbdf349 Fix PR16547.
We should not be asking unique_file to prepend the system temporary directory
when creating the html report. Unfortunately I don't think we can test this
with the current infrastructure since unique_file ignores MakeAbsolute if the
directory is already absolute and the paths provided by lit are.

I will take a quick look at making this api a bit less error prone.

llvm-svn: 185707
2013-07-05 15:05:40 +00:00
Craig Topper 2341c0d3b2 Use SmallVectorImpl instead of SmallVector for iterators and references to avoid specifying the vector size unnecessarily.
llvm-svn: 185610
2013-07-04 03:08:24 +00:00
Anna Zaks a42fb525e4 [analyzer] Suppress reports reported in std::list
The motivation is to suppresses false use-after-free reports that occur when calling
std::list::pop_front() or std::list::pop_back() twice. The analyzer does not
reason about the internal invariants of the list implementation, so just do not report
any of warnings in std::list.

Fixes radar://14317928.

llvm-svn: 185609
2013-07-04 02:38:10 +00:00
Anna Zaks 5673b6567a [analyzer] Make sure that inlined defensive checks work on div by zero.
This suppresses a false positive in std::hash_map.
Fixes  radar://14255587.

llvm-svn: 185608
2013-07-04 02:38:06 +00:00
Jordan Rose 00deddb6d8 [analyzer] Pointers-to-members are (currently) Locs, not NonLocs.
While we don't model pointers-to-members besides "null" and "non-null",
we were using Loc symbols for valid pointers and NonLoc integers for the
null case. This hit the assert committed in r185401.

Fixed by using a true (Loc) null for null member pointers.

llvm-svn: 185444
2013-07-02 16:50:24 +00:00
Pavel Labath 868bebf844 Teach static analyzer about AttributedStmts
Summary:
Static analyzer used to abort when encountering AttributedStmts, because it
asserted that the statements should not appear in the CFG. This is however not
the case, since at least the clang::fallthrough annotation makes it through.

This commit simply makes the analyzer ignore the statement attributes.

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D1030

llvm-svn: 185417
2013-07-02 09:38:48 +00:00
Jordan Rose 686df32216 [analyzer] Explicitly disallow mixed Loc-NonLoc comparisons.
The one bit of code that was using this is gone, and neither C nor C++
actually allows this. Add an assertion and remove dead code.

Found by Matthew Dempsky!

llvm-svn: 185401
2013-07-02 01:37:40 +00:00
Jordan Rose b8e286548c [analyzer] Handle zeroing CXXConstructExprs.
Re-apply r184511, reverted in r184561, with the trivial default constructor
fast path removed -- it turned out not to be necessary here.

Certain expressions can cause a constructor invocation to zero-initialize
its object even if the constructor itself does no initialization. The
analyzer now handles that before evaluating the call to the constructor,
using the same "default binding" mechanism that calloc() uses, rather
than simply ignoring the zero-initialization flag.

<rdar://problem/14212563>

llvm-svn: 184815
2013-06-25 01:56:08 +00:00
Jordan Rose b3b976f061 [analyzer] Don't initialize virtual base classes more than once.
In order to make sure virtual base classes are always initialized once,
the AST contains initializers for the base class in /all/ of its
descendents, not just the immediate descendents. However, at runtime,
the most-derived object is responsible for initializing all the virtual
base classes; all the other initializers will be ignored.

The analyzer now checks to see if it's being called from another base
constructor, and if so does not perform virtual base initialization.

<rdar://problem/14236851>

llvm-svn: 184814
2013-06-25 01:55:59 +00:00
Jordan Rose e83cb0922b Revert "[analyzer] Handle zeroing CXXConstructExprs."
Per review from Anna, this really should have been two commits, and besides
it's causing problems on our internal buildbot. Reverting until these have
been worked out.

This reverts r184511 / 98123284826bb4ce422775563ff1a01580ec5766.

llvm-svn: 184561
2013-06-21 16:30:32 +00:00
Jordan Rose 4ace1a74c0 [analyzer] Handle zeroing CXXConstructExprs.
Certain expressions can cause a constructor invocation to zero-initialize
its object even if the constructor itself does no initialization. The
analyzer now handles that before evaluating the call to the constructor,
using the same "default binding" mechanism that calloc() uses, rather
than simply ignoring the zero-initialization flag.

As a bonus, trivial default constructors are now no longer inlined; they
are instead processed explicitly by ExprEngine. This has a (positive)
effect on the generated path edges: they no longer stop at a default
constructor call unless there's a user-provided implementation.

<rdar://problem/14212563>

llvm-svn: 184511
2013-06-21 00:59:00 +00:00
Pavel Labath cb0b876b39 Fix static analyzer crash when casting from an incomplete type
Summary:
When doing a reinterpret+dynamic cast from an incomplete type, the analyzer
would crash (bug #16308). This fix makes the dynamic cast evaluator ignore
incomplete types, as they can never be used in a dynamic_cast. Also adding a
regression test.

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D1006

llvm-svn: 184403
2013-06-20 07:45:01 +00:00
Pavel Labath 963f91b3a2 Fix a crash in the static analyzer (bug #16307)
Summary:
When processing a call to a function, which got passed less arguments than it
expects, the analyzer would crash.

I've also added a test for that and a analyzer warning which detects these
cases.

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D994

llvm-svn: 184288
2013-06-19 08:19:56 +00:00
Anna Zaks d60a41d941 [analyzer] Do not create a CompoundVal for lvalue InitListExprs.
These should be treated like scalars. This fixes a crash reported in radar://14164698.

llvm-svn: 184257
2013-06-18 23:16:20 +00:00
Reid Kleckner 2ab0ac5360 [AST] Don't include RecursiveASTVisitor.h in ASTContext.h
The untemplated implementation of getParents() doesn't need to be in a
header file.

RecursiveASTVisitor.h is full of repeated macro expansion.  Moving this
include to ASTContext.cpp speeds up compilation of
LambdaMangleContext.cpp, a small C++ file with few includes, from 3.7s
to 2.8s for me locally.  I haven't measured a full build, but it can't
hurt.

I had to fix a few static analyzer files that were depending on
transitive includes of C++ AST headers.

Reviewers: rsmith, klimek

Differential Revision: http://llvm-reviews.chandlerc.com/D982

llvm-svn: 184075
2013-06-17 12:56:08 +00:00
Richard Smith cc1b96d356 PR12086, PR15117
Introduce CXXStdInitializerListExpr node, representing the implicit
construction of a std::initializer_list<T> object from its underlying array.
The AST representation of such an expression goes from an InitListExpr with a
flag set, to a CXXStdInitializerListExpr containing a MaterializeTemporaryExpr
containing an InitListExpr (possibly wrapped in a CXXBindTemporaryExpr).

This more detailed representation has several advantages, the most important of
which is that the new MaterializeTemporaryExpr allows us to directly model
lifetime extension of the underlying temporary array. Using that, this patch
*drastically* simplifies the IR generation of this construct, provides IR
generation support for nested global initializer_list objects, fixes several
bugs where the destructors for the underlying array would accidentally not get
invoked, and provides constant expression evaluation support for
std::initializer_list objects.

llvm-svn: 183872
2013-06-12 22:31:48 +00:00
Benjamin Kramer fbf914ceb0 Port HTMLDiagnostics to PathV2. No intended functionality change.
llvm-svn: 183849
2013-06-12 18:13:05 +00:00
Rafael Espindola be5138885d Include PathV1.h in files that use it.
This is preparation for replacing Path.h with PathV2.h.

llvm-svn: 183781
2013-06-11 19:59:07 +00:00
Anna Zaks 22895473af [analyzer; alternate edges] Fix the edge locations in presence of macros.
We drew the diagnostic edges to wrong statements in cases the note was on a macro.
The fix is simple, but seems to work just fine for a whole bunch of test cases (plist-macros.cpp).

Also, removes an unnecessary edge in edges-new.mm, when function signature starts with a macro.

llvm-svn: 183599
2013-06-08 00:29:24 +00:00
Anna Zaks de2ae19cf6 [analyzer] Address Jordan’s code review for r183451
llvm-svn: 183455
2013-06-06 22:32:11 +00:00
Anna Zaks b1b95d9409 [analyzer] Ensure that pieces with invalid locations always get removed from the BugReport
The function in which we were doing it used to be conditionalized. Add a new unconditional
cleanup step.

This fixes PR16227 (radar://14073870) - a crash when generating html output for one of the test files.

llvm-svn: 183451
2013-06-06 22:02:58 +00:00
Anna Zaks 496312a364 [analyzer] fixup the comment
llvm-svn: 183450
2013-06-06 22:02:55 +00:00
Jordan Rose cf10ea8cb2 [analyzer; new edges] Simplify edges in a C++11 for-range loop.
Previously our edges were completely broken here; now, the final result
is a very simple set of edges in most cases: one up to the "for" keyword
for context, and one into the body of the loop. This matches the behavior
for ObjC for-in loops.

In the AST, however, CXXForRangeStmts are handled very differently from
ObjCForCollectionStmts. Since they are specified in terms of equivalent
statements in the C++ standard, we actually have implicit AST nodes for
all of the semantic statements. This makes evaluation very easy, but
diagnostic locations a bit trickier. Fortunately, the problem can be
generally defined away by marking all of the implicit statements as
part of the top-level for-range statement.

One of the implicit statements in a for-range statement is the declaration
of implicit iterators __begin and __end. The CFG synthesizes two
separate DeclStmts to match each of these decls, but until now these
synthetic DeclStmts weren't in the function's ParentMap. Now, the CFG
keeps track of its synthetic statements, and the AnalysisDeclContext will
make sure to add them to the ParentMap.

<rdar://problem/14038483>

llvm-svn: 183449
2013-06-06 21:53:45 +00:00
Jordan Rose d2f8a6a993 [analyzer] Improve debug output for PathDiagnosticPieces.
You can now dump a single PathDiagnosticPiece or PathDiagnosticLocation.

llvm-svn: 183367
2013-06-06 01:57:19 +00:00
Anna Zaks 148974d678 [analyzer] Fix a crash that occurs when processing an rvalue array.
When processing ArrayToPointerDecay, we expect the array to be a location, not a LazyCompoundVal.
Special case the rvalue arrays by using a location to represent them. This case is handled similarly
elsewhere in the code.

Fixes PR16206.

llvm-svn: 183359
2013-06-06 00:19:36 +00:00
Jordan Rose 7a8bd94365 [analyzer; new edges] Don't crash if the top-level entry edge is missing.
We previously asserted that there was a top-level function entry edge, but
if the function decl's location is invalid (or within a macro) this edge
might not exist. Change the assertion to an actual check, and don't drop
the first path piece if it doesn't match.

<rdar://problem/14070304>

llvm-svn: 183358
2013-06-06 00:12:41 +00:00
Jordan Rose b67b7b201f [analyzer; new edges] Ignore self-edges, not all edges with the same location.
The edge optimizer needs to see edges for, say, implicit casts (which have
the same source location as their operand) to uniformly simplify the
entire path. However, we still don't want to produce edges from a statement
to /itself/, which could occur when two nodes in a row have the same
statement location.

This necessitated moving the check for redundant notes to after edge
optimization, since the check relies on notes being adjacent in the path.

<rdar://problem/14061675>

llvm-svn: 183357
2013-06-06 00:12:37 +00:00
Jordan Rose 5e2b3a30a0 [analyzer] Enable the new edge algorithm by default.
...but don't yet migrate over the existing plist tests. Some of these
would be trivial to migrate; others could use a bit of inspection first.
In any case, though, the new edge algorithm seems to have proven itself,
and we'd like more coverage (and more usage) of it going forwards.

llvm-svn: 183165
2013-06-03 23:00:19 +00:00
Jordan Rose 7ce598aeee [analyzer; new edges] Omit subexpression back-edges that span multiple lines.
A.1 -> A -> B
becomes
A.1 -> B

This only applies if there's an edge from a subexpression to its parent
expression, and that is immediately followed by another edge from the
parent expression to a subsequent expression. Normally this is useful for
bringing the edges back to the left side of the code, but when the
subexpression is on a different line the backedge ends up looking strange,
and may even obscure code. In these cases, it's better to just continue
to the next top-level statement.

llvm-svn: 183164
2013-06-03 23:00:09 +00:00
Jordan Rose 5f16849b34 [analyzer; new edges] Don't eliminate subexpr edge cycles if the line is long.
Specifically, if the line is over 80 characters, or if the top-level
statement spans mulitple lines, we should preserve sub-expression edges
even if they form a simple cycle as described in the last commit, because
it's harder to infer what's going on than it is for shorter lines.

llvm-svn: 183163
2013-06-03 23:00:05 +00:00
Jordan Rose 8c54b44fb3 [analyzer; new edges] Eliminate "cycle edges" for a single subexpression.
Generating context arrows can result in quite a few arrows surrounding a
relatively simple expression, often containing only a single path note.

|
1 +--2---+
v/       v
auto m = new m // 3 (the path note)
|\       |
5 +--4---+
v

Note also that 5 and 1 are two ends of the "same" arrow, i.e. they go from
event to event. 3 is not an arrow but the path note itself.

Now, if we see a pair of edges like 2 and 4---where 4 is the reverse of 2
and there is optionally a single path note between them---we will
eliminate /both/ edges. Anything more complicated will be left as is
(more edges involved, an inlined call, etc).

The next commit will refine this to preserve the arrows in a larger
expression, so that we don't lose all context.

llvm-svn: 183162
2013-06-03 23:00:00 +00:00
Jordan Rose 06e800727e [analyzer; new edges] Improve enclosing contexts for logical expressions.
The old edge builder didn't have a notion of nested statement contexts,
so there was no special treatment of a logical operator inside an if
(or inside another logical operator). The new edge builder always tries
to establish the full context up to the top-level statement, so it's
important to know how much context has been established already rather
than just checking the innermost context.

This restores some of the old behavior for the old edge generation:
the context of a logical operator's non-controlling expression is the
subexpression in the old edge algorithm, but the entire operator
expression in the new algorithm.

llvm-svn: 183160
2013-06-03 22:59:53 +00:00
Jordan Rose b1db073dac [analyzer; new edges] Include context for edges to sub-expressions.
The current edge-generation algorithm sometimes creates edges from a
top-level statement A to a sub-expression B.1 that's not at the start of B.
This creates a "swoosh" effect where the arrow is drawn on top of the
text at the start of B. In these cases, the results are clearer if we see
an edge from A to B, then another one from B to B.1.

Admittedly, this does create a /lot/ of arrows, some of which merely hop
into a subexpression and then out again for a single note. The next commit
will eliminate these if the subexpression is simple enough.

This updates and reuses some of the infrastructure from the old edge-
generation algorithm to find the "enclosing statement" context for a
given expression. One change in particular marks the context of the
LHS or RHS of a logical binary operator (&&, ||) as the entire operator
expression, rather than the subexpression itself. This matches our behavior
for ?:, and allows us to handle nested context information.

<rdar://problem/13902816>

llvm-svn: 183159
2013-06-03 22:59:48 +00:00
Jordan Rose c892bb04ca [analyzer; new edges] Include a top-level function entry edge while optimizing.
Although we don't want to show a function entry edge for a top-level path,
having it makes optimizing edges a little more uniform.

This does not affect any edges now, but will affect context edge generation
(next commit).

llvm-svn: 183158
2013-06-03 22:59:45 +00:00
Ted Kremenek 7c6b4084dd [analyzer; new edges] add simplifySimpleBranches() to reduce edges for branches.
In many cases, the edge from the "if" to the condition, followed by an edge from the branch condition to the target code, is uninteresting.

In such cases, we should fold the two edges into one from the "if" to the target.

This also applies to loops.

Implements <rdar://problem/14034763>.

llvm-svn: 183018
2013-05-31 16:56:54 +00:00
Ted Kremenek 263595f4f3 [analyzer; new edges] in splitBranchConditionEdges() do not check that predecessor edge has source in the same lexical scope as the target branch.
Fixes <rdar://problem/14031292>.

llvm-svn: 182987
2013-05-31 06:11:17 +00:00
Ted Kremenek d59fbca225 [analyzer;alternate arrows] Rename 'adjustBranchEdges' to 'splitBranchConditionEdges'.
llvm-svn: 182986
2013-05-31 06:11:11 +00:00
Jordan Rose ca0ecb61e1 Revert "[analyzer; alternate edges] don't add an edge incoming from the start of a function"
...and make this work correctly in the current codebase.

After living on this for a while, it turns out to look very strange for
inlined functions that have only a single statement, and somewhat strange
for inlined functions in general (since they are still conceptually in the
middle of the path, and there is a function-entry path note).

It's worth noting that this only affects inlined functions; in the new
arrow generation algorithm, the top-level function still starts at the
first real statement in the function body, not the enclosing CompoundStmt.

This reverts r182078 / dbfa950abe0e55b173286a306ee620eff5f72ea.

llvm-svn: 182963
2013-05-30 21:30:17 +00:00
Jordan Rose 278d9de314 [analyzer] Don't crash if a block's signature just has the return type.
It is okay to declare a block without an argument list: ^ {} or ^void {}.
In these cases, the BlockDecl's signature-as-written will just contain
the return type, rather than the entire function type. It is unclear if
this is intentional, but the analyzer shouldn't crash because of it.

<rdar://problem/14018351>

llvm-svn: 182948
2013-05-30 18:14:27 +00:00
Jordan Rose 543bdd1237 [analyzer; new edges] In for(;;), use the ForStmt itself for loop notes.
Most loop notes (like "entering loop body") are attached to the condition
expression guarding a loop or its equivalent. For loops may not have a
condition expression, though. Rather than crashing, just use the entire
ForStmt as the location. This is probably the best we can do.

<rdar://problem/14016063>

llvm-svn: 182904
2013-05-30 01:05:58 +00:00
Jordan Rose 1bd1927a14 [analyzer] Accept references to variables declared "extern void" (C only).
In C, 'void' is treated like any other incomplete type, and though it is
never completed, you can cast the address of a void-typed variable to do
something useful. (In C++ it's illegal to declare a variable with void type.)

Previously we asserted on this code; now we just treat it like any other
incomplete type.

And speaking of incomplete types, we don't know their extent. Actually
check that in TypedValueRegion::getExtent, though that's not being used
by any checkers that are on by default.

llvm-svn: 182880
2013-05-29 20:50:34 +00:00
Anna Zaks 5416ab0156 [analyzer] Use the expression’s type instead of region’s type in ArrayToPointer decay evaluation
This gives slightly better precision, specifically, in cases where a non-typed region represents the array
or when the type is a non-array type, which can happen when an array is a result of a reinterpret_cast.

llvm-svn: 182810
2013-05-28 23:24:01 +00:00
Anna Zaks 6477e97af7 [analyzer] Re-enable reasoning about CK_LValueBitCast
It’s important for us to reason about the cast as it is used in std::addressof. The reason we did not
handle the cast previously was a crash on a test case (see commit r157478). The crash was in
processing array to pointer decay when the region type was not an array. Address the issue, by
just returning an unknown in that case.

llvm-svn: 182808
2013-05-28 22:32:08 +00:00
Anna Zaks bac964e14f [analyzer] Use a more generic MemRegion.getAsOffset to evaluate bin operators on MemRegions
In addition to enabling more code reuse, this suppresses some false positives by allowing us to
compare an element region to its base. See the ptr-arith.cpp test cases for an example.

llvm-svn: 182780
2013-05-28 17:31:43 +00:00
Jordan Rose 56138268b0 [analyzer] Treat analyzer-synthesized function bodies like implicit bodies.
When generating path notes, implicit function bodies are shown at the call
site, so that, say, copying a POD type in C++ doesn't jump you to a header
file. This is especially important when the synthesized function itself
calls another function (or block), in which case we should try to jump the
user around as little as possible.

By checking whether a called function has a body in the AST, we can tell
if the analyzer synthesized the body, and if we should therefore collapse
the call down to the call site like a true implicitly-defined function.

<rdar://problem/13978414>

llvm-svn: 182677
2013-05-24 21:43:11 +00:00
Jordan Rose 7291666cd9 [analyzer; new edges] Properly set location after exiting an inlined call.
The new edge algorithm would keep track of the previous location in each
location context, so that it could draw arrows coming in and out of each
inlined call. However, it tried to access the location of the call before
it was actually set (at the CallEnter node). This only affected
unterminated calls at the end of a path; calls with visible exit nodes
already had a valid location.

This patch ditches the location context map, since we're processing the
nodes in order anyway, and just unconditionally updates the PrevLoc
variable after popping out of an inlined call.

<rdar://problem/13983470>

llvm-svn: 182676
2013-05-24 21:43:05 +00:00
Benjamin Kramer bf8d2540a3 Make helper functions static.
llvm-svn: 182589
2013-05-23 15:53:44 +00:00
Ted Kremenek a76dd19507 [analyzer;alternate edges] fix type that was causing the wrong path piece to get removed.
llvm-svn: 182562
2013-05-23 06:41:58 +00:00
Pete Cooper f2ec16eb8b Insert explicit casts to try appease overload resolution in the buildbots
llvm-svn: 182514
2013-05-22 21:02:38 +00:00
Ted Kremenek f63269fdc5 Use scope-resolution operator to hopefully unbreak Windows builds.
llvm-svn: 182509
2013-05-22 20:01:35 +00:00
Ted Kremenek 561060b7f5 Simplifiy code using return value of erase().
llvm-svn: 182506
2013-05-22 19:25:03 +00:00
Ted Kremenek 0962e56f00 [analyzer; alternate edges] remove redundant adjacent "events" with the same text.
Fixes <rdar://problem/13949982>

llvm-svn: 182505
2013-05-22 19:10:41 +00:00
Ted Kremenek 55efcadc1c [analyzer;alternate edges] remove puny edges on the same line that span less than 3 columns.
These are legitimate control-flow edges, but visually they add
no value.

Implements <rdar://problem/13941325>.

llvm-svn: 182502
2013-05-22 18:52:35 +00:00
Ted Kremenek d2d2a9f17b Remove unnecessary assignment.
llvm-svn: 182501
2013-05-22 18:52:32 +00:00
Jordan Rose 1bfe9c787f [analyzer] Don't crash if a block doesn't have a type signature.
Currently, blocks instantiated in templates lose their "signature as
written"; it's not clear if this is intentional. Change the analyzer's
use of BlockDecl::getSignatureAsWritten to check whether or not the
signature is actually there.

<rdar://problem/13954714>

llvm-svn: 182497
2013-05-22 18:09:44 +00:00
Anna Zaks 2f74ff1b3c [analyzer] Do not assert on reports ending in calls within macros.
The crash is triggered by the newly added option (-analyzer-config report-in-main-source-file=true) introduced in r182058.

Note, ideally, we’d like to report the issue within the main source file here as well.
For now, just do not crash.

llvm-svn: 182445
2013-05-22 01:54:34 +00:00
Ted Kremenek 9f0629f669 [analyzer;alternate edges] prune out extra edges to a subexpression where we dive-in and out of a subexpression.
Fixes <rdar://problem/13941891>.

llvm-svn: 182426
2013-05-21 21:38:05 +00:00
Ted Kremenek 6d5bbec32e [analyzer; alternated edges] look through expressions just like Environment does.
llvm-svn: 182425
2013-05-21 21:38:02 +00:00
Ted Kremenek d4167a66e1 [analyzer; alternate edges] optimize edges for ObjC fast enumeration loops.
Fixes <rdar://problem/13942300>.

llvm-svn: 182342
2013-05-21 00:34:40 +00:00
Jordan Rose 933e5ba722 [analyzer] New edges: include an edge to the end-of-path location.
llvm-svn: 182188
2013-05-18 02:27:13 +00:00
Jordan Rose 7c40d078b5 [analyzer] Add a debug dump for PathPieces, a list of PathDiagnosticPieces.
Originally implemented by Ted, extended by me.

llvm-svn: 182186
2013-05-18 02:26:59 +00:00
Jordan Rose 433b0f5455 Revert "[analyzer; alternate edges] improve support for edges with PseudoObjectExprs."
Ted and I spent a long time discussing this today and found out that neither
the existing code nor the new code was doing what either of us thought it
was, which is never good. The good news is we found a much simpler way to
fix the motivating test case (an ObjCSubscriptExpr).

This reverts r182083, but pieces of it will come back in subsequent commits.

llvm-svn: 182185
2013-05-18 02:26:50 +00:00
Anna Zaks 6334579623 [analyzer] Address Jordan's review comments for r182058
llvm-svn: 182156
2013-05-17 20:51:16 +00:00
Ted Kremenek 35de14540f [analyzer; alternate edges] improve support for edges with PseudoObjectExprs.
This optimizes some spurious edges resulting from PseudoObjectExprs.
This required far more changes than I anticipated.  The current
ParentMap does not record any hierarchy information between
a PseudoObjectExpr and its *semantic* expressions that may be
wrapped in OpaqueValueExprs, which are the expressions actually
laid out in the CFG.  This means the arrow pruning logic could
not map from an expression to its containing PseudoObjectExprs.

To solve this, this patch adds a variant of ParentMap that
returns the "semantic" parentage of expressions (essentially
as they are viewed by the CFG).  This alternate ParentMap is then
used by the arrow reducing logic to identify edges into pseudo
object expressions, and then eliminate them.

llvm-svn: 182083
2013-05-17 09:41:40 +00:00
Ted Kremenek fd8f4b0171 [analyzer; alternate edges] treat 'if' statements the same way we do as 'for' or 'while'.
This means adding an extra edge from the 'if' to the condition,
which aesthetically looks more pleasing.

llvm-svn: 182079
2013-05-17 06:48:27 +00:00
Ted Kremenek 504eb552a8 [analyzer; alternate edges] don't add an edge incoming from the start of a function
for a nested call.  This matches what we do with the first stack frame.

llvm-svn: 182078
2013-05-17 06:48:22 +00:00
Jordan Rose fbe4d85035 [analyzer] Don't inline ~shared_ptr.
The analyzer can't see the reference count for shared_ptr, so it doesn't
know whether a given destruction is going to delete the referenced object.
This leads to spurious leak and use-after-free warnings.

For now, just ban destructors named '~shared_ptr', which catches
std::shared_ptr, std::tr1::shared_ptr, and boost::shared_ptr.

PR15987

llvm-svn: 182071
2013-05-17 02:16:49 +00:00
Anna Zaks c5e2eca042 [analyzer] Add an option to use the last location in the main source file as the report location.
Previously, we’ve used the last location of the analyzer issue path as the location of the
report. This might not provide the best user experience, when one analyzer a source
file and the issue appears in the header. Introduce an option to use the last location
of the path that is in the main source file as the report location.

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

llvm-svn: 182058
2013-05-16 22:30:45 +00:00
David Blaikie d4da8728ba Provide operator<< for stream output of DeclarationNames
ASTDumper was already trying to do this & instead got an implicit bool
conversion by surprise (thus printing out 0 or 1 instead of the name of
the declaration). To avoid that issue & simplify call sites, simply make
it the normal/expected operator<<(raw_ostream&, ...) overload & simplify
all the existing call sites. (bonus: this function doesn't need to be a
member or friend, it's just using public API in DeclarationName)

llvm-svn: 181832
2013-05-14 21:04:00 +00:00
Rafael Espindola 3ae00052cd Cleanup handling of UniqueExternalLinkage.
This patch renames getLinkage to getLinkageInternal. Only code that
needs to handle UniqueExternalLinkage specially should call this.

Linkage, as defined in the c++ standard, is provided by
getFormalLinkage. It maps UniqueExternalLinkage to ExternalLinkage.

Most places in the compiler actually want isExternallyVisible, which
handles UniqueExternalLinkage as internal.

llvm-svn: 181677
2013-05-13 00:12:11 +00:00
Anna Zaks 3feb2cd5bb [analyzer] Do not check if sys/queue.h file is a system header.
In most cases it is, by just looking at the name. Also, this check prevents the heuristic from working in strange user settings.
radar://13839692

llvm-svn: 181615
2013-05-10 18:04:43 +00:00
Ted Kremenek 399980acf5 [analyzer; alternate arrows] for "loop back" edges add back the extra edge to the closing '}'
llvm-svn: 181505
2013-05-09 06:55:41 +00:00
Ted Kremenek b5999f6321 [analyzer;alternate arrows] adapt 'for' loop aesthetic cleanup to 'while' loops.
llvm-svn: 181504
2013-05-09 06:55:35 +00:00
Ted Kremenek cba4a0c593 [analyzer; alternate edges] insert an extra edge for 'for' statements to conditions.
llvm-svn: 181385
2013-05-08 01:15:24 +00:00
Ted Kremenek 0e84ac83eb [analyzer;alternate edges] edges from subexpressions of "?:" are important to retain
llvm-svn: 181384
2013-05-08 01:15:20 +00:00
Ted Kremenek 68a60451b3 [analyzer;alternate arrows] Fix inconsistencies in recorded location context when handling interprocedural paths.
llvm-svn: 181362
2013-05-07 21:12:06 +00:00
Ted Kremenek 1c382b2936 [analyzer; alternate arrows] add back recording whether we visited the first edge.
llvm-svn: 181361
2013-05-07 21:12:03 +00:00
Ted Kremenek abf617c27e [analyzer; alternate arrows] remove pruning of loop diagnostics.
llvm-svn: 181360
2013-05-07 21:12:00 +00:00
Ted Kremenek b48f5d9d56 [analyzer; alternate arrows] include logical '||' and '&&' as anchors for edges.
llvm-svn: 181359
2013-05-07 21:11:57 +00:00
Ted Kremenek 3a865221c7 [analyzer; alternate arrows] include an edge from the "break" or "continue"
llvm-svn: 181358
2013-05-07 21:11:54 +00:00
Ted Kremenek f3510071f3 [analyzer; alternate arrows] the extra edge to the closing '}' in a loop adds no value.
llvm-svn: 181357
2013-05-07 21:11:52 +00:00
Ted Kremenek 2f2a3042e1 [analyzer; alternate arrows] the initializer of a ForStmt isn't interesting either.
llvm-svn: 181356
2013-05-07 21:11:49 +00:00
Anna Zaks e5e416c6de [analyzer] Fix a crash triggered by printing a note on a default argument
Instead, use the location of the call to print the note.

llvm-svn: 181337
2013-05-07 17:42:42 +00:00
Ted Kremenek ae7c38ddc7 [analyzer; alternate arrows] The ForStmt increment is not a critical anchor for arrows.
llvm-svn: 181333
2013-05-07 17:02:41 +00:00
Ted Kremenek 75b8cdee19 [analyzer; alternate edges] simplify optimization rules to look at control-flow conditions to prune edges.
llvm-svn: 181292
2013-05-07 07:30:07 +00:00
Ted Kremenek 868b55336e [analyzer; alternate arrows] use the terminator condition as the location for 'entering loop body'
llvm-svn: 181291
2013-05-07 07:30:00 +00:00
Ted Kremenek fe516f8924 [analyzer; alternate arrows] provide a diagnostic for entering a loop for the first time.
llvm-svn: 181282
2013-05-07 01:18:10 +00:00
Ted Kremenek 9af6baaaa3 [analyzer; alternate arrows] don't increment the path iterator when we just deleted the next iterator.
This is an optimization.  It is possible that by deleting the next
edge we will pattern match again at the current spot.

llvm-svn: 181256
2013-05-06 21:59:37 +00:00
Jordan Rose 50e5db8a6b [analyzer] Remove now-unused bindCompoundLiteral helper function.
The one user has been changed to use getLValue on the compound literal
expression and then use the normal bindLoc to assign a value. No need
to special case this in the StoreManager.

llvm-svn: 181214
2013-05-06 16:48:26 +00:00
Jordan Rose 5d2abefb62 [analyzer] Handle CXXTemporaryObjectExprs in compound literals.
This occurs because in C++11 the compound literal syntax can trigger a
constructor call via list-initialization. That is, "Point{x, y}" and
"(Point){x, y}" end up being equivalent. If this occurs, the inner
CXXConstructExpr will have already handled the object construction; the
CompoundLiteralExpr just needs to propagate that value forwards.

<rdar://problem/13804098>

llvm-svn: 181213
2013-05-06 16:48:20 +00:00
Ted Kremenek 7b9b5a2c48 [analyzer;alternate edges] start experimenting with control flow "barriers" to prevent an edge being optimized away.
llvm-svn: 181088
2013-05-04 01:13:22 +00:00
Ted Kremenek 992b3112ce [analyzer;alternate edges] ignore parentheses when determining edge levels.
llvm-svn: 181087
2013-05-04 01:13:12 +00:00
Ted Kremenek bcd6b0d891 [analyzer; alternate edges] - eliminate unnecessary edges where between parents and subexpressions.
llvm-svn: 181086
2013-05-04 01:13:08 +00:00
Ted Kremenek ccb6d1ea0c [analyzer; alternate edges] - merge control edges where we descend to a subexpression and pop back out.
llvm-svn: 181085
2013-05-04 01:13:05 +00:00
Ted Kremenek 6cf3c97c55 [analyzer; alternate edges] prune edges whose end/begin locations have the same statement parents.
This change required some minor changes to LocationContextMap to have it map
from PathPieces to LocationContexts instead of PathDiagnosticCallPieces to
LocationContexts.  These changes are in the other diagnostic
generation logic as well, but are functionally equivalent.

Interestingly, this optimize requires delaying "cleanUpLocation()" until
later; possibly after all edges have been optimized.  This is because
we need PathDiagnosticLocations to refer to the semantic entity (e.g. a statement)
as long as possible.  Raw source locations tell us nothing about
the semantic relationship between two locations in a path.

llvm-svn: 181084
2013-05-04 01:13:01 +00:00
Ted Kremenek 74c0d388e8 [analyzer;alternate edges] - add in events (loop iterations, etc)
These were being dropped due a transcription mistake from the original
algorithm.

llvm-svn: 181083
2013-05-04 01:12:55 +00:00
Ted Kremenek acf99a1a51 [analyzer] Start hacking up alternate control-flow edge generation. WIP. Not guaranteed to do anything useful yet.
llvm-svn: 181040
2013-05-03 18:25:33 +00:00
Jordan Rose 320fbf057c [analyzer] Check the stack frame when looking for a var's initialization.
FindLastStoreBRVisitor is responsible for finding where a particular region
gets its value; if the region is a VarRegion, it's possible that value was
assigned at initialization, i.e. at its DeclStmt. However, if a function is
called recursively, the same DeclStmt may be evaluated multiple times in
multiple stack frames. FindLastStoreBRVisitor was not taking this into
account and just picking the first one it saw.

<rdar://problem/13787723>

llvm-svn: 180997
2013-05-03 05:47:31 +00:00
Jordan Rose cea47b78fc [analyzer] Fix trackNullOrUndef when tracking args that have nil receivers.
There were actually two bugs here:
- if we decided to look for an interesting lvalue or call expression, we
  wouldn't go find its node if we also knew we were at a (different) call.
- if we looked through one message send with a  nil receiver, we thought we
  were still looking at an argument to the original call.

Put together, this kept us from being able to track the right values, which
means sub-par diagnostics and worse false-positive suppression.

Noticed by inspection.

llvm-svn: 180996
2013-05-03 05:47:24 +00:00
Ted Kremenek 57f745ec96 Make cleanUpLocation() a self-contained function.
llvm-svn: 180986
2013-05-03 01:16:26 +00:00
Ted Kremenek f12d9d93fe Re-apply 180974 with the build error fixed. This was the result
of a weird merge error with git.

llvm-svn: 180981
2013-05-03 00:32:44 +00:00
Rafael Espindola c73756b178 Revert "Change LocationContextMap to be a temporary instead of shared variable in BugReporter."
This reverts commit 180974. It broke the build.

llvm-svn: 180979
2013-05-03 00:22:49 +00:00
Ted Kremenek 48bd5fddf3 Change LocationContextMap to be a temporary instead of shared variable in BugReporter.
BugReporter is used to process ALL bug reports.  By using a shared map,
we are having mappings from different PathDiagnosticPieces to LocationContexts
well beyond the point where we are processing a given report.  This
state is inherently error prone, and is analogous to using a global
variable.  Instead, just create a temporary map, one per report,
and when we are done with it we throw it away.  No extra state.

llvm-svn: 180974
2013-05-02 23:56:33 +00:00
Jordan Rose c76d7e3d96 [analyzer] Don't try to evaluate MaterializeTemporaryExpr as a constant.
...and don't consider '0' to be a null pointer constant if it's the
initializer for a float!

Apparently null pointer constant evaluation looks through both
MaterializeTemporaryExpr and ImplicitCastExpr, so we have to be more
careful about types in the callers. For RegionStore this just means giving
up a little more; for ExprEngine this means handling the
MaterializeTemporaryExpr case explicitly.

Follow-up to r180894.

llvm-svn: 180944
2013-05-02 19:51:20 +00:00
Jordan Rose 89bbd1fb64 [analyzer] Consolidate constant evaluation logic in SValBuilder.
Previously, this was scattered across Environment (literal expressions),
ExprEngine (default arguments), and RegionStore (global constants). The
former special-cased several kinds of simple constant expressions, while
the latter two deferred to the AST's constant evaluator.

Now, these are all unified as SValBuilder::getConstantVal(). To keep
Environment fast, the special cases for simple constant expressions have
been left in, but the main benefits are that (a) unusual constants like
ObjCStringLiterals now work as default arguments and global constant
initializers, and (b) we're not duplicating code between ExprEngine and
RegionStore.

This actually caught a bug in our test suite, which is awesome: we stop
tracking allocated memory if it's passed as an argument along with some
kind of callback, but not if the callback is 0. We were testing this in
a case where the callback parameter had a default value, but that value
was 0. After this change, the analyzer now (correctly) flags that as a
leak!

<rdar://problem/13773117>

llvm-svn: 180894
2013-05-01 23:10:44 +00:00
Jordan Rose 7023a90378 [analyzer] Don't inline the [cd]tors of C++ iterators.
This goes with r178516, which instructed the analyzer not to inline the
constructors and destructors of C++ container classes. This goes a step
further and does the same thing for iterators, so that the analyzer won't
falsely decide we're trying to construct an iterator pointing to a
nonexistent element.

The heuristic for determining whether something is an iterator is the
presence of an 'iterator_category' member. This is controlled under the
same -analyzer-config option as container constructor/destructor inlining:
'c++-container-inlining'.

<rdar://problem/13770187>

llvm-svn: 180890
2013-05-01 22:39:31 +00:00
Jordan Rose dc16628c93 Re-apply "[analyzer] Model casts to bool differently from other numbers."
This doesn't appear to be the cause of the slowdown. I'll have to try a
manual bisect to see if there's really anything there, or if it's just
the bot itself taking on additional load. Meanwhile, this change helps
with correctness.

This changes an assertion and adds a test case, then re-applies r180638,
which was reverted in r180714.

<rdar://problem/13296133> and PR15863

llvm-svn: 180864
2013-05-01 18:19:59 +00:00
Ted Kremenek eba09facff Revert "[analyzer] Change PathPieces to be a wrapper around an ilist of (through indirection) PathDiagnosticPieces."
Jordan rightly pointed out that we can do the same with std::list.

llvm-svn: 180746
2013-04-29 23:12:59 +00:00
Ted Kremenek 03ae57b5af [analyzer] Change PathPieces to be a wrapper around an ilist of (through indirection) PathDiagnosticPieces.
Much of this patch outside of PathDiagnostics.h are just minor
syntactic changes due to the return type for operator* and the like
changing for the iterator, so the real focus should be on
PathPieces itself.

This change is motivated so that we can do efficient insertion
and removal of individual pieces from within a PathPiece, just like
this was a kind of "IR" for static analyzer diagnostics.  We
currently implement path transformations by iterating over an
entire PathPiece and making a copy.  This isn't very natural for
some algorithms.

We use an ilist here instead of std::list because we want operations
to rip out/insert nodes in place, just like IR manipulation.  This
isn't being used yet, but opens the door for more powerful
transformation algorithms on diagnostic paths.

llvm-svn: 180741
2013-04-29 22:38:26 +00:00
Ted Kremenek 518e781256 [analyzer] Remove comparePath's dependency on subscript operator.
llvm-svn: 180740
2013-04-29 22:38:22 +00:00
Jordan Rose 49f888bbab Revert "[analyzer] Model casts to bool differently from other numbers."
This seems to be causing quite a slowdown on our internal analyzer bot,
and I'm not sure why. Needs further investigation.

This reverts r180638 / 9e161ea981f22ae017b6af09d660bfc3ddf16a09.

llvm-svn: 180714
2013-04-29 17:23:03 +00:00
Jordan Rose 9661c1d18a [analyzer] Model casts to bool differently from other numbers.
Casts to bool (and _Bool) are equivalent to checks against zero,
not truncations to 1 bit or 8 bits.

This improved reasoning does cause a change in the behavior of the alpha
BoolAssignment checker. Previously, this checker complained about statements
like "bool x = y" if 'y' was known not to be 0 or 1. Now it does not, since
that conversion is well-defined. It's hard to say what the "best" behavior
here is: this conversion is safe, but might be better written as an explicit
comparison against zero.

More usefully, besides improving our model of booleans, this fixes spurious
warnings when returning the address of a local variable cast to bool.

<rdar://problem/13296133>

llvm-svn: 180638
2013-04-26 21:42:55 +00:00
Anton Yartsev 67f1ab0de8 [analyzer] Refactoring + explanatory comment.
llvm-svn: 180181
2013-04-24 10:24:38 +00:00
Anna Zaks 4e16b29c13 [analyzer] Refactor BugReport::getLocation and PathDiagnosticLocation::createEndOfPath for greater code reuse
The 2 functions were computing the same location using different logic (each one had edge case bugs that the other
one did not). Refactor them to rely on the same logic.

The location of the warning reported in text/command line output format will now match that of the plist file.

There is one change in the plist output as well. When reporting an error on a BinaryOperator, we use the location of the
operator instead of the beginning of the BinaryOperator expression. This matches our output on command line and
looks better in most cases.

llvm-svn: 180165
2013-04-23 23:57:43 +00:00
Jordan Rose b957113b3f [analyzer] Treat reinterpret_cast like a base cast in certain cases.
The analyzer represents all pointer-to-pointer bitcasts the same way, but
this can be problematic if an implicit base cast gets layered on top of a
manual base cast (performed with reinterpret_cast instead of static_cast).
Fix this (and avoid a valid assertion) by looking through cast regions.

Using reinterpret_cast this way is only valid if the base class is at the
same offset as the derived class; this is checked by -Wreinterpret-base-class.
In the interest of performance, the analyzer doesn't repeat this check
anywhere; it will just silently do the wrong thing (use the wrong offsets
for fields of the base class) if the user code is wrong.

PR15394

llvm-svn: 180052
2013-04-22 21:36:49 +00:00
Richard Smith 852c9db72b C++1y: Allow aggregates to have default initializers.
Add a CXXDefaultInitExpr, analogous to CXXDefaultArgExpr, and use it both in
CXXCtorInitializers and in InitListExprs to represent a default initializer.

There's an additional complication here: because the default initializer can
refer to the initialized object via its 'this' pointer, we need to make sure
that 'this' points to the right thing within the evaluation.

llvm-svn: 179958
2013-04-20 22:23:05 +00:00
Anna Zaks 6c0c47ede5 [analyzer] Ensure BugReporterTracking works on regions with pointer arithmetic
Introduce a new helper function, which computes the first symbolic region in
the base region chain. The corresponding symbol has been used for assuming that
a pointer is null. Now, it will also be used for checking if it is null.

This ensures that we are tracking a null pointer correctly in the BugReporter.

llvm-svn: 179916
2013-04-20 01:15:42 +00:00
Anna Zaks 390fb10a9b [analyzer] Flip printPretty and printPrettyAsExpr as per suggestion from Jordan (r179572)
llvm-svn: 179915
2013-04-20 01:15:36 +00:00
Anton Yartsev 3976656e08 [analyzer] Call proper callback for const regions escaped other then on call.
llvm-svn: 179846
2013-04-19 09:39:51 +00:00
Ted Kremenek d51ad8c125 [analyzer] Refine 'nil receiver' diagnostics to mention the name of the method not called.
llvm-svn: 179776
2013-04-18 17:44:15 +00:00
Jordan Rose 3720e2f006 [analyzer] "Force" LazyCompoundVals on bind when they are simple enough.
The analyzer uses LazyCompoundVals to represent rvalues of aggregate types,
most importantly structs and arrays. This allows us to efficiently copy
around an entire struct, rather than doing a memberwise load every time a
struct rvalue is encountered. This can also keep memory usage down by
allowing several structs to "share" the same snapshotted bindings.

However, /lookup/ through LazyCompoundVals can be expensive, especially
since they can end up chaining back to the original value. While we try
to reuse LazyCompoundVals whenever it's safe, and cache information about
this transitivity, the fact is it's sometimes just not a good idea to
perpetuate LazyCompoundVals -- the tradeoffs just aren't worth it.

This commit changes RegionStore so that binding a LazyCompoundVal to struct
will do a memberwise copy if the struct is simple enough. Today's definition
of "simple enough" is "up to N scalar members" (see below), but that could
easily be changed in the future. This is enough to bring the test case in
PR15697 back down to a manageable analysis time (within 20% of its original
time, in an unfair test where the new analyzer is not compiled with LTO).

The actual value of "N" is controlled by a new -analyzer-config option,
'region-store-small-struct-limit'. It defaults to "2", meaning structs with
zero, one, or two scalar members will be considered "simple enough" for
this code path.

It's worth noting that a more straightforward implementation would do this
on load, not on bind, and make use of the structure we already have for this:
CompoundVal. A long time ago, this was actually how RegionStore modeled
aggregate-to-aggregate copies, but today it's only used for compound literals.
Unfortunately, it seems that we've special-cased LazyCompoundVal in certain
places (such as liveness checks) but failed to similarly special-case
CompoundVal in all of them. Until we're confident that CompoundVal is
handled properly everywhere, this solution is safer, since the entire
optimization is just an implementation detail of RegionStore.

<rdar://problem/13599304>

llvm-svn: 179767
2013-04-18 16:33:46 +00:00
Jordan Rose cdb44bdb3d [analyzer] Don't crash if we cache out after making a temporary region.
A C++ overloaded operator may be implemented as an instance method, and
that instance method may be called on an rvalue object, which has no
associated region. The analyzer handles this by creating a temporary region
just for the evaluation of this call; however, it is possible that /by
creating the region/, the analyzer ends up in a previously-explored state.
In this case we don't need to continue along this path.

This doesn't actually show any behavioral change now, but it starts being
used with the next commit and prevents an assertion failure there.

llvm-svn: 179766
2013-04-18 16:33:40 +00:00
Anna Zaks 05139fff42 [analyzer] Tweak getDerefExpr more to track DeclRefExprs to references.
In the committed example, we now see a note that tells us when the pointer
was assumed to be null.

This is the only case in which getDerefExpr returned null (failed to get
the dereferenced expr) throughout our regression tests. (There were multiple
occurrences of this one.)

llvm-svn: 179736
2013-04-18 00:15:15 +00:00
Anna Zaks 1baf545fa6 [analyzer] Improve dereferenced expression tracking for MemberExpr with a dot and non-reference base
llvm-svn: 179734
2013-04-17 23:17:43 +00:00
Anna Zaks 4f59835182 [analyzer] Gain more precision retrieving the right SVal by specifying the type of the expression.
Thanks to Jordan for suggesting the fix.

llvm-svn: 179732
2013-04-17 22:29:51 +00:00
Anna Zaks 54f4d01bd3 [analyzer] Allow TrackConstraintBRVisitor to work when the value it’s tracking is not live in the last node of the path
We always register the visitor on a node in which the value we are tracking is live and constrained. However,
the visitation can restart at a node, later on the path, in which the value is under constrained because
it is no longer live. Previously, we just silently stopped tracking in that case.

llvm-svn: 179731
2013-04-17 22:29:47 +00:00
Jordan Rose add14263ea [analyzer] Don't warn for returning void expressions in void blocks.
This was slightly tricky because BlockDecls don't currently store an
inferred return type. However, we can rely on the fact that blocks with
inferred return types will have return statements that match the inferred
type.

<rdar://problem/13665798>

llvm-svn: 179699
2013-04-17 18:03:48 +00:00
Tareq A. Siraj 24110cc733 Implement CapturedStmt AST
CapturedStmt can be used to implement generic function outlining as described in
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-January/027540.html.

CapturedStmt is not exposed to the C api.

Serialization and template support are pending.

Author: Wei Pan <wei.pan@intel.com>

Differential Revision: http://llvm-reviews.chandlerc.com/D370

llvm-svn: 179615
2013-04-16 18:53:08 +00:00
John McCall 5e77d76c95 Basic support for Microsoft property declarations and
references thereto.

Patch by Tong Shen!

llvm-svn: 179585
2013-04-16 07:28:30 +00:00
Anna Zaks 8591aa78db [analyzer] Do not crash when processing binary "?:" in C++
When computing the value of ?: expression, we rely on the last expression in
the previous basic block to be the resulting value of the expression. This is
not the case for binary "?:" operator (GNU extension) in C++. As the last
basic block has the expression for the condition subexpression, which is an
R-value, whereas the true subexpression is the L-value.

Note the operator evaluation just happens to work in C since the true
subexpression is an R-value (like the condition subexpression). CFG is the
same in C and C++ case, but the AST nodes are different, which the LValue to
Rvalue conversion happening after the BinaryConditionalOperator evaluation.

Changed the logic to only use the last expression from the predecessor only
if it matches either true or false subexpression. Note, the logic needed
fortification anyway: L and R were passed but not even used by the function.

Also, change the conjureSymbolVal to correctly compute the type, when the
expression is an LG-value.

llvm-svn: 179574
2013-04-15 22:38:07 +00:00
Anna Zaks 7460deb15d [analyzer] Add pretty printing to CXXBaseObjectRegion.
llvm-svn: 179573
2013-04-15 22:38:04 +00:00
Anna Zaks e2e8ea62df [analyzer] Address code review for r179395
Mostly refactoring + handle the nested fields by printing the innermost field only.

llvm-svn: 179572
2013-04-15 22:37:59 +00:00
Anna Zaks 0881b8882e [analyzer] Add more specialized error messages for corner cases as per Jordan's code review for r179396
llvm-svn: 179571
2013-04-15 22:37:53 +00:00
Jordan Rose 27ae8a2800 [analyzer] Don't assert on a temporary of pointer-to-member type.
While we don't do anything intelligent with pointers-to-members today,
it's perfectly legal to need a temporary of pointer-to-member type to, say,
pass by const reference. Tweak an assertion to allow this.

PR15742 and PR15747

llvm-svn: 179563
2013-04-15 22:03:38 +00:00
Jordan Rose 2820e3dfca [analyzer] Be lazy about struct/array global invalidation too.
Structs and arrays can take advantage of the single top-level global
symbol optimization (described in the previous commit) just as well
as scalars.

No intended behavioral change.

llvm-svn: 179555
2013-04-15 20:39:48 +00:00
Jordan Rose fa80736bca [analyzer] Re-enable using global regions as a symbolic base.
Now that we're invalidating global regions properly, we want to continue
taking advantage of a particular optimization: if all global regions are
invalidated together, we can represent the bindings of each region with
a "derived region value" symbol. Essentially, this lazily links each
global region with a single symbol created at invalidation time, rather
than binding each region with a new symbolic value.

We used to do this, but haven't been for a while; the previous commit
re-enabled this code path, and this handles the fallout.

<rdar://problem/13464044>

llvm-svn: 179554
2013-04-15 20:39:45 +00:00
Jordan Rose 577749a337 [analyzer] Properly invalidate global regions on opaque function calls.
This fixes a regression where a call to a function we can't reason about
would not actually invalidate global regions that had explicit bindings.

  void test_that_now_works() {
    globalInt = 42;
    clang_analyzer_eval(globalInt == 42); // expected-warning{{TRUE}}

    invalidateGlobals();
    clang_analyzer_eval(globalInt == 42); // expected-warning{{UNKNOWN}}
  }

This has probably been around since the initial "cluster" refactoring of
RegionStore, if not longer.

<rdar://problem/13464044>

llvm-svn: 179553
2013-04-15 20:39:41 +00:00
Anna Zaks 685e913d71 [analyzer] Print a diagnostic note even if the region cannot be printed.
There are few cases where we can track the region, but cannot print the note,
which makes the testing limited. (Though, I’ve tested this manually by making
all regions non-printable.) Even though the applicability is limited now, the enhancement
will be more relevant as we start tracking more regions.

llvm-svn: 179396
2013-04-12 18:40:27 +00:00
Anna Zaks 6cea7d9e5e [analyzer]Print field region even when the base region is not printable
llvm-svn: 179395
2013-04-12 18:40:21 +00:00
Jordan Rose 526d93c55d [analyzer] Show "Returning from ..." note at caller's depth, not callee's.
Before:
  1. Calling 'foo'
    2. Doing something interesting
    3. Returning from 'foo'
  4. Some kind of error here

After:
  1. Calling 'foo'
    2. Doing something interesting
  3. Returning from 'foo'
  4. Some kind of error here

The location of the note is already in the caller, not the callee, so this
just brings the "depth" attribute in line with that.

This only affects plist diagnostic consumers (i.e. Xcode). It's necessary
for Xcode to associate the control flow arrows with the right stack frame.

<rdar://problem/13634363>

llvm-svn: 179351
2013-04-12 00:44:17 +00:00
Jordan Rose ce781ae6ae [analyzer] Don't emit extra context arrow after returning from an inlined call.
In this code

  int getZero() {
    return 0;
  }

  void test() {
    int problem = 1 / getZero(); // expected-warning {{Division by zero}}
  }

we generate these arrows:

    +-----------------+
    |                 v
    int problem = 1 / getZero();
                  ^   |
                  +---+

where the top one represents the control flow up to the first call, and the
bottom one represents the flow to the division.* It turns out, however, that
we were generating the top arrow twice, as if attempting to "set up context"
after we had already returned from the call. This resulted in poor
highlighting in Xcode.

* Arguably the best location for the division is the '/', but that's a
  different problem.

<rdar://problem/13326040>

llvm-svn: 179350
2013-04-12 00:44:01 +00:00
Jordan Rose 61e221f68d [analyzer] Replace isIntegerType() with isIntegerOrEnumerationType().
Previously, the analyzer used isIntegerType() everywhere, which uses the C
definition of "integer". The C++ predicate with the same behavior is
isIntegerOrUnscopedEnumerationType().

However, the analyzer is /really/ using this to ask if it's some sort of
"integrally representable" type, i.e. it should include C++11 scoped
enumerations as well. hasIntegerRepresentation() sounds like the right
predicate, but that includes vectors, which the analyzer represents by its
elements.

This commit audits all uses of isIntegerType() and replaces them with the
general isIntegerOrEnumerationType(), except in some specific cases where
it makes sense to exclude scoped enumerations, or any enumerations. These
cases now use isIntegerOrUnscopedEnumerationType() and getAs<BuiltinType>()
plus BuiltinType::isInteger().

isIntegerType() is hereby banned in the analyzer - lib/StaticAnalysis and
include/clang/StaticAnalysis. :-)

Fixes real assertion failures. PR15703 / <rdar://problem/12350701>

llvm-svn: 179081
2013-04-09 02:30:33 +00:00
Jordan Rose 4db7c1e7e5 [analyzer] When creating a trimmed graph, preserve whether a node is a sink.
This is important because sometimes two nodes are identical, except the
second one is a sink.

This bug has probably been around for a while, but it wouldn't have been an
issue in the old report graph algorithm. I'm ashamed to say I actually looked
at this the first time around and thought it would never be a problem...and
then didn't include an assertion to back that up.

PR15684

llvm-svn: 178944
2013-04-06 01:42:02 +00:00
Anna Zaks a4fdefffd0 [analyzer] Remove another redundancy from trackNullOrUndef
llvm-svn: 178934
2013-04-05 23:50:14 +00:00
Anna Zaks 94b48bdbba [analyzer] Fix null tracking for the given test case, by using the proper state and removing redundant code.
llvm-svn: 178933
2013-04-05 23:50:11 +00:00
Anna Zaks ece622ab46 [analyzer] Show path diagnostic for C++ initializers
Also had to modify the PostInitializer ProgramLocation to contain the field region.

llvm-svn: 178826
2013-04-05 00:59:33 +00:00
Jordan Rose 2de3daa0a2 [analyzer] Enable destructor inlining by default (c++-inlining=destructors).
This turns on not only destructor inlining, but inlining of constructors
for types with non-trivial destructors. Per r178516, we will still not
inline the constructor or destructor of anything that looks like a
container unless the analyzer-config option 'c++-container-inlining' is
set to 'true'.

In addition to the more precise path-sensitive model, this allows us to
catch simple smart pointer issues:

  #include <memory>

  void test() {
    std::auto_ptr<int> releaser(new int[4]);
  } // memory allocated with 'new[]' should not be deleted with 'delete'

<rdar://problem/12295363>

llvm-svn: 178805
2013-04-04 23:10:29 +00:00
Anna Zaks d3254b4462 [analyzer] Allow tracknullOrUndef look through the ternary operator even when condition is unknown
Improvement of r178684 and r178685.

Jordan has pointed out that I should not rely on the value of the condition to know which expression branch
has been taken. It will not work in cases the branch condition is an unknown value (ex: we do not track the constraints for floats).
The better way of doing this would be to find out if the current node is the right or left successor of the node
that has the ternary operator as a terminator (which is how this is done in other places, like ConditionBRVisitor).

llvm-svn: 178701
2013-04-03 21:34:12 +00:00
Jordan Rose 8647ffcda5 [analyzer] Correctly handle destructors for lifetime-extended temporaries.
The lifetime of a temporary can be extended when it is immediately bound
to a local reference:

  const Value &MyVal = Value("temporary");

In this case, the temporary object's lifetime is extended for the entire
scope of the reference; at the end of the scope it is destroyed.

The analyzer was modeling this improperly in two ways:
- Since we don't model temporary constructors just yet, we create a fake
  temporary region when it comes time to "materialize" a temporary into
  a real object (lvalue). This wasn't taking base casts into account when
  the bindings being materialized was Unknown; now it always respects base
  casts except when the temporary region is itself a pointer.
- When actually destroying the region, the analyzer did not actually load
  from the reference variable -- it was basically destroying the reference
  instead of its referent. Now it does do the load.

This will be more useful whenever we finally start modeling temporaries,
or at least those that get bound to local reference variables.

<rdar://problem/13552274>

llvm-svn: 178697
2013-04-03 21:16:58 +00:00
Anna Zaks b5d2fe8a1d [analyzer] make peelOffOuterExpr in BugReporterVisitors recursively peel off select Exprs
llvm-svn: 178685
2013-04-03 19:28:15 +00:00
Anna Zaks ede0983f88 [analyzer] Properly handle the ternary operator in trackNullOrUndefValue
1) Look for the node where the condition expression is live when checking if
it is constrained to true or false.

2) Fix a bug in ProgramState::isNull, which was masking the problem. When
the expression is not a symbol (,which is the case when it is Unknown) return
unconstrained value, instead of value constrained to “false”!
(Thankfully other callers of isNull have not been effected by the bug.)

llvm-svn: 178684
2013-04-03 19:28:12 +00:00
Anna Zaks ddef54cad6 [analyzer] Fix typo.
Thanks Jordan!

llvm-svn: 178683
2013-04-03 19:28:05 +00:00
Jordan Rose bc74eb1c90 [analyzer] Better model for copying of array fields in implicit copy ctors.
- Find the correct region to represent the first array element when
  constructing a CXXConstructorCall.
- If the array is trivial, model the copy with a primitive load/store.
- Don't warn about the "uninitialized" subscript in the AST -- we don't use
  the helper variable that Sema provides.

<rdar://problem/13091608>

llvm-svn: 178602
2013-04-03 01:39:08 +00:00
Aaron Ballman 235af9c1f5 Silencing warnings in MSVC due to duplicate identifiers.
llvm-svn: 178591
2013-04-02 23:47:53 +00:00
Anna Zaks 60bf5f45f7 [analyzer] Teach invalidateRegions that regions within LazyCompoundVal need to be invalidated
Refactor invalidateRegions to take SVals instead of Regions as input and teach RegionStore
about processing LazyCompoundVal as a top-level “escaping” value.

This addresses several false positives that get triggered by the NewDelete checker, but the
underlying issue is reproducible with other checkers as well (for example, MallocChecker).

llvm-svn: 178518
2013-04-02 01:28:24 +00:00
Jordan Rose e189b869c5 [analyzer] For now, don't inline [cd]tors of C++ containers.
This is a heuristic to make up for the fact that the analyzer doesn't
model C++ containers very well. One example is modeling that
'std::distance(I, E) == 0' implies 'I == E'. In the future, it would be
nice to model this explicitly, but for now it just results in a lot of
false positives.

The actual heuristic checks if the base type has a member named 'begin' or
'iterator'. If so, we treat the constructors and destructors of that type
as opaque, rather than inlining them.

This is intended to drastically reduce the number of false positives
reported with experimental destructor support turned on. We can tweak the
heuristic in the future, but we'd rather err on the side of false negatives
for now.

<rdar://problem/13497258>

llvm-svn: 178516
2013-04-02 00:26:35 +00:00
Jordan Rose 19440f58e9 [analyzer] Cache whether a function is generally inlineable.
Certain properties of a function can determine ahead of time whether or not
the function is inlineable, such as its kind, its signature, or its
location. We can cache this value in the FunctionSummaries map to avoid
rechecking these static properties for every call.

Note that the analyzer may still decide not to inline a specific call to
a function because of the particular dynamic properties of the call along
the current path.

No intended functionality change.

llvm-svn: 178515
2013-04-02 00:26:29 +00:00
Jordan Rose 33a1063cab [analyzer] Use inline storage in the FunctionSummary DenseMap.
The summaries lasted for the lifetime of the map anyway; no reason to
include an extra allocation.

Also, use SmallBitVector instead of BitVector to track the visited basic
blocks -- most functions will have less than 64 basic blocks -- and
use bitfields for the other fields to reduce the size of the structure.

No functionality change.

llvm-svn: 178514
2013-04-02 00:26:26 +00:00
Jordan Rose d11ef1aaf7 [analyzer] Allow suppressing diagnostics reported within the 'std' namespace
This is controlled by the 'suppress-c++-stdlib' analyzer-config flag.
It is currently off by default.

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

llvm-svn: 178513
2013-04-02 00:26:15 +00:00
Jordan Rose b8cb8359ce [analyzer] Restructure ExprEngine::VisitCXXNewExpr to do a bit less work.
No functionality change.

llvm-svn: 178402
2013-03-30 01:31:48 +00:00
Jordan Rose 8f6b4b043a [analyzer] Handle caching out while evaluating a C++ new expression.
Evaluating a C++ new expression now includes generating an intermediate
ExplodedNode, and this node could very well represent a previously-
reachable state in the ExplodedGraph. If so, we can short-circuit the
rest of the evaluation.

Caught by the assertion a few lines later.

<rdar://problem/13510065>

llvm-svn: 178401
2013-03-30 01:31:42 +00:00
Anna Zaks 8e492c2380 [analyzer] Address Jordan’s review of r178309 - do not register an extra visitor for nil receiver
We can check if the receiver is nil in the node that corresponds to the StmtPoint of the message send.
At that point, the receiver is guaranteed to be live. We will find at least one unreclaimed node due to
my previous commit (look for StmtPoint instead of PostStmt) and the fact that the nil receiver nodes are tagged.

+ a couple of extra tests.

llvm-svn: 178381
2013-03-29 22:32:38 +00:00
Anna Zaks 8d0dcd8add [analyzer] Look for a StmtPoint node instead of PostStmt in trackNullOrUndefValue.
trackNullOrUndefValue tries to find the first node that matches the statement it is tracking.
Since we collect PostStmt nodes (in node reclamation), none of those might be on the
current path, so relax the search to look for any StmtPoint.

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

llvm-svn: 178317
2013-03-29 00:09:22 +00:00
Anna Zaks 333481b90b [analyzer] Add support for escape of const pointers and use it to allow “newed” pointers to escape
Add a new callback that notifies checkers when a const pointer escapes. Currently, this only works
for const pointers passed as a top level parameter into a function. We need to differentiate the const
pointers escape from regular escape since the content pointed by const pointer will not change;
if it’s a file handle, a file cannot be closed; but delete is allowed on const pointers.

This should suppress several false positives reported by the NewDelete checker on llvm codebase.

llvm-svn: 178310
2013-03-28 23:15:29 +00:00
Anna Zaks 05fb371efc [analyzer] Apply the suppression rules to the nil receiver only if the value participates in the computation of the nil we warn about.
We should only suppress a bug report if the IDCed or null returned nil value is directly related to the value we are warning about. This was
not the case for nil receivers - we would suppress a bug report that had an IDCed nil receiver on the path regardless of how it’s
related to the warning.

1) Thread EnableNullFPSuppression parameter through the visitors to differentiate between tracking the value which
is directly responsible for the bug and other values that visitors are tracking (ex: general tracking of nil receivers).
2) in trackNullOrUndef specifically address the case when a value of the message send is nil due to the receiver being nil.

llvm-svn: 178309
2013-03-28 23:15:22 +00:00
Anton Yartsev 8b662704dc [analyzer] For now assume all standard global 'operator new' functions allocate memory in heap.
+ Improved test coverage for cplusplus.NewDelete checker.

llvm-svn: 178244
2013-03-28 16:10:38 +00:00
Jordan Rose 3503cb3572 [analyzer] Use evalBind for C++ new of scalar types.
These types will not have a CXXConstructExpr to do the initialization for
them. Previously we just used a simple call to ProgramState::bindLoc, but
that doesn't trigger proper checker callbacks (like pointer escape).

Found by Anton Yartsev.

llvm-svn: 178160
2013-03-27 18:10:35 +00:00
Anna Zaks 54417f6d68 [analyzer] Cleanup: only get the PostStmt when we need the underlying Stmt + comment
llvm-svn: 178153
2013-03-27 17:36:01 +00:00
Anna Zaks 22fa6ecc14 [analyzer] Ensure that the node NilReceiverBRVisitor is looking for is not reclaimed
The visitor should look for the PreStmt node as the receiver is nil in the PreStmt and this is the node. Also, tag the nil
receiver nodes with a special tag for consistency.

llvm-svn: 178152
2013-03-27 17:35:58 +00:00
Anna Zaks bd8f60d6d1 [analyzer] Make sure IDC works for ‘NSContainer value/key is nil’ checks.
Register the nil tracking visitors with the region and refactor trackNullOrUndefValue a bit.

Also adds the cast and paren stripping before checking if the value is an OpaqueValueExpr
or ExprWithCleanups.

llvm-svn: 178093
2013-03-26 23:58:49 +00:00
Anna Zaks b13d21b6e1 [analyzer] Change inlining policy to inline small functions when reanalyzing ObjC methods as top level.
This allows us to better reason about(inline) small wrapper functions.

llvm-svn: 178063
2013-03-26 18:57:58 +00:00
Anna Zaks ea47959c2f [analyzer] micro optimization as per Jordan’s feedback on r177905.
llvm-svn: 178062
2013-03-26 18:57:51 +00:00
Anna Zaks f60f2fb142 [analyzer] Set concrete offset bindings to UnknownVal when processing symbolic offset binding, even if no bindings are present.
This addresses an undefined value false positive from concreteOffsetBindingIsInvalidatedBySymbolicOffsetAssignment.

Fixes PR14877; radar://12991168.

llvm-svn: 177905
2013-03-25 20:43:24 +00:00
Jordan Rose d1d8929370 [analyzer] Teach ConstraintManager to ignore NonLoc <> NonLoc comparisons.
These aren't generated by default, but they are needed when either side of
the comparison is tainted.

Should fix our internal buildbot.

llvm-svn: 177846
2013-03-24 20:25:22 +00:00
Jordan Rose 8828d356fb [analyzer] Teach constraint managers about unsigned comparisons.
In C, comparisons between signed and unsigned numbers are always done in
unsigned-space. Thus, we should know that "i >= 0U" is always true, even
if 'i' is signed. Similarly, "u >= 0" is also always true, even though '0'
is signed.

Part of <rdar://problem/13239003> (false positives related to std::vector)

llvm-svn: 177806
2013-03-23 01:21:33 +00:00
Jordan Rose 8eca04b24e [analyzer] Loc-Loc operations (subtraction or comparison) produce a NonLoc.
For two concrete locations, we were producing another concrete location and
then casting it to an integer. We should just create a nonloc::ConcreteInt
to begin with.

No functionality change.

llvm-svn: 177805
2013-03-23 01:21:29 +00:00
Jordan Rose 59d179e9d2 [analyzer] Also transform "a < b" to "(b - a) > 0" in the constraint manager.
We can support the full range of comparison operations between two locations
by canonicalizing them as subtraction, as in the previous commit.

This won't work (well) if either location includes an offset, or (again)
if the comparisons are not consistent about which region comes first.

<rdar://problem/13239003>

llvm-svn: 177803
2013-03-23 01:21:23 +00:00
Jordan Rose 604d0bbba5 Add reverseComparisonOp and negateComparisonOp to BinaryOperator.
...and adopt them in the analyzer.

llvm-svn: 177802
2013-03-23 01:21:20 +00:00
Jordan Rose 8e6b6c0c2f [analyzer] Translate "a != b" to "(b - a) != 0" in the constraint manager.
Canonicalizing these two forms allows us to better model containers like
std::vector, which use "m_start != m_finish" to implement empty() but
"m_finish - m_start" to implement size(). The analyzer should have a
consistent interpretation of these two symbolic expressions, even though
it's not properly reasoning about either one yet.

The other unfortunate thing is that while the size() expression will only
ever be written "m_finish - m_start", the comparison may be written
"m_finish == m_start" or "m_start == m_finish". Right now the analyzer does
not attempt to canonicalize those two expressions, since it doesn't know
which length expression to pick. Doing this correctly will probably require
implementing unary minus as a new SymExpr kind (<rdar://problem/12351075>).

For now, the analyzer inverts the order of arguments in the comparison to
build the subtraction, on the assumption that "begin() != end()" is
written more often than "end() != begin()". This is purely speculation.

<rdar://problem/13239003>

llvm-svn: 177801
2013-03-23 01:21:16 +00:00
Jordan Rose 3b4c3ea2fb [analyzer] Use SymExprs to represent '<loc> - <loc>' and '<loc> == <loc>'.
We just treat this as opaque symbols, but even that allows us to handle
simple cases where the same condition is tested twice. This is very common
in the STL, which means that any project using the STL gets spurious errors.

Part of <rdar://problem/13239003>.

llvm-svn: 177800
2013-03-23 01:21:05 +00:00
Anna Zaks 911a7c9e03 [analyzer] Correct the stale comment.
llvm-svn: 177788
2013-03-23 00:39:17 +00:00
Jordan Rose 25fac2f6dc Revert "[analyzer] Break cycles (optionally) when trimming an ExplodedGraph."
The algorithm used here was ridiculously slow when a potential back-edge
pointed to a node that already had a lot of successors. The previous commit
makes this feature unnecessary anyway.

This reverts r177468 / f4cf6b10f863b9bc716a09b2b2a8c497dcc6aa9b.

Conflicts:

	lib/StaticAnalyzer/Core/BugReporter.cpp

llvm-svn: 177765
2013-03-22 21:15:33 +00:00
Jordan Rose f342adef47 [analyzer] Use a forward BFS instead of a reverse BFS to find shortest paths.
For a given bug equivalence class, we'd like to emit the report with the
shortest path. So far to do this we've been trimming the ExplodedGraph to
only contain relevant nodes, then doing a reverse BFS (starting at all the
error nodes) to find the shortest paths from the root. However, this is
fairly expensive when we are suppressing many bug reports in the same
equivalence class.

r177468-9 tried to solve this problem by breaking cycles during graph
trimming, then updating the BFS priorities after each suppressed report
instead of recomputing the whole thing. However, breaking cycles is not
a cheap operation because an analysis graph minus cycles is still a DAG,
not a tree.

This fix changes the algorithm to do a single forward BFS (starting from the
root) and to use that to choose the report with the shortest path by looking
at the error nodes with the lowest BFS priorities. This was Anna's idea, and
has the added advantage of requiring no update step: we can just pick the
error node with the next lowest priority to produce the next bug report.

<rdar://problem/13474689>

llvm-svn: 177764
2013-03-22 21:15:28 +00:00
Jordan Rose 73aa6f2178 [analyzer] Fix ExprEngine::ViewGraph to handle C++ initializers.
Debugging aid only, no functionality change.

llvm-svn: 177762
2013-03-22 21:15:16 +00:00
Jordan Rose 3b6af191d8 [analyzer] Appease buildbots: include template arguments in base class ref.
llvm-svn: 177583
2013-03-20 21:44:17 +00:00
Jordan Rose 28c68a2d07 [analyzer] Don't invalidate globals when there's no call involved.
This fixes some mistaken condition logic in RegionStore that caused
global variables to be invalidated when /any/ region was invalidated,
rather than only as part of opaque function calls. This was only
being used by CStringChecker, and so users will now see that strcpy()
and friends do not invalidate global variables.

Also, add a test case we don't handle properly: explicitly-assigned
global variables aren't being invalidated by opaque calls. This is
being tracked by <rdar://problem/13464044>.

llvm-svn: 177572
2013-03-20 20:36:01 +00:00
Jordan Rose 5d22fcb257 [analyzer] Track malloc'd memory into struct fields.
Due to improper modelling of copy constructors (specifically, their
const reference arguments), we were producing spurious leak warnings
for allocated memory stored in structs. In order to silence this, we
decided to consider storing into a struct to be the same as escaping.
However, the previous commit has fixed this issue and we can now properly
distinguish leaked memory that happens to be in a struct from a buffer
that escapes within a struct wrapper.

Originally applied in r161511, reverted in r174468.
<rdar://problem/12945937>

llvm-svn: 177571
2013-03-20 20:35:57 +00:00
Jordan Rose 5413aaa791 [analyzer] Invalidate regions indirectly accessible through const pointers.
In this case, the value of 'x' may be changed after the call to indirectAccess:

  struct Wrapper {
    int *ptr;
  };

  void indirectAccess(const Wrapper &w);

  void test() {
    int x = 42;
    Wrapper w = { x };

    clang_analyzer_eval(x == 42); // TRUE
    indirectAccess(w);
    clang_analyzer_eval(x == 42); // UNKNOWN
  }

This is important for modelling return-by-value objects in C++, to show
that the contents of the struct are escaping in the return copy-constructor.

<rdar://problem/13239826>

llvm-svn: 177570
2013-03-20 20:35:53 +00:00
Jordan Rose 153c81b7c4 [analyzer] Remove strip of ElementRegion in CallEvent::invalidateRegions.
This is a bit of old code trying to deal with the fact that functions that
take pointers often use them to access an entire array via pointer
arithmetic. However, RegionStore already conservatively assumes you can use
pointer arithmetic to access any part of a region.

Some day we may want to go back to handling this specifically for calls,
but we can do that in the future.

No functionality change.

llvm-svn: 177569
2013-03-20 20:35:48 +00:00
Jordan Rose 86e04ceaad [analyzer] Re-apply "Do part of the work to find shortest bug paths up front".
With the assurance that the trimmed graph does not contain cycles,
this patch is safe (with a few tweaks), and provides the performance
boost it was intended to.

Part of performance work for <rdar://problem/13433687>.

llvm-svn: 177469
2013-03-20 00:35:37 +00:00
Jordan Rose 34e19a1d1d [analyzer] Break cycles (optionally) when trimming an ExplodedGraph.
Having a trimmed graph with no cycles (a DAG) is much more convenient for
trying to find shortest paths, which is exactly what BugReporter needs to do.

Part of the performance work for <rdar://problem/13433687>.

llvm-svn: 177468
2013-03-20 00:35:31 +00:00
Anna Zaks 3f4fad92fe [analyzer] Do not believe lazy binding when symbolic region types do not match
This fixes a crash when analyzing LLVM that was exposed by r177220 (modeling of
trivial copy/move assignment operators).

When we look up a lazy binding for “Builder”, we see the direct binding of Loc at offset 0.
Previously, we believed the binding, which led to a crash. Now, we do not believe it as
the types do not match.

llvm-svn: 177453
2013-03-19 22:38:09 +00:00
Jordan Rose 431e4e4d0e Revert "[analyzer] Do part of the work to find shortest bug paths up front."
The whole reason we were doing a BFS in the first place is because an
ExplodedGraph can have cycles. Unfortunately, my removeErrorNode "update"
doesn't work at all if there are cycles.

I'd still like to be able to avoid doing the BFS every time, but I'll come
back to it later.

This reverts r177353 / 481fa5071c203bc8ba4f88d929780f8d0f8837ba.

llvm-svn: 177448
2013-03-19 22:10:35 +00:00
Jordan Rose 2d0edec994 [analyzer] Do part of the work to find shortest bug paths up front.
Splitting the graph trimming and the path-finding (r177216) already
recovered quite a bit of performance lost to increased suppression.
We can still do better by also performing the reverse BFS up front
(needed for shortest-path-finding) and only walking the shortest path
for each report. This does mean we have to walk back up the path and
invalidate all the BFS numbers if the report turns out to be invalid,
but it's probably still faster than redoing the full BFS every time.

More performance work for <rdar://problem/13433687>

llvm-svn: 177353
2013-03-18 23:34:37 +00:00
Jordan Rose ee47a5bd92 [analyzer] Replace uses of assume() with isNull() in BR visitors.
Also, replace a std::string with a SmallString.

No functionality change.

llvm-svn: 177352
2013-03-18 23:34:32 +00:00
Jordan Rose 3d7b7f5268 [analyzer] Model trivial copy/move assignment operators with a bind as well.
r175234 allowed the analyzer to model trivial copy/move constructors as
an aggregate bind. This commit extends that to trivial assignment
operators as well. Like the last commit, one of the motivating factors here
is not warning when the right-hand object is partially-initialized, which
can have legitimate uses.

<rdar://problem/13405162>

llvm-svn: 177220
2013-03-16 02:14:06 +00:00
Jordan Rose 7a007bfbad [analyzer] Separate graph trimming from creating the single-path graph.
When we generate a path diagnostic for a bug report, we have to take the
full ExplodedGraph and limit it down to a single path. We do this in two
steps: "trimming", which limits the graph to all the paths that lead to
this particular bug, and "creating the report graph", which finds the
shortest path in the trimmed path to any error node.

With BugReporterVisitor false positive suppression, this becomes more
expensive: it's possible for some paths through the trimmed graph to be
invalid (i.e. likely false positives) but others to be valid. Therefore
we have to run the visitors over each path in the graph until we find one
that is valid, or until we've ruled them all out. This can become quite
expensive.

This commit separates out graph trimming from creating the report graph,
performing the first only once per bug equivalence class and the second
once per bug report. It also cleans up that portion of the code by
introducing some wrapper classes.

This seems to recover most of the performance regression described in my
last commit.

<rdar://problem/13433687>

llvm-svn: 177216
2013-03-16 01:07:58 +00:00
Jordan Rose 0833c84a50 [analyzer] Eliminate InterExplodedGraphMap class and NodeBackMap typedef.
...in favor of this typedef:

  typedef llvm::DenseMap<const ExplodedNode *, const ExplodedNode *>
          InterExplodedGraphMap;

Use this everywhere the previous class and typedef were used.

Took the opportunity to ArrayRef-ize ExplodedGraph::trim while I'm at it.

No functionality change.

llvm-svn: 177215
2013-03-16 01:07:53 +00:00
Jordan Rose 5315931105 [analyzer] Don't repeat a bug equivalence class if every report is invalid.
I removed this check in the recursion->iteration commit, but forgot that
generatePathDiagnostic may be called multiple times if there are multiple
PathDiagnosticConsumers.

llvm-svn: 177214
2013-03-16 01:07:47 +00:00
Anna Zaks bda130f02a [analyzer] Use isLiveRegion to determine when SymbolRegionValue is dead.
Fixes a FIXME, improves dead symbol collection, suppresses a false positive,
which resulted from reusing the same symbol twice for simulation of 2 calls to the same function.

Fixing this lead to 2 possible false negatives in CString checker. Since the checker is still alpha and
the solution will not require revert of this commit, move the tests to a FIXME section.

llvm-svn: 177206
2013-03-15 23:34:29 +00:00
Anna Zaks e0f1a0f0d8 [analyzer] BugReporterVisitors: handle the case where a ternary operator is wrapped in a cast.
llvm-svn: 177205
2013-03-15 23:34:25 +00:00
Anna Zaks 313b101e27 [analyzer] Address Jordan’s review of r177138 (a micro optimization)
llvm-svn: 177204
2013-03-15 23:34:22 +00:00
Jordan Rose e42122e6b4 [analyzer] Make GRBugReporter::generatePathDiagnostic iterative, not recursive.
The previous generatePathDiagnostic() was intended to be tail-recursive,
restarting and trying again if a report was marked invalid. However:
 (1) this leaked all the cloned visitors, which weren't being deleted, and
 (2) this wasn't actually tail-recursive because some local variables had
     non-trivial destructors.

This was causing us to overflow the stack on inputs with large numbers of
reports in the same equivalence class, such as sqlite3.c. Being iterative
at least prevents us from blowing out the stack, but doesn't solve the
performance issue: suppressing thousands (yes, thousands) of paths in the
same equivalence class is expensive. I'm looking into that now.

<rdar://problem/13423498>

llvm-svn: 177189
2013-03-15 21:41:55 +00:00
Jordan Rose a45fc81180 [analyzer] Collect stats on the max # of bug reports in an equivalence class.
We discovered that sqlite3.c currently has 2600 reports in a single
equivalence class; it would be good to know if this is a recent
development or what.

(For the curious, the different reports in an equivalence class represent
the same bug found along different paths. When we're suppressing false
positives, we need to go through /every/ path to make sure there isn't a
valid path to a bug. This is a flaw in our after-the-fact suppression,
made worse by the fact that that function isn't particularly optimized.)

llvm-svn: 177188
2013-03-15 21:41:53 +00:00
Jordan Rose 06f68eda4d [analyzer] Include opcode in dumping a SymSymExpr.
For debugging use only; no functionality change.

llvm-svn: 177187
2013-03-15 21:41:50 +00:00
Jordan Rose ecaa7d2c3d [analyzer] Look through ExprWhenCleanups when trying to track a NULL.
Silences a few false positives in LLVM.

llvm-svn: 177186
2013-03-15 21:41:46 +00:00
Anna Zaks e9c43ffb2a [analyzer] Refactor checks in IDC visitor for consistency and speed
llvm-svn: 177138
2013-03-15 01:15:14 +00:00
Anna Zaks 913b0d0078 [analyzer] Teach trackNullOrUndef to look through ternary operators
Allows the suppression visitors trigger more often.

llvm-svn: 177137
2013-03-15 01:15:12 +00:00
Anna Zaks 2672a4cc0c [analyzer] Change the way in which IDC Visitor decides to kick in and make sure it attaches in the given edge case
In the test case below, the value V is not constrained to 0 in ErrorNode but it is in node N.
So we used to fail to register the Suppression visitor.

We also need to change the way we determine that the Visitor should kick in because the node N belongs to
the ExplodedGraph and might not be on the BugReporter path that the visitor sees. Instead of trying to match the node,
turn on the visitor when we see the last node in which the symbol is ‘0’.

llvm-svn: 177121
2013-03-14 22:31:56 +00:00
Anna Zaks e9989bd4df [analyzer] BugReporter - more precise tracking of C++ references
When BugReporter tracks C++ references involved in a null pointer violation, we
want to differentiate between a null reference and a reference to a null pointer. In the
first case, we want to track the region for the reference location; in the second, we want
to track the null pointer.

In addition, the core creates CXXTempObjectRegion to represent the location of the
C++ reference, so teach FindLastStoreBRVisitor about it.

This helps null pointer suppression to kick in.

(Patch by Anna and Jordan.)

llvm-svn: 176969
2013-03-13 20:20:14 +00:00
Ted Kremenek 650a69e988 Remove stray space.
llvm-svn: 176966
2013-03-13 20:05:52 +00:00
Ted Kremenek 2fb5f09e15 [analyzer] Handle Objc Fast enumeration for "loop is executed 0 times".
Fixes <rdar://problem/12322528>

llvm-svn: 176965
2013-03-13 20:03:31 +00:00
Jordan Rose 2eb7195ee6 [analyzer] Look for calls along with lvalue nodes in trackNullOrUndefValue.
r176737 fixed bugreporter::trackNullOrUndefValue to find nodes for an lvalue
even if the rvalue node had already been collected. This commit extends that
to call statement nodes as well, so that if a call is contained within
implicit casts we can still track the return value.

No test case because node reclamation is extremely finicky (dependent on
how the AST and CFG are built, and then on our current reclamation rules,
and /then/ on how many nodes were generated by the analyzer core and the
current set of checkers). I consider this a low-risk change, though, and
it will only happen in cases of reclamation when the rvalue node isn't
available.

<rdar://problem/13340764>

llvm-svn: 176829
2013-03-11 21:31:46 +00:00
Anna Zaks 5497d1a55c [analyzer] Make Suppress IDC checker aware that it might not start from the same node it was registered at
The visitor used to assume that the value it’s tracking is null in the first node it examines. This is not true.
If we are registering the Suppress Inlined Defensive checks visitor while traversing in another visitor
(such as FindlastStoreVisitor). When we restart with the IDC visitor, the invariance of the visitor does
not hold since the symbol we are tracking no longer exists at that point.

I had to pass the ErrorNode when creating the IDC visitor, because, in some cases, node N is
neither the error node nor will be visible along the path (we had not finalized the path at that point
and are dealing with ExplodedGraph.)

We should revisit the other visitors which might not be aware that they might get nodes, which are
later in path than the trigger point.

This suppresses a number of inline defensive checks in JavaScriptCore.

llvm-svn: 176756
2013-03-09 03:23:19 +00:00
Jordan Rose bce0583627 [analyzer] Look for lvalue nodes when tracking a null pointer.
r176010 introduced the notion of "interesting" lvalue expressions, whose
nodes are guaranteed never to be reclaimed by the ExplodedGraph. This was
used in bugreporter::trackNullOrUndefValue to find the region that contains
the null or undef value being tracked.

However, the /rvalue/ nodes (i.e. the loads from these lvalues that produce
a null or undef value) /are/ still being reclaimed, and if we couldn't
find the node for the rvalue, we just give up. This patch changes that so
that we look for the node for either the rvalue or the lvalue -- preferring
the former, since it lets us fall back to value-only tracking in cases
where we can't get a region, but allowing the latter as well.

<rdar://problem/13342842>

llvm-svn: 176737
2013-03-08 23:30:56 +00:00
Jordan Rose 5ca395462e [analyzer] Don't rely on finding the correct return statement for suppression.
Previously, ReturnVisitor waited to suppress a null return path until it
had found the inlined "return" statement. Now, it checks up front whether
the return value was NULL, and suppresses the warning right away if so.

We still have to wait until generating the path notes to invalidate the bug
report, or counter-suppression will never be triggered. (Counter-suppression
happens while generating path notes, but the generation won't happen for
reports already marked invalid.)

This isn't actually an issue today because we never reclaim nodes for
top-level statements (like return statements), but it could be an issue
some day in the future. (But, no expected behavioral change and no new
test case.)

llvm-svn: 176736
2013-03-08 23:30:53 +00:00
Jordan Rose 6f09928d5b [analyzer] Clean up a few doc comments for ProgramState and CallEvent.
No functionality change.

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

llvm-svn: 176576
2013-03-06 20:25:59 +00:00
Jordan Rose d03d99da16 Silence a number of static analyzer warnings with assertions and such.
No functionality change.

llvm-svn: 176469
2013-03-05 01:27:54 +00:00
Anna Zaks 8d7c8a4dd6 [analyzer] Simple inline defensive checks suppression
Inlining brought a few "null pointer use" false positives, which occur because
the callee defensively checks if a pointer is NULL, whereas the caller knows
that the pointer cannot be NULL in the context of the given call.

This is a first attempt to silence these warnings by tracking the symbolic value
along the execution path in the BugReporter. The new visitor finds the node
in which the symbol was first constrained to NULL. If the node belongs to
a function on the active stack, the warning is reported, otherwise, it is
suppressed.

There are several areas for follow up work, for example:
 - How do we differentiate the cases where the first check is followed by
another one, which does happen on the active stack?

Also, this only silences a fraction of null pointer use warnings. For example, it
does not do anything for the cases where NULL was assigned inside a callee.

llvm-svn: 176402
2013-03-02 03:20:52 +00:00
Jordan Rose e185d73101 [analyzer] Special-case bitfields when finding sub-region bindings.
Previously we were assuming that we'd never ask for the sub-region bindings
of a bitfield, since a bitfield cannot have subregions. However,
unification of code paths has made that assumption invalid. While we could
take advantage of this by just checking for the single possible binding,
it's probably better to do the right thing, so that if/when we someday
support unions we'll do the right thing there, too.

This fixes a handful of false positives in analyzing LLVM.

<rdar://problem/13325522>

llvm-svn: 176388
2013-03-01 23:03:17 +00:00
Jordan Rose 801916baf1 [analyzer] Suppress paths involving a reference whose rvalue is null.
Most map types have an operator[] that inserts a new element if the key
isn't found, then returns a reference to the value slot so that you can
assign into it. However, if the value type is a pointer, it will be
initialized to null. This is usually no problem.

However, if the user /knows/ the map contains a value for a particular key,
they may just use it immediately:

   // From ClangSACheckersEmitter.cpp
   recordGroupMap[group]->Checkers

In this case the analyzer reports a null dereference on the path where the
key is not in the map, even though the user knows that path is impossible
here. They could silence the warning by adding an assertion, but that means
splitting up the expression and introducing a local variable. (Note that
the analyzer has no way of knowing that recordGroupMap[group] will return
the same reference if called twice in a row!)

We already have logic that says a null dereference has a high chance of
being a false positive if the null came from an inlined function. This
patch simply extends that to references whose rvalues are null as well,
silencing several false positives in LLVM.

<rdar://problem/13239854>

llvm-svn: 176371
2013-03-01 19:45:10 +00:00
Jordan Rose a22cd706a2 [analyzer] RegionStore: collectSubRegionKeys -> collectSubRegionBindings
By returning the (key, value) binding pairs, we save lookups afterwards.
This also enables further work later on.

No functionality change.

llvm-svn: 176230
2013-02-28 01:53:08 +00:00
Jordan Rose f7f32d5202 [analyzer] Teach FindLastStoreBRVisitor to understand stores of the same value.
Consider this case:

  int *p = 0;
  p = getPointerThatMayBeNull();
  *p = 1;

If we inline 'getPointerThatMayBeNull', we might know that the value of 'p'
is NULL, and thus emit a null pointer dereference report. However, we
usually want to suppress such warnings as error paths, and we do so by using
FindLastStoreBRVisitor to see where the NULL came from. In this case, though,
because 'p' was NULL both before and after the assignment, the visitor
would decide that the "last store" was the initialization, not the
re-assignment.

This commit changes FindLastStoreBRVisitor to consider all PostStore nodes
that assign to this region. This still won't catches changes made directly
by checkers if they re-assign the same value, but it does handle the common
case in user-written code and will trigger ReturnVisitor's suppression
machinery as expected.

<rdar://problem/13299738>

llvm-svn: 176201
2013-02-27 18:49:57 +00:00
Jordan Rose 4587b28758 [analyzer] Turn on C++ constructor inlining by default.
This enables constructor inlining for types with non-trivial destructors.
The plan is to enable destructor inlining within the next month, but that
needs further verification.

<rdar://problem/12295329>

llvm-svn: 176200
2013-02-27 18:49:43 +00:00
Ted Kremenek f352d8c7ec [analyzer] Add stop-gap patch to prevent assertion failure when analyzing LLVM codebase.
This potentially reduces a performance optimization of throwing away
PreStmtPurgeDeadSymbols nodes.  I'll investigate the performance impact
soon and see if we need something better.

llvm-svn: 176149
2013-02-27 01:26:58 +00:00
Jordan Rose 4a7bf49bd4 [analyzer] If a struct has a partial lazy binding, its fields aren't Undef.
This is essentially the same problem as r174031: a lazy binding for the first
field of a struct may stomp on an existing default binding for the
entire struct. Because of the way RegionStore is set up, we can't help
but lose the top-level binding, but then we need to make sure that accessing
one of the other fields doesn't come back as Undefined.

In this case, RegionStore is now correctly detecting that the lazy binding
we have isn't the right type, but then failing to follow through on the
implications of that: we don't know anything about the other fields in the
aggregate. This fix adds a test when searching for other kinds of default
values to see if there's a lazy binding we rejected, and if so returns
a symbolic value instead of Undefined.

The long-term fix for this is probably a new Store model; see
<rdar://problem/12701038>.

Fixes <rdar://problem/13292559>.

llvm-svn: 176144
2013-02-27 00:05:29 +00:00
Ted Kremenek 37c777ecc0 [analyzer] Use 'MemRegion::printPretty()' instead of assuming the region is a VarRegion.
Fixes PR15358 and <rdar://problem/13295437>.

Along the way, shorten path diagnostics that say "Variable 'x'" to just
be "'x'".  By the context, it is obvious that we have a variable,
and so this just consumes text space.

llvm-svn: 176115
2013-02-26 19:44:38 +00:00
Jordan Rose 861a174018 [analyzer] Don't look through casts when creating pointer temporaries.
Normally, we need to look through derived-to-base casts when creating
temporary object regions (added in r175854). However, if the temporary
is a pointer (rather than a struct/class instance), we need to /preserve/
the base casts that have been applied.

This also ensures that we really do create a new temporary region when
we need to: MaterializeTemporaryExpr and lvalue CXXDefaultArgExprs.

Fixes PR15342, although the test case doesn't include the crash because
I couldn't isolate it.

llvm-svn: 176069
2013-02-26 01:21:27 +00:00
Ted Kremenek 8f5640588a [analyzer] Recover all PreStmtPurgeDeadSymbols nodes with a single successor or predecessor.
These nodes are never consulted by any analyzer client code, so they are
used only for machinery for removing dead bindings.  Once successor nodes
are generated they can be safely removed.

This greatly reduces the amount of nodes that are generated in some case,
lowering the memory regression when analyzing Sema.cpp introduced by
r176010 from 14% to 2%.

llvm-svn: 176050
2013-02-25 21:32:40 +00:00
Anna Zaks 2d773b8138 [analyzer] Address Jordan's code review of r175857.
llvm-svn: 176043
2013-02-25 19:50:50 +00:00
Jordan Rose 77cdb53cdf [analyzer] Handle reference parameters with default values.
r175026 added support for default values, but didn't take reference
parameters into account, which expect the default argument to be an
lvalue. Use createTemporaryRegionIfNeeded if we can evaluate the default
expr as an rvalue but the expected result is an lvalue.

Fixes the most recent report of PR12915. The original report predates
default argument support, so that can't be it.

llvm-svn: 176042
2013-02-25 19:45:34 +00:00
Jordan Rose f9e9a9f41b [analyzer] Base regions may be invalid when layered on symbolic regions.
While RegionStore checks to make sure casts on TypedValueRegions are valid,
it does not do the same for SymbolicRegions, which do not have perfect type
info anyway. Additionally, MemRegion::getAsOffset does not take a
ProgramState, so it can't use dynamic type info to determine a better type
for the regions. (This could also be dangerous if the type of a super-region
changes!)

Account for this by checking that a base object region is valid on top of a
symbolic region, and falling back to "symbolic offset" mode if not.

Fixes PR15345.

llvm-svn: 176034
2013-02-25 18:36:15 +00:00
Ted Kremenek 9db5f52c47 [analyzer] Relax assumption in FindLastStoreBRVisitor that the thing we are looking for is always a VarRegion.
This was triggering assertion failures when analyzing the LLVM codebase.  This
is fallout from r175988.

I've got delta chewing away on a test case, but I wanted the fix to go
in now.

llvm-svn: 176011
2013-02-25 07:37:18 +00:00
Ted Kremenek 04fa9e3d80 [analyzer] add the notion of an "interesting" lvalue expression for ExplodedNode pruning.
r175988 modified the ExplodedGraph trimming algorithm to retain all
nodes for "lvalue" expressions.  This patch refines that notion to
only "interesting" expressions that would be used for diagnostics.

llvm-svn: 176010
2013-02-25 07:37:13 +00:00
Ted Kremenek 9625048278 [analyzer] tracking stores/constraints now works for ObjC ivars or struct fields.
This required more changes than I originally expected:

- ObjCIvarRegion implements "canPrintPretty" et al
- DereferenceChecker indicates the null pointer source is an ivar
- bugreporter::trackNullOrUndefValue() uses an alternate algorithm
  to compute the location region to track by scouring the ExplodedGraph.
  This allows us to get the actual MemRegion for variables, ivars,
  fields, etc.  We only hand construct a VarRegion for C++ references.
- ExplodedGraph no longer drops nodes for expressions that are marked
  'lvalue'.  This is to facilitate the logic in the previous bullet.
  This may lead to a slight increase in size in the ExplodedGraph,
  which I have not measured, but it is likely not to be a big deal.

I have validated each of the changed plist output.

Fixes <rdar://problem/12114812>

llvm-svn: 175988
2013-02-24 07:21:01 +00:00
Ted Kremenek e3cf171730 Add "KnownSVal" to represent SVals that cannot be UnknownSVal.
This provides a few sundry cleanups, and allows us to provide
a compile-time check for a case that was a runtime assertion.

llvm-svn: 175987
2013-02-24 07:20:53 +00:00
David Blaikie 00be69ab5c Remove the CFGElement "Invalid" state.
Use Optional<CFG*> where invalid states were needed previously. In the one case
where that's not possible (beginAutomaticObjDtorsInsert) just use a dummy
CFGAutomaticObjDtor.

Thanks for the help from Jordan Rose & discussion/feedback from Ted Kremenek
and Doug Gregor.

Post commit code review feedback on r175796 by Ted Kremenek.

llvm-svn: 175938
2013-02-23 00:29:34 +00:00
Jordan Rose 893d73b7e9 [analyzer] Don't canonicalize the RecordDecl used in CXXBaseObjectRegion.
This Decl shouldn't be the canonical Decl; it should be the Decl used by
the CXXBaseSpecifier in the subclass. Unfortunately, that means continuing
to throw getCanonicalDecl() on all comparisons.

This fixes MemRegion::getAsOffset's use of ASTRecordLayout when redeclarations
are involved.

llvm-svn: 175913
2013-02-22 19:33:13 +00:00
Ted Kremenek efb41d23a6 [analyzer] Implement "Loop executed 0 times" diagnostic correctly.
Fixes <rdar://problem/13236549>

llvm-svn: 175863
2013-02-22 05:45:33 +00:00