Commit Graph

7 Commits

Author SHA1 Message Date
Artem Dergachev 11cadc3e6b [analyzer] MoveChecker Pt.6: Suppress the warning for the move-safe STL classes.
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
2018-12-14 20:52:57 +00:00
Artem Dergachev 12f7c2bacc [analyzer] MoveChecker: Improve invalidation policies.
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
2018-12-14 20:47:58 +00:00
Artem Dergachev f3f0366296 [analyzer] MoveChecker: Add more common state resetting methods.
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
2018-12-04 03:38:08 +00:00
Artem Dergachev 60eb8c113b [analyzer] MoveChecker: Improve warning and note messages.
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
2018-12-04 02:00:29 +00:00
Artem Dergachev eb4692582a [analyzer] MoveChecker: Restrict to locals and std:: objects.
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
2018-12-03 23:06:07 +00:00
Artem Dergachev 6c0b2ce1be [analyzer] MoveChecker: NFC: Remove the workaround for the "zombie symbols" bug.
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
2018-12-03 22:44:16 +00:00
Artem Dergachev 2c5945ca20 [analyzer] Rename MisusedMovedObjectChecker to MoveChecker
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
2018-12-03 22:32:32 +00:00