Commit Graph

741 Commits

Author SHA1 Message Date
Ted Kremenek 5bc38bad73 Rework how PathDiagnosticConsumers pass knowledge of what files they
generated for a given diagnostic to another.  Because PathDiagnostics
are specific to a give PathDiagnosticConsumer, store in
a FoldingSet a unique hash for a PathDiagnostic (that will be the same
for the same bug for different PathDiagnosticConsumers) that
stores a list of files generated.  This can then be read by the
other PathDiagnosticConsumers.

This fixes breakage in the PLIST-HTML output.

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

<rdar://problem/12114609>

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

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

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

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

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

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

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

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

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

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

Add some doxygen comments along the way.

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

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

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

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

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

This functionality is already covered by bindLoc().

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

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

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

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

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

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

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

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

No functionality change.

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

PR13461

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

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

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

<rdar://problem/12113713>

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

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

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

No functionality change.

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

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

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

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

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

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

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

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

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

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

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

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

Reverts r161911, which merely masked the problem.

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

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

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

llvm-svn: 161911
2012-08-15 00:36:44 +00:00
Anna Zaks 6ddb6b1a9a [analyzer]Assume that the properties cannot be overridden when dot
syntax is used.

llvm-svn: 161889
2012-08-14 19:19:18 +00:00
Benjamin Kramer 9299d8c298 Do NOT use inline functions with LLVM_ATTRIBUTE_USED.
The function will be emitted into every single TU including the header!

llvm-svn: 161872
2012-08-14 14:50:32 +00:00
Jordan Rose e521f93225 [analyzer] Look up DynamicTypeInfo by region instead of symbol.
This allows us to store type info for non-symbolic regions.

No functionality change.

llvm-svn: 161811
2012-08-13 23:59:07 +00:00
Jordan Rose ce6c99a559 [analyzer] Reduce code duplication: make CXXDestructorCall a CXXInstanceCall.
While there is now some duplication between SimpleCall and the CXXInstanceCall
sub-hierarchy, this is much better than copy-and-pasting the devirtualization
logic shared by both instance methods and destructors.

An unfortunate side effect is that there is no longer a single CallEvent type
that corresponds to "calls written as CallExprs". For the most part this is a
good thing, but the checker callback eval::Call still takes a CallExpr rather
than a CallEvent (since we're not sure if we want to allow checkers to
evaluate other kinds of calls). A mistake here will be caught by a cast<> in
CheckerManager::runCheckersForEvalCall.

No functionality change.

llvm-svn: 161809
2012-08-13 23:46:05 +00:00
Jordan Rose 710f6b1259 [analyzer] Be more careful when downcasting for devirtualization.
Virtual base regions are never layered, so simply stripping them off won't
necessarily get you to the correct casted class. Instead, what we want is
the same logic for evaluating dynamic_cast: strip off base regions if possible,
but add new base regions if necessary.

llvm-svn: 161808
2012-08-13 23:46:01 +00:00
Jordan Rose 574ef152fc [analyzer] Handle dynamic_casts that turn out to be upcasts.
This can occur with multiple inheritance, which jumps from one parent to
the other, and with virtual inheritance, since virtual base regions always
wrap the actual object and can't be nested within other base regions.

This also exposed some incorrect logic for multiple inheritance: even if B
is known not to derive from C, D might still derive from both of them.

llvm-svn: 161798
2012-08-13 22:11:42 +00:00
Jordan Rose 07a7ed80cb [analyzer] Don't strip CXXBaseObjectRegions when checking dynamic_casts.
...and /do/ strip CXXBaseObjectRegions when casting to a virtual base class.

This allows us to enforce the invariant that a CXXBaseObjectRegion can always
provide an offset for its base region if its base region has a known class
type, by only allowing virtual bases and direct non-virtual bases to form
CXXBaseObjectRegions.

This does mean some slight problems for our modeling of dynamic_cast, which
needs to be resolved by finding a path from the current region to the class
we're trying to cast to.

llvm-svn: 161797
2012-08-13 22:11:34 +00:00
Jordan Rose 02e5309b35 [analyzer] Strip CXXBaseObjectRegions when devirtualizing method calls.
This was causing a crash when we tried to re-apply a base object region to
itself. It probably also caused incorrect offset calculations in RegionStore.

PR13569 / <rdar://problem/12076683>

llvm-svn: 161710
2012-08-10 22:26:46 +00:00
Jordan Rose 51bcb226a2 [analyzer] Try to devirtualize even if the static callee has no definition.
This mostly affects pure virtual methods, but would also affect parent
methods defined inline in the header when analyzing the child's source file.

llvm-svn: 161709
2012-08-10 22:26:43 +00:00
Anna Zaks 75f49a9c07 [analyzer] Track if a region can be a subclass in the dynamic type info.
When object is allocated with alloc or init, we assume it cannot be a
subclass (currently used only for bifurcation purposes).

llvm-svn: 161682
2012-08-10 18:55:58 +00:00
Anna Zaks 920af014c1 [analyzer] Optimize dynamic dispatch bifurcation by detecting the cases
when we don't need to split.

In some cases we know that a method cannot have a different
implementation in a subclass:
 - the class is declared in the main file (private)
 - all the method declarations (including the ones coming from super
classes) are in the main file.

This can be improved further, but might be enough for the heuristic.
(When we are too aggressive splitting the state, efficiency suffers.
When we fail to split the state coverage might suffer.)

llvm-svn: 161681
2012-08-10 18:55:53 +00:00
Benjamin Kramer 3a913ed805 Fix a couple of pedantic gcc warnings.
llvm-svn: 161656
2012-08-10 10:06:13 +00:00