Commit Graph

3564 Commits

Author SHA1 Message Date
Kristof Umann f282d27215 [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp
Renaming collectCheckers to getEnabledCheckers
Changing the functionality to acquire all enabled checkers, rather then collect
checkers for a specific CheckerOptInfo (for example, collecting all checkers for
{ "core", true }, which meant enabling all checkers from the core package, which
was an unnecessary complication).
Removing CheckerOptInfo, instead of storing whether the option was claimed via a
field, we handle errors immediately, as getEnabledCheckers can now access a
DiagnosticsEngine. Realize that the remaining information it stored is directly
accessible through AnalyzerOptions.CheckerControlList.
Fix a test with -analyzer-disable-checker -verify accidentally left in.

llvm-svn: 349274
2018-12-15 15:44:05 +00:00
Gabor Horvath 21aa8db606 [analyzer] Assume that we always have a SubEngine available
The removed codepath was dead.

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

llvm-svn: 349266
2018-12-15 13:20:33 +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
Richard Trieu 41b1960a89 Move static analyzer core diagnostics to common.
llvm-svn: 349230
2018-12-15 02:30:16 +00:00
Artem Dergachev 46f34624d2 [analyzer] Fix unknown block calls to have zero parameters.
Right now they report to have one parameter with null decl,
because initializing an ArrayRef of pointers with a nullptr
yields an ArrayRef to an array of one null pointer.

Fixes a crash in the OSObject section of RetainCountChecker.

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

llvm-svn: 349229
2018-12-15 02:13:26 +00:00
Artem Dergachev 0ce45fae72 [analyzer] ObjCDealloc: Fix a crash when a class attempts to deallocate a class.
The checker wasn't prepared to see the dealloc message sent to the class itself
rather than to an instance, as if it was +dealloc.

Additionally, it wasn't prepared for pure-unknown or undefined self values.
The new guard covers that as well, but it is annoying to test because
both kinds of values shouldn't really appear and we generally want to
get rid of all of them (by modeling unknown values with symbols and
by warning on use of undefined values before they are used).

The CHECK: directive for FileCheck at the end of the test looks useless,
so i removed it.

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

llvm-svn: 349228
2018-12-15 02:09:02 +00:00
Artem Dergachev 5f500a33c1 [analyzer] ObjCContainers: Track index values.
Use trackExpressionValue() (previously known as trackNullOrUndefValue())
to track index value in the report, so that the user knew
what Static Analyzer thinks the index is.

Additionally, implement printState() to help debugging the checker later.

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

llvm-svn: 349227
2018-12-15 02:06:13 +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 b5b974e4b3 [analyzer] MoveChecker: NFC: De-duplicate a few checks.
No functional change intended.

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

llvm-svn: 349225
2018-12-15 01:50:58 +00:00
Aaron Ballman 847e73d69c Using llvm::find_if() instead of a range-based for loop; NFC.
This addresses post-commit review feedback from r349188.

llvm-svn: 349197
2018-12-14 21:14:44 +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
Aaron Ballman 3ccec59ec2 Update our SARIF support from 10-10 to 11-28.
Functional changes include:

* The run.files property is now an array instead of a mapping.
* fileLocation objects now have a fileIndex property specifying the array index into run.files.
* The resource.rules property is now an array instead of a mapping.
* The result object was given a ruleIndex property that is an index into the resource.rules array.
* rule objects now have their "id" field filled out in addition to the name field.
* Updated the schema and spec version numbers to 11-28.

llvm-svn: 349188
2018-12-14 20:34:23 +00:00
David Carlier 37a22ea063 [analyzer][CStringChecker] evaluate explicit_bzero
- explicit_bzero has limited scope/usage only for security/crypto purposes but is non-optimisable version of memset/0 and bzero.
- explicit_memset has similar signature and semantics as memset but is also a non-optimisable version.

Reviewers: NoQ

Reviewed By: NoQ

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

llvm-svn: 348884
2018-12-11 18:57:07 +00:00
George Karpenkov e8240f4df0 [analyzer] Remove memoization from RunLoopAutoreleaseLeakChecker
Memoization dose not seem to be necessary, as other statement visitors
run just fine without it,
and in fact seems to be causing memory corruptions.
Just removing it instead of investigating the root cause.

rdar://45945002

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

llvm-svn: 348822
2018-12-11 01:14:17 +00:00
George Karpenkov d1081ec508 [analyzer] Hack for backwards compatibility for options for RetainCountChecker.
To be removed once the clients update.

llvm-svn: 348821
2018-12-11 01:13:58 +00:00
George Karpenkov ff01486753 [analyzer] Display a diagnostics when an inlined function violates its os_consumed summary
This is currently a diagnostics, but might be upgraded to an error in the future,
especially if we introduce os_return_on_success attributes.

rdar://46359592

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

llvm-svn: 348820
2018-12-11 01:13:40 +00:00
George Karpenkov 79ed11c12e [analyzer] Resolve another bug where the name of the leaked object was not printed properly
Differential Revision: https://reviews.llvm.org/D55528

llvm-svn: 348819
2018-12-11 01:13:20 +00:00
Raphael Isemann b23ccecbb0 Misc typos fixes in ./lib folder
Summary: Found via `codespell -q 3 -I ../clang-whitelist.txt -L uint,importd,crasher,gonna,cant,ue,ons,orign,ned`

Reviewers: teemperor

Reviewed By: teemperor

Subscribers: teemperor, jholewinski, jvesely, nhaehnle, whisperity, jfb, cfe-commits

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

llvm-svn: 348755
2018-12-10 12:37:46 +00:00
George Karpenkov 041c9fa8ba Stop tracking retain count of OSObject after escape to void * / other primitive types
Escaping to void * / uint64_t / others non-OSObject * should stop tracking,
as such functions can have heterogeneous semantics depending on context,
and can not always be annotated.

rdar://46439133

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

llvm-svn: 348675
2018-12-08 01:18:40 +00:00
George Karpenkov 27db33075c [analyzer] Move out tracking retain count for OSObjects into a separate checker
Allow enabling and disabling tracking of ObjC/CF objects
separately from tracking of OS objects.

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

llvm-svn: 348638
2018-12-07 20:21:51 +00:00
George Karpenkov 936a9c978c [analyzer] RetainCountChecker: remove untested, unused, incorrect option IncludeAllocationLine
The option has no tests, is not used anywhere, and is actually
incorrect: it prints the line number without the reference to a file,
which can be outright incorrect.

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

llvm-svn: 348637
2018-12-07 20:21:37 +00:00
Gabor Marton 9419eb42c4 [CTU] Add DisplayCTUProgress analyzer switch
Summary:
With a new switch we may be able to print to stderr if a new TU is being loaded
during CTU.  This is very important for higher level scripts (like CodeChecker)
to be able to parse this output so they can create e.g. a zip file in case of
a Clang crash which contains all the related TU files.

Reviewers: xazax.hun, Szelethus, a_sidorin, george.karpenkov

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

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

llvm-svn: 348594
2018-12-07 14:56:02 +00:00
George Karpenkov b0b61955a1 [analyzer] Rely on os_consumes_this attribute to signify that the method call consumes a reference for "this"
Differential Revision: https://reviews.llvm.org/D55158

llvm-svn: 348533
2018-12-06 22:07:12 +00:00
George Karpenkov a71ec6c00a [analyzer] Fix an infinite recursion bug while checking parent methods in RetainCountChecker
Differential Revision: https://reviews.llvm.org/D55351

llvm-svn: 348531
2018-12-06 22:06:44 +00:00
George Karpenkov a717bc78b7 [analyzer] Attribute for RetainCountChecker for OSObject should propagate with inheritance
rdar://46388388

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

llvm-svn: 348396
2018-12-05 18:34:54 +00:00
Adam Balogh d5bd3f6354 [Analyzer] Iterator Checker - Forbid decrements past the begin() and increments past the end() of containers
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
2018-12-04 10:27:27 +00:00
Adam Balogh 42d241fc0b [Analyzer] Iterator Checkers - Use the region of the topmost base class for iterators stored in a region
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
2018-12-04 10:22:28 +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
Artem Dergachev ca3ace55dc [analyzer] Dump stable identifiers for objects under construction.
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
2018-12-03 22:23:21 +00:00
Artem Dergachev 057647d878 [AST] [analyzer] NFC: Reuse code in stable ID dumping methods.
Use the new fancy method introduced in r348197 to simplify some code.

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

llvm-svn: 348199
2018-12-03 22:19:05 +00:00
Artem Dergachev cf439eda2d Re-apply r347954 "[analyzer] Nullability: Don't detect post factum violation..."
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
2018-12-03 21:04:30 +00:00
Fangrui Song 407659ab0a Revert "Revert r347417 "Re-Reinstate 347294 with a fix for the failures.""
It seems the two failing tests can be simply fixed after r348037

Fix 3 cases in Analysis/builtin-functions.cpp
Delete the bad CodeGen/builtin-constant-p.c for now

llvm-svn: 348053
2018-11-30 23:41:18 +00:00
Fangrui Song f5d3335d75 Revert r347417 "Re-Reinstate 347294 with a fix for the failures."
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
2018-11-30 21:26:09 +00:00
Kristof Umann 549f9cd46f [analyzer] Evaluate all non-checker config options before analysis
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
2018-11-30 20:44:00 +00:00
George Karpenkov be3f4bd36b Revert "Reverting r347949-r347951 because they broke the test bots."
This reverts commit 5bad6129c012fbf186eb055be49344e790448ecc.

Hopefully fixing the issue which was breaking the bots.

llvm-svn: 348030
2018-11-30 20:43:42 +00:00
Kristof Umann 5f9981f8a5 [analyzer][PlistMacroExpansion] Part 5.: Support for # and ##
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
2018-11-30 19:21:35 +00:00
Aaron Ballman cd5115b74d Reverting r347949-r347951 because they broke the test bots.
http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/440/steps/ninja%20check%202/logs/FAIL%3A%20Clang%3A%3Aosobject-retain-release.cpp

llvm-svn: 348020
2018-11-30 18:52:51 +00:00
Mikael Holmen ebf787b138 Fix warning about unused variable [NFC]
llvm-svn: 347987
2018-11-30 13:38:33 +00:00
Adam Balogh 471d0864df lyzer] [HOTFIX!] SValBuilder crash when `aggressive-binary-operation-simplification` enabled
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
2018-11-30 10:37:44 +00:00
Haojian Wu ceff730fef Fix a use-after-scope bug.
llvm-svn: 347970
2018-11-30 09:23:01 +00:00
Artem Dergachev c076907384 Revert r344580 "[analyzer] Nullability: Don't detect post factum violation..."
Fails under ASan!

llvm-svn: 347956
2018-11-30 04:26:17 +00:00
Artem Dergachev e2b5438a73 [analyzer] MallocChecker: Avoid redundant transitions.
Don't generate a checker-tagged node unconditionally on the first
checkDeadSymbols callback when no pointers are tracked.

This is a tiny performance optimization; it may change the behavior slightly
by making Static Analyzer bail out on max-nodes one node later (which is good)
but any test would either break for no good reason or become useless
every time someone sneezes.

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

llvm-svn: 347955
2018-11-30 03:52:42 +00:00
Artem Dergachev 34d3576736 [analyzer] Nullability: Don't detect post factum violation on concrete values.
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
2018-11-30 03:39:58 +00:00
Artem Dergachev bbc6d68297 [analyzer] Fix the "Zombie Symbols" bug.
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
2018-11-30 03:27:50 +00:00
George Karpenkov 2620c60545 [analyzer] RetainCountChecker for OSObject model the "free" call
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
2018-11-30 02:19:16 +00:00