A refinement of r204730, itself a refinement of r198953, to better handle
cases where an object is accessed both through a property getter and
through direct ivar access. An object accessed through a property should
always be treated as +0, i.e. not owned by the caller. However, an object
accessed through an ivar may be at +0 or at +1, depending on whether the
ivar is a strong reference. Outside of ARC, we don't always have that
information.
The previous attempt would clear out the +0 provided by a getter, but only
if that +0 hadn't already participated in other retain counting operations.
(That is, "self.foo" is okay, but "[[self.foo retain] autorelease]" is
problematic.) This turned out to not be good enough when our synthesized
getters get involved.
This commit drops the notion of "overridable" reference counting and instead
just tracks whether a value ever came from a (strong) ivar. If it has, we
allow one more release than we otherwise would. This has the added benefit
of being able to catch /some/ overreleases of instance variables, though
it's not likely to come up in practice.
We do still get some false negatives because we currently throw away
refcount state upon assigning a value into an ivar. We should probably
improve on that in the future, especially once we synthesize setters as
well as getters.
rdar://problem/18075108
llvm-svn: 228174
Richard rejected my Sema change to interpret an integer literal zero in
a varargs context as a null pointer, so -Wsentinel sees an integer
literal zero and fires off a warning. Only CodeGen currently knows that
it promotes integer literal zeroes in this context to pointer size on
Windows. I didn't want to teach -Wsentinel about that compatibility
hack. Therefore, I'm migrating to C++11 nullptr.
llvm-svn: 223079
Fixes a crash in Retain Count checker error reporting logic by handing
the allocation statement retrieval from a BlockEdge program point.
Also added a simple CFG dump routine for debugging.
llvm-svn: 210960
This also includes some infrastructure to make it easier to build multi-argument
selectors, rather than trying to use string matching on each piece. There's a bit
more setup code, but less cost at runtime.
PR18908
llvm-svn: 205827
A refinement of r198953 to handle cases where an object is accessed both through
a property getter and through direct ivar access. An object accessed through a
property should always be treated as +0, i.e. not owned by the caller. However,
an object accessed through an ivar may be at +0 or at +1, depending on whether
the ivar is a strong reference. Outside of ARC, we don't have that information,
so we just don't track objects accessed through ivars.
With this change, accessing an ivar directly will deliberately override the +0
provided by a getter, but only if the +0 hasn't participated in other retain
counting yet. That isn't perfect, but it's already unusual for people to be
mixing property access with direct ivar access. (The primary use case for this
is in setters, init methods, and -dealloc.)
Thanks to Ted for spotting a few mistakes in private review.
<rdar://problem/16333368>
llvm-svn: 204730
This implements FIXME from Checker.cpp (FIXME: We want to return the package + name of the checker here.) and replaces hardcoded checker names with the new ones obtained via getCheckName().getName().
llvm-svn: 201525
Summary:
In clang-tidy we'd like to know the name of the checker producing each
diagnostic message. PathDiagnostic has BugType and Category fields, which are
both arbitrary human-readable strings, but we need to know the exact name of the
checker in the form that can be used in the CheckersControlList option to
enable/disable the specific checker.
This patch adds the CheckName field to the CheckerBase class, and sets it in
the CheckerManager::registerChecker() method, which gets them from the
CheckerRegistry.
Checkers that implement multiple checks have to store the names of each check
in the respective registerXXXChecker method.
Reviewers: jordan_rose, krememek
Reviewed By: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D2557
llvm-svn: 201186
A return type is the declared or deduced part of the function type specified in
the declaration.
A result type is the (potentially adjusted) type of the value of an expression
that calls the function.
Rule of thumb:
* Declarations have return types and parameters.
* Expressions have result types and arguments.
llvm-svn: 200082
Fix a perennial source of confusion in the clang type system: Declarations and
function prototypes have parameters to which arguments are supplied, so calling
these 'arguments' was a stretch even in C mode, let alone C++ where default
arguments, templates and overloading make the distinction important to get
right.
Readability win across the board, especially in the casting, ADL and
overloading implementations which make a lot more sense at a glance now.
Will keep an eye on the builders and update dependent projects shortly.
No functional change.
llvm-svn: 199686
Per discussion with Anna a /long/ time ago, it was way too easy to misuse
BlockCall: because it inherited from AnyFunctionCall (through SimpleCall),
getDecl() was constrained to return a FunctionDecl, and you had to call
getBlockDecl() instead. This goes against the whole point of CallEvent
(to abstract over different ways to invoke bodies of code).
Now, BlockCall just inherits directly from CallEvent. There's a bit of
duplication in getting things out of the origin expression (which is still
known to be a CallExpr), but nothing significant.
llvm-svn: 199321
...by synthesizing their body to be "return self->_prop;", with an extra
nudge to RetainCountChecker to still treat the value as +0 if we have no
other information.
This doesn't handle weak properties, but that's mostly correct anyway,
since they can go to nil at any time. This also doesn't apply to properties
whose implementations we can't see, since they may not be backed by an
ivar at all. And finally, this doesn't handle properties of C++ class type,
because we can't invoke the copy constructor. (Sema has actually done this
work already, but the AST it synthesizes is one the analyzer doesn't quite
handle -- it has an rvalue DeclRefExpr.)
Modeling setters is likely to be more difficult (since it requires
handling strong/copy), but not impossible.
<rdar://problem/11956898>
llvm-svn: 198953
RetainCountChecker has to track returned object values to know if they are
retained or not. Under ARC, even methods that return +1 are tracked by the
system and should be treated as +0. However, this effect behaves exactly
like NotOwned(ObjC), i.e. a generic Objective-C method that actually returns
+0, so we don't need a special case for it.
No functionality change.
llvm-svn: 198709
encodes the canonical rules for LLVM's style. I noticed this had drifted
quite a bit when cleaning up LLVM, so wanted to clean up Clang as well.
llvm-svn: 198686
This is intended to be a simplified API, whose internals are
deliberately less efficient for the purpose of a simplified interface,
for use with clients that want to query the analyzer's heuristics for
determining retain count semantics.
There are no immediate clients, but it is intended to be used
by the ObjC modernizer.
llvm-svn: 188433
We process autorelease counts when we exit functions, but if there's an
issue in a synthesized body the report will get dropped. Just skip the
processing for now and let it get handled when the caller gets around to
processing autoreleases.
(This is still suboptimal: objects autoreleased in the caller context
should never be warned about when exiting a callee context, synthesized
or not.)
Second half of <rdar://problem/14611722>
llvm-svn: 187625
It is unfortunate that we have to mark these exceptions in multiple places.
This was already in CallEvent. I suppose it does let us be more precise
about saying /which/ arguments have their retain counts invalidated -- the
connection's is still valid even though the context object's isn't -- but
we're not tracking the retain count of XPC objects anyway.
<rdar://problem/13783514>
llvm-svn: 180904
We get a CallEnter with a null expression, when processing a destructor. All other users of
CallEnter::getCallExpr work fine with null as return value.
(Addresses PR15832, Thanks to Jordan for reducing the test case!)
llvm-svn: 180234
The uniqueing location is the location which is part of the hash used to determine if two reports are
the same. This is used by the CmpRuns.py script to compare two analyzer runs and determine which
warnings are new.
llvm-svn: 180166
This is an opt-in tweak for leak diagnostics to reference the allocation
site if the diagnostic consumer only wants a pithy amount of information,
and not the entire path.
This is a strawman enhancement that I expect to see some experimentation
with over the next week, and can go away if we don't want it.
Currently it is only used by RetainCountChecker, but could be used
by MallocChecker if and when we decide this should stay in.
llvm-svn: 179634
The heuristic here (proposed by Jordan) is that, usually, if a leak is due to an early exit from init, the allocation site will be
a call to alloc. Note that in other cases init resets self to [super init], which becomes the allocation site of the object.
llvm-svn: 179221
...and add a new test case.
I thought this was broken, but it isn't; refactoring and reformatting anyway
so that I don't make the same mistake again. No functionality change.
llvm-svn: 178799
We weren't treating a cf_audited_transfer CFRetain as returning +1 because
its name doesn't contain "Create" or "Copy". Oops! Fortunately, the
standard definitions of these functions are not marked audited.
<rdar://problem/13339601>
llvm-svn: 176463
This matches our behavior for autorelease pools created by +alloc. Some
people like to create autorelease pools in one method and release them
somewhere else.
If you want safe autorelease pool semantics, use the new ARC-compatible
syntax: @autoreleasepool { ... }
<rdar://problem/13121353>
llvm-svn: 174096
This was previously added to support -[NSAutoreleasePool drain], which
behaves like -release under non-GC and "please collect" under GC. We're
not currently modeling the autorelease pool stack, though, so we can
just take this out entirely.
Fixes PR14927.
llvm-svn: 172444
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
Previously we made three passes over the set of dead symbols, and removed
them from the state /twice/. Now we combine the autorelease pass and the
symbol death pass, and only have to remove the bindings for the symbols
that leaked.
llvm-svn: 169527
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
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
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
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
Any future exceptions need to go INSIDE the test that checks if the
IdentifierInfo is non-null!
No functionality change. Thanks for the review, Ted.
llvm-svn: 163067
Fixes a hard-to-reach crash when calling a non-member overloaded operator
with arguments that may be callbacks.
Future-proofing: don't make the same assumption in MallocSizeofChecker.
Aside from possibly respecting attributes in the future, it might be
possible to call 'malloc' through a function pointer.
I audited all other uses of FunctionDecl::getIdentifier() in the analyzer;
they all now correctly test to see if the identifier is present before
using it.
llvm-svn: 163012
inlined function.
This resolves retain count checker false positives that are caused by
inlining ObjC and other methods. Essentially, if we are passing an
object to a method with "delegate" in the selector or a function pointer
as another argument, we should stop tracking the other parameters/return
value as far as the retain count checker is concerned.
llvm-svn: 162876
This code has been added a while ago and removing it does not trigger
any test failures. The false positives it was trying to suppress are
probably handled by other logic (ex: special handling of delegates).
llvm-svn: 162529
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
Also rename 'getCurrentBlockCounter()' to 'blockCount()'.
This ripples a bunch of code simplifications; mostly aesthetic,
but makes the code a bit tighter.
llvm-svn: 162349
No need to have the "get", the word "conjure" is a verb too!
Getting a conjured symbol is the same as conjuring one up.
This shortening is largely cosmetic, but just this simple changed
cleaned up a handful of lines, making them less verbose.
llvm-svn: 162348
Generating a sink is significantly different behavior from generating a
normal node, and a simple boolean parameter can be rather opaque. Per
offline discussion with Anna, adding new generation methods is the
clearest way to communicate intent.
No functionality change.
llvm-svn: 162215
This was once an adapter class between callbacks that had CheckerContexts
and those that don't, but for a while now it's essentially just been a
wrapper around a ProgramPointTag. We can just pass the tag around instead.
No functionality change.
llvm-svn: 162155
The autorelease pool has not been implemented completely: we were adding
the autoreleased symbols to the state, but never looking at them. Until
we have a complete implementation, remove the overhead and comment out
the unused code.
llvm-svn: 161821
to set/get/remove the RefBinding.
No functional change here. Having these setter and getter methods will
make it much easier when replacing the underlining representation of
RefBindings (I just went through the exercise). It makes the code more
readable as well.
llvm-svn: 161820
The frameworks correctly use the 'cf_consumed' and 'ns_returns_retained'
attributes for NSMakeCollectable, but we can model the behavior under
garbage collection more precisely than that.
No functionality change.
llvm-svn: 161349
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
As pointed out by Anna, we only differentiate between explicit message sends
This also adds support for ObjCSubscriptExprs, which are basically the same
as properties in many ways. We were already checking these, but not emitting
nice messages for them.
This depends on the llvm::PointerIntPair change in r160456.
llvm-svn: 160461
This enables the faster SmallVector in clang and also allows clang's unused
variable warnings to be more effective. Fix the two instances that popped up.
The RetainCountChecker change actually changes functionality, it would be nice
if someone from the StaticAnalyzer folks could look at it.
llvm-svn: 160444
While this work is still fairly tentative (destructors are still left out of
the CFG by default), we now handle destructors in the same way as any other
calls, instead of just automatically trying to inline them.
llvm-svn: 160020
These are currently unused, but are intended to be used in lieu of PreStmt
and PostStmt when the call is implicit (e.g. an automatic object destructor).
This also modifies the Data1 field of ProgramPoints to allow storing any
pointer-sized value, as opposed to only aligned pointers. This is necessary
to store SourceLocations.
There is currently no BugReporter support for these; they should be skipped
over in any diagnostic output.
This commit also tags checkers that currently rely on function calls only
occurring at StmtPoints.
llvm-svn: 160019
Our current inlining support (specifically RegionStore::enterStackFrame)
doesn't know that calls to overloaded operators may be calls to non-static
member functions, and that in these cases the first argument should be
treated as 'this'. This caused incorrect results and sometimes crashes.
The long-term fix will be to rewrite RegionStore::enterStackFrame to use
CallEvent and its subclasses, but for now we can just disable these
problematic calls by classifying them under a new CallEvent,
CXXMemberOperatorCall.
llvm-svn: 159692
The preObjCMessage and postObjCMessage callbacks now take an ObjCMethodCall
argument, which can represent an explicit message send (ObjCMessageSend) or an
implicit message generated by a property access (ObjCPropertyAccess).
llvm-svn: 159559
Previously, the CallEvent subclass ObjCMessageInvocation was just a wrapper
around the existing ObjCMessage abstraction (over message sends and property
accesses). Now, we have abstract CallEvent ObjCMethodCall with subclasses
ObjCMessageSend and ObjCPropertyAccess.
In addition to removing yet another wrapper object, this should make it easy
to add a ObjCSubscriptAccess call event soon.
llvm-svn: 159558
Jordan Rose