Commit Graph

20 Commits

Author SHA1 Message Date
Csaba Dabis 2e896b8b39 [analyzer] ConditionBRVisitor: Boolean support
Summary: -

Reviewers: NoQ, george.karpenkov

Reviewed By: NoQ, george.karpenkov

Subscribers: cfe-commits, xazax.hun, baloghadamsoftware, szepet, a.sidorin,
             mikhail.ramalho, Szelethus, donat.nagy, dkrupp

Tags: #clang

Differential Revision: https://reviews.llvm.org/D58207

llvm-svn: 362027
2019-05-29 20:34:29 +00:00
Csaba Dabis 4b0184b2d3 [analyzer] ConditionBRVisitor: Enhance to write out more information
Summary:
Add extra messages to the bug report to inform the user why the analyzer
`Taking true/false branch`.

Reviewers: NoQ, george.karpenkov

Reviewed By: NoQ

Subscribers: gerazo, gsd, dkrupp, whisperity, baloghadamsoftware, xazax.hun,
             eraman, szepet, a.sidorin, mikhail.ramalho, Szelethus,
             donat.nagy, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D53076

llvm-svn: 362020
2019-05-29 20:06:09 +00:00
Csaba Dabis b7ca72a113 [analyzer] print() JSONify: Checker messages implementation
Summary: -

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Reviewed By: NoQ

Subscribers: szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy,
             dkrupp

Tags: #clang

Differential Revision: https://reviews.llvm.org/D62086

llvm-svn: 361982
2019-05-29 16:02:33 +00:00
Artem Dergachev 8c6119a442 [analyzer] PR41269: Add a bit of C++ smart pointer modeling.
Implement cplusplus.SmartPtrModeling, a new checker that doesn't
emit any warnings but models methods of smart pointers more precisely.

For now the only thing it does is make `(bool) P` return false when `P`
is a freshly moved pointer. This addresses a false positive in the
use-after-move-checker.

Differential Revision: https://reviews.llvm.org/D60796

llvm-svn: 358944
2019-04-23 02:45:42 +00:00
Artem Dergachev 6b71e27c94 [analyzer] NFC: MoveChecker: Refactor tests to use -verify=prefix.
This -verify=prefix feature is quite underrated.

Differential Revision: https://reviews.llvm.org/D60732

llvm-svn: 358719
2019-04-18 23:17:58 +00:00
Kristof Umann 748c139ade [analyzer] Emit an error rather than assert on invalid checker option input
Asserting on invalid input isn't very nice, hence the patch to emit an error
instead.

This is the first of many patches to overhaul the way we handle checker options.

Differential Revision: https://reviews.llvm.org/D57850

llvm-svn: 355704
2019-03-08 16:00:42 +00:00
Artem Dergachev 342a7ac8d6 [analyzer] MoveChecker: Add one more common resetting method, "append".
This is especially crucial for reports related to use-after-move of
standard library objects.

rdar://problem/47338505

Differential Revision: https://reviews.llvm.org/D56824

llvm-svn: 351500
2019-01-18 00:16:25 +00:00
Artem Dergachev ce42bd6765 [analyzer] MoveChecker: Enable by default as cplusplus.Move.
This checker warns you when you re-use an object after moving it.

Mostly developed by Peter Szecsi!

Differential Revision: https://reviews.llvm.org/D38675

llvm-svn: 349328
2018-12-17 06:30:39 +00:00
Artem Dergachev 2b500cbdf1 [analyzer] MoveChecker: Add an option to suppress warnings on locals.
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
2018-12-17 06:19:32 +00:00
Artem Dergachev 69909540a7 Speculatively re-apply "[analyzer] MoveChecker: Add checks for dereferencing..."
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
2018-12-17 05:25:23 +00:00
Artem Dergachev dda42164ec [analyzer] Fix some expressions staying live too long. Add a debug checker.
StaticAnalyzer uses the CFG-based RelaxedLiveVariables analysis in order to,
in particular, figure out values of which expressions are still needed.
When the expression becomes "dead", it is garbage-collected during
the dead binding scan.

Expressions that constitute branches/bodies of control flow statements,
eg. `E1' in `if (C1) E1;' but not `E2' in `if (C2) { E2; }', were kept alive
for too long. This caused false positives in MoveChecker because it relies
on cleaning up loop-local variables when they go out of scope, but some of those
live-for-too-long expressions were keeping a reference to those variables.

Fix liveness analysis to correctly mark these expressions as dead.

Add a debug checker, debug.DumpLiveStmts, in order to test expressions liveness.

Differential Revision: https://reviews.llvm.org/D55566

llvm-svn: 349320
2018-12-16 23:44:06 +00:00
Artem Dergachev fe5be58162 Revert "[analyzer] MoveChecker: Add checks for dereferencing a smart pointer..."
This reverts commit r349226.

Fails on an MSVC buildbot.

llvm-svn: 349233
2018-12-15 02:55:55 +00:00
Artem Dergachev ffba750a0e [analyzer] MoveChecker: Add checks for dereferencing a smart pointer after move.
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
2018-12-15 01:53:38 +00:00
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