Previously, the iterator range checker only warned upon dereferencing of
iterators outside their valid range as well as increments and decrements of
out-of-range iterators where the result remains out-of-range. However, the C++
standard is more strict than this: decrementing begin() or incrementing end()
results in undefined behaviour even if the iterator is not dereferenced
afterwards. Coming back to the range once out-of-range is also undefined.
This patch corrects the behaviour of the iterator range checker: warnings are
given for any operation whose result is ahead of begin() or past the end()
(which is the past-end iterator itself, thus now we are speaking of past
past-the-end).
Differential Revision: https://reviews.llvm.org/D53812
llvm-svn: 348245
If an iterator is represented by a derived C++ class but its comparison operator
is for its base the iterator checkers cannot recognize the iterators compared.
This results in false positives in very straightforward cases (range error when
dereferencing an iterator after disclosing that it is equal to the past-the-end
iterator).
To overcome this problem we always use the region of the topmost base class for
iterators stored in a region. A new method called getMostDerivedObjectRegion()
was added to the MemRegion class to get this region.
Differential Revision: https://reviews.llvm.org/D54466
llvm-svn: 348244
Includes "resize" and "shrink" because they can reset the object to a known
state in certain circumstances.
Differential Revision: https://reviews.llvm.org/D54563
llvm-svn: 348235
The warning piece traditionally describes the bug itself, i.e.
"The bug is a _____", eg. "Attempt to delete released memory",
"Resource leak", "Method call on a moved-from object".
Event pieces produced by the visitor are usually in a present tense, i.e.
"At this moment _____": "Memory is released", "File is closed",
"Object is moved".
Additionally, type information is added into the event pieces for STL objects
(in order to highlight that it is in fact an STL object), and the respective
event piece now mentions that the object is left in an unspecified state
after it was moved, which is a vital piece of information to understand the bug.
Differential Revision: https://reviews.llvm.org/D54560
llvm-svn: 348229
In general case there use-after-move is not a bug. It depends on how the
move-constructor or move-assignment is implemented.
In STL, the convention that applies to most classes is that the move-constructor
(-assignment) leaves an object in a "valid but unspecified" state. Using such
object without resetting it to a known state first is likely a bug. Objects
Local value-type variables are special because due to their automatic lifetime
there is no intention to reuse space. If you want a fresh object, you might
as well make a new variable, no need to move from a variable and than re-use it.
Therefore, it is not always a bug, but it is obviously easy to suppress when it
isn't, and in most cases it indeed is - as there's no valid intention behind
the intentional use of a local after move.
This applies not only to local variables but also to parameter variables,
not only of value type but also of rvalue reference type (but not to lvalue
references).
Differential Revision: https://reviews.llvm.org/D54557
llvm-svn: 348210
The checker had extra code to clean up memory regions that were sticking around
in the checker without ever being cleaned up due to the bug that was fixed in
r347953. Because of that, if a region was moved from, then became dead,
and then reincarnated, there were false positives.
Why regions are even allowed to reincarnate is a separate story. Luckily, this
only happens for local regions that don't produce symbols when loaded from.
No functional change intended. The newly added test demonstrates that even
though no cleanup is necessary upon destructor calls, the early return
cannot be removed. It was not failing before the patch.
Differential Revision: https://reviews.llvm.org/D54372
llvm-svn: 348208
This follows the Static Analyzer's tradition to name checkers after
things in which they find bugs, not after bugs they find.
Differential Revision: https://reviews.llvm.org/D54556
llvm-svn: 348201
This continues the work that was started in r342313, which now gets applied to
object-under-construction tracking in C++. Makes it possible to debug
temporaries by dumping exploded graphs again.
Differential Revision: https://reviews.llvm.org/D54459
llvm-svn: 348200
Buildbot failures were caused by an unrelated UB that was introduced in r347943
and fixed in r347970.
Also the revision was incorrectly specified as r344580 during revert.
Differential Revision: https://reviews.llvm.org/D54017
llvm-svn: 348188
Kept the "indirect_builtin_constant_p" test case in test/SemaCXX/constant-expression-cxx1y.cpp
while we are investigating why the following snippet fails:
extern char extern_var;
struct { int a; } a = {__builtin_constant_p(extern_var)};
llvm-svn: 348039
In earlier patches regarding AnalyzerOptions, a lot of effort went into
gathering all config options, and changing the interface so that potential
misuse can be eliminited.
Up until this point, AnalyzerOptions only evaluated an option when it was
querried. For example, if we had a "-no-false-positives" flag, AnalyzerOptions
would store an Optional field for it that would be None up until somewhere in
the code until the flag's getter function is called.
However, now that we're confident that we've gathered all configs, we can
evaluate off of them before analysis, so we can emit a error on invalid input
even if that prticular flag will not matter in that particular run of the
analyzer. Another very big benefit of this is that debug.ConfigDumper will now
show the value of all configs every single time.
Also, almost all options related class have a similar interface, so uniformity
is also a benefit.
The implementation for errors on invalid input will be commited shorty.
Differential Revision: https://reviews.llvm.org/D53692
llvm-svn: 348031
From what I can see, this should be the last patch needed to replicate macro
argument expansions.
Differential Revision: https://reviews.llvm.org/D52988
llvm-svn: 348025
During the review of D41938 a condition check with an early exit accidentally
slipped into a branch, leaving the other branch unprotected. This may result in
an assertion later on. This hotfix moves this contition check outside of the
branch.
Differential Revision: https://reviews.llvm.org/D55051
llvm-svn: 347981
The checker suppresses warnings on paths on which a nonnull value is assumed
to be nullable. This probably deserves a warning, but it's a separate story.
Now, because dead symbol collection fires in pretty random moments,
there sometimes was a situation when dead symbol collection fired after
computing a parameter but before actually evaluating call enter into the
function, which triggered the suppression when the argument was null
in the first place earlier than the obvious warning for null-to-nonnull
was emitted, causing false negatives.
Only trigger the suppression for symbols, not for concrete values.
It is impossible to constrain a concrete value post-factum because
it is impossible to constrain a concrete value at all.
This covers all the necessary cases because by the time we reach the call,
symbolic values should be either not constrained to null, or already collapsed
into concrete null values. Which in turn happens because they are passed through
the Store, and the respective collapse is implemented as part of getSVal(),
which is also weird.
Differential Revision: https://reviews.llvm.org/D54017
llvm-svn: 347954
It's an old bug that consists in stale references to symbols remaining in the
GDM if they disappear from other program state sections as a result of any
operation that isn't the actual dead symbol collection. The most common example
here is:
FILE *fp = fopen("myfile.txt", "w");
fp = 0; // leak of file descriptor
In this example the leak were not detected previously because the symbol
disappears from the public part of the program state due to evaluating
the assignment. For that reason the checker never receives a notification
that the symbol is dead, and never reports a leak.
This patch not only causes leak false negatives, but also a number of other
problems, including false positives on some checkers.
What's worse, even though the program state contains a finite number of symbols,
the set of symbols that dies is potentially infinite. This means that is
impossible to compute the set of all dead symbols to pass off to the checkers
for cleaning up their part of the GDM.
No longer compute the dead set at all. Disallow iterating over dead symbols.
Disallow querying if any symbols are dead. Remove the API for marking symbols
as dead, as it is no longer necessary. Update checkers accordingly.
Differential Revision: https://reviews.llvm.org/D18860
llvm-svn: 347953
The "free" call frees the object immediately, ignoring the reference count.
Sadly, it is actually used in a few places, so we need to model it.
Differential Revision: https://reviews.llvm.org/D55092
llvm-svn: 347950
Move visitors to the implementation file, move a complicated logic into
a function.
Differential Revision: https://reviews.llvm.org/D55036
llvm-svn: 347946
Attempt to get a fully qualified name from AST if an SVal corresponding
to the object is not available.
Differential Revision: https://reviews.llvm.org/D55034
llvm-svn: 347944
If the object is a temporary, and there is no variable it binds to,
let's at least print out the object name in order to help differentiate
it from other temporaries.
rdar://45175098
Differential Revision: https://reviews.llvm.org/D55033
llvm-svn: 347943
This was reverted in r347656 due to me thinking it caused a miscompile of
Chromium. Turns out it was the Chromium code that was broken.
llvm-svn: 347756
This caused a miscompile in Chrome (see crbug.com/908372) that's
illustrated by this small reduction:
static bool f(int *a, int *b) {
return !__builtin_constant_p(b - a) || (!(b - a));
}
int arr[] = {1,2,3};
bool g() {
return f(arr, arr + 3);
}
$ clang -O2 -S -emit-llvm a.cc -o -
g() should return true, but after r347417 it became false for some reason.
This also reverts the follow-up commits.
r347417:
> Re-Reinstate 347294 with a fix for the failures.
>
> Don't try to emit a scalar expression for a non-scalar argument to
> __builtin_constant_p().
>
> Third time's a charm!
r347446:
> The result of is.constant() is unsigned.
r347480:
> A __builtin_constant_p() returns 0 with a function type.
r347512:
> isEvaluatable() implies a constant context.
>
> Assume that we're in a constant context if we're asking if the expression can
> be compiled into a constant initializer. This fixes the issue where a
> __builtin_constant_p() in a compound literal was diagnosed as not being
> constant, even though it's always possible to convert the builtin into a
> constant.
r347531:
> A "constexpr" is evaluated in a constant context. Make sure this is reflected
> if a __builtin_constant_p() is a part of a constexpr.
llvm-svn: 347656
Summary:
A __builtin_constant_p may end up with a constant after inlining. Use
the is.constant intrinsic if it's a variable that's in a context where
it may resolve to a constant, e.g., an argument to a function after
inlining.
Reviewers: rsmith, shafik
Subscribers: jfb, kristina, cfe-commits, nickdesaulniers, jyknight
Differential Revision: https://reviews.llvm.org/D54355
llvm-svn: 347294
Especially with pointees, a lot of meaningless reports came from uninitialized
regions that were already reported. This is fixed by storing all reported fields
to the GDM.
Differential Revision: https://reviews.llvm.org/D51531
llvm-svn: 347153
Extend the alpha.core.Conversion checker to handle implicit converions
where a too large integer value is converted to a floating point type. Each
floating point type has a range where it can exactly represent all integers; we
emit a warning when the integer value is above this range. Although it is
possible to exactly represent some integers which are outside of this range
(those that are divisible by a large enough power of 2); we still report cast
involving those, because their usage may lead to bugs. (For example, if 1<<24
is stored in a float variable x, then x==x+1 holds.)
Patch by: Donát Nagy!
Differential Revision: https://reviews.llvm.org/D52730
llvm-svn: 347006
This patch adds a couple new functions to acquire the macro's name, and also
expands it, although it doesn't expand the arguments, as seen from the test files
Differential Revision: https://reviews.llvm.org/D52794
llvm-svn: 346095
This test checks the entire output of a help option, the problem
is that on Windows, the line break occurs in a different place
causing the CHECK to fail because it is not expecting a line break.
llvm-svn: 346070
George Karpenkov