Commit Graph

68 Commits

Author SHA1 Message Date
Anton Yartsev 424ad95fa7 [analyzer] This patch removes passing around of const-invalidation vs regular-invalidation info by passing around a datastructure that maps regions and symbols to the type of invalidation they experience. This simplifies the code and would allow to associate more different invalidation types in the future.
With this patch things like preserving contents of regions (either hi- or low-level ones) or processing of the only top-level region can be implemented easily without passing around extra parameters.

This patch is a first step towards adequate modeling of memcpy() by the CStringChecker checker and towards eliminating of majority of false-positives produced by the NewDeleteLeaks checker.

llvm-svn: 191342
2013-09-24 23:47:29 +00:00
Jordan Rose 5770c038fe [analyzer] Use getParentIgnoreParenCasts instead of doing it by hand.
Apart from being more compact and already implemented, this also handles the
case where the parent is null. (It does also ignore all casts, not just
implicit ones, but this is more efficient to test and in the case we care
about---a message in a PseudoObjectExpr---there should only be implicit casts
anyway.

This should fix our internal buildbot.

llvm-svn: 191094
2013-09-20 16:51:50 +00:00
Anna Zaks 226a56fa1d [analyzer] More reliably detect property accessors.
This has a side effect of preventing a crash, which occurs because we get a
property getter declaration, which is overriding but is declared inside
@protocol. Will file a bug about this inconsistency internally. Getting a
small test case is very challenging.

llvm-svn: 190836
2013-09-17 01:30:57 +00:00
Eli Friedman 5ba37d5282 Split isFromMainFile into two functions.
Basically, isInMainFile considers line markers, and isWrittenInMainFile
doesn't.  Distinguishing between the two is useful when dealing with
files which are preprocessed files or rewritten with -frewrite-includes
(so we don't, for example, print useless warnings).

llvm-svn: 188968
2013-08-22 00:27:10 +00:00
Pavel Labath 963f91b3a2 Fix a crash in the static analyzer (bug #16307)
Summary:
When processing a call to a function, which got passed less arguments than it
expects, the analyzer would crash.

I've also added a test for that and a analyzer warning which detects these
cases.

CC: cfe-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D994

llvm-svn: 184288
2013-06-19 08:19:56 +00:00
Jordan Rose 278d9de314 [analyzer] Don't crash if a block's signature just has the return type.
It is okay to declare a block without an argument list: ^ {} or ^void {}.
In these cases, the BlockDecl's signature-as-written will just contain
the return type, rather than the entire function type. It is unclear if
this is intentional, but the analyzer shouldn't crash because of it.

<rdar://problem/14018351>

llvm-svn: 182948
2013-05-30 18:14:27 +00:00
Jordan Rose 1bfe9c787f [analyzer] Don't crash if a block doesn't have a type signature.
Currently, blocks instantiated in templates lose their "signature as
written"; it's not clear if this is intentional. Change the analyzer's
use of BlockDecl::getSignatureAsWritten to check whether or not the
signature is actually there.

<rdar://problem/13954714>

llvm-svn: 182497
2013-05-22 18:09:44 +00:00
Jordan Rose add14263ea [analyzer] Don't warn for returning void expressions in void blocks.
This was slightly tricky because BlockDecls don't currently store an
inferred return type. However, we can rely on the fact that blocks with
inferred return types will have return statements that match the inferred
type.

<rdar://problem/13665798>

llvm-svn: 179699
2013-04-17 18:03:48 +00:00
Anna Zaks 60bf5f45f7 [analyzer] Teach invalidateRegions that regions within LazyCompoundVal need to be invalidated
Refactor invalidateRegions to take SVals instead of Regions as input and teach RegionStore
about processing LazyCompoundVal as a top-level “escaping” value.

This addresses several false positives that get triggered by the NewDelete checker, but the
underlying issue is reproducible with other checkers as well (for example, MallocChecker).

llvm-svn: 178518
2013-04-02 01:28:24 +00:00
Jordan Rose 5413aaa791 [analyzer] Invalidate regions indirectly accessible through const pointers.
In this case, the value of 'x' may be changed after the call to indirectAccess:

  struct Wrapper {
    int *ptr;
  };

  void indirectAccess(const Wrapper &w);

  void test() {
    int x = 42;
    Wrapper w = { x };

    clang_analyzer_eval(x == 42); // TRUE
    indirectAccess(w);
    clang_analyzer_eval(x == 42); // UNKNOWN
  }

This is important for modelling return-by-value objects in C++, to show
that the contents of the struct are escaping in the return copy-constructor.

<rdar://problem/13239826>

llvm-svn: 177570
2013-03-20 20:35:53 +00:00
Jordan Rose 153c81b7c4 [analyzer] Remove strip of ElementRegion in CallEvent::invalidateRegions.
This is a bit of old code trying to deal with the fact that functions that
take pointers often use them to access an entire array via pointer
arithmetic. However, RegionStore already conservatively assumes you can use
pointer arithmetic to access any part of a region.

Some day we may want to go back to handling this specifically for calls,
but we can do that in the future.

No functionality change.

llvm-svn: 177569
2013-03-20 20:35:48 +00:00
Jordan Rose 6f09928d5b [analyzer] Clean up a few doc comments for ProgramState and CallEvent.
No functionality change.

llvm-svn: 176600
2013-03-07 01:23:14 +00:00
David Blaikie 00be69ab5c Remove the CFGElement "Invalid" state.
Use Optional<CFG*> where invalid states were needed previously. In the one case
where that's not possible (beginAutomaticObjDtorsInsert) just use a dummy
CFGAutomaticObjDtor.

Thanks for the help from Jordan Rose & discussion/feedback from Ted Kremenek
and Doug Gregor.

Post commit code review feedback on r175796 by Ted Kremenek.

llvm-svn: 175938
2013-02-23 00:29:34 +00:00
David Blaikie 2a01f5d426 Replace CFGElement llvm::cast support to be well-defined.
See r175462 for another example/more details.

llvm-svn: 175796
2013-02-21 20:58:29 +00:00
David Blaikie 05785d1622 Include llvm::Optional in clang/Basic/LLVM.h
Post-commit CR feedback from Jordan Rose regarding r175594.

llvm-svn: 175679
2013-02-20 22:23:23 +00:00
David Blaikie 2fdacbc5b0 Replace SVal llvm::cast support to be well-defined.
See r175462 for another example/more details.

llvm-svn: 175594
2013-02-20 05:52:05 +00:00
Anna Zaks 9747febba9 [analyzer] Address Jordan's nitpicks as per code review of r170625.
llvm-svn: 170832
2012-12-21 01:50:14 +00:00
Anna Zaks dc15415da4 [analyzer] Add the pointer escaped callback.
Instead of using several callbacks to identify the pointer escape event,
checkers now can register for the checkPointerEscape.

Converted the Malloc checker to use the new callback.
SimpleStreamChecker will be converted next.

llvm-svn: 170625
2012-12-20 00:38:25 +00:00
Ted Kremenek 245e45af7d Cache queries to lookupPrivateMethod() within ObjCMethodCall::getRuntimeDefinition().
The same queries can happen thousands of times.  This reduces the analysis
time on one heavy Objective-C file by 2.4%.

llvm-svn: 169589
2012-12-07 07:30:19 +00:00
Chandler Carruth 3a02247dc9 Sort all of Clang's files under 'lib', and fix up the broken headers
uncovered.

This required manually correcting all of the incorrect main-module
headers I could find, and running the new llvm/utils/sort_includes.py
script over the files.

I also manually added quite a few missing headers that were uncovered by
shuffling the order or moving headers up to be main-module-headers.

llvm-svn: 169237
2012-12-04 09:13:33 +00:00
Benjamin Kramer 444a1304ad Include pruning and general cleanup.
llvm-svn: 169095
2012-12-01 17:12:56 +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
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 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
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
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
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
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
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
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 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