summaryrefslogtreecommitdiffstats
path: root/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
Commit message (Collapse)AuthorAgeFilesLines
* [analyzer] Nullability: Don't infer nullable when passing as nullable parameter.Artem Dergachev2019-11-081-5/+0
| | | | You can't really infer anything from that.
* [analyzer][NFC] Fix inconsistent references to checkers as "checks"Kristof Umann2019-09-121-7/+7
| | | | | | | | | | | | | | Traditionally, clang-tidy uses the term check, and the analyzer uses checker, but in the very early years, this wasn't the case, and code originating from the early 2010's still incorrectly refer to checkers as checks. This patch attempts to hunt down most of these, aiming to refer to checkers as checkers, but preserve references to callback functions (like checkPreCall) as checks. Differential Revision: https://reviews.llvm.org/D67140 llvm-svn: 371760
* [analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic.Artem Dergachev2019-09-111-1/+1
| | | | | | | | | | These static functions deal with ExplodedNodes which is something we don't want the PathDiagnostic interface to know anything about, as it's planned to be moved out of libStaticAnalyzerCore. Differential Revision: https://reviews.llvm.org/D67382 llvm-svn: 371659
* [analyzer] NFC: Re-implement stack hints as a side map in BugReport.Artem Dergachev2019-09-111-2/+1
| | | | | | | | | | That's one of the few random entities in the PathDiagnostic interface that are specific to the Static Analyzer. By moving them out we could let everybody use path diagnostics without linking against Static Analyzer. Differential Revision: https://reviews.llvm.org/D67381 llvm-svn: 371658
* [analyzer] NFC: Introduce sub-classes for path-sensitive and basic reports.Artem Dergachev2019-09-091-3/+4
| | | | | | | | | | | | | Checkers are now required to specify whether they're creating a path-sensitive report or a path-insensitive report by constructing an object of the respective type. This makes BugReporter more independent from the rest of the Static Analyzer because all Analyzer-specific code is now in sub-classes. Differential Revision: https://reviews.llvm.org/D66572 llvm-svn: 371450
* [Clang] Migrate llvm::make_unique to std::make_uniqueJonas Devlieghere2019-08-141-2/+2
| | | | | | | | | | Now that we've moved to C++14, we no longer need the llvm::make_unique implementation from STLExtras.h. This patch is a mechanical replacement of (hopefully) all the llvm::make_unique instances across the monorepo. Differential revision: https://reviews.llvm.org/D66259 llvm-svn: 368942
* [analyzer][NFC] Refactoring BugReporter.cpp P3.: ↵Kristof Umann2019-08-131-7/+5
| | | | | | | | | | | | | std::shared_pointer<PathDiagnosticPiece> -> PathDiagnosticPieceRef find clang/ -type f -exec sed -i 's/std::shared_ptr<PathDiagnosticPiece>/PathDiagnosticPieceRef/g' {} \; git diff -U3 --no-color HEAD^ | clang-format-diff-6.0 -p1 -i Just as C++ is meant to be refactored, right? Differential Revision: https://reviews.llvm.org/D65381 llvm-svn: 368717
* Fix parameter name comments using clang-tidy. NFC.Rui Ueyama2019-07-161-1/+1
| | | | | | | | | | | | | | | | | | | | | This patch applies clang-tidy's bugprone-argument-comment tool to LLVM, clang and lld source trees. Here is how I created this patch: $ git clone https://github.com/llvm/llvm-project.git $ cd llvm-project $ mkdir build $ cd build $ cmake -GNinja -DCMAKE_BUILD_TYPE=Debug \ -DLLVM_ENABLE_PROJECTS='clang;lld;clang-tools-extra' \ -DCMAKE_EXPORT_COMPILE_COMMANDS=On -DLLVM_ENABLE_LLD=On \ -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ ../llvm $ ninja $ parallel clang-tidy -checks='-*,bugprone-argument-comment' \ -config='{CheckOptions: [{key: StrictMode, value: 1}]}' -fix \ ::: ../llvm/lib/**/*.{cpp,h} ../clang/lib/**/*.{cpp,h} ../lld/**/*.{cpp,h} llvm-svn: 366177
* [analyzer] Remove the default value arg from getChecker*OptionKristof Umann2019-05-171-1/+1
| | | | | | | | | | | | | | | | | | | Since D57922, the config table contains every checker option, and it's default value, so having it as an argument for getChecker*Option is redundant. By the time any of the getChecker*Option function is called, we verified the value in CheckerRegistry (after D57860), so we can confidently assert here, as any irregularities detected at this point must be a programmer error. However, in compatibility mode, verification won't happen, so the default value must be restored. This implies something else, other than adding removing one more potential point of failure -- debug.ConfigDumper will always contain valid values for checker/package options! Differential Revision: https://reviews.llvm.org/D59195 llvm-svn: 361042
* [analyzer] Enable subcheckers to possess checker optionsKristof Umann2019-03-041-1/+1
| | | | | | | | | | | | | | | | Under the term "subchecker", I mean checkers that do not have a checker class on their own, like unix.MallocChecker to unix.DynamicMemoryModeling. Since a checker object was required in order to retrieve checker options, subcheckers couldn't possess options on their own. This patch is also an excuse to change the argument order of getChecker*Option, it always bothered me, now it resembles the actual command line argument (checkername:option=value). Differential Revision: https://reviews.llvm.org/D57579 llvm-svn: 355297
* Fix file headers. NFCFangrui Song2019-03-011-1/+1
| | | | llvm-svn: 355176
* [analyzer] Add CheckerManager::getChecker, make sure that a registry ↵Kristof Umann2019-01-261-1/+1
| | | | | | | | | | | | | function registers no more than 1 checker This patch effectively fixes the almost decade old checker naming issue. The solution is to assert when CheckerManager::getChecker is called on an unregistered checker, and assert when CheckerManager::registerChecker is called on a checker that is already registered. Differential Revision: https://reviews.llvm.org/D55429 llvm-svn: 352292
* [analyzer] Reimplement dependencies between checkersKristof Umann2019-01-261-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Unfortunately, up until now, the fact that certain checkers depended on one another was known, but how these actually unfolded was hidden deep within the implementation. For example, many checkers (like RetainCount, Malloc or CString) modelled a certain functionality, and exposed certain reportable bug types to the user. For example, while MallocChecker models many many different types of memory handling, the actual "unix.MallocChecker" checker the user was exposed to was merely and option to this modeling part. Other than this being an ugly mess, this issue made resolving the checker naming issue almost impossible. (The checker naming issue being that if a checker registered more than one checker within its registry function, both checker object recieved the same name) Also, if the user explicitly disabled a checker that was a dependency of another that _was_ explicitly enabled, it implicitly, without "telling" the user, reenabled it. Clearly, changing this to a well structured, declarative form, where the handling of dependencies are done on a higher level is very much preferred. This patch, among the detailed things later, makes checkers declare their dependencies within the TableGen file Checkers.td, and exposes the same functionality to plugins and statically linked non-generated checkers through CheckerRegistry::addDependency. CheckerRegistry now resolves these dependencies, makes sure that checkers are added to CheckerManager in the correct order, and makes sure that if a dependency is disabled, so will be every checker that depends on it. In detail: * Add a new field to the Checker class in CheckerBase.td called Dependencies, which is a list of Checkers. * Move unix checkers before cplusplus, as there is no forward declaration in tblgen :/ * Add the following new checkers: - StackAddrEscapeBase - StackAddrEscapeBase - CStringModeling - DynamicMemoryModeling (base of the MallocChecker family) - IteratorModeling (base of the IteratorChecker family) - ValistBase - SecuritySyntaxChecker (base of bcmp, bcopy, etc...) - NSOrCFErrorDerefChecker (base of NSErrorChecker and CFErrorChecker) - IvarInvalidationModeling (base of IvarInvalidation checker family) - RetainCountBase (base of RetainCount and OSObjectRetainCount) * Clear up and registry functions in MallocChecker, happily remove old FIXMEs. * Add a new addDependency function to CheckerRegistry. * Neatly format RUN lines in files I looked at while debugging. Big thanks to Artem Degrachev for all the guidance through this project! Differential Revision: https://reviews.llvm.org/D54438 llvm-svn: 352287
* [analyzer] Supply all checkers with a shouldRegister functionKristof Umann2019-01-261-1/+5
| | | | | | | | | | | | | | | | | | Introduce the boolean ento::shouldRegister##CHECKERNAME(const LangOptions &LO) function very similarly to ento::register##CHECKERNAME. This will force every checker to implement this function, but maybe it isn't that bad: I saw a lot of ObjC or C++ specific checkers that should probably not register themselves based on some LangOptions (mine too), but they do anyways. A big benefit of this is that all registry functions now register their checker, once it is called, registration is guaranteed. This patch is a part of a greater effort to reinvent checker registration, more info here: D54438#1315953 Differential Revision: https://reviews.llvm.org/D55424 llvm-svn: 352277
* Update the file headers across all of the LLVM projects in the monorepoChandler Carruth2019-01-191-4/+3
| | | | | | | | | | | | | | | | | to reflect the new license. We understand that people may be surprised that we're moving the header entirely to discuss the new license. We checked this carefully with the Foundation's lawyer and we believe this is the correct approach. Essentially, all code in the project is now made available by the LLVM project under our new license, so you will see that the license headers include that license only. Some of our contributors have contributed code under our old license, and accordingly, we have retained a copy of our old license notice in the top-level files in each project and repository. llvm-svn: 351636
* [analyzer][NFC] Move CheckerRegistry from the Core directory to FrontendKristof Umann2018-12-151-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Misc typos fixes in ./lib folderRaphael Isemann2018-12-101-1/+1
| | | | | | | | | | | | | | 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
* Re-apply r347954 "[analyzer] Nullability: Don't detect post factum violation..."Artem Dergachev2018-12-031-6/+16
| | | | | | | | | | | 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
* Revert r344580 "[analyzer] Nullability: Don't detect post factum violation..."Artem Dergachev2018-11-301-16/+6
| | | | | | Fails under ASan! llvm-svn: 347956
* [analyzer] Nullability: Don't detect post factum violation on concrete values.Artem Dergachev2018-11-301-6/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* [analyzer] Fix the "Zombie Symbols" bug.Artem Dergachev2018-11-301-3/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* [analyzer] Restrict AnalyzerOptions' interface so that non-checker objects ↵Kristof Umann2018-11-051-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | have to be registered One of the reasons why AnalyzerOptions is so chaotic is that options can be retrieved from the command line whenever and wherever. This allowed for some options to be forgotten for a looooooong time. Have you ever heard of "region-store-small-struct-limit"? In order to prevent this in the future, I'm proposing to restrict AnalyzerOptions' interface so that only checker options can be retrieved without special getters. I would like to make every option be accessible only through a getter, but checkers from plugins are a thing, so I'll have to figure something out for that. This also forces developers who'd like to add a new option to register it properly in the .def file. This is done by * making the third checker pointer parameter non-optional, and checked by an assert to be non-null. * I added new, but private non-checkers option initializers, meant only for internal use, * Renamed these methods accordingly (mind the consistent name for once with getBooleanOption!): - getOptionAsString -> getCheckerStringOption, - getOptionAsInteger -> getCheckerIntegerOption * The 3 functions meant for initializing data members (with the not very descriptive getBooleanOption, getOptionAsString and getOptionAsUInt names) were renamed to be overloads of the getAndInitOption function name. * All options were in some way retrieved via getCheckerOption. I removed it, and moved the logic to getStringOption and getCheckerStringOption. This did cause some code duplication, but that's the only way I could do it, now that checker and non-checker options are separated. Note that the non-checker version inserts the new option to the ConfigTable with the default value, but the checker version only attempts to find already existing entries. This is how it always worked, but this is clunky and I might end reworking that too, so we can eventually get a ConfigTable that contains the entire configuration of the analyzer. Differential Revision: https://reviews.llvm.org/D53483 llvm-svn: 346113
* [analyzer] Rename trackNullOrUndefValue to trackExpressionValueGeorge Karpenkov2018-10-231-1/+2
| | | | | | | | | | | | trackNullOrUndefValue is a long and confusing name, and it does not actually reflect what the function is doing. Give a function a new name, with a relatively clear semantics. Also remove some dead code. Differential Revision: https://reviews.llvm.org/D52758 llvm-svn: 345064
* [analyzer] [NFC] Remove unused parameters, as found by -Wunused-parameterGeorge Karpenkov2018-09-281-3/+1
| | | | | | Differential Revision: https://reviews.llvm.org/D52640 llvm-svn: 343353
* Port getLocStart -> getBeginLocStephen Kelly2018-08-091-2/+2
| | | | | | | | | | Reviewers: teemperor! Subscribers: jholewinski, whisperity, jfb, cfe-commits Differential Revision: https://reviews.llvm.org/D50350 llvm-svn: 339385
* [analyzer] Do not run visitors until the fixpoint, run only once.George Karpenkov2018-06-261-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* 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
* [analyzer] Trust _Nonnull annotations for system frameworkGeorge Karpenkov2018-03-231-29/+4
| | | | | | | | | | | Changes the analyzer to believe that methods annotated with _Nonnull from system frameworks indeed return non null objects. Local methods with such annotation are still distrusted. rdar://24291919 Differential Revision: https://reviews.llvm.org/D44341 llvm-svn: 328282
* [analyzer] introduce getSVal(Stmt *) helper on ExplodedNode, make sure the ↵George Karpenkov2018-01-171-4/+2
| | | | | | | | | | | | | | | | | | 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] Nullability: fix notes around synthesized ObjC property accessors.Artem Dergachev2017-06-051-1/+1
| | | | | | | | | | | | | Nullable-to-nonnull checks used to crash when the custom bug visitor was trying to add its notes to autosynthesized accessors of Objective-C properties. Now we avoid this, mostly automatically outside of checker control, by moving the diagnostic to the parent stack frame where the accessor has been called. Differential revision: https://reviews.llvm.org/D32437 llvm-svn: 304710
* [analyzer] Fix memory error bug category capitalization.Artem Dergachev2017-05-031-1/+1
| | | | | | | | | | | | It was written as "Memory Error" in most places and as "Memory error" in a few other places, however it is the latter that is more consistent with other categories (such as "Logic error"). rdar://problem/31718115 Differential Revision: https://reviews.llvm.org/D32702 llvm-svn: 302016
* Spelling mistakes in comments. NFCI. (PR27635)Simon Pilgrim2017-03-301-2/+2
| | | | llvm-svn: 299083
* Migrate PathDiagnosticPiece to std::shared_ptrDavid Blaikie2017-01-051-8/+11
| | | | | | | Simplifies and makes explicit the memory ownership model rather than implicitly passing/acquiring ownership. llvm-svn: 291143
* [analyzer] Refine the diagnostics in the nullability checker to ↵Anna Zaks2016-12-151-5/+10
| | | | | | | | | | | | differentiate between nil and null This is a big deal for ObjC, where nullability annotations are extensively used. I've also changed "Null" -> "null" and removed "is" as this is the pattern that Sema is using. Differential Revision: https://reviews.llvm.org/D27600 llvm-svn: 289885
* [analyzer] Fix typo in nullability checker diagnosticDevin Coughlin2016-12-071-1/+1
| | | | | | 'infered' --> 'inferred' llvm-svn: 288922
* [analyzer] Fix crash in NullabilityChecker calling block with too few argumentsDevin Coughlin2016-11-141-3/+4
| | | | | | | | | Fix a crash when checking parameter nullability on a block invocation with fewer arguments than the block declaration requires. rdar://problem/29237566 llvm-svn: 286901
* [analyzer] Small cleanups when checkers retrieving statements from explodedGabor Horvath2016-08-181-4/+1
| | | | | | | | nodes. Differential Revision: https://reviews.llvm.org/D23550 llvm-svn: 279037
* [analyzer] Nullability: Suppress diagnostic on bind with cast.Devin Coughlin2016-04-131-6/+28
| | | | | | | | | | | | | | Update the nullability checker to allow an explicit cast to nonnull to suppress a warning on an assignment of nil to a nonnull: id _Nonnull x = (id _Nonnull)nil; // no-warning This suppression as already possible for diagnostics on returns and function/method arguments. rdar://problem/25381178 llvm-svn: 266219
* [analyzer] Nullability: Treat nil _Nonnull ivar as invariant violation.Devin Coughlin2016-04-131-14/+56
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Treat a _Nonnull ivar that is nil as an invariant violation in a similar fashion to how a nil _Nonnull parameter is treated as a precondition violation. This avoids warning on defensive returns of nil on defensive internal checks, such as the following common idiom: @class InternalImplementation @interface PublicClass { InternalImplementation * _Nonnull _internal; } -(id _Nonnull)foo; @end @implementation PublicClass -(id _Nonnull)foo { if (!_internal) return nil; // no-warning return [_internal foo]; } @end rdar://problem/24485171 llvm-svn: 266157
* [analyzer] Nullability: Suppress return diagnostics in inlined functions.Devin Coughlin2016-04-121-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The nullability checker can sometimes miss detecting nullability precondition violations in inlined functions because the binding for the parameter that violated the precondition becomes dead before the return: int * _Nonnull callee(int * _Nonnull p2) { if (!p2) // p2 becomes dead here, so binding removed. return 0; // warning here because value stored in p2 is symbolic. else return p2; } int *caller(int * _Nonnull p1) { return callee(p1); } The fix, which is quite blunt, is to not warn about null returns in inlined methods/functions. This won’t lose much coverage for ObjC because the analyzer always analyzes each ObjC method at the top level in addition to inlined. It *will* lose coverage for C — but there aren’t that many codebases with C nullability annotations. rdar://problem/25615050 llvm-svn: 266109
* [analyzer] Nullability: Don't warn along paths where null returned from ↵Devin Coughlin2016-03-281-55/+79
| | | | | | | | | | | | | | | | non-null. Change the nullability checker to not warn along paths where null is returned from a method with a non-null return type, even when the diagnostic for this return has been suppressed. This prevents warning from methods with non-null return types that inline methods that themselves return nil but that suppressed the diagnostic. Also change the PreconditionViolated state component to be called "InvariantViolated" because it is set when a post-condition is violated, as well. rdar://problem/25393539 llvm-svn: 264647
* [analyzer] Nullability: add option to not report on calls to system headers.Devin Coughlin2016-03-051-2/+25
| | | | | | | | | | | | | | Add an -analyzer-config 'nullability:NoDiagnoseCallsToSystemHeaders' option to the nullability checker. When enabled, this option causes the analyzer to not report about passing null/nullable values to functions and methods declared in system headers. This option is motivated by the observation that large projects may have many nullability warnings. These projects may find warnings about nullability annotations that they have explicitly added themselves higher priority to fix than warnings on calls to system libraries. llvm-svn: 262763
* [analyzer] Improve Nullability checker diagnosticsAnna Zaks2016-01-291-32/+60
| | | | | | | | | - Include the position of the argument on which the nullability is violated - Differentiate between a 'method' and a 'function' in the message wording - Test for the error message text in the tests - Fix a bug with setting 'IsDirectDereference' which resulted in regular dereferences assumed to have call context. llvm-svn: 259221
* [analyzer] NullabilityChecker: Remove unused isReturnSelf() function.Devin Coughlin2016-01-281-16/+0
| | | | | | | Remove the now-unused isReturnSelf() function so we don't get a compiler warning. Apologies for not doing this in r259099. llvm-svn: 259118
* [analyzer] Suppress nullability warnings in copy, mutableCopy, and init ↵Devin Coughlin2016-01-281-11/+10
| | | | | | | | | | | | | | families. There are multiple, common idioms of defensive nil-checks in copy, mutableCopy, and init methods in ObjC. The analyzer doesn't currently have the capability to distinguish these idioms from true positives, so suppress all warnings about returns in those families. This is a pretty blunt suppression that we should improve later. rdar://problem/24395811 llvm-svn: 259099
* [analyzer] Suppress nullability warning for defensive super initializer idiom.Devin Coughlin2016-01-221-4/+33
| | | | | | | | | | | | | | | A common idiom in Objective-C initializers is for a defensive nil-check on the result of a call to a super initializer: if (self = [super init]) { ... } return self; To avoid warning on this idiom, the nullability checker now suppress diagnostics for returns of nil on syntactic 'return self' even in initializers with non-null return types. llvm-svn: 258461
* [analyzer] Nullability: Look through implicit casts when suppressing ↵Devin Coughlin2016-01-181-1/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | warnings on return. In r256567 I changed the nullability checker to suppress warnings about returning a null value from a function/method with a non-null return type when the type of the returned expression is itself nonnull. This enables the programmer to silence nullability warnings by casting to _Nonnull: return (SomeObject * _Nonnull)nil; Unfortunately, under ObjC automated reference counting, Sema adds implicit casts to _Nonnull to return expressions of nullable or unspecified types in functions with non-null function/method return types. With r256567, these casts cause all nullability warnings for returns of reference-counted types to be suppressed under ARC, leading to false negatives. This commit updates the nullability checker to look through implicit casts before determining the type of the returned expression. It also updates the tests to turn on ARC for the nullability_nullonly.mm testfile and adds a new testfile to test when ARC is turned off. rdar://problem/24200117 llvm-svn: 258061
* [analyzer] Check for return of nil in ObjC methods with nonnull return type.Devin Coughlin2016-01-151-20/+20
| | | | | | | | | | | | Update NullabilityChecker so that it checks return statements in ObjC methods. Previously it was returning early because methods do not have a function type. Also update detection of violated parameter _Nonnull preconditions to handle ObjC methods. rdar://problem/24200560 llvm-svn: 257938
* [analyzer] Suppress nullability warning for _Nonnull locals zero-initialized ↵Devin Coughlin2015-12-291-1/+44
| | | | | | | | | | | | | | | | | | | | | | | | | | by ObjC ARC. Prevent the analyzer from warning when a _Nonnnull local variable is implicitly zero-initialized because of Objective-C automated reference counting. This avoids false positives in cases where a _Nonnull local variable cannot be initialized with an initialization expression, such as: NSString * _Nonnull s; // no-warning @autoreleasepool { s = ...; } The nullability checker will still warn when a _Nonnull local variable is explicitly initialized with nil. This suppression introduces the potential for false negatives if the local variable is used before it is assigned a _Nonnull value. Based on a discussion with Anna Zaks, Jordan Rose, and John McCall, I've added a FIXME to treat implicitly zero-initialized _Nonnull locals as uninitialized in Sema's UninitializedValues analysis to avoid these false negatives. rdar://problem/23522311 llvm-svn: 256603
* [analyzer] Nullability: allow cast to _Nonnull to suppress warning about ↵Devin Coughlin2015-12-291-12/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | returning nil. The nullability checker currently allows casts to suppress warnings when a nil literal is passed as an argument to a parameter annotated as _Nonnull: foo((NSString * _Nonnull)nil); // no-warning It does so by suppressing the diagnostic when the *type* of the argument expression is _Nonnull -- even when the symbolic value returned is known to be nil. This commit updates the nullability checker to similarly honor such casts in the analogous scenario when nil is returned from a function with a _Nonnull return type: return (NSString * _Nonnull)nil; // no-warning This commit also normalizes variable naming between the parameter and return cases and adds several tests demonstrating the limitations of this suppression mechanism (such as when nil is cast to _Nonnull and then stored into a local variable without a nullability qualifier). These tests are marked with FIXMEs. rdar://problem/23176782 llvm-svn: 256567
OpenPOWER on IntegriCloud