Commit Graph

36 Commits

Author SHA1 Message Date
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
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 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
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 e537cc05f5 [analyzer] Rename CallEvent::mayBeInlined to CallEvent::isCallStmt.
The two callers are using this in order to be conservative, so let's just
clarify the information that's actually being provided here. This is not
related to inlining decisions in any way.

No functionality change.

llvm-svn: 162717
2012-08-28 00:50:38 +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
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 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 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
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 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 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
Anna Zaks 85383182ec [analyzer] Improve readability of the dyn. dispatch bifurcation patch
r161552.

As per Jordan's feedback.

llvm-svn: 161603
2012-08-09 18:43:00 +00:00
Anna Zaks bc6d0ccf92 Unbreak the build.
Declaring "const Decl *Decl" is not a good idea.

llvm-svn: 161567
2012-08-09 02:57:02 +00:00
Anna Zaks 123af098b8 [analyzer] Bifurcate the path with dynamic dispatch.
This is an initial (unoptimized) version. We split the path when
inlining ObjC instance methods. On one branch we always assume that the
type information for the given memory region is precise. On the other we
assume that we don't have the exact type info. It is important to check
since the class could be subclassed and the method can be overridden. If
we always inline we can loose coverage.

Had to refactor some of the call eval functions.

llvm-svn: 161552
2012-08-09 00:21:33 +00:00
Anna Zaks 75930b65b4 [analyzer] Address Jordan's review of DynamicTypePropagation.
llvm-svn: 161391
2012-08-07 05:12:24 +00:00
Anna Zaks 472dbcf156 [analyzer] Add a checker to manage dynamic type propagation.
Instead of sprinkling dynamic type info propagation throughout
ExprEngine, the added checker would add the more precise type
information on known APIs (Ex: ObjC alloc, new) and propagate
the type info in other cases (ex: ObjC init method, casts (the second is
not implemented yet)).

Add handling of ObjC alloc, new and init to the checker.

llvm-svn: 161357
2012-08-06 23:25:39 +00:00
Jordan Rose 92e1449b55 [analyzer] Track null/uninitialized C++ objects used in method calls.
llvm-svn: 161278
2012-08-03 23:08:49 +00:00
Jordan Rose 6a97d92ef5 [analyzer] Don't try to inline if there's no region for a message receiver.
While usually we'd use a symbolic region rather than a straight-up Unknown,
we can still generate unknowns via array subscripts with symbolic indexes.
(And if this ever changes in the future, we still shouldn't crash.)

llvm-svn: 161059
2012-07-31 18:04:53 +00:00
Jordan Rose 1f8c0b4587 [analyzer] Add a FIXME about devirtualization in ctors/dtors.
llvm-svn: 161058
2012-07-31 18:04:49 +00:00
Jordan Rose 42e8d6497d [analyzer] Let CallEvent decide what goes in an inital stack frame.
This removes explicit checks for 'this' and 'self' from
Store::enterStackFrame. It also removes getCXXThisRegion() as a virtual
method on all CallEvents; it's now only implemented in the parts of the
hierarchy where it is relevant. Finally, it removes the option to ask
for the ParmVarDecls attached to the definition of an inlined function,
saving a recomputation of the result of getRuntimeDefinition().

No visible functionality change!

llvm-svn: 161017
2012-07-31 01:07:55 +00:00
Anna Zaks 5808eb8029 [analyzer] Handle inlining of instance calls to super.
Use self-init.m for testing. (It used to have a bunch of failing tests
with dynamic inlining turned on.)

llvm-svn: 161012
2012-07-30 23:48:36 +00:00
Jordan Rose c2d249ce2c [analyzer] Perform post-call checks for all inlined calls.
Previously, we were only checking the origin expressions of inlined calls.
Checkers using the generic postCall and older postObjCMessage callbacks were
ignored. Now that we have CallEventManager, it is much easier to create
a CallEvent generically when exiting an inlined function, which we can then
use for post-call checks.

No test case because we don't (yet) have any checkers that depend on this
behavior (which is why it hadn't been fixed before now).

llvm-svn: 161005
2012-07-30 23:39:47 +00:00
Anna Zaks 63282aefb9 [analyzer] Very simple ObjC instance method inlining
- Retrieves the type of the object/receiver from the state.
- Binds self during stack setup.
- Only explores the path on which the method is inlined (no
bifurcation to explore the path on which the method is not inlined).

llvm-svn: 160991
2012-07-30 20:31:29 +00:00
Jordan Rose fcd016e57e [analyzer] Only allow CallEvents to be created by CallEventManager.
This ensures that it is valid to reference-count any CallEvents, and we
won't accidentally try to reclaim a CallEvent that lives on the stack.
It also hides an ugly switch statement for handling CallExprs!

There should be no functionality change here.

llvm-svn: 160986
2012-07-30 20:22:09 +00:00
Jordan Rose 72ce8e2d42 [analyzer] CallEvent is no longer a value object.
After discussion, the type-based dispatch was decided to be bad for
maintenance and made it very easy for subtle bugs to creep in. Instead,
we'll just be very careful when we do have to allocate these on the heap.

llvm-svn: 160817
2012-07-26 21:41:15 +00:00
Jordan Rose 4f7df9be69 [analyzer] Rename Calls.{h,cpp} to CallEvent.{h,cpp}. No functionality change.
llvm-svn: 160815
2012-07-26 21:39:41 +00:00