Commit Graph

3608 Commits

Author SHA1 Message Date
Brian Gesiak 5488ab4ddd [AST] Remove ASTContext from getThisType (NFC)
Summary:
https://reviews.llvm.org/D54862 removed the usages of `ASTContext&` from
within the `CXXMethodDecl::getThisType` method. Remove the parameter
altogether, as well as all usages of it. This does not result in any
functional change because the parameter was unused since
https://reviews.llvm.org/D54862.

Test Plan: check-clang

Reviewers: akyrtzi, mikael

Reviewed By: mikael

Subscribers: mehdi_amini, dexonsmith, cfe-commits

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

llvm-svn: 350914
2019-01-11 01:54:53 +00:00
Richard Trieu f8b8b39c60 Fix header issues.
Several headers would fail to compile if other headers were not previously
included.  The usual issue is that a class is forward declared, but the
full definition is needed.  The requirement for the definition is use of
isa/dyn_cast or calling functions of pointer-packed data types such as
DenseMap or PointerIntPair.  Add missing includes to these headers.

SVals.h required an out-of-line method definition in the .cpp file to avoid
circular inclusion of headers with BasicValueFactory.h

llvm-svn: 350913
2019-01-11 01:32:35 +00:00
Artem Dergachev fc72007f43 [analyzer] pr38838, pr39976: Fix crash on diagnosing before implicit destructor.
We need to be able to emit the diagnostic at PreImplicitCall,
and the patch implements this functionality.

However, for now the need for emitting such diagnostics is not all that great:
it is only necessary to not crash when emitting a false positive due to an
unrelated issue of having dead symbol collection not working properly.

Coming up with a non-false-positive test seems impossible with the current
set of checkers, though it is likely to be needed for good things as well
in the future.

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

rdar://problem/46911462

llvm-svn: 350907
2019-01-10 23:44:44 +00:00
George Karpenkov 3c1e066dca [analyzer] [hotfix] Fix the tests
The error must have crept during the cherry-pick.

llvm-svn: 350870
2019-01-10 18:28:10 +00:00
George Karpenkov 0bb17c4634 [analyzer] Update the category name for RetainCountChecker reports
..now that it includes OSObjects

rdar://46509986

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

llvm-svn: 350869
2019-01-10 18:16:25 +00:00
George Karpenkov 42c6ac748e [analyzer] [NFC] [RetainCountChecker] Remove dead unused map
Differential Revision: https://reviews.llvm.org/D56402

llvm-svn: 350868
2019-01-10 18:16:10 +00:00
George Karpenkov 4f64b38f56 [analyzer] Quote the type of the leaked/problematic object in diagnostics for readability
Differential Revision: https://reviews.llvm.org/D56344

llvm-svn: 350867
2019-01-10 18:15:57 +00:00
George Karpenkov 7bac331b7e [analyzer] [NFC] Reduce redundancy in RetainSummaryManager by using a function
Differential Revision: https://reviews.llvm.org/D56282

llvm-svn: 350865
2019-01-10 18:15:30 +00:00
George Karpenkov 717c4c0e2b [analyzer] [RetainCountChecker] [NFC] Remove SummaryLog
The complicated machinery for passing the summary log around is actually
only used for one thing! To figure out whether the "dealloc" message was
sent.

Since I have tried to extend it for other uses and failed (it's actually
very hard to use), I think it's much better to simply use a tag and
remove the summary log altogether.

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

llvm-svn: 350864
2019-01-10 18:15:17 +00:00
George Karpenkov c2d8f1235b [analyzer] [RetainCountChecker] [NFC] Another minor cleanup
Differential Revision: https://reviews.llvm.org/D56224

llvm-svn: 350863
2019-01-10 18:15:04 +00:00
George Karpenkov 371bfd7295 [analyzer] [RetainCountChecker] [NFC] Refactor the way attributes are handled
Make sure all checks for attributes go through a centralized function,
which checks whether attribute handling is enabled, and performs
validation.  The type of the attribute is returned.

Sadly, metaprogramming is required as attributes have no sensible static
getters.

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

llvm-svn: 350862
2019-01-10 18:14:51 +00:00
George Karpenkov 4cb992e254 [analyzer] [RetainCountChecker] Remove redundant enum UnarySummaryKind
Differential Revision: https://reviews.llvm.org/D56072

llvm-svn: 350861
2019-01-10 18:14:38 +00:00
George Karpenkov 29e1ca83dd [analyzer] [RetainCountChecker] Remove obsolete "MakeCollectable" enum value
Differential Revision: https://reviews.llvm.org/D56071

llvm-svn: 350860
2019-01-10 18:14:25 +00:00
George Karpenkov 9cbcc21aed [analyzer] [RetainCountChecker] [NFC] Remove redundant enum items *Msg, as the object type is already communicated by a separate field
Differential Revision: https://reviews.llvm.org/D56070

llvm-svn: 350859
2019-01-10 18:14:12 +00:00
George Karpenkov 585a210e5f [analyzer] [NFC] Track object type with ArgEffect in RetainCountChecker.
This would be needed in the future.

https://reviews.llvm.org/D56040

llvm-svn: 350858
2019-01-10 18:13:59 +00:00
George Karpenkov 7e3016dec4 [analyzer] [NFC] Move ObjKind into a separate top-level enum in RetainSummaryManager.
Allows using it in future outside of RetEffect.

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

llvm-svn: 350857
2019-01-10 18:13:46 +00:00
Aaron Ballman 2c49462725 Correct the spelling of helpURI to helpUri.
JSON is case sensitive and the SARIF spec uses the corrected spelling.

llvm-svn: 350817
2019-01-10 13:19:48 +00:00
Rafael Stahl cc19f921b5 [analyzer] Pass the correct loc Expr from VisitIncDecOp to evalStore
Summary: The LocationE parameter of evalStore is documented as "The location expression that is stored to". When storing from an increment / decrement operator this was not satisfied. In user code this causes an inconsistency between the SVal and Stmt parameters of checkLocation.

Reviewers: NoQ, dcoughlin, george.karpenkov

Reviewed By: NoQ

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

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

llvm-svn: 350528
2019-01-07 15:07:01 +00:00
George Karpenkov b945db8afc [analyzer] [NFC] Clean up the mess of constructing argument effects in RetainCountChecker
Previously, argument effects were stored in a method variable, which was
effectively global.
The global state was reset at each (hopefully) entrance point to the
summary construction,
and every function could modify it.

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

llvm-svn: 350057
2018-12-24 18:45:18 +00:00
Artem Dergachev 02955afbb4 [analyzer] pr38668: Do not attempt to cast loaded integers to floats.
This patch is a different approach to landing the reverted r349701.

It is expected to have the same object (memory region) treated as if it has
different types in different program points. The correct behavior for
RegionStore when an object is stored as an object of type T1 but loaded as
an object of type T2 is to store the object as if it has type T1 but cast it
to T2 during load.

Note that the cast here is some sort of a "reinterpret_cast" (even in C). For
instance, if you store an integer and load a float, you won't get your integer
represented as a float; instead, you will get garbage.

Admit that we cannot perform the cast and return an unknown value.

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

rdar://problem/45062567

llvm-svn: 349984
2018-12-22 02:06:51 +00:00
George Karpenkov 255b05820c Revert "Revert rL349876 from cfe/trunk: [analyzer] Perform escaping in RetainCountChecker on type mismatch even for inlined functions"
This reverts commit b44b33f6e020a2c369da2b0c1d53cd52975f2526.

Revert the revert with the fix.

llvm-svn: 349939
2018-12-21 19:13:40 +00:00
George Karpenkov 79f0340c53 [analyzer] Correct the summary violation diagnostics for the retain count checker
It should be in the past tense.

llvm-svn: 349938
2018-12-21 19:13:28 +00:00
Simon Pilgrim 82fbb66465 Revert rL349876 from cfe/trunk: [analyzer] Perform escaping in RetainCountChecker on type mismatch even for inlined functions
The fix done in D55465 did not previously apply when the function was inlined.

rdar://46889541

Differential Revision: https://reviews.llvm.org/D55976
........
Fixes broken buildbot: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/14764

llvm-svn: 349894
2018-12-21 10:11:23 +00:00
George Karpenkov aecb371a0a [analyzer] Perform escaping in RetainCountChecker on type mismatch even for inlined functions
The fix done in D55465 did not previously apply when the function was inlined.

rdar://46889541

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

llvm-svn: 349876
2018-12-21 02:16:36 +00:00
George Karpenkov f508532627 [analyzer] Fix a bug in RetainCountDiagnostics while printing a note on mismatched summary in inlined functions
Previously, we were not printing a note at all if at least one of the parameters was not annotated.

rdar://46888422

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

llvm-svn: 349875
2018-12-21 02:16:23 +00:00
Artem Dergachev 212bbfad25 [analyzer] RetainCount: Suppress retain detection heuristic on some CM methods.
If it ends with "Retain" like CFRetain and returns a CFTypeRef like CFRetain,
then it is not necessarily a CFRetain. But it is indeed true that these two
return something retained.

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

rdar://problem/39390714

llvm-svn: 349862
2018-12-21 00:18:58 +00:00
Aaron Ballman 2f234cbfb0 Allow direct navigation to static analysis checker documentation through SARIF exports.
This adds anchors to all of the documented checks so that you can directly link to a check by a stable name. This is useful because the SARIF file format has a field for specifying a URI to documentation for a rule and some viewers, like CodeSonar, make use of this information. These links are then exposed through the SARIF exporter.

llvm-svn: 349812
2018-12-20 20:20:20 +00:00
Artem Dergachev b40e99af08 Revert "[analyzer] pr38668: Do not attempt to cast loaded values..."
This reverts commit r349701.

The patch was incorrect. The whole point of CastRetrievedVal()
is to handle the case in which the type from which the cast is made
(i.e., the "type" of value `V`) has nothing to do with the type of
the region it was loaded from (i.e., `R->getValueType()`).

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

rdar://problem/45062567

llvm-svn: 349798
2018-12-20 19:36:06 +00:00
Michal Gorny 5a409d0e30 Replace getOS() == llvm::Triple::*BSD with isOS*BSD() [NFCI]
Replace multiple comparisons of getOS() value with FreeBSD, NetBSD,
OpenBSD and DragonFly with matching isOS*BSD() methods.  This should
improve the consistency of coding style without changing the behavior.
Direct getOS() comparisons were left whenever used in switch or switch-
like context.

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

llvm-svn: 349752
2018-12-20 13:09:30 +00:00
Artem Dergachev 173f55693e [analyzer] pr38668: Do not attempt to cast loaded values of non-scalar types.
It is expected to have the same object (memory region) treated as if it has
different types in different program points. The correct behavior for
RegionStore when an object is stored as an object of type T1 but loaded as
an object of type T2 is to store the object as if it has type T1 but cast it
to T2 during load.

Note that the cast here is some sort of a "reinterpret_cast" (even in C). For
instance, if you store a float and load an integer, you won't have your float
rounded to an integer; instead, you will have garbage.

Admit that we cannot perform the cast as long as types we're dealing with are
non-trivial (neither integers, nor pointers).

Of course, if the cast is not necessary (eg, T1 == T2), we can still load the
value just fine.

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

rdar://problem/45062567

llvm-svn: 349701
2018-12-19 23:48:44 +00:00
Artem Dergachev b68cb5498b [analyzer] GenericTaint: Fix formatting to prepare for incoming improvements.
Patch by Gábor Borsik!

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

llvm-svn: 349698
2018-12-19 23:35:08 +00:00
Artem Dergachev 179064983a [analyzer] Improve modeling for returning an object from the top frame with RVO.
Static Analyzer processes the program function-by-function, sometimes diving
into other functions ("inlining" them). When an object is returned from an
inlined function, Return Value Optimization is modeled, and the returned object
is constructed at its return location directly.

When an object is returned from the function from which the analysis has started
(the top stack frame of the analysis), the return location is unknown. Model it
with a SymbolicRegion based on a conjured symbol that is specifically tagged for
that purpose, because this is generally the correct way to symbolicate
unknown locations in Static Analyzer.

Fixes leak false positives when an object is returned from top frame in C++17:
objects that are put into a SymbolicRegion-based memory region automatically
"escape" and no longer get reported as leaks. This only applies to C++17 return
values with destructors, because it produces a redundant CXXBindTemporaryExpr
in the call site, which confuses our liveness analysis. The actual fix
for liveness analysis is still pending, but it is no longer causing problems.

Additionally, re-enable temporary destructor tests in C++17.

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

rdar://problem/46217550

llvm-svn: 349696
2018-12-19 23:14:06 +00:00
Artem Dergachev be2c0c1968 [analyzer] CStringChecker: Fix a crash on C++ overloads of standard functions.
It turns out that it's not all that uncommon to have a C++ override of, say,
memcpy that receives a structure (or two) by reference (or by value, if it's
being copied from) and copies memory from it (or into it, if it's passed
by reference). In this case the argument will be of structure type (recall that
expressions of reference type do not exist: instead, C++ classifies expressions
into prvalues and lvalues and xvalues).

In this scenario we crash because we are trying to assume that, say,
a memory region is equal to an empty CompoundValue (the non-lazy one; this is
what makeZeroVal() return for compound types and it represents prvalue of
an object that is initialized with an empty initializer list).

Add defensive checks.

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

rdar://problem/45366551

llvm-svn: 349682
2018-12-19 21:50:46 +00:00
Artem Dergachev b4bde2acee [analyzer] MoveChecker: Squash the bit field because it causes a GCC warning.
The warning seems spurious (GCC bug 51242), but the bit field is
simply not worth the hassle.

rdar://problem/41349073

llvm-svn: 349394
2018-12-17 21:07:38 +00:00
Kristof Umann 9af1d2d10f Revert rC349281 '[analyzer][MallocChecker][NFC] Document and reorganize some functions'
Accidentally commited earlier with the same commit title, but really it
should've been
"Revert rC349283 '[analyzer][MallocChecker] Improve warning messages on double-delete errors'"

llvm-svn: 349344
2018-12-17 12:25:48 +00:00
Kristof Umann 09e86e77c9 Revert rC349281 '[analyzer][MallocChecker][NFC] Document and reorganize some functions'
llvm-svn: 349340
2018-12-17 12:07:57 +00:00
Kristof Umann a033466602 Reverting bitfield size to attempt to fix a windows buildbot
llvm-svn: 349336
2018-12-17 10:31:35 +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
Kristof Umann a82810c56b [analyzer][MallocChecker] Improve warning messages on double-delete errors
Differential Revision: https://reviews.llvm.org/D54834

llvm-svn: 349283
2018-12-15 18:41:37 +00:00
Kristof Umann cf6bb77f65 [analyzer][MallocChecker][NFC] Document and reorganize some functions
This patch merely reorganizes some things, and features no functional change.

In detail:

* Provided documentation, or moved existing documentation in more obvious
places.
* Added dividers. (the //===----------===// thing).
* Moved getAllocationFamily, printAllocDeallocName, printExpectedAllocName and
printExpectedDeallocName in the global namespace on top of the file where
AllocationFamily is declared, as they are very strongly related.
* Moved isReleased and MallocUpdateRefState near RefState's definition for the
same reason.
* Realloc modeling was very poor in terms of variable and structure naming, as
well as documentation, so I renamed some of them and added much needed docs.
* Moved function IdentifierInfos to a separate struct, and moved isMemFunction,
isCMemFunction adn isStandardNewDelete inside it. This makes the patch affect
quite a lot of lines, should I extract it to a separate one?
* Moved MallocBugVisitor out of MallocChecker.
* Preferred switches to long else-if branches in some places.
* Neatly organized some RUN: lines.

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

llvm-svn: 349281
2018-12-15 18:34:00 +00:00
Kristof Umann b0be2ab4d0 [analyzer][NFC] Merge ClangCheckerRegistry to CheckerRegistry
Now that CheckerRegistry lies in Frontend, we can finally eliminate
ClangCheckerRegistry. Fortunately, this also provides us with a
DiagnosticsEngine, so I went ahead and removed some parameters from it's
methods.

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

llvm-svn: 349280
2018-12-15 18:11:49 +00:00
Kristof Umann 76a21502fd [analyzer][NFC] Move CheckerRegistry from the Core directory to Frontend
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 &registry);

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
2018-12-15 16:23:51 +00:00
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