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
Consider this example:
char *p = malloc(sizeof(char));
systemFunction(&p);
free(p);
In this case, when we call systemFunction, we know (because it's a system
function) that it won't free 'p'. However, we /don't/ know whether or not
it will /change/ 'p', so the analyzer is forced to invalidate 'p', wiping
out any bindings it contains. But now the malloc'd region looks like a
leak, since there are no more bindings pointing to it, and we'll get a
spurious leak warning.
The fix for this is to notice when something is becoming inaccessible due
to invalidation (i.e. an imperfect model, as opposed to being explicitly
overwritten) and stop tracking it at that point. Currently, the best way
to determine this for a call is the "indirect escape" pointer-escape kind.
In practice, all the patch does is take the "system functions don't free
memory" special case and limit it to direct parameters, i.e. just the
arguments to a call and not other regions accessible to them. This is a
conservative change that should only cause us to escape regions more
eagerly, which means fewer leak warnings.
This isn't perfect for several reasons, the main one being that this
example is treated the same as the one above:
char **p = malloc(sizeof(char *));
systemFunction(p + 1);
// leak
Currently, "addresses accessible by offsets of the starting region" and
"addresses accessible through bindings of the starting region" are both
considered "indirect" regions, hence this uniform treatment.
Another issue is our longstanding problem of not distinguishing const and
non-const bindings; if in the first example systemFunction's parameter were
a char * const *, we should know that the function will not overwrite 'p',
and thus we can safely report the leak.
<rdar://problem/13758386>
llvm-svn: 181607
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
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
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
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
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
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
...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
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
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
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
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
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
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
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
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
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
- 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
Anna Zaks