summaryrefslogtreecommitdiffstats
path: root/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
Commit message (Collapse)AuthorAgeFilesLines
* 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
* [analyzer] Don't special-case ivars backing +0 properties.Jordan Rose2015-03-301-82/+1
| | | | | | | | | Give up this checking in order to continue tracking that these values came from direct ivar access, which will be important in the next commit. Part of rdar://problem/20335433 llvm-svn: 233591
* [analyzer] RetainCountChecker: Don't assume +0 for ivars backing readonly ↵Jordan Rose2015-03-201-12/+52
| | | | | | | | | | | | properties. Similarly, don't assume +0 if the property's setter is manually implemented. In both cases, if the property's ownership is explicitly written, then we /do/ assume the ivar has the same ownership. rdar://problem/20218183 llvm-svn: 232849
* [analyzer] RetainCountChecker: CF properties are always manually retain-counted.Jordan Rose2015-03-071-8/+13
| | | | | | | | | | | In theory we could assume a CF property is stored at +0 if there's not a custom setter, but that's not really worth the complexity. What we do know is that a CF property can't have ownership attributes, and so we shouldn't assume anything about the ownership of the ivar. rdar://problem/20076963 llvm-svn: 231553
* Sema: Parenthesized bound destructor member expressions can be calledDavid Majnemer2015-02-251-1/+1
| | | | | | | | | We would wrongfully reject (a.~A)() in both the destructor and pseudo-destructor cases. This fixes PR22668. llvm-svn: 230512
* [analyzer] RetainCountChecker: don't try to track ivars known to be nil.Jordan Rose2015-02-191-2/+4
| | | | | | | | | | | We expect in general that any nil value has no retain count information associated with it; violating this results in unexpected state unification /later/ when we decide to throw the information away. Unexpectedly caching out can lead to an assertion failure or crash. rdar://problem/19862648 llvm-svn: 229934
* Update APIs that return a pair of iterators to return an iterator_range instead.Benjamin Kramer2015-02-061-3/+2
| | | | | | Convert uses of those APIs into ranged for loops. NFC. llvm-svn: 228404
* [analyzer] Look for allocation site in the parent frames as well as the ↵Anna Zaks2015-02-051-20/+13
| | | | | | | | | | | | | current one. Instead of handling edge cases (mostly involving blocks), where we have difficulty finding an allocation statement, allow the allocation site to be in a parent node. Previously we assumed that the allocation site can always be found in the same frame as allocation, but there are scenarios in which an element is leaked in a child frame but is allocated in the parent. llvm-svn: 228247
* [analyzer] RetainCountChecker: be forgiving when ivars are accessed directly.Jordan Rose2015-02-041-82/+216
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A refinement of r204730, itself a refinement of r198953, to better handle cases where an object is accessed both through a property getter and through direct ivar access. An object accessed through a property should always be treated as +0, i.e. not owned by the caller. However, an object accessed through an ivar may be at +0 or at +1, depending on whether the ivar is a strong reference. Outside of ARC, we don't always have that information. The previous attempt would clear out the +0 provided by a getter, but only if that +0 hadn't already participated in other retain counting operations. (That is, "self.foo" is okay, but "[[self.foo retain] autorelease]" is problematic.) This turned out to not be good enough when our synthesized getters get involved. This commit drops the notion of "overridable" reference counting and instead just tracks whether a value ever came from a (strong) ivar. If it has, we allow one more release than we otherwise would. This has the added benefit of being able to catch /some/ overreleases of instance variables, though it's not likely to come up in practice. We do still get some false negatives because we currently throw away refcount state upon assigning a value into an ivar. We should probably improve on that in the future, especially once we synthesize setters as well as getters. rdar://problem/18075108 llvm-svn: 228174
* Use nullptr to silence -Wsentinel when self-hosting on WindowsReid Kleckner2014-12-011-7/+7
| | | | | | | | | | | Richard rejected my Sema change to interpret an integer literal zero in a varargs context as a null pointer, so -Wsentinel sees an integer literal zero and fires off a warning. Only CodeGen currently knows that it promotes integer literal zeroes in this context to pointer size on Windows. I didn't want to teach -Wsentinel about that compatibility hack. Therefore, I'm migrating to C++11 nullptr. llvm-svn: 223079
OpenPOWER on IntegriCloud