If a method is resetting the state of an object that was moved from, it should
be safe to use this object again. However if the method was defined in a parent
class, but used in a child class, the reset didn't happen from the checker's
perspective.
Differential Revision: https://reviews.llvm.org/D31538
llvm-svn: 315301
This method injects additional information into program state dumps,
describing which objects have been moved from.
Differential Revision: https://reviews.llvm.org/D31541
llvm-svn: 315300
This method injects additional information into program state dumps,
describing states of mutexes tracked by the checker.
Differential Revision: https://reviews.llvm.org/D37805
llvm-svn: 315298
The analyzer now realizes that C++ std::initializer_list objects and
Objective-C boxed structure/array/dictionary expressions can potentially
maintain a reference to the objects that were put into them. This avoids
false memory leak posivites and a few other issues.
This is a conservative behavior; for now, we do not model what actually happens
to the objects after being passed into such initializer lists.
rdar://problem/32918288
Differential Revision: https://reviews.llvm.org/D35216
llvm-svn: 314975
In ProgramState::getSVal(Location, Type) API which dereferences a pointer value,
when the optional Type parameter is not supplied and the Location is not typed,
type should have been guessed on a best-effort basis by inspecting the Location
more deeply. However, this never worked; the auto-detected type was instead
a pointer type to the correct type.
Fixed the issue and added various test cases to demonstrate which parts of the
analyzer were affected (uninitialized pointer argument checker, C++ trivial copy
modeling, Google test API modeling checker).
Additionally, autodetected void types are automatically replaced with char,
in order to simplify checker APIs. Which means that if the location is a void
pointer, getSVal() would read the first byte through this pointer
and return its symbolic value.
Fixes pr34305.
Differential Revision: https://reviews.llvm.org/D38358
llvm-svn: 314910
Fixes the test failure: temporary is now bound to std::string, tests
fully pass on Linux.
This reverts commit b36ee0924038e1d95ea74230c62d46e05f80587e.
llvm-svn: 314859
Only assume that IOBSDNameMatching and friends increment a reference counter
if their return type is a CFMutableDictionaryRef.
Differential Revision: https://reviews.llvm.org/D38487
llvm-svn: 314820
This function can now track null pointer through simple pointer arithmetic,
such as '*&*(p + 2)' => 'p' and so on, displaying intermediate diagnostic pieces
for the user to understand where the null pointer is coming from.
Differential Revision: https://reviews.llvm.org/D37025
llvm-svn: 314290
This API is used by checkers (and other entities) in order to track where does
a value originate from, by jumping from an expression value of which is equal
to that value to the expression from which this value has "appeared". For
example, it may be an lvalue from which the rvalue was loaded, or a function
call from which the dereferenced pointer was returned.
The function now avoids incorrectly unwrapping implicit lvalue-to-rvalue casts,
which caused crashes and incorrect intermediate diagnostic pieces. It also no
longer relies on how the expression is written when guessing what it means.
Fixes pr34373 and pr34731.
rdar://problem/33594502
Differential Revision: https://reviews.llvm.org/D37023
llvm-svn: 314287
This patch fixes analyzer's crash on the newly added test case
(see also https://bugs.llvm.org/show_bug.cgi?id=34374).
Pointers subtraction appears to be modeled incorrectly
in the following example:
char* p;
auto n = p - reinterpret_cast<char*>((unsigned long)1);
In this case the analyzer (built without this patch)
tries to create a symbolic value for the difference
treating reinterpret_cast<char*>((unsigned long)1)
as an integer, that is not correct.
Differential revision: https://reviews.llvm.org/D38214
Test plan: make check-all
llvm-svn: 314141
The implementation is in AnalysisDeclContext.cpp and the class is called
AnalysisDeclContext.
Making those match up has numerous benefits, including:
- Easier jump from header to/from implementation.
- Easily identify filename from class.
Differential Revision: https://reviews.llvm.org/D37500
llvm-svn: 312671
Summary:
So far we used a value of 10 which was useful for testing but produces many false-positives in real programs. The usual suspicious clones we find seem to be at around a complexity value of 70 and for normal clone-reporting everything above 50 seems to be a valid normal clone for users, so let's just go with 50 for now and set this as the new default value.
This patch also explicitly sets the complexity value for the regression tests as they serve more of a regression testing/debugging purpose and shouldn't really be reported by default in real programs. I'll add more tests that reflect actual found bugs that then need to pass with the default setting in the future.
Reviewers: NoQ
Subscribers: cfe-commits, javed.absar, xazax.hun, v.g.vassilev
Differential Revision: https://reviews.llvm.org/D34178
llvm-svn: 312468
Summary:
This patch aims at optimizing the CloneChecker for larger programs. Before this
patch we took around 102 seconds to analyze sqlite3 with a complexity value of
50. After this patch we now take 2.1 seconds to analyze sqlite3.
The biggest performance optimization is that we now put the constraint for group
size before the constraint for the complexity. The group size constraint is much
faster in comparison to the complexity constraint as it only does a simple
integer comparison. The complexity constraint on the other hand actually
traverses each Stmt and even checks the macro stack, so it is obviously not able
to handle larger amounts of incoming clones. The new order filters out all the
single-clone groups that the type II constraint generates in a faster way before
passing the fewer remaining clones to the complexity constraint. This reduced
runtime by around 95%.
The other change is that we also delay the verification part of the type II
clones back in the chain of constraints. This required to split up the
constraint into two parts - a verification and a hash constraint (which is also
making it more similar to the original design of the clone detection algorithm).
The reasoning for this is the same as before: The verification constraint has to
traverse many statements and shouldn't be at the start of the constraint chain.
However, as the type II hashing has to be the first step in our algorithm, we
have no other choice but split this constrain into two different ones. Now our
group size and complexity constrains filter out a chunk of the clones before
they reach the slow verification step, which reduces the runtime by around 8%.
I also kept the full type II constraint around - that now just calls it's two
sub-constraints - in case someone doesn't care about the performance benefits
of doing this.
Reviewers: NoQ
Reviewed By: NoQ
Subscribers: klimek, v.g.vassilev, xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D34182
llvm-svn: 312222
This diff fixes modeling of arithmetic
expressions where pointers are treated as integers
(i.e. via C-style / reinterpret casts).
For now we return UnknownVal unless the operation is a comparison.
Test plan: make check-all
Differential revision: https://reviews.llvm.org/D37120
llvm-svn: 311935
This way the unrolling can be restricted for loops which will take at most a
given number of steps. It is defined as 128 in this patch and it seems to have
a good number for that purpose.
Differential Revision: https://reviews.llvm.org/D37181
llvm-svn: 311883
Added check if the execution of the last step of the given unrolled loop has
generated more branches. If yes, than treat it as a normal (non-unrolled) loop
in the remaining part of the analysis.
Differential Revision: https://reviews.llvm.org/D36962
llvm-svn: 311881
1. The LoopUnrolling feature needs the LoopExit included in the CFG so added this
dependency via the config options
2. The LoopExit element can be encountered even if we haven't encountered the
block of the corresponding LoopStmt. So the asserts were not right.
3. If we are caching out the Node then we get a nullptr from generateNode which
case was not handled.
Differential Revision: https://reviews.llvm.org/D37103
llvm-svn: 311880
The LoopExit CFG information provides the opportunity to not mark the loops but
having a stack which tracks if a loop is unrolled or not. So in case of
simulating a loop we just add it and the information if it meets the
requirements to be unrolled to the top of the stack.
Differential Revision: https://reviews.llvm.org/D35684
llvm-svn: 311346
This patch adds handling of the LoopExit CFGElements to the StaticAnalyzer.
This is reached by introducing a new ProgramPoint.
Tests will be added in a following commit.
Differential Revision: https://reviews.llvm.org/D35670
llvm-svn: 311344
This patch introduces a new CFG element CFGLoopExit that indicate when a loop
ends. It does not deal with returnStmts yet (left it as a TODO).
It hidden behind a new analyzer-config flag called cfg-loopexit (false by
default).
Test cases added.
The main purpose of this patch right know is to make loop unrolling and loop
widening easier and more efficient. However, this information can be useful for
future improvements in the StaticAnalyzer core too.
Differential Revision: https://reviews.llvm.org/D35668
llvm-svn: 311235
Adding escape check for the counter variable of the loop.
It is achieved by jumping back on the ExplodedGraph to its declStmt.
Differential Revision: https://reviews.llvm.org/D35657
llvm-svn: 311234
This diff fixes analyzer's crash (triggered assert) on the newly added test case.
The assert being discussed is assert(!B.lookup(R, BindingKey::Direct))
in lib/StaticAnalyzer/Core/RegionStore.cpp, however the root cause is different.
For classes with empty bases the offsets might be tricky.
For example, let's assume we have
struct S: NonEmptyBase, EmptyBase {
...
};
In this case Clang applies empty base class optimization and
the offset of EmptyBase will be 0, it can be verified via
clang -cc1 -x c++ -v -fdump-record-layouts main.cpp -emit-llvm -o /dev/null.
When the analyzer tries to perform zero initialization of EmptyBase
it will hit the assert because that region
has already been "written" by the constructor of NonEmptyBase.
Test plan:
make check-all
Differential revision: https://reviews.llvm.org/D36851
llvm-svn: 311182
This commit adds the functionality of performing reference counting on the
callee side for Integer Set Library (ISL) to Clang Static Analyzer's
RetainCountChecker.
Reference counting on the callee side can be extensively used to perform
debugging within a function (For example: Finding leaks on error paths).
Patch by Malhar Thakkar!
Differential Revision: https://reviews.llvm.org/D36441
llvm-svn: 311063
This diff fixes a crash (triggered assert) on the newly added test case.
In the method Simplifier::VisitSymbolData we check the type of S and return
Loc/NonLoc accordingly.
Differential revision: https://reviews.llvm.org/D36564
llvm-svn: 310887
This change adds support for cross-file diagnostic paths in html output. If the
diagnostic path is not cross-file, there is no change in the output.
Patch by Vlad Tsyrklevich!
Differential Revision: https://reviews.llvm.org/D30406
llvm-svn: 309968
This feature allows the analyzer to consider loops to completely unroll.
New requirements/rules (for unrolling) can be added easily via ASTMatchers.
Right now it is hidden behind a flag, the aim is to find the correct heuristic
and create a solution which results higher coverage % and more precise
analysis, thus can be enabled by default.
Right now the blocks which belong to an unrolled loop are marked by the
LoopVisitor which adds them to the ProgramState.
Then whenever we encounter a CFGBlock in the processCFGBlockEntrance which is
marked then we skip its investigating. That means, it won't be considered to
be visited more than the maximal bound for visiting since it won't be checked.
llvm-svn: 309006
Add a 'Generalized' object kind to the retain-count checker and suitable
generic diagnostic text for retain-count diagnostics involving those objects.
For now the object kind is introduced in summaries by 'annotate' attributes.
Once we have more experience with these annotations we will propose explicit
attributes.
Patch by Malhar Thakkar!
Differential Revision: https://reviews.llvm.org/D35613
llvm-svn: 308990
Because since r308957 the suppress-on-sink feature contains its own
mini-analysis, it also needs to become aware that C++ unhandled exceptions
cause sinks. Unfortunately, for now we treat all exceptions as unhandled in
the analyzer, so suppress-on-sink needs to do the same.
rdar://problem/28157554
Differential Revision: https://reviews.llvm.org/D35674
llvm-svn: 308961
If a certain memory leak (or other similar bug) found by the analyzer is known
to be happening only before abnormal termination of the program ("sink", eg.
assertion failure in the code under analysis, or another bug that introduces
undefined behavior), such leak warning is discarded. However, if the analysis
has never reaches completion (due to complexity of the code), it may be
failing to notice the sink.
This commit further extends the partial solution introduced in r290341 to cover
cases when a complicated control flow occurs before encountering a no-return
statement (which anyway inevitably leads to such statement(s)) by traversing
the respective section of the CFG in a depth-first manner. A complete solution
still seems elusive.
rdar://problem/28157554
Differential Revision: https://reviews.llvm.org/D35673
llvm-svn: 308957
requirements/rules (for unrolling) can be added easily via ASTMatchers.
The current implementation is hidden behind a flag.
Right now the blocks which belong to an unrolled loop are marked by the
LoopVisitor which adds them to the ProgramState. Then whenever we encounter a
CFGBlock in the processCFGBlockEntrance which is marked then we skip its
investigating. That means, it won't be considered to be visited more than the
maximal bound for visiting since it won't be checked.
Differential Revision: https://reviews.llvm.org/D34260
llvm-svn: 308558
Add support to the retain-count checker for an annotation indicating that a
function's implementation should be trusted by the retain count checker.
Functions with these attributes will not be inlined and the arguments will
be treating as escaping.
Adding this annotation avoids spurious diagnostics when the implementation of
a reference counting operation is visible but the analyzer can't reason
precisely about the ref count.
Patch by Malhar Thakkar!
Differential Revision: https://reviews.llvm.org/D34937
llvm-svn: 308416
There was already a returns_localized_nsstring annotation to indicate
that the return value could be passed to UIKit methods that would
display them. However, those UIKit methods were hard-coded, and it was
not possible to indicate that other classes/methods in a code-base would
do the same.
The takes_localized_nsstring annotation can be put on function
parameters and selector parameters to indicate that those will also show
the string to the user.
Differential Revision: https://reviews.llvm.org/D35186
llvm-svn: 308012
Summary:
This mimics the implementation for the implicit destructors. The
generation of this scope leaving elements is hidden behind
a flag to the CFGBuilder, thus it should not affect existing code.
Currently, I'm missing a test (it's implicitly tested by the clang-tidy
lifetime checker that I'm proposing).
I though about a test using debug.DumpCFG, but then I would
have to add an option to StaticAnalyzer/Core/AnalyzerOptions
to enable the scope leaving CFGElement,
which would only be useful to that particular test.
Any other ideas how I could make a test for this feature?
Reviewers: krememek, jordan_rose
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D15031
llvm-svn: 307759
This is a follow up for one of
the previous diffs https://reviews.llvm.org/D32328.
getTypeSize and with getIntWidth are not equivalent for bool
(see https://clang.llvm.org/doxygen/ASTContext_8cpp_source.html#l08444),
this causes a number of issues
(for instance, if APint X representing a bool is created
with the wrong bit width then X is not comparable against Min/Max
(because of the different bit width), that results in crashes
(triggered asserts) inside assume* methods),
for examples see the newly added test cases.
Test plan: make check-all
Differential revision: https://reviews.llvm.org/D35041
llvm-svn: 307604
This is a new checker package. It contains checkers that highlight
well-documented implementation-defined behavior. Such checkers are only useful
to developers that intend to write portable code. Code that is only compiled for
a single platform should be allowed to rely on this platform's specific
documented behavior.
rdar://problem/30545046
Differential Revision: https://reviews.llvm.org/D34102
llvm-svn: 306396
This makes the analyzer around 10% slower by default,
allowing it to find deeper bugs.
Default values for the following -analyzer-config change:
max-nodes: 150000 -> 225000;
max-inlinable-size: 50 -> 100.
rdar://problem/32539666
Differential Revision: https://reviews.llvm.org/D34277
llvm-svn: 305900
Add support for new methods that were added in macOS High Sierra & iOS 11
and require a localized string.
Patch by Kulpreet Chilana!
rdar://problem/32795210
Differential Revision: https://reviews.llvm.org/D34266
llvm-svn: 305896
Memory region allocated by alloca() carries no implicit type information.
Don't crash when resolving the init message for an Objective-C object
that is being constructed in such region.
rdar://problem/32517077
Differential Revision: https://reviews.llvm.org/D33828
llvm-svn: 305211
In plist output mode with alternate path diagnostics, when entering a function,
we draw an arrow from the caller to the beginning of the callee's declaration.
Upon exiting, however, we draw the arrow from the last statement in the
callee function. The former makes little sense when the declaration is
not a definition, i.e. has no body, which may happen in case the body
is coming from a body farm, eg. Objective-C autosynthesized property accessor.
Differential Revision: https://reviews.llvm.org/D33671
llvm-svn: 304713
Nullable-to-nonnull checks used to crash when the custom bug visitor was trying
to add its notes to autosynthesized accessors of Objective-C properties.
Now we avoid this, mostly automatically outside of checker control, by
moving the diagnostic to the parent stack frame where the accessor has been
called.
Differential revision: https://reviews.llvm.org/D32437
llvm-svn: 304710
This should fix the leaks found by asan buildbot in r304162.
Also don't store a reference to the factory with every map value,
which is the only difference between ImmutableMap and ImmutableMapRef.
llvm-svn: 304170
The analyzer's taint analysis can now reason about structures or arrays
originating from taint sources in which only certain sections are tainted.
In particular, it also benefits modeling functions like read(), which may
read tainted data into a section of a structure, but RegionStore is incapable of
expressing the fact that the rest of the structure remains intact, even if we
try to model read() directly.
Patch by Vlad Tsyrklevich!
Differential revision: https://reviews.llvm.org/D28445
llvm-svn: 304162
The new checker currently contains the very core infrastructure for tracking
the state of iterator-type objects in the analyzer: relating iterators to
their containers, tracking symbolic begin and end iterator values for
containers, and solving simple equality-type constraints over iterators.
A single specific check over this infrastructure is capable of finding usage of
out-of-range iterators in some simple cases.
Patch by Ádám Balogh!
Differential revision: https://reviews.llvm.org/D32592
llvm-svn: 304160
pthread_mutex_destroy() may fail, returning a non-zero error number, and
keeping the mutex untouched. The mutex can be used on the execution branch
that follows such failure, so the analyzer shouldn't warn on using
a mutex that was previously destroyed, when in fact the destroy call has failed.
Patch by Malhar Thakkar!
Differential revision: https://reviews.llvm.org/D32449
llvm-svn: 304159
Even though the shouldInlineCall function returns true, it can happen that the
function is not going to be inlined (as it can be seen at line 913 and below).
Moved the bumpNumTimesInlined(D) (the counter increaser) call to the inlineCall
function where it logically belongs.
Differential Revision: https://reviews.llvm.org/D32179
llvm-svn: 303158
Use variadic templates instead of relying on <cstdarg> + sentinel.
This enforces better type checking and makes code more readable.
Differential revision: https://reviews.llvm.org/D32550
llvm-svn: 302572
It was written as "Memory Error" in most places and as "Memory error" in a few
other places, however it is the latter that is more consistent with
other categories (such as "Logic error").
rdar://problem/31718115
Differential Revision: https://reviews.llvm.org/D32702
llvm-svn: 302016
Array-to-pointer cast now works correctly when the pointer to the array
is concrete, eg. null, which allows further symbolic calculations involving
such values.
Inlined defensive checks are now detected correctly when the resulting null
symbol is being array-subscripted before dereference.
Differential Revision: https://reviews.llvm.org/D32291
llvm-svn: 301251
Null dereferences are suppressed if the lvalue was constrained to 0 for the
first time inside a sub-function that was inlined during analysis, because
such constraint is a valid defensive check that does not, by itself,
indicate that null pointer case is anyhow special for the caller.
If further operations on the lvalue are performed, the symbolic lvalue is
collapsed to concrete null pointer, and we need to track where does the null
pointer come from.
Improve such tracking for lvalue operations involving operator &.
rdar://problem/27876009
Differential Revision: https://reviews.llvm.org/D31982
llvm-svn: 301224
This diff replaces getTypeSize(CondE->getType()))
with getIntWidth(CondE->getType())) in ExprEngine::processSwitch.
These calls are not equivalent for bool, see ASTContext.cpp
Add a test case.
Test plan:
make check-clang-analysis
make check-clang
Differential revision: https://reviews.llvm.org/D32328
llvm-svn: 300936
We now check the type of the super-region pointer for most SubRegion classes
in compile time; some checks are run-time though.
This is an API-breaking change (we now require explicit casts to specific region
sub-classes), but in practice very few checkers are affected.
Differential Revision: https://reviews.llvm.org/D26838
llvm-svn: 300189
Clean up vtable anchors (remove anchors for regions that have regular
out-of-line virtual methods, add anchors for regions that don't have those).
Fix private/public methods (all constructors should now be private for leaf
classes, protected for abstract classes).
No functional change intended, only extra sanity checks and cleanups.
Differential Revision: https://reviews.llvm.org/D26837
llvm-svn: 300187
SValBuilder tries to constant-fold symbols in the left-hand side of the symbolic
expression whenever it fails to evaluate the expression directly. However, it
only constant-folds them when they are atomic expressions, not when they are
complicated expressions themselves. This patch adds recursive constant-folding
to the left-hand side subexpression (there's a lack of symmetry because we're
trying to have symbols on the left and constants on the right). As an example,
we'd now be able to handle operations similar to "$x + 1 < $y", when $x is
constrained to a constant.
rdar://problem/31354676
Differential Revision: https://reviews.llvm.org/D31886
llvm-svn: 300178
This diff adds a defensive check in getExtraInvalidatedValues
for the case when there are no regions for the ivar associated with
a property. Corresponding test case added.
Test plan:
make check-clang
make check-clang-analysis
llvm-svn: 300114
Hopefully fix crashes by unshadowing the variable.
Original commit message:
A big part of the clone detection code is functionality for filtering clones and
clone groups based on different criteria. So far this filtering process was
hardcoded into the CloneDetector class, which made it hard to understand and,
ultimately, to extend.
This patch splits the CloneDetector's logic into a sequence of reusable
constraints that are used for filtering clone groups. These constraints
can be turned on and off and reodreder at will, and new constraints are easy
to implement if necessary.
Unit tests are added for the new constraint interface.
This is a refactoring patch - no functional change intended.
Patch by Raphael Isemann!
Differential Revision: https://reviews.llvm.org/D23418
llvm-svn: 299653
A big part of the clone detection code is functionality for filtering clones and
clone groups based on different criteria. So far this filtering process was
hardcoded into the CloneDetector class, which made it hard to understand and,
ultimately, to extend.
This patch splits the CloneDetector's logic into a sequence of reusable
constraints that are used for filtering clone groups. These constraints
can be turned on and off and reodreder at will, and new constraints are easy
to implement if necessary.
Unit tests are added for the new constraint interface.
This is a refactoring patch - no functional change intended.
Patch by Raphael Isemann!
Differential Revision: https://reviews.llvm.org/D23418
llvm-svn: 299544
Summary:
The alpha.core.Conversion was too strict about compound assignments and could warn even though there is no problem.
Differential Revision: https://reviews.llvm.org/D25596
llvm-svn: 299523
If the value is known, but we cannot increment it, conjure a symbol to
represent the result of the operation based on the operator expression,
not on the sub-expression.
In particular, no longer crash on comparing a result of a LocAsInteger increment
to a constant integer.
rdar://problem/31067356
Differential Revision: https://reviews.llvm.org/D31289
llvm-svn: 298927
Adjustments should be considered properly; we should copy the unadjusted object
over the whole temporary base region. If the unadjusted object is no longer
available in the Environment, invalidate the temporary base region, and then
copy the adjusted object into the adjusted sub-region of the temporary region.
This fixes a regression introduced by r288263, that caused various
false positives, due to copying only adjusted object into the adjusted region;
the rest of the base region therefore remained undefined.
Before r288263, the adjusted value was copied over the unadjusted region,
which is incorrect, but accidentally worked better due to how region store
disregards compound value bindings to non-base regions.
An additional test machinery is introduced to make sure that despite making
two binds, we only notify checkers once for both of them, without exposing
the partially copied objects.
This fix is a hack over a hack. The proper fix would be to model C++ temporaries
in the CFG, and after that dealing with adjustments would no longer be
necessary, and the values we need would no longer disappear from the
Environment.
rdar://problem/30658168
Differential Revision: https://reviews.llvm.org/D30534
llvm-svn: 298924
The checker currently warns on copying, moving, or calling methods on an object
that was recently std::move'd from. It understands a set of "state reset"
methods that bring a moved-from object back to a well-specified state.
Patch by Peter Szecsi!
Differential Revision: https://reviews.llvm.org/D24246
llvm-svn: 298698
It looks like on some host-triples the result of a valist related expr can be
a LazyCompoundVal. Handle that case in the check.
Patch by Abramo Bagnara!
llvm-svn: 297619
Daniel Marjamaki