Somehow both Daniel and I missed the fact that while loops are only identical
if they have identical bodies.
Patch by Daniel Fahlgren!
llvm-svn: 201829
IdenticalExprChecker now warns if any expressions in a logical or bitwise
chain (&&, ||, &, |, or ^) are the same. Unlike the previous patch, this
actually checks all subexpressions against each other (an O(N^2) operation,
but N is likely to be small).
Patch by Daniel Fahlgren!
llvm-svn: 201702
This extends the checks for identical expressions to handle identical
statements, and compares the consequent and alternative ("then" and "else")
branches of an if-statement to see if they are identical, treating a single
statement surrounded by braces as equivalent to one without braces.
This does /not/ check subsequent branches in an if/else chain, let alone
branches that are not consecutive. This may improve in a future patch, but
it would certainly take more work.
Patch by Daniel Fahlgren!
llvm-svn: 201701
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
...rather somewhere in the destructor when we try to access something and
realize the object has already been deleted. This is necessary because
the destructor is processed before the 'delete' itself.
Patch by Karthik Bhat!
llvm-svn: 198779
...even though the argument is declared "const void *", because this is
just a way to pass pointers around as objects. (Though NSData is often
a better one.)
PR18262
llvm-svn: 198710
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 has the dual effect of (1) enabling more dead-stripping in release builds
and (2) ensuring that debug helper functions aren't stripped away in debug
builds, as they're intended to be called from the debugger.
Note that the attribute is applied to definitions rather than declarations in
headers going forward because it's now conditional on NDEBUG:
/// \brief Mark debug helper function definitions like dump() that should not be
/// stripped from debug builds.
Requires corresponding macro added in LLVM r198456.
llvm-svn: 198489
This checker has not been updated to work with interprocedural analysis,
and actually contains both logical correctness issues but also
memory bugs. We can resuscitate it from version control once there
is focused interest in making it a real viable checker again.
llvm-svn: 198476
There's nothing special about type traits accepting two arguments.
This commit eliminates BinaryTypeTraitExpr and switches all related handling
over to TypeTraitExpr.
Also fixes a CodeGen failure with variadic type traits appearing in a
non-constant expression.
The BTT/TT prefix and evaluation code is retained as-is for now but will soon
be further cleaned up.
This is part of the ongoing work to unify type traits.
llvm-svn: 197273
Warn if both result expressions of a ternary operator (? :) are the same.
Because only one of them will be executed, this warning will fire even if
the expressions have side effects.
Patch by Anders Rönnholm and Per Viberg!
llvm-svn: 196937
look at the attribute spelling instead. The 'ownership_*' attributes should
probably be split into separate *Attr classes, but that's more than I wanted to
do here.
llvm-svn: 195805
New rules of invalidation/escape of the source buffer of memcpy: the source buffer contents is invalidated and escape while the source buffer region itself is neither invalidated, nor escape.
In the current modeling of memcpy the information about allocation state of regions, accessible through the source buffer, is not copied to the destination buffer and we can not track the allocation state of those regions anymore. So we invalidate/escape the source buffer indirect regions in anticipation of their being invalidated for real later. This eliminates false-positive leaks reported by the unix.Malloc and alpha.cplusplus.NewDeleteLeaks checkers for the cases like
char *f() {
void *x = malloc(47);
char *a;
memcpy(&a, &x, sizeof a);
return a;
}
llvm-svn: 194953
I've added the missing ImutProfileInfo [sic] specialization for bool,
so this patch on r194235 is no longer needed.
This reverts r194244 / 2baea2887dfcf023c8e3560e5d4713c42eed7b6b.
llvm-svn: 194265
In ADT/ImmutableSet, ImutProfileInfo<bool> cannot be matched to ImutProfileInteger.
I didn't have idea it'd the right way if PROFILE_INTEGER_INFO(bool) could be added there.
llvm-svn: 194244
This syntactic checker looks for expressions on both sides of comparison
operators that are structurally the same. As a special case, the
floating-point idiom "x != x" for "isnan(x)" is left alone.
Currently this only checks comparison operators, but in the future we could
extend this to include logical operators or chained if-conditionals.
Checker by Per Viberg!
llvm-svn: 194236
An Objective-C for-in loop will have zero iterations if the collection is
empty. Previously, we could only detect this case if the program asked for
the collection's -count /before/ the for-in loop. Now, the analyzer
distinguishes for-in loops that had zero iterations from those with at
least one, and can use this information to constrain the result of calling
-count after the loop.
In order to make this actually useful, teach the checker that methods on
NSArray, NSDictionary, and the other immutable collection classes don't
change the count.
<rdar://problem/14992886>
llvm-svn: 194235
Re-commit r191910 (reverted in r191936) with layering violation fixed, by
moving the bug categories to StaticAnalyzerCore instead of ...Checkers.
llvm-svn: 191937
One small functionality change is to bring the sizeof-pointer checker in
line with the other checkers by making its category be "Logic error"
instead of just "Logic". There should be no other functionality changes.
Patch by Daniel Marjamäki!
llvm-svn: 191910
This will emit a warning if a call to clang_analyzer_warnIfReached is
executed, printing REACHABLE. This is a more explicit way to declare
expected reachability than using clang_analyzer_eval or triggering
a bug (divide-by-zero or null dereference), and unlike the former will
work the same in inlined functions and top-level functions. Like the
other debug helpers, it is part of the debug.ExprInspection checker.
Patch by Jared Grubb!
llvm-svn: 191909
Also add some tests that there is actually a message and that the bug is
actually a hard error. This actually behaved correctly before, because:
- addTransition() doesn't actually add a transition if the new state is null;
it assumes you want to propagate the predecessor forward and does nothing.
- generateSink() is called in order to emit a bug report.
- If at least one new node has been generated, the predecessor node is /not/
propagated forward.
But now it's spelled out explicitly.
Found by Richard Mazorodze, who's working on a patch that may require this.
llvm-svn: 191805
Now that the CFG includes nodes for the destructors in a delete-expression,
process them in the analyzer using the same common destructor interface
currently used for local, member, and base destructors. Also, check for when
the value is known to be null, in which case no destructor is actually run.
This does not yet handle destructors for deleted /arrays/, which may need
more CFG work. It also causes a slight regression in the location of
double delete warnings; the double delete is detected at the destructor
call, which is implicit, and so is reported on the first access within the
destructor instead of at the 'delete' statement. This will be fixed soon.
Patch by Karthik Bhat!
llvm-svn: 191381
"+method_name: cannot take ownership of memory allocated by 'new'."
instead of the old
"Memory allocated by 'new' should be deallocated by 'delete', not +method_name"
llvm-svn: 190800
This is still an alpha checker, but we use it in certain tests to make sure
something is not being executed.
This should fix the buildbots.
llvm-svn: 188682
This keeps the analyzer from making silly assumptions, like thinking
strlen(foo)+1 could wrap around to 0. This fixes PR16558.
Patch by Karthik Bhat!
llvm-svn: 188680
This builtin does not actually evaluate its arguments for side effects,
so we shouldn't include them in the CFG. In the analyzer, rely on the
constant expression evaluator to get the proper semantics, at least for
now. (In the future, we could get ambitious and try to provide path-
sensitive size values.)
In theory, this does pose a problem for liveness analysis: a variable can
be used within the __builtin_object_size argument expression but not show
up as live. However, it is very unlikely that such a value would be used
to compute the object size and not used to access the object in some way.
<rdar://problem/14760817>
llvm-svn: 188679
When a region is realloc()ed, MallocChecker records whether it was known
to be allocated or not. If it is, and the reallocation fails, the original
region has to be freed. Previously, when an allocated region escaped,
MallocChecker completely stopped tracking it, so a failed reallocation
still (correctly) wouldn't require freeing the original region. Recently,
however, MallocChecker started tracking escaped symbols, so that if it were
freed we could check that the deallocator matched the allocator. This
broke the reallocation model for whether or not a symbol was allocated.
Now, MallocChecker will actually check if a symbol is owned, and only
require freeing after a failed reallocation if it was owned before.
PR16730
llvm-svn: 188468
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
This field is just IsDefaulted && !IsDeleted; in all places it's used,
a simple check for isDefaulted() is superior anyway, and we were forgetting
to set it in a few cases.
Also eliminate CXXDestructorDecl::IsImplicitlyDefined, for the same reasons.
No intended functionality change.
llvm-svn: 187891
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
This is the same way GenericSelectionExpr works, and it's generally a
more consistent approach.
A large part of this patch is devoted to caching the value of the condition
of a ChooseExpr; it's needed to avoid threading an ASTContext into
IgnoreParens().
Fixes <rdar://problem/14438917>.
llvm-svn: 186738
Summary:
The analyzer incorrectly handled noreturn destructors which were hidden inside
function calls. This happened because NoReturnFunctionChecker only listened for
PostStmt events, which are not executed for destructor calls. I've changed it to
listen to PostCall events, which should catch both cases.
Reviewers: jordan_rose
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1056
llvm-svn: 185522
Add a debug checker that is useful to understand how the ExplodedGraph is
built; it can be triggered using the following command:
clang -cc1 -analyze -analyzer-checker=debug.ViewExplodedGraph my_program.c
A patch by Béatrice Creusillet!
llvm-svn: 184768
This fixes false positives by allowing us to know that a loop is always entered if
the collection count method returns a positive value and vice versa.
Addresses radar://14169391.
llvm-svn: 184618
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
This silences warnings that could occur when one is swapping partially initialized structs. We suppress
not only the assignments of uninitialized members, but any values inside swap because swap could
potentially be used as a subroutine to swap class members.
This silences a warning from std::try::function::swap() on partially initialized objects.
llvm-svn: 184256
The untemplated implementation of getParents() doesn't need to be in a
header file.
RecursiveASTVisitor.h is full of repeated macro expansion. Moving this
include to ASTContext.cpp speeds up compilation of
LambdaMangleContext.cpp, a small C++ file with few includes, from 3.7s
to 2.8s for me locally. I haven't measured a full build, but it can't
hurt.
I had to fix a few static analyzer files that were depending on
transitive includes of C++ AST headers.
Reviewers: rsmith, klimek
Differential Revision: http://llvm-reviews.chandlerc.com/D982
llvm-svn: 184075
Summary:
"register" functions for the checker were caching the checker objects in a
static variable. This caused problems when the function is called with a
different CheckerManager.
Reviewers: klimek
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D955
llvm-svn: 183823
Jordan has pointed out that it is valuable to warn in cases when the arguments to init escape.
For example, NSData initWithBytes id not going to free the memory.
llvm-svn: 183062
This class is a StmtVisitor that distinguishes between block-level and
non-block-level statements in a CFG. However, it does so using a hard-coded
idea of which statements might be block-level, which probably isn't accurate
anymore. The only implementer of the CFGStmtVisitor hierarchy was the
analyzer's DeadStoresChecker, and the analyzer creates a linearized CFG
anyway (every non-trivial statement is a block-level statement).
This also allows us to remove the block-expr map ("BlkExprMap"), which
mapped statements to positions in the CFG. Apart from having a helper type
that really should have just been Optional<unsigned>, it was only being
used to ask /if/ a particular expression was block-level, for traversal
purposes in CFGStmtVisitor.
llvm-svn: 181945
Consider this example:
char *p = malloc(sizeof(char));
systemFunction(&p);
free(p);
In this case, when we call systemFunction, we know (because it's a system
function) that it won't free 'p'. However, we /don't/ know whether or not
it will /change/ 'p', so the analyzer is forced to invalidate 'p', wiping
out any bindings it contains. But now the malloc'd region looks like a
leak, since there are no more bindings pointing to it, and we'll get a
spurious leak warning.
The fix for this is to notice when something is becoming inaccessible due
to invalidation (i.e. an imperfect model, as opposed to being explicitly
overwritten) and stop tracking it at that point. Currently, the best way
to determine this for a call is the "indirect escape" pointer-escape kind.
In practice, all the patch does is take the "system functions don't free
memory" special case and limit it to direct parameters, i.e. just the
arguments to a call and not other regions accessible to them. This is a
conservative change that should only cause us to escape regions more
eagerly, which means fewer leak warnings.
This isn't perfect for several reasons, the main one being that this
example is treated the same as the one above:
char **p = malloc(sizeof(char *));
systemFunction(p + 1);
// leak
Currently, "addresses accessible by offsets of the starting region" and
"addresses accessible through bindings of the starting region" are both
considered "indirect" regions, hence this uniform treatment.
Another issue is our longstanding problem of not distinguishing const and
non-const bindings; if in the first example systemFunction's parameter were
a char * const *, we should know that the function will not overwrite 'p',
and thus we can safely report the leak.
<rdar://problem/13758386>
llvm-svn: 181607
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
In an Objective-C for-in loop "for (id element in collection) {}", the loop
will run 0 times if the collection is nil. This is because the for-in loop
is implemented using a protocol method that returns 0 when there are no
elements to iterate, and messages to nil will result in a 0 return value.
At some point we may want to actually model this message send, but for now
we may as well get the nil case correct, and avoid the false positives that
would come with this case.
<rdar://problem/13744632>
llvm-svn: 180639
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
- If only partial invalidators exist and there are no full invalidators in @implementation, report every ivar that has
not been invalidated. (Previously, we reported the first Ivar in the list, which could actually have been invalidated
by a partial invalidator. The code assumed you cannot have only partial invalidators.)
- Do not report missing invalidation method declaration if a partial invalidation method declaration exists.
llvm-svn: 180170
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 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
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
Previously, the analyzer used isIntegerType() everywhere, which uses the C
definition of "integer". The C++ predicate with the same behavior is
isIntegerOrUnscopedEnumerationType().
However, the analyzer is /really/ using this to ask if it's some sort of
"integrally representable" type, i.e. it should include C++11 scoped
enumerations as well. hasIntegerRepresentation() sounds like the right
predicate, but that includes vectors, which the analyzer represents by its
elements.
This commit audits all uses of isIntegerType() and replaces them with the
general isIntegerOrEnumerationType(), except in some specific cases where
it makes sense to exclude scoped enumerations, or any enumerations. These
cases now use isIntegerOrUnscopedEnumerationType() and getAs<BuiltinType>()
plus BuiltinType::isInteger().
isIntegerType() is hereby banned in the analyzer - lib/StaticAnalysis and
include/clang/StaticAnalysis. :-)
Fixes real assertion failures. PR15703 / <rdar://problem/12350701>
llvm-svn: 179081
Test that the path notes do not change. I don’t think we should print a note on escape.
Also, I’ve removed a check that assumed that the family stored in the RefStete could be
AF_None and added an assert in the constructor.
llvm-svn: 179075
As mentioned in the previous commit message, the use-after-free and
double-free warnings for 'delete' are worth enabling even while the
leak warnings still have false positives.
llvm-svn: 178891
This splits the leak-checking part of alpha.cplusplus.NewDelete into a
separate user-level checker, alpha.cplusplus.NewDeleteLeaks. All the
difficult false positives we've seen with the new/delete checker have been
spurious leak warnings; the use-after-free warnings and mismatched
deallocator warnings, while rare, have always been valid.
<rdar://problem/6194569>
llvm-svn: 178890
The statement passed to isTrackedFamily() might be a user defined function calling malloc; in this case we got AF_NONE family for this function.
Now the allocation family is derived from Sym, that holds a family of a real allocator.
This commit is also a movement towards getting rid of tracking memory allocating by unknown means.
llvm-svn: 178834
This fixes an issue pointed to by Jordan: if unix.Malloc and unix.MismatchedDeallocator are both on, then we end up still tracking leaks of memory allocated by new.
Moved the guards right before emitting the bug reports to unify and simplify the logic of handling of multiple checkers. Now all the checkers perform their checks regardless of if they were enabled, or not, and it is decided just before the emitting of the report, if it should be emitted. (idea from Anna).
Additional changes:
improved test coverage for checker correlations;
refactoring: BadDealloc -> MismatchedDealloc
llvm-svn: 178814
...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
- Find the correct region to represent the first array element when
constructing a CXXConstructorCall.
- If the array is trivial, model the copy with a primitive load/store.
- Don't warn about the "uninitialized" subscript in the AST -- we don't use
the helper variable that Sema provides.
<rdar://problem/13091608>
llvm-svn: 178602
Add a new callback that notifies checkers when a const pointer escapes. Currently, this only works
for const pointers passed as a top level parameter into a function. We need to differentiate the const
pointers escape from regular escape since the content pointed by const pointer will not change;
if it’s a file handle, a file cannot be closed; but delete is allowed on const pointers.
This should suppress several false positives reported by the NewDelete checker on llvm codebase.
llvm-svn: 178310
We should only suppress a bug report if the IDCed or null returned nil value is directly related to the value we are warning about. This was
not the case for nil receivers - we would suppress a bug report that had an IDCed nil receiver on the path regardless of how it’s
related to the warning.
1) Thread EnableNullFPSuppression parameter through the visitors to differentiate between tracking the value which
is directly responsible for the bug and other values that visitors are tracking (ex: general tracking of nil receivers).
2) in trackNullOrUndef specifically address the case when a value of the message send is nil due to the receiver being nil.
llvm-svn: 178309
+ Improved display names for allocators and deallocators
The checker checks if a deallocation function matches allocation one. ('free' for 'malloc', 'delete' for 'new' etc.)
llvm-svn: 178250
The visitor should look for the PreStmt node as the receiver is nil in the PreStmt and this is the node. Also, tag the nil
receiver nodes with a special tag for consistency.
llvm-svn: 178152
Register the nil tracking visitors with the region and refactor trackNullOrUndefValue a bit.
Also adds the cast and paren stripping before checking if the value is an OpaqueValueExpr
or ExprWithCleanups.
llvm-svn: 178093
Previously, MallocChecker's pointer escape check and its post-call state
update for Objective-C method calls had a fair amount duplicated logic
and not-entirely-consistent checks. This commit restructures all this to
be more consistent and possibly allow us to be more aggressive in warning
about double-frees.
New policy (applies to system header methods only):
(1) If this is a method we know about, model it as taking/holding ownership
of the passed-in buffer.
(1a) ...unless there's a "freeWhenDone:" parameter with a zero (NO) value.
(2) If there's a "freeWhenDone:" parameter (but it's not a method we know
about), treat the buffer as escaping if the value is non-zero (YES) and
non-escaping if it's zero (NO).
(3) If the first selector piece ends with "NoCopy" (but it's not a method we
know about and there's no "freeWhenDone:" parameter), treat the buffer
as escaping.
The reason that (2) and (3) don't explicitly model the ownership transfer is
because we can't be sure that they will actually free the memory using free(),
and we wouldn't want to emit a spurious "mismatched allocator" warning
(coming in Anton's upcoming patch). In the future, we may have an idea of a
"generic deallocation", i.e. we assume that the deallocator is correct but
still continue tracking the region so that we can warn about double-frees.
Patch by Anton Yartsev, with modifications from me.
llvm-svn: 176744
Warn about null pointer dereference earlier when a reference to a null pointer is
passed in a call. The idea is that even though the standard might allow this, reporting
the issue earlier is better for diagnostics (the error is reported closer to the place where
the pointer was set to NULL). This also simplifies analyzer’s diagnostic logic, which has
to track “where the null came from”. As a consequence, some of our null pointer
warning suppression mechanisms started triggering more often.
TODO: Change the name of the file and class to reflect the new check.
llvm-svn: 176612
Officially in the C++ standard, a null reference cannot exist. However,
it's still very easy to create one:
int &getNullRef() {
int *p = 0;
return *p;
}
We already check that binds to reference regions don't create null references.
This patch checks that we don't create null references by returning, either.
<rdar://problem/13364378>
llvm-svn: 176601
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
With the new support for trivial copy constructors, we are not always
consistent about whether a CXXTempObjectRegion gets reused or created
from scratch, which affects whether qualifiers are preserved. However,
we probably don't care anyway.
This also switches to using the current PrintingPolicy for the type,
which means C++ types don't get a spurious 'struct' prefix anymore.
llvm-svn: 176068
This addresses a case when we inline a wrong method due to incorrect
dynamic type inference. Specifically, when user code contains a method from init
family, which creates an instance of another class.
Use hasRelatedResultType() to find out if our inference rules should be triggered.
llvm-svn: 176054
This required more changes than I originally expected:
- ObjCIvarRegion implements "canPrintPretty" et al
- DereferenceChecker indicates the null pointer source is an ivar
- bugreporter::trackNullOrUndefValue() uses an alternate algorithm
to compute the location region to track by scouring the ExplodedGraph.
This allows us to get the actual MemRegion for variables, ivars,
fields, etc. We only hand construct a VarRegion for C++ references.
- ExplodedGraph no longer drops nodes for expressions that are marked
'lvalue'. This is to facilitate the logic in the previous bullet.
This may lead to a slight increase in size in the ExplodedGraph,
which I have not measured, but it is likely not to be a big deal.
I have validated each of the changed plist output.
Fixes <rdar://problem/12114812>
llvm-svn: 175988
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
The missing definition check should be in the same category as the
missing ivar validation - in this case, the intent is to invalidate in
the given class, as described in the declaration, but the implementation
does not perform the invalidation. Whereas the MissingInvalidationMethod
checker checks the cases where the method intention is not to
invalidate. The second checker has potential to have a much higher false
positive rate.
llvm-svn: 174787
The new annotation allows having methods that only partially invalidate
IVars and might not be called from the invalidation methods directly
(instead, are guaranteed to be called before the invalidation occurs).
The checker is going to trust the programmer to call the partial
invalidation method before the invalidator.This is common in cases when
partial object tear down happens before the death of the object.
llvm-svn: 174779
The malloc checker will now catch the case when a previously malloc'ed
region is freed, but the pointer passed to free does not point to the
start of the allocated memory. For example:
int *p1 = malloc(sizeof(int));
p1++;
free(p1); // warn
From the "memory.LeakPtrValChanged enhancement to unix.Malloc" entry
in the list of potential checkers.
A patch by Branden Archer!
llvm-svn: 174678
The checkPointerEscape callback previously did not specify how a
pointer escaped. This change includes an enum which describes the
different ways a pointer may escape. This enum is passed to the
checkPointerEscape callback when a pointer escapes. If the escape
is due to a function call, the call is passed. This changes
previous behavior where the call is passed as NULL if the escape
was due to indirectly invalidating the region the pointer referenced.
A patch by Branden Archer!
llvm-svn: 174677
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
The expression 'a->b.c()' contains a call to the 'c' method of 'a->b'.
We emit an error if 'a' is NULL, but previously didn't actually track
the null value back through the 'a->b' expression, which caused us to
miss important false-positive-suppression cases, including
<rdar://problem/12676053>.
llvm-svn: 173547
it apart from [[gnu::noreturn]] / __attribute__((noreturn)), since their
semantics are not equivalent (for instance, we treat [[gnu::noreturn]] as
affecting the function type, whereas [[noreturn]] does not).
llvm-svn: 172691