Commit Graph

1434 Commits

Author SHA1 Message Date
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
Anna Zaks 92898a79eb [analyzer] Ivar invalidation: identify properties declared in protocols.
llvm-svn: 166211
2012-10-18 19:17:57 +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 b642fc5db4 [analyzer] Ivar Invalidation: track ivars in continuations and
@implementation.

llvm-svn: 166047
2012-10-16 19:36:37 +00:00
Jordan Rose 968a1b58fa [analyzer] ObjCContainersASTChecker: minor cleanup and an extra test case.
Follow-up to r165838, which fixed a potential crash.

llvm-svn: 166002
2012-10-16 00:47:25 +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
Anna Zaks 09ffeba552 [analyzer] Enhance the error message.
llvm-svn: 165993
2012-10-15 22:48:17 +00:00
Anna Zaks f4fd1455e0 [analyzer] Do not warn on direct ivar assignments within copy methods.
llvm-svn: 165992
2012-10-15 22:48:14 +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
Ted Kremenek be4008472d Move assertion to not crash tests.
llvm-svn: 165842
2012-10-12 22:56:45 +00:00
Ted Kremenek 26a3661a10 Silence static analyzer issue by documenting that in this context
that a DeclRefExpr can never return a null decl.  We possibly should
hoist this into getDecl() itself.

llvm-svn: 165841
2012-10-12 22:56:42 +00:00
Ted Kremenek b045b01840 Silence null dereference warnings by documenting context-specific
invariants using assertions.

llvm-svn: 165840
2012-10-12 22:56:40 +00:00
Ted Kremenek d0b9770399 Fix potential crash in ObjCContainersChecker by properly validating
the number of arguments.

llvm-svn: 165838
2012-10-12 22:56:36 +00:00
Ted Kremenek c9822ebc97 Remove OSAtomicChecker.
llvm-svn: 165744
2012-10-11 20:58:21 +00:00
Ted Kremenek 089ffd0889 Switch over to BodyFarm implementation of OSAtomicCompareAndSwap and
objc_atomicCompareAndSwap.

llvm-svn: 165743
2012-10-11 20:58:18 +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 d8665b3d1a [analyzer] Don't run non-path-sensitive checks on system headers...
...but do run them on user headers.

Previously, we were inconsistent here: non-path-sensitive checks on code
/bodies/ were only run in the main source file, but checks on
/declarations/ were run in /all/ headers. Neither of those is the
behavior we want.

Thanks to Sujit for pointing this out!

<rdar://problem/12454226>

llvm-svn: 165635
2012-10-10 17:55:40 +00:00
Jordan Rose 85c8c81924 [analyzer] Fix typo: s/HandleDeclsGallGraph/HandleDeclsCallGraph/g
No functionality change.

llvm-svn: 165634
2012-10-10 17:55:37 +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
Anna Zaks 97c7ce3368 Move isObjCSelf into Expr.
llvm-svn: 164966
2012-10-01 20:34:04 +00:00
Anna Zaks bfacf17b8b [analyzer] Address Jordan's review for r164868.
llvm-svn: 164965
2012-10-01 20:33:58 +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
Ted Kremenek 86917fdbe0 Add checker debug.ConfigDumper to dump the contents of the configuration table.
The format of this output is a WIP; largely I'm bringing it up now
for regression testing.  We can evolve the output format over time.

llvm-svn: 164953
2012-10-01 18:28:14 +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
Anna Zaks 00a896e7a2 [analyzer] Do not visit ObjCMethodDecl twice in the AST checkers.
llvm-svn: 164869
2012-09-29 00:20:40 +00:00
Anna Zaks 0353aad5a9 [analyzer] Re-implement IvarInvalidationChecker so that it verifies that
the validation occurred.

The original implementation was pessimistic - we assumed that ivars
which escape are invalidated. This version is optimistic, it assumes
that the ivars will always be explicitly invalidated: either set to nil
or sent an invalidation message.

llvm-svn: 164868
2012-09-29 00:20:38 +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