Commit Graph

1001 Commits

Author SHA1 Message Date
Jordan Rose 2d98b97e10 [analyzer] Make sure calls in synthesized functions have valid path locations.
We do this by using the "most recent" good location: if a synthesized
function 'A' calls another function 'B', the path notes for the call to 'B'
will be placed at the same location as the path note for calling 'A'.

Similarly, the call to 'A' will have a note saying "Entered call from...",
and now we just don't emit that (since the user doesn't have a body to look
at anyway).

Previously, we were doing this for the "Calling..." notes, but not for the
"Entered call from..." or "Returning to caller". This caused a crash when
the path entered and then exiting a call within a synthesized body.

<rdar://problem/12657843>

llvm-svn: 168019
2012-11-15 02:07:23 +00:00
Anna Zaks abdc72d970 [analyzer] Address Jordan's feedback for r167780.
llvm-svn: 167790
2012-11-13 00:13:44 +00:00
Anna Zaks 6ec9c3cbc1 [analyzer] Follow up to r167762 - precisely determine the adjustment
conditions.

The adjustment is needed only in case of dynamic dispatch performed by
the analyzer - when the runtime declaration is different from the static
one.

Document this explicitly in the code (by adding a helper). Also, use
canonical Decls to avoid matching against the case where the definition
is different from found declaration.

This fix suppresses the testcase I added in r167762, so add another
testcase to make sure we do test commit r167762.

llvm-svn: 167780
2012-11-12 23:40:29 +00:00
Anna Zaks 4e255b62f1 [analyzer] Fix a regression (from r 165079): compare canonical types.
Suppresses a leak false positive (radar://12663777).

In addition, we'll need to rewrite the adjustReturnValue() method not to
return UnknownVal by default, but rather assert in cases we cannot
handle. To make it possible, we need to correctly handle some of the
edge cases we already know about.

llvm-svn: 167762
2012-11-12 22:06:24 +00:00
Jordan Rose 9eb409ace9 [analyzer] When invalidating symbolic offset regions, take fields into account.
Previously, RegionStore was being VERY conservative in saying that because
p[i].x and p[i].y have a concrete base region of 'p', they might overlap.
Now, we check the chain of fields back up to the base object and check if
they match.

This only kicks in when dealing with symbolic offset regions because
RegionStore's "base+offset" representation of concrete offset regions loses
all information about fields. In cases where all offsets are concrete
(s.x and s.y), RegionStore will already do the right thing, but mixing
concrete and symbolic offsets can cause bindings to be invalidated that
are known to not overlap (e.g. p[0].x and p[i].y).
This additional refinement is tracked by <rdar://problem/12676180>.

<rdar://problem/12530149>

llvm-svn: 167654
2012-11-10 01:40:08 +00:00
Jordan Rose 520a30fd05 [analyzer] Move convenience REGISTER_*_WITH_PROGRAMSTATE to CheckerContext.h
As Anna pointed out, ProgramStateTrait.h is a relatively obscure header,
and checker writers may not know to look there to add their own custom
state.

The base macro that specializes the template remains in ProgramStateTrait.h
(REGISTER_TRAIT_WITH_PROGRAMSTATE), which allows the analyzer core to keep
using it.

llvm-svn: 167385
2012-11-05 16:58:00 +00:00
NAKAMURA Takumi ba15a7974a StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp: Appease msvc.
0 (as nullptr) is incompatible to pointer in type matching on msvc.

llvm-svn: 167355
2012-11-03 13:59:36 +00:00
Anna Zaks 8d1f6ed9a8 [analyzer] Run remove dead on end of path.
This will simplify checkers that need to register for leaks. Currently,
they have to register for both: check dead and check end of path.

I've modified the SymbolReaper to consider everything on the stack dead
if the input StackLocationContext is 0.

(This is a bit disruptive, so I'd like to flash out all the issues
asap.)

llvm-svn: 167352
2012-11-03 02:54:20 +00:00
Anna Zaks 2510608e81 [analyzer] Refactor: Remove Pred from NodeBuilderContext.
Node builders should manage the nodes, not the context.

llvm-svn: 167350
2012-11-03 02:54:11 +00:00
Jordan Rose 829c383114 [analyzer] Add some convenience accessors to CallEvent, and use them.
These are CallEvent-equivalents of helpers already accessible in
CheckerContext, as part of making it easier for new checkers to be written
using CallEvent rather than raw CallExprs.

llvm-svn: 167338
2012-11-02 23:49:29 +00:00
Jordan Rose 0da6747901 [analyzer] isCLibraryFunction: check that the function is at TU-scope.
Also, Decls already carry a pointer to the ASTContext, so there's no need
to pass an extra argument to the predicate.

llvm-svn: 167337
2012-11-02 23:49:24 +00:00
Jordan Rose 0c153cb277 [analyzer] Use nice macros for the common ProgramStateTraits (map, set, list).
Also, move the REGISTER_*_WITH_PROGRAMSTATE macros to ProgramStateTrait.h.

This doesn't get rid of /all/ explicit uses of ProgramStatePartialTrait,
but it does get a lot of them.

llvm-svn: 167276
2012-11-02 01:54:06 +00:00
Jordan Rose e10d5a7659 [analyzer] Rename 'EmitReport' to 'emitReport'.
No functionality change.

llvm-svn: 167275
2012-11-02 01:53:40 +00:00
Jordan Rose 417591fba7 [analyzer] Let ConstraintManager subclasses provide a more efficient checkNull.
Previously, every call to a ConstraintManager's isNull would do a full
assumeDual to test feasibility. Now, ConstraintManagers can override
checkNull if they have a cheaper way to do the same thing.
RangeConstraintManager can do this in less than half the work.

<rdar://problem/12608209>

llvm-svn: 167138
2012-10-31 16:44:55 +00:00
Anna Zaks 7bd0674dea [analyzer]Don't invalidate const arguments when there is no
IdentifierInfo.

Ee: C++ copy constructors.
llvm-svn: 167092
2012-10-31 01:18:26 +00:00
Jordan Rose ec44ac6a59 [analyzer] New option to not suppress null return paths if an argument is null.
Our one basic suppression heuristic is to assume that functions do not
usually return NULL. However, when one of the arguments is NULL it is
suddenly much more likely that NULL is a valid return value. In this case,
we don't suppress the report here, but we do attach /another/ visitor to
go find out if this NULL argument also comes from an inlined function's
error path.

This new behavior, controlled by the 'avoid-suppressing-null-argument-paths'
analyzer-config option, is turned off by default. Turning it on produced
two false positives and no new true positives when running over LLVM/Clang.

This is one of the possible refinements to our suppression heuristics.
<rdar://problem/12350829>

llvm-svn: 166941
2012-10-29 17:31:59 +00:00
Jordan Rose 199fdd825f [analyzer] Use the CallEnter node to get a value for tracked null arguments.
Additionally, don't collect PostStore nodes -- they are often used in
path diagnostics.

Previously, we tried to track null arguments in the same way as any other
null values, but in many cases the necessary nodes had already been
collected (a memory optimization in ExplodedGraph). Now, we fall back to
using the value of the argument at the time of the call, which may not
always match the actual contents of the region, but often will.

This is a precursor to improving our suppression heuristic.
<rdar://problem/12350829>

llvm-svn: 166940
2012-10-29 17:31:53 +00:00
Ted Kremenek 808102685b Add comments for RemoveRedundantMsgs, rename it to removeRedundantMsgs() per Jordan's feedback.
llvm-svn: 166778
2012-10-26 16:02:36 +00:00
Ted Kremenek a5958869f6 TrackConstraintBRVisitor and ConditionBRVisitor can emit similar
path notes for cases where a value may be assumed to be null, etc.
Instead of having redundant diagnostics, do a pass over the generated
PathDiagnostic pieces and remove notes from TrackConstraintBRVisitor
that are already covered by ConditionBRVisitor, whose notes tend
to be better.

Fixes <rdar://problem/12252783>

llvm-svn: 166728
2012-10-25 22:07:10 +00:00
Jordan Rose 1bbd143945 [analyzer] Handle 'SomeVar.SomeEnumConstant', which is legal in C++.
This caused assertion failures analyzing LLVM.

<rdar://problem/12560282>

llvm-svn: 166529
2012-10-23 23:59:08 +00:00
Jordan Rose 746c06d0bc [analyzer] Replace -analyzer-no-eagerly-trim-egraph with graph-trim-interval.
After every 1000 CFGElements processed, the ExplodedGraph trims out nodes
that satisfy a number of criteria for being "boring" (single predecessor,
single successor, and more). Rather than controlling this with a cc1 option,
which can only disable this behavior, we now have an analyzer-config option,
'graph-trim-interval', which can change this interval from 1000 to something
else. Setting the value to 0 disables reclamation.

The next commit relies on this behavior to actually test anything.

llvm-svn: 166528
2012-10-23 23:59:05 +00:00
Jordan Rose 3957fd5858 [analyzer] Assume 'new' never returns NULL if it could throw an exception.
This is actually required by the C++ standard in
[basic.stc.dynamic.allocation]p3:

  If an allocation function declared with a non-throwing
  exception-specification fails to allocate storage, it shall return a
  null pointer. Any other allocation function that fails to allocate
  storage shall indicate failure only by throwing an exception of a type
  that would match a handler of type std::bad_alloc.

We don't bother checking for the specific exception type, but just go off
the operator new prototype. This should help with a certain class of lazy
initalization false positives.

<rdar://problem/12115221>

llvm-svn: 166363
2012-10-20 02:32:51 +00:00
Jordan Rose 8e785e214b [analyzer] When binding to a ParenExpr, bind to its inner expression instead.
This actually looks through several kinds of expression, such as
OpaqueValueExpr and ExprWithCleanups. The idea is that binding and lookup
should be consistent, and so if the environment needs to be modified later,
the code doing the modification will not have to manually look through these
"transparent" expressions to find the real binding to change.

This is necessary for proper updating of struct rvalues as described in
the previous commit.

llvm-svn: 166121
2012-10-17 19:35:44 +00:00
Jordan Rose 29fc261cd7 [analyzer] Create a temporary region when accessing a struct rvalue.
In C++, rvalues that need to have their address taken (for example, to be
passed to a function by const reference) will be wrapped in a
MaterializeTemporaryExpr, which lets CodeGen know to create a temporary
region to store this value. However, MaterializeTemporaryExprs are /not/
created when a method is called on an rvalue struct, even though the 'this'
pointer needs a valid value. CodeGen works around this by creating a
temporary region anyway; now, so does the analyzer.

The analyzer also does this when accessing a field of a struct rvalue.
This is a little unfortunate, since the rest of the struct will soon be
thrown away, but it does make things consistent with the rest of the
analyzer.

This allows us to bring back the assumption that all known 'this' values
are Locs. This is a revised version of r164828-9, reverted in r164876-7.

<rdar://problem/12137950>

llvm-svn: 166120
2012-10-17 19:35:37 +00:00
Anna Zaks f2546f6726 [analyzer] Embed the analyzer version into the plist output.
llvm-svn: 165994
2012-10-15 22:48:19 +00:00
Jordan Rose 690c063b73 [analyzer] Remove the "direct bindings only" Environment lookup.
This was only used by OSAtomicChecker and makes it more
difficult to update values for expressions that the environment
may look through instead (it's not the same as IgnoreParens).
With this gone, we can have bindExpr bind to the inner
expression that getSVal will find.

Groundwork for <rdar://problem/12137950>

llvm-svn: 165866
2012-10-13 05:05:20 +00:00
Jordan Rose 88b690dd2e [analyzer] Remove unneeded 'inlineCall' checker callback.
I believe the removed assert in CheckerManager says it best:

	InlineCall is a special hacky callback to allow intrusive
	evaluation of the call (which simulates inlining). It is
	currently only used by OSAtomicChecker and should go away
	at some point.

OSAtomicChecker has gone away; inlineCall can now go away as well!

llvm-svn: 165865
2012-10-13 05:05:13 +00:00
Jordan Rose e15fb77df8 Reapply "[analyzer] Treat fields of unions as having symbolic offsets."
This time, actually uncomment the code that's supposed to fix the problem.

This reverts r165671 / 8ceb837585ed973dc36fba8dfc57ef60fc8f2735.

llvm-svn: 165676
2012-10-10 23:23:21 +00:00
Eric Christopher a529f8c9c2 Temporarily Revert "[analyzer] Treat fields of unions as having symbolic offsets."
Author: Jordan Rose <jordan_rose@apple.com>
Date:   Wed Oct 10 21:31:21 2012 +0000

    [analyzer] Treat fields of unions as having symbolic offsets.

    This allows only one field to be active at a time in RegionStore.
    This isn't quite the correct behavior for unions, but it at least
    would handle the case of "value goes in, value comes out" from the
    same field.

    RegionStore currently has a number of places where any access to a union
    results in UnknownVal being returned. However, it is clearly missing
    some cases, or the original issue wouldn't have occurred. It is probably
    now safe to remove those changes, but that's a potentially destabilizing
    change that should wait for more thorough testing.

    Fixes PR14054.

    git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@165660 91177308-0d34-0410-b5e6-96231b3b80d8

This reverts commit cf9030e480f77ab349672f00ad302e216c26c92c.

llvm-svn: 165671
2012-10-10 22:49:05 +00:00
Jordan Rose fb29410c85 [analyzer] Treat fields of unions as having symbolic offsets.
This allows only one field to be active at a time in RegionStore.
This isn't quite the correct behavior for unions, but it at least
would handle the case of "value goes in, value comes out" from the
same field.

RegionStore currently has a number of places where any access to a union
results in UnknownVal being returned. However, it is clearly missing
some cases, or the original issue wouldn't have occurred. It is probably
now safe to remove those changes, but that's a potentially destabilizing
change that should wait for more thorough testing.

Fixes PR14054.

llvm-svn: 165660
2012-10-10 21:31:21 +00:00
Jordan Rose c8a78a37bb [analyzer] Handle implicit statements used for end-of-path nodes' source locs.
Some implicit statements, such as the implicit 'self' inserted for "free"
Objective-C ivar access, have invalid source locations. If one of these
statements is the location where an issue is reported, we'll now look at
the enclosing statements for a valid source location.

<rdar://problem/12446776>

llvm-svn: 165354
2012-10-06 01:19:30 +00:00
Jordan Rose 1dd2afd876 [analyzer] Adjust the return type of an inlined devirtualized method call.
In C++, overriding virtual methods are allowed to specify a covariant
return type -- that is, if the return type of the base method is an
object pointer type (or reference type), the overriding method's return
type can be a pointer to a subclass of the original type. The analyzer
was failing to take this into account when devirtualizing a method call,
and anything that relied on the return value having the proper type later
would crash.

In Objective-C, overriding methods are allowed to specify ANY return type,
meaning we can NEVER be sure that devirtualizing will give us a "safe"
return value. Of course, a program that does this will most likely crash
at runtime, but the analyzer at least shouldn't crash.

The solution is to check and see if the function/method being inlined is
the function that static binding would have picked. If not, check that
the return value has the same type. If the types don't match, see if we
can fix it with a derived-to-base cast (the C++ case). If we can't,
return UnknownVal to avoid crashing later.

<rdar://problem/12409977>

llvm-svn: 165079
2012-10-03 01:08:35 +00:00
Jordan Rose 9aa9980217 [analyzer] Push evalDynamicCast and evalDerivedToBase up to Store.
These functions are store-agnostic, and would benefit from information in
DynamicTypeInfo but gain nothing from the store type.

No intended functionality change.

llvm-svn: 165078
2012-10-03 01:08:32 +00:00
Jordan Rose 7bb2611400 Teach getCXXRecordDeclForPointerType about references.
Then, rename it getPointeeCXXRecordDecl and give it a nice doc comment,
and actually use it.

No intended functionality change.

llvm-svn: 165077
2012-10-03 01:08:28 +00:00
Ted Kremenek f1245ddc78 Silence -Wunused-value warning.
llvm-svn: 165059
2012-10-02 21:50:18 +00:00
Ted Kremenek 4924a0161b Refactor clients of AnalyzerOptions::getBooleanOption() to have
an intermediate helper method to query and populate the Optional value.

llvm-svn: 165043
2012-10-02 20:42:16 +00:00
Ted Kremenek 3c6932922e Tweak AnalyzerOptions::getOptionAsInteger() to populate the string
table, making it printable with the ConfigDump checker.  Along the
way, fix a really serious bug where the value was getting parsed
from the string in code that was in an assert() call.  This means
in a Release-Asserts build this code wouldn't work as expected.

llvm-svn: 165041
2012-10-02 20:31:56 +00:00
Ted Kremenek 5faa5e04a3 Change AnalyzerOptions::mayInlineCXXMemberFunction to default populate
the config string table.  Also setup a test for dumping the analyzer
configuration for C++.

llvm-svn: 165040
2012-10-02 20:31:52 +00:00
Jordan Rose 92375adafb [analyzer] Allow ObjC ivar lvalues where the base is nil.
By analogy with C structs, this seems to be legal, if probably discouraged.
It's only if the ivar is read from or written to that there's a problem.
Running a program that gets the "address" of an instance variable does in
fact return the offset when the base "object" is nil.

This isn't a full revert because r164442 includes some diagnostic tweaks
as well; those have been kept.

This partially reverts r164442 / 08965091770c9b276c238bac2f716eaa4da2dca4.

llvm-svn: 164960
2012-10-01 19:07:22 +00:00
Jordan Rose 12024f8776 Revert "[analyzer] Check that a member expr is valid even when the result is an lvalue."
The original intent of this commit was to catch potential null dereferences
early, but it breaks the common "home-grown offsetof" idiom (PR13927):

 (((struct Foo *)0)->member - ((struct foo *)0))

As it turns out, this appears to be legal in C, per a footnote in
C11 6.5.3.2: "Thus, &*E is equivalent to E (even if E is a null pointer)".
In C++ this issue is still open:
  http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#232

We'll just have to make sure we have good path notes in the future.

This reverts r164441 / 9be016dcd1ca3986873a7b66bd4bc027309ceb59.

llvm-svn: 164958
2012-10-01 19:07:15 +00:00
Ted Kremenek 4a5b35eeec Have AnalyzerOptions::getBooleanOption() stick the matching config
string in the config table so that it can be dumped as part of the 
config dumper.  Add a test to show that these options are sticking
and can be cross-checked using FileCheck.

llvm-svn: 164954
2012-10-01 18:28:19 +00:00
Jordan Rose 88dd13fdca Reapply "[analyzer] Handle inlined constructors for rvalue temporaries correctly."
This is related to but not blocked by <rdar://problem/12137950>
("Return-by-value structs do not have associated regions")

This reverts r164875 / 3278d41e17749dbedb204a81ef373499f10251d7.

llvm-svn: 164952
2012-10-01 17:51:35 +00:00
Jordan Rose d63f04d8a7 [analyzer] Make ProgramStateManager's SubEngine parameter optional.
It is possible and valid to have a state manager and associated objects
without having a SubEngine or checkers.

Patch by Olaf Krzikalla!

llvm-svn: 164947
2012-10-01 16:53:40 +00:00
Jordan Rose d60b9168fa Revert "[analyzer] Create a temporary region for rvalue structs when accessing fields"
This reverts commit 6f61df3e7256413dcb99afb9673f4206e3c4992c.

llvm-svn: 164877
2012-09-29 01:36:51 +00:00
Jordan Rose d9b0268401 Revert "[analyzer] Create a temp region when a method is called on a struct rvalue."
This reverts commit 0006ba445962621ed82ec84400a6b978205a3fbc.

llvm-svn: 164876
2012-09-29 01:36:47 +00:00
Jordan Rose cd9000e840 Revert "[analyzer] Handle inlined constructors for rvalue temporaries correctly."
This reverts commit 580cd17f256259f39a382e967173f34d68e73859.

llvm-svn: 164875
2012-09-29 01:36:42 +00:00
Jordan Rose 19ed6748ea [analyzer] Handle inlined constructors for rvalue temporaries correctly.
Previously the analyzer treated all inlined constructors like lvalues,
setting the value of the CXXConstructExpr to the newly-constructed
region. However, some CXXConstructExprs behave like rvalues -- in
particular, the implicit copy constructor into a pass-by-value argument.
In this case, we want only the /contents/ of a temporary object to be
passed, so that we can use the same "copy each argument into the
parameter region" algorithm that we use for scalar arguments.

This may change when we start modeling destructors of temporaries,
but for now this is the last part of <rdar://problem/12137950>.

llvm-svn: 164830
2012-09-28 17:15:25 +00:00
Jordan Rose b559f18584 [analyzer] Create a temp region when a method is called on a struct rvalue.
An rvalue has no address, but calling a C++ member function requires a
'this' pointer. This commit makes the analyzer create a temporary region
in which to store the struct rvalue and use as a 'this' pointer whenever
a member function is called on an rvalue, which is essentially what
CodeGen does.

More of <rdar://problem/12137950>. The last part is tracking down the
C++ FIXME in array-struct-region.cpp.

llvm-svn: 164829
2012-09-28 17:15:21 +00:00
Jordan Rose e7126582a4 [analyzer] Create a temporary region for rvalue structs when accessing fields
Struct rvalues are represented in the analyzer by CompoundVals,
LazyCompoundVals, or plain ConjuredSymbols -- none of which have associated
regions. If the entire structure is going to persist, this is not a
problem -- either the rvalue will be assigned to an existing region, or
a MaterializeTemporaryExpr will be present to create a temporary region.
However, if we just need a field from the struct, we need to create the
temporary region ourselves.

This is inspired by the way CodeGen handles calls to temporaries;
support for that in the analyzer is coming next.

Part of <rdar://problem/12137950>

llvm-svn: 164828
2012-09-28 17:15:12 +00:00
Ted Kremenek 8971b028e7 Revert "Use sep instead of ' '."
This isn't correct, as Jordan correctly points out.

llvm-svn: 164711
2012-09-26 18:06:08 +00:00
Ted Kremenek 2de0a9919b Use sep instead of ' '.
llvm-svn: 164709
2012-09-26 17:23:31 +00:00
Ted Kremenek a808e165b2 Remove unnecessary ASTContext& parameter from SymExpr::getType().
llvm-svn: 164661
2012-09-26 06:00:14 +00:00
Jordan Rose db72e2fc37 Reapply "[analyzer] Remove constraints on dead symbols as part of removeDeadBindings."
Previously, we'd just keep constraints around forever, which means we'd
never be able to merge paths that differed only in constraints on dead
symbols.

Because we now allow constraints on symbolic expressions, not just single
symbols, this requires changing SymExpr::symbol_iterator to include
intermediate symbol nodes in its traversal, not just the SymbolData leaf
nodes.

This depends on the previous commit to be correct. Originally applied in
r163444, reverted in r164275, now being re-applied.

llvm-svn: 164622
2012-09-25 19:03:06 +00:00
Jordan Rose 60d704ab4a [analyzer] Calculate liveness for symbolic exprs as well as atomic symbols.
No tests, but this allows the optimization of removing dead constraints.
We can then add tests that we don't do this prematurely.

<rdar://problem/12333297>

Note: the added FIXME to investigate SymbolRegionValue liveness is
tracked by <rdar://problem/12368183>. This patch does not change the
existing behavior.

llvm-svn: 164621
2012-09-25 19:03:01 +00:00
Anna Zaks 3533a54a97 [analyzer]Prevent infinite recursion(assume->checker:evalAssume->assume)
(Unfortunately, I do not have a good reduced test case for this.)

llvm-svn: 164541
2012-09-24 17:43:41 +00:00
Jordan Rose 52de8eec01 [analyzer] Suppress bugs whose paths go through the return of a null pointer.
This is a heuristic intended to greatly reduce the number of false
positives resulting from inlining, particularly inlining of generic,
defensive C++ methods that live in header files. The suppression is
triggered in the cases where we ask to track where a null pointer came
from, and it turns out that the source of the null pointer was an inlined
function call.

This change brings the number of bug reports in LLVM from ~1500 down to
around ~300, a much more manageable number. Yes, some true positives may
be hidden as well, but from what I looked at the vast majority of silenced
reports are false positives, and many of the true issues found by the
analyzer are still reported.

I'm hoping to improve this heuristic further by adding some exceptions
next week (cases in which a bug should still be reported).

llvm-svn: 164449
2012-09-22 01:25:06 +00:00
Jordan Rose 4ac7cba404 [analyzer] Track a null value back through FindLastStoreBRVisitor.
Also, tidy up the other tracking visitors so that they mark the right
things as interesting and don't do extra work.

llvm-svn: 164448
2012-09-22 01:25:00 +00:00
Jordan Rose fa92f0f298 [analyzer] Always allow BugReporterVisitors to see the bug path.
Before, PathDiagnosticConsumers that did not support actual path output
would (sensibly) cause the generation of the full path to be skipped.
However, BugReporterVisitors may want to see the path in order to mark a
BugReport as invalid.

Now, even for a path generation scheme of 'None' we will still create a
trimmed graph and walk backwards through the bug path, doing no work other
than passing the nodes to the BugReporterVisitors. This isn't cheap, but
it's necessary to properly do suppression when the first path consumer does
not support path notes.

In the future, we should try only generating the path and visitor-provided
path notes once, or at least only creating the trimmed graph once.

llvm-svn: 164447
2012-09-22 01:24:56 +00:00
Jordan Rose 5a751b993f [analyzer] Allow a BugReport to be marked "invalid" during path generation.
This is intended to allow visitors to make decisions about whether a
BugReport is likely a false positive. Currently there are no visitors
making use of this feature, so there are no tests.

When a BugReport is marked invalid, the invalidator must provide a key
that identifies the invaliation (intended to be the visitor type and a
context pointer of some kind). This allows us to reverse the decision
later on. Being able to reverse a decision about invalidation gives us more
flexibility, and allows us to formulate conditions like "this report is
invalid UNLESS the original argument is 'foo'". We can use this to
fine-tune our false-positive suppression (coming soon).

llvm-svn: 164446
2012-09-22 01:24:53 +00:00
Jordan Rose 6f3d2f0acd [analyzer] Look through OpaqueValueExprs when tracking a nil value.
This allows us to show /why/ a particular object is nil, even when it is
wrapped in an OpaqueValueExpr.

llvm-svn: 164445
2012-09-22 01:24:49 +00:00
Jordan Rose 106b037a85 [analyzer] Better path notes for null pointers passed as arguments.
Rather than saying "Null pointer value stored to 'foo'", we now say
"Passing null pointer value via Nth parameter 'foo'", which is much better.
The note is also now on the argument expression as well, rather than the
entire call.

This paves the way for continuing to track arguments back to their sources.

<rdar://problem/12211490>

llvm-svn: 164444
2012-09-22 01:24:46 +00:00
Jordan Rose c102b35b44 Use llvm::getOrdinalSuffix to print ordinal numbers in diagnostics.
Just a refactoring of common infrastructure. No intended functionality change.

llvm-svn: 164443
2012-09-22 01:24:42 +00:00
Jordan Rose 1d64a49855 [analyzer] Check that an ObjCIvarRefExpr's base is non-null even as an lvalue.
Like with struct fields, we want to catch cases like this early,
so that we can produce better diagnostics and path notes:

  PointObj *p = nil;
  int *px = &p->_x; // should warn here
  *px = 1;

llvm-svn: 164442
2012-09-22 01:24:38 +00:00
Jordan Rose 04dcb7235f [analyzer] Check that a member expr is valid even when the result is an lvalue.
We want to catch cases like this early, so that we can produce better
diagnostics and path notes:

  Point *p = 0;
  int *px = &p->x; // should warn here
  *px = 1;

llvm-svn: 164441
2012-09-22 01:24:33 +00:00
Ted Kremenek 61e2f2d6ec Re-enable faux-bodies by default.
Try this again, now that r164392 is in place.

llvm-svn: 164393
2012-09-21 17:55:34 +00:00
NAKAMURA Takumi 443eef47ef Revert r164364, "Flip "faux-bodies" in the analyzer on by default to flush out bugs."
It crashed test/Analysis/Output/blocks.m on some hosts.

llvm-svn: 164368
2012-09-21 12:00:42 +00:00
Ted Kremenek e460a4ea2d Flip "faux-bodies" in the analyzer on by default to flush out bugs.
llvm-svn: 164364
2012-09-21 06:14:37 +00:00
Ted Kremenek 089c5510b8 Simplify getRuntimeDefinition() back to taking no arguments.
llvm-svn: 164363
2012-09-21 06:13:13 +00:00
Ted Kremenek 14f779c4d6 Implement faux-body-synthesis of well-known functions in the static analyzer when
their implementations are unavailable.  Start by simulating dispatch_sync().

This change is largely a bunch of plumbing around something very simple.  We
use AnalysisDeclContext to conjure up a fake function body (using the
current ASTContext) when one does not exist.  This is controlled
under the analyzer-config option "faux-bodies", which is off by default.

The plumbing in this patch is largely to pass the necessary machinery
around.  CallEvent needs the AnalysisDeclContextManager to get
the function definition, as one may get conjured up lazily.

BugReporter and PathDiagnosticLocation needed to be relaxed to handle
invalid locations, as the conjured body has no real source locations.
We do some primitive recovery in diagnostic generation to generate
some reasonable locations (for arrows and events), but it can be
improved.

llvm-svn: 164339
2012-09-21 00:09:11 +00:00
Jordan Rose ae134c6449 Revert "[analyzer] Remove constraints on dead symbols as part of removeDeadBindings."
While we definitely want this optimization in the future, we're not
currently handling constraints on symbolic /expressions/ correctly.
These should stay live even if the SymExpr itself is no longer referenced
because could recreate an identical SymExpr later. Only once the SymExpr
can no longer be recreated -- i.e. a component symbol is dead -- can we
safely remove the constraints on it.

This liveness issue is tracked by <rdar://problem/12333297>.

This reverts r163444 / 24c7f98828e039005cff3bd847e7ab404a6a09f8.

llvm-svn: 164275
2012-09-20 01:54:56 +00:00
Anna Zaks 4278234360 [analyzer] Teach the analyzer about implicit initialization of statics
in ObjCMethods.

Extend FunctionTextRegion to represent ObjC methods as well as
functions. Note, it is not clear what type ObjCMethod region should
return. Since the type of the FunctionText region is not currently used,
defer solving this issue.

llvm-svn: 164046
2012-09-17 19:13:56 +00:00
Anna Zaks f6a5d793d2 [analyzer] Don't reimplement an existing function.
Thanks Jordan.

llvm-svn: 163762
2012-09-13 00:37:12 +00:00
Ted Kremenek 8b3f938697 Refactor logic in ExprEngine for detecting 'noreturn' methods
in NSException to a helper object in libAnalysis that can also
be used by Sema.  Not sure if the predicate name 'isImplicitNoReturn'
is the best one, but we can massage that later.

No functionality change.

llvm-svn: 163759
2012-09-13 00:21:31 +00:00
Anna Zaks 5d2964e770 [analyzer] Do not report use of undef on "return foo();" when the return type is void.
Fixes a false positive found by analyzing LLVM code base.

llvm-svn: 163750
2012-09-12 22:57:40 +00:00
Anna Zaks e663b80975 [analyzer] Teach UndefOrNullArgVisitor to track parent regions.
llvm-svn: 163748
2012-09-12 22:57:30 +00:00
Jordan Rose 5297748e3f [analyzer] Fix another use of the address of a temporary, like r163402.
Again, GCC is more aggressive about reusing temporary space than we are,
leading to Release build crashes for this undefined behavior.

PR13710 (though it may not be the only problem there)

llvm-svn: 163747
2012-09-12 22:48:08 +00:00
Jordan Rose d44977ef64 [analyzer] Handle when the dynamic type is worse than the static type.
Currently we don't update the dynamic type of a C++ object when it is
cast. This can cause the situation above, where the static type of the
region is now known to be a subclass of the dynamic type.

Once we start updating DynamicTypeInfo in response to the various kinds
of casts in C++, we can re-add this assert to make sure we don't miss
any cases. This work is tracked by <rdar://problem/12287087>.

In -Asserts builds, we will simply not return any runtime definition
when our DynamicTypeInfo is known to be incorrect like this.

llvm-svn: 163745
2012-09-12 21:48:17 +00:00
Jordan Rose 99c6c2b4e2 Revert "[analyzer] Use the static type for a virtual call if the dynamic type is worse."
Using the static type may be inconsistent with later calls. We should just
report that there is no inlining definition available if the static type is
better than the dynamic type. See next commit.

This reverts r163644 / 19d5886d1704e24282c86217b09d5c6d35ba604d.

llvm-svn: 163744
2012-09-12 21:48:13 +00:00
Ted Kremenek ba22a035ad Fix regression where "looping back to the head of" PathDiagnosticEvents
were not emitted.

Fixes <rdar://problem/12280665>.

llvm-svn: 163683
2012-09-12 06:22:18 +00:00
Richard Smith b15fe3a5e4 PR13811: Add a FunctionParmPackExpr node to handle references to function
parameter packs where the reference is not being expanded but the pack has
been. Previously, Clang would segfault in such cases.

llvm-svn: 163672
2012-09-12 00:56:43 +00:00
Jordan Rose a522f1cf8b Revert "[analyzer] Disable STL inlining. Blocked by PR13724."
While PR13724 is still an issue, it's not actually an issue in the STL.
We can keep this option around in case there turn out to be widespread
false positives due to poor modeling of the C++ standard library functions,
but for now we'd like to get more data.

This reverts r163633 / c6baadceec1d5148c20ee6c902a102233c547f62.

llvm-svn: 163647
2012-09-11 20:26:49 +00:00
Jordan Rose e35fdeb330 [analyzer] Use the static type for a virtual call if the dynamic type is worse.
reinterpret_cast does not provide any of the usual type information that
static_cast or dynamic_cast provide -- only the new type. This can get us
in a situation where the dynamic type info for an object is actually a
superclass of the static type, which does not match what CodeGen does at all.
In these cases, just fall back to the static type as the best possible type
for devirtualization.

Should fix the crashes on our internal buildbot.

llvm-svn: 163644
2012-09-11 18:47:13 +00:00
Anna Zaks 464493fbf4 [analyzer] Disable STL inlining. Blocked by PR13724.
llvm-svn: 163633
2012-09-11 17:15:39 +00:00
Jordan Rose 12f669e3cd [analyzer] Member function calls that use qualified names are non-virtual.
C++11 [expr.call]p1: ...If the selected function is non-virtual, or if the
  id-expression in the class member access expression is a qualified-id,
  that function is called. Otherwise, its final overrider in the dynamic type
  of the object expression is called.

<rdar://problem/12255556>

llvm-svn: 163577
2012-09-11 00:31:02 +00:00
Anna Zaks 1ded453e36 [analyzer] Turn stl inlining back on.
The one reported bug, which was exposed by stl inlining, is addressed in
r163558.

llvm-svn: 163574
2012-09-10 23:59:02 +00:00
Anna Zaks 4f9c460874 [analyzer] Do not count calls to small functions when computing stack
depth.

We only want to count how many substantial functions we inlined. This
is an improvement to r163558.

llvm-svn: 163571
2012-09-10 23:35:11 +00:00
Anna Zaks 5446f4dfb1 [analyzer] Add an option to enable/disable objc inlining.
llvm-svn: 163562
2012-09-10 22:56:41 +00:00
Anna Zaks 14ce52492f [analyzer] Add ipa-always-inline-size option (with 3 as the default).
The option allows to always inline very small functions, whose size (in
number of basic blocks) is set using -analyzer-config
ipa-always-inline-size option.

llvm-svn: 163558
2012-09-10 22:37:19 +00:00
Jordan Rose c6fcbf06a6 [analyzer] Make the defaults explicit for each of the new config options.
Also, document both new inlining options in IPA.txt.

llvm-svn: 163551
2012-09-10 21:54:24 +00:00
Jordan Rose 1e0e4001c8 [analyzer] For now, don't inline C++ standard library functions.
This is a (heavy-handed) solution to PR13724 -- until we know we can do
a good job inlining the STL, it's best to be consistent and not generate
more false positives than we did before. We can selectively whitelist
certain parts of the 'std' namespace that are known to be safe.

This is controlled by analyzer config option 'c++-stdlib-inlining', which
can be set to "true" or "false".

This commit also adds control for whether or not to inline any templated
functions (member or non-member), under the config option
'c++-template-inlining'. This option is currently on by default.

llvm-svn: 163548
2012-09-10 21:27:35 +00:00
Ted Kremenek a0fa5d6564 Fix another case where we should be using isBeforeInTranslationUnit().
llvm-svn: 163533
2012-09-10 19:07:56 +00:00
Ted Kremenek 54fd079265 Add a few more cases where we should be using isBeforeInTranslationUnit().
llvm-svn: 163531
2012-09-10 19:02:33 +00:00
Ted Kremenek 6c7a5eae6d Revert "Revert Ted's r163489 and r163490, due to breakage."
I need to see how this breaks on other platforms when I fix the issue
that Benjamin Kramer pointed out.

This includes r163489 and r163490, plus a two line change.

llvm-svn: 163512
2012-09-10 14:50:55 +00:00
NAKAMURA Takumi 6eb1399088 Revert Ted's r163489 and r163490, due to breakage.
r163489, "Take another crack at stabilizing the emission order of analyzer"
r163490, "Use isBeforeInTranslationUnitThan() instead of operator<."

llvm-svn: 163497
2012-09-10 09:17:27 +00:00
Ted Kremenek f1fc8ce65d Use isBeforeInTranslationUnitThan() instead of operator<.
llvm-svn: 163490
2012-09-10 06:56:07 +00:00
Ted Kremenek 3d92699d3c Take another crack at stabilizing the emission order of analyzer
diagnostics without using FoldingSetNodeIDs.  This is done
by doing a complete recursive comparison of the PathDiagnostics.

Note that the previous method of comparing FoldingSetNodeIDs did
not end up relying on unstable things such as pointer addresses, so
I suspect this may still have some issues on various buildbots because
I'm not sure if the true source of non-determinism has been eliminated.
The tests pass for me, so the only way to know is to commit this change
and see what happens.

llvm-svn: 163489
2012-09-10 06:20:06 +00:00
Ted Kremenek 9b9ee2a616 Indent the "message" key in analyzer plist output.
llvm-svn: 163487
2012-09-10 06:19:43 +00:00
Ted Kremenek e9764d8f91 Remove dead method ProgramState::MarshalState().
llvm-svn: 163479
2012-09-09 14:55:59 +00:00
Ted Kremenek e7ec4ef48d Fix bug in BugReporter::RemoveUneededCalls() where "prunable"
PathDiagnosticEventPieces were *always* pruned.  Instead, they
are suppose to only be pruned if the entire call gets pruned.

llvm-svn: 163460
2012-09-08 07:18:18 +00:00
Ted Kremenek b0d1c70258 Attempt (again) to stabilize the order of the emission of diagnostics
of the analyzer by using the FullProfile() of a PathDiagnostic
for ordering them.

llvm-svn: 163455
2012-09-08 04:26:37 +00:00
Jordan Rose 5481cfefa6 [analyzer] ObjCSelfInitChecker should always clean up in postCall checks.
ObjCSelfInitChecker stashes information in the GDM to persist it across
function calls; it is stored in pre-call checks and retrieved post-call.
The post-call check is supposed to clear out the stored state, but was
failing to do so in cases where the call did not have a symbolic return
value.

This was actually causing the inappropriate cache-out from r163361.
Per discussion with Anna, we should never actually cache out when
assuming the receiver of an Objective-C message is non-nil, because
we guarded that node generation by checking that the state has changed.
Therefore, the only states that could reach this exact ExplodedNode are
ones that should have merged /before/ making this assumption.

r163361 has been reverted and the test case removed, since it won't
actually test anything interesting now.

llvm-svn: 163449
2012-09-08 01:47:28 +00:00
Ted Kremenek 1fdcfcdf13 Revert "Attempt to make the PathDiagnostic emission order more deterministic by"
llvm-svn: 163446
2012-09-08 01:25:00 +00:00
Ted Kremenek af4cc7eab1 Revert "Further tweaks to hopefully make the PathDiagnostic emission more deterministic."
llvm-svn: 163445
2012-09-08 01:24:53 +00:00
Jordan Rose 5860e329a4 [analyzer] Remove constraints on dead symbols as part of removeDeadBindings.
Previously, we'd just keep constraints around forever, which means we'd
never be able to merge paths that differed only in constraints on dead
symbols.

Because we now allow constraints on symbolic expressions, not just single
symbols, this requires changing SymExpr::symbol_iterator to include
intermediate symbol nodes in its traversal, not just the SymbolData leaf
nodes.

llvm-svn: 163444
2012-09-08 01:24:53 +00:00
Jordan Rose dd5e8c4975 [analyzer] Symbolic regions are live if any subregions are live.
RegionStoreManager was only treating a SymbolicRegion's symbel as live
if there was a binding referring to the region itself.

No test case because constraints are currently not being cleaned out
of the constraint manager at all (even if the symbol is legitimately dead).

llvm-svn: 163443
2012-09-08 01:24:49 +00:00
Jordan Rose aaf8318480 [analyzer] Cast the result of a placement new-expression to the correct type.
This is necessary because further analysis will assume that the SVal's
type matches the AST type. This caused a crash when trying to perform
a derived-to-base cast on a C++ object that had been new'd to be another
object type.

Yet another crash in PR13763.

llvm-svn: 163442
2012-09-08 01:24:38 +00:00
Ted Kremenek a11a741e2f Further tweaks to hopefully make the PathDiagnostic emission more deterministic.
llvm-svn: 163430
2012-09-07 23:13:11 +00:00
Ted Kremenek 244e1d7d0f Remove ProgramState::getSymVal(). It was being misused by Checkers,
with at least one subtle bug in MacOSXKeyChainAPIChecker where the
calling the method was a substitute for assuming a symbolic value
was null (which is not the case).

We still keep ConstraintManager::getSymVal(), but we use that as
an optimization in SValBuilder and ProgramState::getSVal() to
constant-fold SVals.  This is only if the ConstraintManager can
provide us with that information, which is no longer a requirement.
As part of this, introduce a default implementation of
ConstraintManager::getSymVal() which returns null.

For Checkers, introduce ConstraintManager::isNull(), which queries
the state to see if the symbolic value is constrained to be a null
value.  It does this without assuming it has been implicitly constant
folded.

llvm-svn: 163428
2012-09-07 22:31:01 +00:00
Ted Kremenek 334ad6ac13 Attempt to make the PathDiagnostic emission order more deterministic by
looking at PathPieces.

llvm-svn: 163427
2012-09-07 22:24:24 +00:00
Ted Kremenek 58ec11612c Remove ConstraintManager:isEqual(). It is no longer used.
llvm-svn: 163425
2012-09-07 22:24:18 +00:00
Jordan Rose 8dc77398a1 [analyzer] Use cast<> instead of getAs<> for a CFGElement known to be a CFGStmt.
When adding the next statement to the CoreEngine's work list, we take care
of all the special cases first. We certainly shouldn't be building
PostStmts with null statements (the diagnostics machinery assumes such
StmtPoints do not exist), and we should find out sooner if we're missing
a special case.

A refinement of r163402 that should help prevent further issues like PR13760.

llvm-svn: 163409
2012-09-07 19:48:09 +00:00
Jordan Rose 3c2713accf [analyzer] Don't use the address of a temporary CFGElement.
GCC destroys temporary objects more aggressively than clang, so this
results in incorrect behavior when compiling GCC Release builds.

We could avoid this issue under C++11 by preventing getAs from being
called when 'this' is an rvalue:

  template<class ElemTy> const ElemTy *getAs() const & { ... }
  template<class ElemTy> const ElemTy *getAs() const && = delete;

Unfortunately, we do not have compatibility macros for this behavior yet.

This will hopefully fix PR13760 and PR13762.

llvm-svn: 163402
2012-09-07 18:36:17 +00:00
Anna Zaks 67e0062b7c [analyzer] Explain why we need condition 8.
llvm-svn: 163394
2012-09-07 16:22:09 +00:00
Ted Kremenek 891bcdb644 ExplodedGraph::shouldCollectNode() should not collect nodes for non-Expr Stmts
(as this previously was the case before this was refactored).  We also shouldn't
need to specially handle BinaryOperators since the eagerly-assume heuristic tags
such nodes.

llvm-svn: 163374
2012-09-07 06:56:18 +00:00
Ted Kremenek 7c15040e98 Fix bug in ConditionBRVisitor where for C++ (and not C) we were not ignoring
implicit pointer-to-boolean conversions in condition expressions.  This would
result in inconsistent diagnostic emission between C and C++.

A consequence of this is now ConditionBRVisitor and TrackConstraintBRVisitor may
emit redundant diagnostics, for example:

  "Assuming pointer value is null" (TrackConstraintBRVisitor)
  "Assuming 'p' is null" (ConditionBRVisitor)

We need to reconcile the two, and perhaps prefer one over the other in some
cases.

llvm-svn: 163372
2012-09-07 06:51:37 +00:00
Jordan Rose 81456d9f6d [analyzer] Fail gracefully when the dynamic type is outside the hierarchy.
With some particularly evil casts, we can get an object whose dynamic type
is not actually a subclass of its static type. In this case, we won't even
find the statically-resolved method as a devirtualization candidate.

Rather than assert that this situation cannot occur, we now simply check
that the dynamic type is not an ancestor or descendent of the static type,
and leave it at that.

This error actually occurred analyzing LLVM: CallEventManager uses a
BumpPtrAllocator to allocate a concrete subclass of CallEvent
(FunctionCall), but then casts it to the actual subclass requested
(such as ObjCMethodCall) to perform the constructor.

Yet another crash in PR13763.

llvm-svn: 163367
2012-09-07 01:19:42 +00:00
Jordan Rose 7e97996f4e [analyzer] Don't crash if we cache out while evaluating an ObjC message.
A bizarre series of coincidences led us to generate a previously-seen
node in the middle of processing an Objective-C message, where we assume
the receiver is non-nil. We were assuming that such an assumption would
never "cache out" like this, and blithely went on using a null ExplodedNode
as the predecessor for the next step in evaluation.

Although the test case committed here is complicated, this could in theory
happen in other ways as well, so the correct fix is just to test if the
non-nil assumption results in an ExplodedNode we've seen before.

<rdar://problem/12243648>

llvm-svn: 163361
2012-09-06 23:44:36 +00:00
Jordan Rose 2bc9674b0a [analyzer] Don't attempt to devirtualize calls to base class destructors.
CXXDestructorCall now has a flag for when it is a base destructor call.
Other kinds of destructor calls (locals, fields, temporaries, and 'delete')
all behave as "whole-object" destructors and do not behave differently
from one another (specifically, in these cases we /should/ try to
devirtualize a call to a virtual destructor).

This was causing crashes in both our internal buildbot, the crash still
being tracked in PR13765, and some of the crashes being tracked in PR13763,
due to a assertion failure. (The behavior under -Asserts happened to be
correct anyway.)

Adding this knowledge also allows our DynamicTypePropagation checker to do
a bit less work; the special rules about virtual method calls during a
destructor only require extra handling during base destructors.

llvm-svn: 163348
2012-09-06 20:37:08 +00:00
Roman Divacky e637711ae0 Dont cast away const needlessly. Found by gcc48 -Wcast-qual.
llvm-svn: 163325
2012-09-06 15:59:27 +00:00
Anna Zaks 3245e584db [analyzer] Enhance the member expr tracking to account for references.
As per Jordan's suggestion. (Came out of code review for r163261.)

llvm-svn: 163269
2012-09-05 23:41:54 +00:00
Jordan Rose 6d671cc34a [analyzer] Always include destructors in the analysis CFG.
While destructors will continue to not be inlined (unless the analyzer
config option 'c++-inlining' is set to 'destructors'), leaving them out
of the CFG is an incomplete model of the behavior of an object, and
can cause false positive warnings (like PR13751, now working).

Destructors for temporaries are still not on by default, since
(a) we haven't actually checked this code to be sure it's fully correct
    (in particular, we probably need to be very careful with regard to
    lifetime-extension when a temporary is bound to a reference,
    C++11 [class.temporary]p5), and
(b) ExprEngine doesn't actually do anything when it sees a temporary
    destructor in the CFG -- not even invalidate the object region.

To enable temporary destructors, set the 'cfg-temporary-dtors' analyzer
config option to '1'. The old -cfg-add-implicit-dtors cc1 option, which
controlled all implicit destructors, has been removed.

llvm-svn: 163264
2012-09-05 22:55:23 +00:00
Anna Zaks e5cb4981d0 [analyzer] Fix a crash PR13762.
llvm-svn: 163262
2012-09-05 22:31:58 +00:00
Anna Zaks b4b2b57ee0 [analyzer] NullOrUndef diagnostics: track symbols binded to regions.
If a region is binded to a symbolic value, we should track the symbol.

(The code I changed was not previously exercised by the regression
tests.)

llvm-svn: 163261
2012-09-05 22:31:55 +00:00
Jordan Rose fcdda36149 [analyzer] Be more forgiving about calling methods on struct rvalues.
The problem is that the value of 'this' in a C++ member function call
should always be a region (or NULL). However, if the object is an rvalue,
it has no associated region (only a conjured symbol or LazyCompoundVal).
For now, we handle this in two ways:

1) Actually respect MaterializeTemporaryExpr. Before, it was relying on
   CXXConstructExpr to create temporary regions for all struct values.
   Now it just does the right thing: if the value is not in a temporary
   region, create one.

2) Have CallEvent recognize the case where its 'this' pointer is a
   non-region, and just return UnknownVal to keep from confusing clients.

The long-term problem is being tracked internally in <rdar://problem/12137950>,
but this makes many test cases pass.

llvm-svn: 163220
2012-09-05 17:11:26 +00:00
Jordan Rose d1a08b6e43 [analyzer] Clean up a couple uses of getPointeeType().
No intended functionality change.

llvm-svn: 163219
2012-09-05 17:11:22 +00:00
Jordan Rose bc009d4493 Revert "[analyzer] Treat all struct values as regions (even rvalues)."
This turned out to have many implications, but what eventually seemed to
make it unworkable was the fact that we can get struct values (as
LazyCompoundVals) from other places besides return-by-value function calls;
that is, we weren't actually able to "treat all struct values as regions"
consistently across the entire analyzer core.

Hopefully we'll be able to come up with an alternate solution soon.

This reverts r163066 / 02df4f0aef142f00d4637cd851e54da2a123ca8e.

llvm-svn: 163218
2012-09-05 17:11:15 +00:00
Jordan Rose 7523d1a847 [analyzer] Don't use makeIntVal to create a floating-point value.
SimpleSValBuilder processes a couple trivial identities, including 'x - x'
and 'x ^ x' (both 0). However, the former could appear with arguments of
floating-point type, and we weren't checking for that. This started
triggering an assert with r163069, which checks that a constant value is
actually going to be used as an integer or pointer.

llvm-svn: 163159
2012-09-04 19:34:58 +00:00
Joao Matos 566359c0bf Revert r163083 per chandlerc's request.
llvm-svn: 163149
2012-09-04 17:49:35 +00:00
Joao Matos c32a7e4d8e Implemented parsing and AST support for the MS __leave exception statement. Also a minor fix to __except printing in StmtPrinter.cpp. Thanks to Aaron Ballman for review.
llvm-svn: 163083
2012-09-02 03:45:41 +00:00
Jordan Rose d229e39a9a [analyzer] Silence unused variable warnings in NDEBUG builds.
No functionality change.

llvm-svn: 163073
2012-09-01 19:15:13 +00:00
Jordan Rose 21580c2f92 [analyzer] Disallow creation of int vals with explicit bit width / signedness.
All clients of BasicValueFactory should be using QualTypes instead, and
indeed it seems they are. This caught the (fortunately harmless) bug
fixed in the previous commit.

No intended functionality change.

llvm-svn: 163069
2012-09-01 17:39:24 +00:00
Jordan Rose a44ad1b35c [analyzer] Don't attempt to create a floating-point value of "1" for ++/--.
The current logic would actually create a float- or double-sized signed
integer value of 1, which is not at all the same.

No test because the value would be swallowed by an Unknown as soon as it
gets added or subtracted to the original value, but it enables the cleanup
in the next patch.

llvm-svn: 163068
2012-09-01 17:39:17 +00:00
Jordan Rose 82ae9898ef [analyzer] Treat all struct values as regions (even rvalues).
This allows us to correctly symbolicate the fields of structs returned by
value, as well as get the proper 'this' value for when methods are called
on structs returned by value.

This does require a moderately ugly hack in the StoreManager: if we assign
a "struct value" to a struct region, that now appears as a Loc value being
bound to a region of struct type. We handle this by simply "dereferencing"
the struct value region, which should create a LazyCompoundVal.

This should fix recent crashes analyzing LLVM and on our internal buildbot.

<rdar://problem/12137950>

llvm-svn: 163066
2012-09-01 17:39:09 +00:00
Jordan Rose 2da564380a [analyzer] Always derive a CallEvent's return type from its origin expr.
Previously, we preferred to get a result type by looking at the callee's
declared result type. This allowed us to handlereferences, which are
represented in the AST as lvalues of their pointee type. (That is, a call
to a function returning 'int &' has type 'int' and value kind 'lvalue'.)

However, this results in us preferring the original type of a function
over a casted type. This is a problem when a function  pointer is casted
to another type, because the conjured result value will have the wrong
type. AdjustedReturnValueChecker is supposed to handle this, but still
doesn't handle the case where there is no "original function" at all,
i.e. where the callee is unknown.

Now, we instead look at the call expression's value kind (lvalue, xvalue,
or prvalue), and adjust the expr's type accordingly. This will have no
effect when the function is inlined, and will conjure the value that will
actually be used when it is not.

This makes AdjustedReturnValueChecker /nearly/ unnecessary; unfortunately,
the cases where it would still be useful are where we need to cast the
result of an inlined function or a checker-evaluated function, and in these
cases we don't know what we're casting /from/ by the time we can do post-
call checks. In light of that, remove AdjustedReturnValueChecker, which
was already not checking quite a few calls.

llvm-svn: 163065
2012-09-01 17:39:00 +00:00
Ted Kremenek cdf814900d Split library clangRewrite into clangRewriteCore and clangRewriteFrontend.
This is similar to how we divide up the StaticAnalyzer libraries to separate
core functionality to what is clearly associated with Frontend actions.

llvm-svn: 163050
2012-09-01 05:09:24 +00:00
Jordan Rose 219c9d0dd3 [analyzer] Though C++ inlining is enabled, don't inline ctors and dtors.
More generally, this adds a new configuration option 'c++-inlining', which
controls which C++ member functions can be considered for inlining. This
uses the new -analyzer-config table, so the cc1 arguments will look like this:

... -analyzer-config c++-inlining=[none|methods|constructors|destructors]

Note that each mode implies that all the previous member function kinds
will be inlined as well; it doesn't make sense to inline destructors
without inlining constructors, for example.

The default mode is 'methods'.

llvm-svn: 163004
2012-08-31 17:06:49 +00:00
Jordan Rose cc0b1bfa56 [analyzer] Ensure that PathDiagnostics profile the same regardless of path.
PathDiagnostics are actually profiled and uniqued independently of the
path on which the bug occurred. This is used to merge diagnostics that
refer to the same issue along different paths, as well as by the plist
diagnostics to reference files created by the HTML diagnostics.

However, there are two problems with the current implementation:

1) The bug description is included in the profile, but some
   PathDiagnosticConsumers prefer abbreviated descriptions and some
   prefer verbose descriptions. Fixed by including both descriptions in
   the PathDiagnostic objects and always using the verbose one in the profile.

2) The "minimal" path generation scheme provides extra information about
   which events came from macros that the "extensive" scheme does not.
   This resulted not only in different locations for the plist and HTML
   diagnostics, but also in diagnostics being uniqued in the plist output
   but not in the HTML output. Fixed by storing the "end path" location
   explicitly in the PathDiagnostic object, rather than trying to find the
   last piece of the path when the diagnostic is requested.

This should hopefully finish unsticking our internal buildbot.

llvm-svn: 162965
2012-08-31 00:36:26 +00:00
Jordan Rose 7444f5d826 [analyzer] Fix a crash in plist-html generation introduced in r162939.
Basically, do the correct thing to fix the XML generation error, rather
than making it even worse by unilaterally dereferencing a null pointer.

llvm-svn: 162964
2012-08-31 00:36:20 +00:00
Eli Friedman 34866c7719 Change the representation of builtin functions in the AST
(__builtin_* etc.) so that it isn't possible to take their address.
Specifically, introduce a new type to represent a reference to a builtin
function, and a new cast kind to convert it to a function pointer in the
operand of a call.  Fixes PR13195.

llvm-svn: 162962
2012-08-31 00:14:07 +00:00
Anna Zaks a8017eca1a [analyzer] Refactor the logic that determines if a functions should be
reanalyzed.

The policy on what to reanalyze should be in AnalysisConsumer with the
rest of visitation order logic.

There is no reason why ExprEngine needs to pass the Visited set to
CoreEngine, it can populate it itself.

llvm-svn: 162957
2012-08-30 23:42:02 +00:00
Jordan Rose 03fac27bab [analyzer] Plist diagnostics: Fix a case where we fail to close an XML tag.
If the current path diagnostic does /not/ have files associated with it, we
were simply skipping on to the next diagnostic with 'continue'. But that
also skipped the close tag for the diagnostic's <dict> node.

Part of fixing our internal analyzer buildbot.

llvm-svn: 162939
2012-08-30 20:43:09 +00:00
Ted Kremenek efca7a7e1b Rename 'MaxLoop' to 'maxBlockVisitOnPath' to reflect reality. We
should consider renaming the command line option as well.

llvm-svn: 162932
2012-08-30 19:26:56 +00:00
Ted Kremenek 6f5131f149 Rename AnalyzerOptions 'EagerlyAssume' to 'eagerlyAssumeBinOpBifurcation'.
llvm-svn: 162930
2012-08-30 19:26:48 +00:00
Ted Kremenek 8756c4a1a9 Store const& to AnalyzerOptions in AnalysisManager instead of copying
individual flags.

llvm-svn: 162929
2012-08-30 19:26:43 +00:00
Anna Zaks 07a821fb17 [analyzer] Fixup 162863.
Thanks Jordan.

llvm-svn: 162875
2012-08-29 23:23:39 +00:00
Anna Zaks 5d4ec36323 [analyzer] Improved diagnostic pruning for calls initializing values.
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
2012-08-29 21:22:37 +00:00
Ted Kremenek fb5351eed3 Add new -cc1 driver option -analyzer-config, which allows one to specify
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
2012-08-29 05:55:00 +00:00
Jordan Rose 8d48938bf3 [analyzer] Teach CallEventManager that CXXTemporaryObjectExpr is also a ctor.
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
2012-08-28 20:52:21 +00:00
Jordan Rose 2be6e30d96 [analyzer] When we look for the last stmt in a function, skip implicit dtors.
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
2012-08-28 20:52:13 +00:00
Jordan Rose 595c131460 [analyzer] Don't purge dead symbols at the end of calls if -analyzer-purge=none.
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
2012-08-28 18:16:45 +00:00