summaryrefslogtreecommitdiffstats
path: root/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
Commit message (Collapse)AuthorAgeFilesLines
* [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
* [analyzer] Pass value expression for inlined defensive checks when binding ↵Devin Coughlin2015-11-241-1/+30
| | | | | | | | | | | | | | | | null to nonnull. The nullability checker was not suppressing false positives resulting from inlined defensive checks when null was bound to a nonnull variable because it was passing the entire bind statement rather than the value expression to trackNullOrUndefValue(). This commit changes that checker to synactically match on the bind statement to extract the value expression so it can be passed to trackNullOrUndefValue(). rdar://problem/23575439 llvm-svn: 254007
* [analyzer] Add generateErrorNode() APIs to CheckerContext.Devin Coughlin2015-09-161-3/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* [Static Analyzer] Nullability checker optimization.Gabor Horvath2015-09-141-1/+6
| | | | | | Differential Revision: http://reviews.llvm.org/D12848 llvm-svn: 247612
* [Static Analyzer] Moving nullability checkers to a top level package.Gabor Horvath2015-09-141-10/+27
| | | | | | Differential Revision: http://reviews.llvm.org/D12852 llvm-svn: 247590
* [Static Analyzer] Fixed a typo in a diagnostic message.Gabor Horvath2015-09-111-1/+1
| | | | llvm-svn: 247444
* [Static Analyzer] Minor cleanups for the nullability checker.Gabor Horvath2015-09-111-46/+40
| | | | | | Differential Revision: http://reviews.llvm.org/D12619 llvm-svn: 247423
* [Static Analyzer] Remove sinks from nullability checks.Gabor Horvath2015-09-031-26/+146
| | | | | | Differential Revision: http://reviews.llvm.org/D12445 llvm-svn: 246818
* [Static Analyzer] Make NonNullParamChecker emit implicit null dereference ↵Gabor Horvath2015-08-271-1/+4
| | | | | | | | events. Differential Revision: http://reviews.llvm.org/D11433 llvm-svn: 246182
* [Static Analyzer] Checks to catch nullability related issues.Gabor Horvath2015-08-261-0/+838
Differential Revision: http://reviews.llvm.org/D11468 llvm-svn: 246105
OpenPOWER on IntegriCloud