Re-using a moved-from local variable is most likely a bug because there's
rarely a good motivation for not introducing a separate variable instead.
We plan to keep emitting such warnings by default.
Introduce a flag that allows disabling warnings on local variables that are
not of a known move-unsafe type. If it doesn't work out as we expected,
we'll just flip the flag.
We still warn on move-unsafe objects and unsafe operations on known move-safe
objects.
Differential Revision: https://reviews.llvm.org/D55730
llvm-svn: 349327
This re-applies commit r349226 that was reverted in r349233 due to failures
on clang-x64-windows-msvc.
Specify enum type as unsigned for use in bit field. Otherwise overflows
may cause UB.
Differential Revision: https://reviews.llvm.org/D55388
llvm-svn: 349326
ClangCheckerRegistry is a very non-obvious, poorly documented, weird concept.
It derives from CheckerRegistry, and is placed in lib/StaticAnalyzer/Frontend,
whereas it's base is located in lib/StaticAnalyzer/Core. It was, from what I can
imagine, used to circumvent the problem that the registry functions of the
checkers are located in the clangStaticAnalyzerCheckers library, but that
library depends on clangStaticAnalyzerCore. However, clangStaticAnalyzerFrontend
depends on both of those libraries.
One can make the observation however, that CheckerRegistry has no place in Core,
it isn't used there at all! The only place where it is used is Frontend, which
is where it ultimately belongs.
This move implies that since
include/clang/StaticAnalyzer/Checkers/ClangCheckers.h only contained a single function:
class CheckerRegistry;
void registerBuiltinCheckers(CheckerRegistry ®istry);
it had to re purposed, as CheckerRegistry is no longer available to
clangStaticAnalyzerCheckers. It was renamed to BuiltinCheckerRegistration.h,
which actually describes it a lot better -- it does not contain the registration
functions for checkers, but only those generated by the tblgen files.
Differential Revision: https://reviews.llvm.org/D54436
llvm-svn: 349275
Calling operator*() or operator->() on a null STL smart pointer is
undefined behavior.
Smart pointers are specified to become null after being moved from.
So we can't warn on arbitrary method calls, but these two operators
definitely make no sense.
The new bug is fatal because it's an immediate UB,
unlike other use-after-move bugs.
The work on a more generic null smart pointer dereference checker
is still pending.
Differential Revision: https://reviews.llvm.org/D55388
llvm-svn: 349226
Some C++ standard library classes provide additional guarantees about their
state after move. Suppress warnings on such classes until a more precise
behavior is implemented. Warnings for locals are not suppressed anyway
because it's still most likely a bug.
Differential Revision: https://reviews.llvm.org/D55307
llvm-svn: 349191
If a moved-from object is passed into a conservatively evaluated function
by pointer or by reference, we assume that the function may reset its state.
Make sure it doesn't apply to const pointers and const references. Add a test
that demonstrates that it does apply to rvalue references.
Additionally, make sure that the object is invalidated when its contents change
for reasons other than invalidation caused by evaluating a call conservatively.
In particular, when the object's fields are manipulated directly, we should
assume that some sort of reset may be happening.
Differential Revision: https://reviews.llvm.org/D55289
llvm-svn: 349190
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
Artem Dergachev