summaryrefslogtreecommitdiffstats
path: root/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
Commit message (Collapse)AuthorAgeFilesLines
* [analyzer] [NFC] Split up RetainCountCheckerGeorge Karpenkov2018-08-171-3893/+0
| | | | | | | | At some point, staring at 4k+ LOC file becomes a bit hard. Differential Revision: https://reviews.llvm.org/D50821 llvm-svn: 340092
* [analyzer] Drop support for GC mode in RetainCountCheckerGeorge Karpenkov2018-08-171-324/+61
| | | | | | | | | | | | A lot of code in RetainCountChecker deals with GC mode. Given that GC mode is deprecated, Apple does not ship runtime for it, and modern compiler toolchain does not support it, it makes sense to remove the code dealing with it in order to aid understanding of RetainCountChecker. Differential Revision: https://reviews.llvm.org/D50747 llvm-svn: 340091
* Port getLocStart -> getBeginLocStephen Kelly2018-08-091-1/+1
| | | | | | | | | | Reviewers: teemperor! Subscribers: jholewinski, whisperity, jfb, cfe-commits Differential Revision: https://reviews.llvm.org/D50350 llvm-svn: 339385
* Remove trailing spaceFangrui Song2018-07-301-1/+1
| | | | | | sed -Ei 's/[[:space:]]+$//' include/**/*.{def,h,td} lib/**/*.{cpp,h} llvm-svn: 338291
* [analyzer] Make checkEndFunction() give access to the return statement.Reka Kovacs2018-07-161-2/+3
| | | | | | Differential Revision: https://reviews.llvm.org/D49387 llvm-svn: 337215
* [analyzer] [NFC] A convenient getter for getting a current stack frameGeorge Karpenkov2018-06-271-3/+3
| | | | | | Differential Revision: https://reviews.llvm.org/D44756 llvm-svn: 335701
* [analyzer] Do not run visitors until the fixpoint, run only once.George Karpenkov2018-06-261-16/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | In the current implementation, we run visitors until the fixed point is reached. That is, if a visitor adds another visitor, the currently processed path is destroyed, all diagnostics is discarded, and it is regenerated again, until it's no longer modified. This pattern has a few negative implications: - This loop does not even guarantee to terminate. E.g. just imagine two visitors bouncing a diagnostics around. - Performance-wise, e.g. for sqlite3 all visitors are being re-run at least 10 times for some bugs. We have already seen a few reports where it leads to timeouts. - If we want to add more computationally intense visitors, this will become worse. - From architectural standpoint, the current layout requires copying visitors, which is conceptually wrong, and can be annoying (e.g. no unique_ptr on visitors allowed). The proposed change is a much simpler architecture: the outer loop processes nodes upwards, and whenever the visitor is added it only processes current nodes and above, thus guaranteeing termination. Differential Revision: https://reviews.llvm.org/D47856 llvm-svn: 335666
* [analyzer] RetainCount: Accept more "safe" CFRetain wrappers.Artem Dergachev2018-04-191-4/+5
| | | | | | | | | | | | | | | | | | | | | | | | r315736 added support for the misplaced CF_RETURNS_RETAINED annotation on CFRetain() wrappers. It works by trusting the function's name (seeing if it confirms to the CoreFoundation naming convention) rather than the annotation. There are more false positives caused by users using a different naming convention, namely starting the function name with "retain" or "release" rather than suffixing it with "retain" or "release" respectively. Because this isn't according to the naming convention, these functions are usually inlined and the annotation is therefore ignored, which is correct. But sometimes we run out of inlining stack depth and the function is evaluated conservatively and then the annotation is trusted. Add support for the "alternative" naming convention and test the situation when we're running out of inlining stack depth. rdar://problem/18270122 Differential Revision: https://reviews.llvm.org/D45117 llvm-svn: 330375
* Fix typos in clangAlexander Kornienko2018-04-061-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Found via codespell -q 3 -I ../clang-whitelist.txt Where whitelist consists of: archtype cas classs checkk compres definit frome iff inteval ith lod methode nd optin ot pres statics te thru Patch by luzpaz! (This is a subset of D44188 that applies cleanly with a few files that have dubious fixes reverted.) Differential revision: https://reviews.llvm.org/D44188 llvm-svn: 329399
* Resolve unused variable 'VR' warning in RetainCountChecker.cppBjorn Pettersson2018-03-181-1/+1
| | | | | | | | | Getting rid of error: unused variable 'VR' [-Werror,-Wunused-variable] warning/error at lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp:1933 llvm-svn: 327802
* [analyzer] Fix crashes in RetainCountChecker when underlying region is not a varGeorge Karpenkov2018-03-161-8/+12
| | | | | | | | | | | | For other regions, the error message contains a good indication of the problem, and there, in general, nothing helpful we can print. Error pointer to the problematic expression seems enough. rdar://37323555 Differential Revision: https://reviews.llvm.org/D44409 llvm-svn: 327727
* [analyzer] NFC: RetainCount: Protect from dumping raw region to path notes.Artem Dergachev2018-01-181-2/+8
| | | | | | | | | | | | | | | | | | MemRegion::getString() is a wrapper around MemRegion::dump(), which is not user-friendly and should never be used for diagnostic messages. Actual cases where raw dumps were reaching the user were unintentionally fixed in r315736; these were noticed accidentally and shouldn't be reproducible anymore. For now RetainCountChecker only tracks pointers through variable regions, and for those dumps are "fine". However, we should still use a less dangerous method for producing our path notes. This patch replaces the dump with printing a variable name, asserting that this is indeed a variable. Differential Revision: https://reviews.llvm.org/D42015 llvm-svn: 322799
* [analyzer] introduce getSVal(Stmt *) helper on ExplodedNode, make sure the ↵George Karpenkov2018-01-171-7/+4
| | | | | | | | | | | | | | | | | | helper is used consistently In most cases using `N->getState()->getSVal(E, N->getLocationContext())` is ugly, verbose, and also opens up more surface area for bugs if an inconsistent location context is used. This patch introduces a helper on an exploded node, and ensures consistent usage of either `ExplodedNode::getSVal` or `CheckContext::getSVal` across the codebase. As a result, a large number of redundant lines is removed. Differential Revision: https://reviews.llvm.org/D42155 llvm-svn: 322753
* [analyzer] Teach RetainCountChecker about CoreMedia APIsDevin Coughlin2017-11-251-4/+4
| | | | | | | | | | Teach the retain-count checker that CoreMedia reference types use CoreFoundation-style reference counting. This enables the checker to catch leaks and over releases of those types. rdar://problem/33599757 llvm-svn: 318979
* [analyzer] RetainCount: Ignore annotations on user-made CFRetain wrappers.Artem Dergachev2017-10-131-0/+5
| | | | | | | | | | | | | It is not uncommon for the users to make their own wrappers around CoreFoundation's CFRetain and CFRelease functions that are defensive against null references. In such cases CFRetain is often incorrectly marked as CF_RETURNS_RETAINED. Ignore said annotation and treat such wrappers similarly to the regular CFRetain. rdar://problem/31699502 Differential Revision: https://reviews.llvm.org/D38877 llvm-svn: 315736
* [Analyzer] Re-apply r314820 with a fix for StringRef lifetime.George Karpenkov2017-10-031-3/+6
| | | | | | | | | Fixes the test failure: temporary is now bound to std::string, tests fully pass on Linux. This reverts commit b36ee0924038e1d95ea74230c62d46e05f80587e. llvm-svn: 314859
* Revert r314820 "[Analyzer] More granular special casing in RetainCountChecker"Tim Shen2017-10-031-6/+3
| | | | | | | | The test retain-release.m fails with this patch. Differential Revision: https://reviews.llvm.org/D38487 llvm-svn: 314831
* [Analyzer] More granular special casing in RetainCountCheckerGeorge Karpenkov2017-10-031-3/+6
| | | | | | | | | Only assume that IOBSDNameMatching and friends increment a reference counter if their return type is a CFMutableDictionaryRef. Differential Revision: https://reviews.llvm.org/D38487 llvm-svn: 314820
* [Analyzer] Check function name size before indexing.George Karpenkov2017-09-151-1/+2
| | | | | | https://reviews.llvm.org/D37908 llvm-svn: 313385
* [analyzer] Add support for reference counting of parameters on the callee sideDevin Coughlin2017-08-171-9/+90
| | | | | | | | | | | | | | | This commit adds the functionality of performing reference counting on the callee side for Integer Set Library (ISL) to Clang Static Analyzer's RetainCountChecker. Reference counting on the callee side can be extensively used to perform debugging within a function (For example: Finding leaks on error paths). Patch by Malhar Thakkar! Differential Revision: https://reviews.llvm.org/D36441 llvm-svn: 311063
* [analyzer] Add diagnostic text for generalized refcount annotations.Devin Coughlin2017-07-251-11/+13
| | | | | | | | | | | | | | | Add a 'Generalized' object kind to the retain-count checker and suitable generic diagnostic text for retain-count diagnostics involving those objects. For now the object kind is introduced in summaries by 'annotate' attributes. Once we have more experience with these annotations we will propose explicit attributes. Patch by Malhar Thakkar! Differential Revision: https://reviews.llvm.org/D35613 llvm-svn: 308990
* [analyzer] Add annotation attribute to trust retain count implementationDevin Coughlin2017-07-191-4/+31
| | | | | | | | | | | | | | | | | Add support to the retain-count checker for an annotation indicating that a function's implementation should be trusted by the retain count checker. Functions with these attributes will not be inlined and the arguments will be treating as escaping. Adding this annotation avoids spurious diagnostics when the implementation of a reference counting operation is visible but the analyzer can't reason precisely about the ref count. Patch by Malhar Thakkar! Differential Revision: https://reviews.llvm.org/D34937 llvm-svn: 308416
* Suppress all uses of LLVM_END_WITH_NULL. NFC.Serge Guelton2017-05-091-26/+21
| | | | | | | | | | Use variadic templates instead of relying on <cstdarg> + sentinel. This enforces better type checking and makes code more readable. Differential revision: https://reviews.llvm.org/D32550 llvm-svn: 302572
* [analyzer] Add LocationContext as a parameter to checkRegionChangesAnna Zaks2017-01-131-5/+7
| | | | | | | | | | | This patch adds LocationContext to checkRegionChanges and removes wantsRegionChangeUpdate as it was unused. A patch by Krzysztof Wiśniewski! Differential Revision: https://reviews.llvm.org/D27090 llvm-svn: 291869
* Migrate PathDiagnosticPiece to std::shared_ptrDavid Blaikie2017-01-051-11/+10
| | | | | | | Simplifies and makes explicit the memory ownership model rather than implicitly passing/acquiring ownership. llvm-svn: 291143
* [analyzer] Include type name in Retain Count Checker diagnosticsAnna Zaks2016-12-151-2/+14
| | | | | | | | | The more detailed diagnostic will make identifying which object the diagnostics refer to easier. Differential Revision: https://reviews.llvm.org/D27740 llvm-svn: 289883
* [analyzer] Add dispatch_data_create as a special case in RetainCountChecker.Artem Dergachev2016-12-081-1/+4
| | | | | | | | | | | | | This function receives a callback block. The analyzer suspects that this block may be used to take care of releasing the libdispatch object returned from the function. In fact, it doesn't - it only releases the raw data buffer. Inform the analyzer about that. Fixes the resulting false negatives. rdar://problem/22280098 Differential Revision: https://reviews.llvm.org/D27409 llvm-svn: 289047
* [analyzer] Remove an unused enum value in RetainCountChecker.Artem Dergachev2016-12-071-9/+8
| | | | | | | | No functional change intended. Differential Revision: https://reviews.llvm.org/D27408 llvm-svn: 288917
* [analyzer] Remove unused check::RegionChanges::wantsRegionChangeUpdate callbackAnna Zaks2016-11-161-4/+0
| | | | | | | | | | | Remove the check::RegionChanges::wantsRegionChangeUpdate callback as it is no longer used (since checkPointerEscape has been added). A patch by Krzysztof Wiśniewski! Differential Revision: https://reviews.llvm.org/D26759 llvm-svn: 287175
* [analyzer] Update 'Automated' to 'Automatic' from r286694.Devin Coughlin2016-11-121-1/+1
| | | | | | ARC is 'Automatic Reference Counting' and not 'Automated Reference Counting'. llvm-svn: 286700
* [analyzer] Improve misleading RetainCountChcker diagnostic under ARCDevin Coughlin2016-11-121-4/+9
| | | | | | | | | | | | | | | | | | | | Under automated reference counting the analyzer treats a methods -- even those starting with 'copy' and friends -- as returning an unowned value. This is because ownership of CoreFoundation objects must be transferred to ARC with __bridge_transfer or CFBridgingRelease() before being returned as ARC-managed bridged objects. Unfortunately this could lead to a poor diagnostic inside copy methods under ARC where the analyzer would complain about a leak of a returned CF value inside a method "whose name does not start with 'copy'" -- even though the name did start with 'copy'. This commit improves the diagnostic under ARC to say inside a method "returned from a method managed by Automated Reference Counting". rdar://problem/28849667 llvm-svn: 286694
* [analyzer] Teach RetainCountChecker about VTCompressionSessionEncodeFrame()Devin Coughlin2016-11-111-0/+8
| | | | | | | | | | | | | | | The context argument passed to VideoToolbox's VTCompressionSessionEncodeFrame() function is ultimately passed to a callback supplied when creating the compression session and so may be freed by that callback. To suppress false positives in this case, teach the retain count checker to stop tracking that argument. This isn't suppressed by the usual callback context mechanism because the call to VTCompressionSessionEncodeFrame() doesn't include the callback itself. rdar://problem/27685213 llvm-svn: 286633
* [analyzer] Fix crash in RetainCountChecker::checkEndFunctionAlexander Shaposhnikov2016-09-231-1/+1
| | | | | | | | | | | | | | | | | The class BodyFarm creates bodies for OSAtomicCompareAndSwap*, objc_atomicCompareAndSwap*, dispatch_sync*, dispatch_once* and for them the flag isBodyAutosynthesized is set to true. This diff 1. makes AnalysisConsumer::HandleCode skip the autosynthesized code 2. replaces assert(LCtx->getParent()) in RetainCountChecker::checkEndFunction by assert(!LCtx->inTopFrame()) (minor cleanup) Test plan: make -j8 check-clang-analysis Differential revision: https://reviews.llvm.org/D24792 llvm-svn: 282293
* [analyzer] Small cleanups when checkers retrieving statements from explodedGabor Horvath2016-08-181-6/+1
| | | | | | | | nodes. Differential Revision: https://reviews.llvm.org/D23550 llvm-svn: 279037
* [analyzer] Teach RetainCountChecker about CVFooRetainDevin Coughlin2016-08-111-4/+6
| | | | | | | | | | | | Change the retain count checker to treat CoreFoundation-style "CV"-prefixed reference types from CoreVideo similarly to CoreGraphics types. With this change, we treat CVFooRetain() on a CVFooRef type as a retain. CVFooRelease() APIs are annotated as consuming their parameter, so this change prevents false positives about incorrect decrements of reference counts. <rdar://problem/27116090> llvm-svn: 278382
* [analyzer] Implement a methond to discover origin region of a symbol.Artem Dergachev2016-07-131-9/+1
| | | | | | | | | | | | | This encourages checkers to make logical decisions depending on value of which region was the symbol under consideration introduced to denote. A similar technique is already used in a couple of checkers; they were modified to call the new method. Differential Revision: http://reviews.llvm.org/D22242 llvm-svn: 275290
* Apply clang-tidy's misc-move-constructor-init throughout Clang.Benjamin Kramer2016-05-271-2/+3
| | | | | | No functionality change intended, maybe a tiny performance improvement. llvm-svn: 270996
* Put global classes into the appropriate namespace.Benjamin Kramer2015-10-281-0/+2
| | | | | | | Most of the cases belong into an anonymous namespace. No functionality change intended. llvm-svn: 251514
* Fix Clang-tidy modernize-use-nullptr warnings in source directories; other ↵Hans Wennborg2015-10-061-8/+10
| | | | | | | | | | minor cleanups Patch by Eugene Zelenko! Differential Revision: http://reviews.llvm.org/D13406 llvm-svn: 249484
* [analyzer] Add generateErrorNode() APIs to CheckerContext.Devin Coughlin2015-09-161-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The analyzer trims unnecessary nodes from the exploded graph before reporting path diagnostics. However, in some cases it can trim all nodes (including the error node), leading to an assertion failure (see https://llvm.org/bugs/show_bug.cgi?id=24184). This commit addresses the issue by adding two new APIs to CheckerContext to explicitly create error nodes. Unless the client provides a custom tag, these APIs tag the node with the checker's tag -- preventing it from being trimmed. The generateErrorNode() method creates a sink error node, while generateNonFatalErrorNode() creates an error node for a path that should continue being explored. The intent is that one of these two methods should be used whenever a checker creates an error node. This commit updates the checkers to use these APIs. These APIs (unlike addTransition() and generateSink()) do not take an explicit Pred node. This is because there are not any error nodes in the checkers that were created with an explicit different than the default (the CheckerContext's Pred node). It also changes generateSink() to require state and pred nodes (previously these were optional) to reduce confusion. Additionally, there were several cases where checkers did check whether a generated node could be null; we now explicitly check for null in these places. This commit also includes a test case written by Ying Yi as part of http://reviews.llvm.org/D12163 (that patch originally addressed this issue but was reverted because it introduced false positive regressions). Differential Revision: http://reviews.llvm.org/D12780 llvm-svn: 247859
* [analyzer] Apply whitespace cleanups by Honggyu Kim.Ted Kremenek2015-09-081-54/+54
| | | | llvm-svn: 246978
* Wdeprecated: CollectReachableSymbolsCallback are move constructed/returned ↵David Blaikie2015-08-131-1/+1
| | | | | | | | | | | | | | by value, so make sure they're copy/moveable (return by value is in ExprEngine::processPointerEscapedOnBind and any other call to the scanReachableSymbols function template used there) Protect the special members in the base class to avoid slicing, and make derived classes final so these special members don't accidentally become public on an intermediate base which would open up the possibility of slicing again. llvm-svn: 244975
* Rewrite users of Stmt::child_begin/end into for-range loops.Benjamin Kramer2015-07-031-8/+5
| | | | | | No functionality change intended. llvm-svn: 241355
* Clarify pointer ownership semantics by hoisting the std::unique_ptr creation ↵Aaron Ballman2015-06-231-23/+16
| | | | | | to the caller instead of hiding it in emitReport. NFC. llvm-svn: 240400
* Revert r240270 ("Fixed/added namespace ending comments using clang-tidy").Alexander Kornienko2015-06-221-6/+4
| | | | llvm-svn: 240353
* Don't use &* when get() will suffice; NFC.Aaron Ballman2015-06-221-4/+4
| | | | llvm-svn: 240279
* Fixed/added namespace ending comments using clang-tidy. NFCAlexander Kornienko2015-06-221-4/+6
| | | | | | | | | | | | The patch is generated using this command: $ tools/extra/clang-tidy/tool/run-clang-tidy.py -fix \ -checks=-*,llvm-namespace-comment -header-filter='llvm/.*|clang/.*' \ work/llvm/tools/clang To reduce churn, not touching namespaces spanning less than 10 lines. llvm-svn: 240270
* Allow the cf_returns_[not_]retained attributes to appear on out-parameters.Douglas Gregor2015-06-191-5/+69
| | | | | | | | | | | | | | | | Includes a simple static analyzer check and not much else, but we'll also be able to take advantage of this in Swift. This feature can be tested for using __has_feature(cf_returns_on_parameters). This commit also contains two fixes: - Look through non-typedef sugar when deciding whether something is a CF type. - When (cf|ns)_returns(_not)?_retained is applied to invalid properties, refer to "property" instead of "method" in the error message. rdar://problem/18742441 llvm-svn: 240185
* Use 'override/final' instead of 'virtual' for overridden methodsAlexander Kornienko2015-04-111-3/+1
| | | | | | | | | | | | | | | | | | | | Summary: The patch is generated using clang-tidy misc-use-override check. This command was used: tools/clang/tools/extra/clang-tidy/tool/run-clang-tidy.py \ -checks='-*,misc-use-override' -header-filter='llvm|clang' -j=32 -fix Reviewers: dblaikie Reviewed By: dblaikie Subscribers: klimek, cfe-commits Differential Revision: http://reviews.llvm.org/D8926 llvm-svn: 234678
* [analyzer] Disable all retain count diagnostics on values that come from ivars.Jordan Rose2015-03-301-2/+41
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is imitating a pre-r228174 state where ivars are not considered tracked by default, but with the addition that even ivars /with/ retain count information (e.g. "[_ivar retain]; [ivar _release];") are not being tracked as well. This is to ensure that we don't regress on values accessed through both properties and ivars, which is what r228174 was trying to fix. The issue occurs in code like this: [_contentView retain]; [_contentView removeFromSuperview]; [self addSubview:_contentView]; // invalidates 'self' [_contentView release]; In this case, the call to -addSubview: may change the value of self->_contentView, and so the analyzer can't be sure that we didn't leak the original _contentView. This is a correct conservative view of the world, but not a useful one. Until we have a heuristic that allows us to not consider this a leak, not emitting a diagnostic is our best bet. This commit disables all of the ivar-related retain count tests, but does not remove them to ensure that we don't crash trying to evaluate either valid or erroneous code. The next commit will add a new test for the example above so that this commit (and the previous one) can be reverted wholesale when a better solution is implemented. Rest of rdar://problem/20335433 llvm-svn: 233592
OpenPOWER on IntegriCloud