| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|
|
|
| |
llvm-svn: 162927
|
|
|
|
|
|
|
|
|
|
|
|
| |
inlined function.
This resolves retain count checker false positives that are caused by
inlining ObjC and other methods. Essentially, if we are passing an
object to a method with "delegate" in the selector or a function pointer
as another argument, we should stop tracking the other parameters/return
value as far as the retain count checker is concerned.
llvm-svn: 162876
|
|
|
|
|
|
| |
Thanks Jordan.
llvm-svn: 162875
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This heuristic addresses the case when a pointer (or ref) is passed
to a function, which initializes the variable (or sets it to something
other than '0'). On the branch where the inlined function does not
set the value, we report use of undefined value (or NULL pointer
dereference). The access happens in the caller and the path
through the callee would get pruned away with regular path pruning. To
solve this issue, we previously disabled diagnostic pruning completely
on undefined and null pointer dereference checks, which entailed very
verbose diagnostics in most cases. Furthermore, not all of the
undef value checks had the diagnostic pruning disabled.
This patch implements the following heuristic: if we pass a pointer (or
ref) to the region (on which the error is reported) into a function and
it's value is either undef or 'NULL' (and is a pointer), do not prune
the function.
llvm-svn: 162863
|
|
|
|
|
|
|
|
|
| |
a comma separated collection of key:value pairs (which are strings). This
allows a general way to provide analyzer configuration data from the command line.
No clients yet.
llvm-svn: 162827
|
|
|
|
|
|
|
|
|
|
|
|
| |
In C++, objects being returned on the stack are actually copy-constructed into
the return value. That means that when a temporary is returned, it still has
to be destroyed, i.e. the returned expression will be wrapped in an
ExprWithCleanups node. Our "returning stack memory" checker needs to look
through this node to see if we really are returning an object by value.
PR13722
llvm-svn: 162817
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Specifically, CallEventManager::getCaller was looking at the call site for
an inlined call and trying to see what kind of call it was, but it only
checked for CXXConstructExprClass. (It's not using an isa<> here to avoid
doing three more checks on the the statement class.)
This caused an unreachable when we actually did inline the constructor of a
temporary object.
PR13717
llvm-svn: 162792
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When exiting a function, the analyzer looks for the last statement in the
function to see if it's a return statement (and thus bind the return value).
However, the search for "the last statement" was accepting statements that
were in implicitly-generated inlined functions (i.e. destructors). So we'd
go and get the statement from the destructor, and then say "oh look, this
function had no explicit return...guess there's no return value". And /that/
led to the value being returned being declared dead, and all our leak
checkers complaining.
llvm-svn: 162791
|
|
|
|
|
|
|
|
| |
No test case since this is a debug option that we will never turn on by
default since it makes the leak checkers much less useful. (We'll only report
leaks at the end of analysis if -analyzer-purge=none.)
llvm-svn: 162772
|
|
|
|
|
|
|
|
|
|
|
| |
This helper function (in the clang::ento::bugreporter namespace) may add more
than one visitor, but conceptually it's tracking a single use of a null or
undefined value and should do so as best it can.
Also, the BugReport parameter has been made a reference to underscore that
it is non-optional.
llvm-svn: 162720
|
|
|
|
|
|
|
|
| |
As Anna pointed out to me offline, it's a little silly to walk backwards through
the graph to find the store site when BugReporter will do the exact same walk
as part of path diagnostic generation.
llvm-svn: 162719
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously, if we were tracking stores to a variable 'x', and came across this:
x = foo();
...we would simply emit a note here and stop. Now, we'll step into 'foo' and
continue tracking the returned value from there.
<rdar://problem/12114689>
llvm-svn: 162718
|
|
|
|
|
|
|
|
|
|
| |
The two callers are using this in order to be conservative, so let's just
clarify the information that's actually being provided here. This is not
related to inlining decisions in any way.
No functionality change.
llvm-svn: 162717
|
|
|
|
|
|
|
|
| |
Also, add comments to addTrackNullOrUndefValueVisitor.
Thanks for the review, Anna!
llvm-svn: 162695
|
|
|
|
|
|
|
|
|
|
|
| |
Because the CXXNewExpr appears after the CXXConstructExpr in the CFG, we don't
actually have the correct region to construct into at the time we decide
whether or not to inline. The long-term fix (discussed in PR12014) might be to
introduce a new CFG node (CFGAllocator) that appears before the constructor.
Tracking the short-term fix in <rdar://problem/12180598>.
llvm-svn: 162689
|
|
|
|
| |
llvm-svn: 162687
|
|
|
|
|
|
|
|
|
|
|
|
| |
This allows us to better reason about status objects, like Clang's own
llvm::Optional (when its contents are trivially destructible), which are
often intended to be passed around by value.
We still don't inline constructors for temporaries in the general case.
<rdar://problem/11986434>
llvm-svn: 162681
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This allows checkers (like the MallocChecker) to process the effects of the
bind. Previously, using a memory-allocating function (like strdup()) in an
initializer would result in a leak warning.
This does bend the expectations of checkBind a bit; since there is no
assignment expression, the statement being used is the initializer value.
In most cases this shouldn't matter because we'll use a PostInitializer
program point (rather than PostStmt) for any checker-generated nodes, though
we /will/ generate a PostStore node referencing the internal statement.
(In theory this could have funny effects if someone actually does an
assignment within an initializer; in practice, that seems like it would be
very rare.)
<rdar://problem/12171711>
llvm-svn: 162637
|
|
|
|
|
|
| |
No functional change intended.
llvm-svn: 162632
|
|
|
|
|
|
|
| |
this group into "alpha" and "beta" to distinguish between checkers in
different levels of premature state.
llvm-svn: 162582
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
generated for a given diagnostic to another. Because PathDiagnostics
are specific to a give PathDiagnosticConsumer, store in
a FoldingSet a unique hash for a PathDiagnostic (that will be the same
for the same bug for different PathDiagnosticConsumers) that
stores a list of files generated. This can then be read by the
other PathDiagnosticConsumers.
This fixes breakage in the PLIST-HTML output.
llvm-svn: 162580
|
|
|
|
|
|
|
|
|
|
|
|
| |
More generally, any time we try to track where a null value came from, we
should show if it came from a function. This usually isn't necessary if
the value is symbolic, but if the value is just a constant we previously
just ignored its origin entirely. Now, we'll step into the function and
recursively add a visitor to the returned expression.
<rdar://problem/12114609>
llvm-svn: 162563
|
|
|
|
|
|
|
| |
When reallocation of a non-allocated (not owned) symbol fails do not
expect it to be freed.
llvm-svn: 162533
|
|
|
|
|
|
|
|
| |
This code has been added a while ago and removing it does not trigger
any test failures. The false positives it was trying to suppress are
probably handled by other logic (ex: special handling of delegates).
llvm-svn: 162529
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
With inlining, retain count checker starts tracking 'self' through the
init methods. The analyser results were too noisy if the developer
did not follow 'self = [super init]' pattern (which is common
especially in older code bases) - we reported self init anti-pattern AND
possible use-after-free. This patch teaches the retain count
checker to assume that [super init] does not fail when it's not consumed
by another expression. This silences the retain count warning that warns
about possibility of use-after-free when init fails, while preserving
all the other checking on 'self'.
llvm-svn: 162508
|
|
|
|
|
|
|
|
|
|
|
|
| |
Until we have full support for pointers-to-members, we can at least
approximate some of their use by tracking null and non-null values.
We thus treat &A::m_ptr as a non-null void * symbol, and MemberPointer(0)
as a pointer-sized null constant.
This enables support for what is sometimes called the "safe bool" idiom,
demonstrated in the test case.
llvm-svn: 162495
|
|
|
|
|
|
|
|
| |
This is trivial; the UserDefinedConversion always wraps a CXXMemberCallExpr
for the appropriate conversion function, so it's just a matter of
propagating that value to the CastExpr itself.
llvm-svn: 162494
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
A CXXDefaultArgExpr wraps an Expr owned by a ParmVarDecl belonging to the
called function. In general, ExprEngine and Environment ought to treat this
like a ParenExpr or other transparent wrapper expression, with the inside
expression evaluated first.
However, if we call the same function twice, we'd produce a CFG that contains
the same wrapped expression twice, and we're not set up to handle that. I've
added a FIXME to the CFG builder to come back to that, but meanwhile we can
at least handle expressions that don't need to be explicitly evaluated:
literals. This probably handles many common uses of default parameters:
true/false, null, etc.
Part of PR13385 / <rdar://problem/12156507>
llvm-svn: 162453
|
|
|
|
| |
llvm-svn: 162430
|
|
|
|
| |
llvm-svn: 162412
|
|
|
|
| |
llvm-svn: 162405
|
|
|
|
|
|
|
|
| |
The checker adds assumptions that the return values from the known APIs
are non-nil. Teach the checker about NSArray/NSMutableArray/NSOrderedSet
objectAtIndex, objectAtIndexedSubscript.
llvm-svn: 162398
|
|
|
|
|
|
| |
another way to whitelist these special cases. This is an intermediate patch.
llvm-svn: 162386
|
|
|
|
|
|
|
|
| |
As part of this change, I discovered that a few of our tests were not testing
the RangeConstraintManager. Luckily all of those passed when I moved them
over to use that constraint manager.
llvm-svn: 162384
|
|
|
|
|
|
|
|
|
| |
'killBinding()'. The name is more specific, and one just forwarded
to the other.
Add some doxygen comments along the way.
llvm-svn: 162350
|
|
|
|
|
|
|
|
|
| |
Also rename 'getCurrentBlockCounter()' to 'blockCount()'.
This ripples a bunch of code simplifications; mostly aesthetic,
but makes the code a bit tighter.
llvm-svn: 162349
|
|
|
|
|
|
|
|
|
|
| |
No need to have the "get", the word "conjure" is a verb too!
Getting a conjured symbol is the same as conjuring one up.
This shortening is largely cosmetic, but just this simple changed
cleaned up a handful of lines, making them less verbose.
llvm-svn: 162348
|
|
|
|
|
|
|
|
| |
all forwarding methods.
This functionality is already covered by bindLoc().
llvm-svn: 162346
|
|
|
|
|
|
| |
add doxygen comments.
llvm-svn: 162345
|
|
|
|
| |
llvm-svn: 162341
|
|
|
|
| |
llvm-svn: 162330
|
|
|
|
| |
llvm-svn: 162325
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Under -analyzer-ipa=basic-inlining, only C functions, blocks, and C++ static
member functions are inlined -- essentially, the calls that behave like simple
C function calls. This is essentially the behavior in Xcode 4.4.
C++ support still has some rough edges, and we don't want users to be worried
about them if they download and run their own checker. (In particular, the
massive number of false positives for analyzing LLVM comes from inlining
defensively-written code in contexts where more aggressive assumptions are
implicitly made. This problem is not unique to C++, but it is exacerbated by
the higher proportion of code that lives in header files in C++.)
The eventual goal is to be comfortable enough with C++ support (and simple
Objective-C support) to advance to -analyzer-ipa=inlining as the default
behavior. See the IPA design notes for more details.
llvm-svn: 162318
|
|
|
|
|
|
|
|
|
|
| |
This reduces duplication across the Basic and Range constraint managers, and
keeps their internals free of dealing with the semantics of C++. It's still
a little unfortunate that the constraint manager is dealing with this at all,
but this is pretty much the only place to put it so that it will apply to all
symbolic values, even when embedded in larger expressions.
llvm-svn: 162313
|
|
|
|
|
|
|
|
| |
By doing this in the constraint managers, we can ensure that ANY reference
whose value we don't know gets the effect, even if it's not a top-level
parameter.
llvm-svn: 162246
|
|
|
|
|
|
| |
No functionality change.
llvm-svn: 162216
|
|
|
|
|
|
|
|
|
|
|
| |
Generating a sink is significantly different behavior from generating a
normal node, and a simple boolean parameter can be rather opaque. Per
offline discussion with Anna, adding new generation methods is the
clearest way to communicate intent.
No functionality change.
llvm-svn: 162215
|
|
|
|
|
|
|
|
|
|
|
| |
Forgetting to at least cast the result was giving us Loc/NonLoc problems
in SValBuilder (hitting an assertion). But the standard (both C and C++)
does actually guarantee that && and || will result in the actual values
1 and 0, typed as 'int' in C and 'bool' in C++, and we can easily model that.
PR13461
llvm-svn: 162209
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Our current handling of 'throw' is all CFG-based: it jumps to a 'catch' block
if there is one and the function exit block if not. But this doesn't really
get the right behavior when a function is inlined: execution will continue on
the caller's side, which is always the wrong thing to do.
Even within a single function, 'throw' completely skips any destructors that
are to be run. This is essentially the same problem as @finally -- a CFGBlock
that can have multiple entry points, whose exit points depend on whether it
was entered normally or exceptionally.
Representing 'throw' as a sink matches our current (non-)handling of @throw.
It's not a perfect solution, but it's better than continuing analysis in an
inconsistent or even impossible state.
<rdar://problem/12113713>
llvm-svn: 162157
|
|
|
|
|
|
|
|
|
|
|
|
| |
The CFG approximates @throw as a return statement, but that's not good
enough in inlined functions. Moreover, since Objective-C exceptions are
usually considered fatal, we should be suppressing leak warnings like we
do for calls to noreturn functions (like abort()).
The comments indicate that we were probably intending to do this all along;
it may have been inadvertantly changed during a refactor at one point.
llvm-svn: 162156
|