Commit Graph

1856 Commits

Author SHA1 Message Date
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 b147918252 [analyzer] RetainCountChecker: don't track through xpc_connection_set_context.
It is unfortunate that we have to mark these exceptions in multiple places.
This was already in CallEvent. I suppose it does let us be more precise
about saying /which/ arguments have their retain counts invalidated -- the
connection's is still valid even though the context object's isn't -- but
we're not tracking the retain count of XPC objects anyway.

<rdar://problem/13783514>

llvm-svn: 180904
2013-05-02 01:51:40 +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 9de821ebfd [analyzer] An ObjC for-in loop runs 0 times if the collection is nil.
In an Objective-C for-in loop "for (id element in collection) {}", the loop
will run 0 times if the collection is nil. This is because the for-in loop
is implemented using a protocol method that returns 0 when there are no
elements to iterate, and messages to nil will result in a 0 return value.

At some point we may want to actually model this message send, but for now
we may as well get the nil case correct, and avoid the false positives that
would come with this case.

<rdar://problem/13744632>

llvm-svn: 180639
2013-04-26 21:43:01 +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
Anna Zaks 144579e299 [analyzer] Teach DeadStoreChecker to look though BO_Comma and disregard the LHS.
llvm-svn: 180579
2013-04-25 21:52:35 +00:00
Anna Zaks 99394bbd02 [analyzer] Fix a crash in RetainCountChecker - we should not rely on CallEnter::getCallExpr to return non-NULL
We get a CallEnter with a null expression, when processing a destructor. All other users of
CallEnter::getCallExpr work fine with null as return value.

(Addresses PR15832, Thanks to Jordan for reducing the test case!)

llvm-svn: 180234
2013-04-25 00:41:32 +00:00
Anton Yartsev 67f1ab0de8 [analyzer] Refactoring + explanatory comment.
llvm-svn: 180181
2013-04-24 10:24:38 +00:00
Anna Zaks 7712f38978 [analyzer] IvarInvalidation: correctly handle cases where only partial invalidators exist
- If only partial invalidators exist and there are no full invalidators in @implementation, report every ivar that has
not been invalidated. (Previously, we reported the first Ivar in the list, which could actually have been invalidated
by a partial invalidator. The code assumed you cannot have only partial invalidators.)

- Do not report missing invalidation method declaration if a partial invalidation method declaration exists.

llvm-svn: 180170
2013-04-24 02:49:16 +00:00
Anna Zaks 404028798f [analyzer] Set the allocation site to be the uniqueing location for retain count checker leaks.
The uniqueing location is the location which is part of the hash used to determine if two reports are
the same. This is used by the CmpRuns.py script to compare two analyzer runs and determine which
warnings are new.

llvm-svn: 180166
2013-04-23 23:57:50 +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 7467f06533 [analyzer] RetainCountChecker: Clean up path notes for autorelease.
No functionality change.

<rdar://problem/13710586>

llvm-svn: 180075
2013-04-23 01:42:25 +00:00
Jordan Rose 6e3cf2ba85 [analyzer] Model strsep(), particularly that it returns its input.
This handles the false positive leak warning in PR15374, and also serves
as a basic model for the strsep() function.

llvm-svn: 180069
2013-04-22 23:18:42 +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
Jordan Rose 3437669ca9 [analyzer] Type information from C++ new expressions is perfect.
This improves our handling of dynamic_cast and devirtualization for
objects allocated by 'new'.

llvm-svn: 180051
2013-04-22 21:36:44 +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
Ted Kremenek 8671acba95 [analyzer] Add experimental option "leak-diagnostics-reference-allocation".
This is an opt-in tweak for leak diagnostics to reference the allocation
site if the diagnostic consumer only wants a pithy amount of information,
and not the entire path.

This is a strawman enhancement that I expect to see some experimentation
with over the next week, and can go away if we don't want it.

Currently it is only used by RetainCountChecker, but could be used
by MallocChecker if and when we decide this should stay in.

llvm-svn: 179634
2013-04-16 21:44:22 +00:00
Ted Kremenek eb5f089ce1 Properly sort list.
llvm-svn: 179627
2013-04-16 21:10:09 +00:00
Ted Kremenek 8a28295895 Factor CheckerManager to be able to pass AnalyzerOptions to checkers
during checker registration.  There are no immediate clients of this,
but this provides a way for checkers to query the options table
at startup instead.

llvm-svn: 179626
2013-04-16 21:10:05 +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 e4cfcd4e41 [analyzer] Improve the malloc checker stack hint message
llvm-svn: 179580
2013-04-16 00:22:55 +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