Author: dcoughlin Date: Tue Dec 29 11:40:49 2015 New Revision: 256567 URL: http://llvm.org/viewvc/llvm-project?rev=256567&view=rev Log: [analyzer] Nullability: allow cast to _Nonnull to suppress warning about 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 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp cfe/trunk/test/Analysis/nullability.mm Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp?rev=256567&r1=256566&r2=256567&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp Tue Dec 29 11:40:49 2015 @@ -492,12 +492,21 @@ void NullabilityChecker::checkPreStmt(co NullConstraint Nullness = getNullConstraint(*RetSVal, State); - Nullability StaticNullability = + Nullability RequiredNullability = getNullabilityAnnotation(FuncType->getReturnType()); + // If the returned value is null but the type of the expression + // generating it is nonnull then we will suppress the diagnostic. + // This enables explicit suppression when returning a nil literal in a + // function with a _Nonnull return type: + // return (NSString * _Nonnull)0; + Nullability RetExprTypeLevelNullability = + getNullabilityAnnotation(RetExpr->getType()); + if (Filter.CheckNullReturnedFromNonnull && Nullness == NullConstraint::IsNull && - StaticNullability == Nullability::Nonnull) { + RetExprTypeLevelNullability != Nullability::Nonnull && + RequiredNullability == Nullability::Nonnull) { static CheckerProgramPointTag Tag(this, "NullReturnedFromNonnull"); ExplodedNode *N = C.generateErrorNode(State, &Tag); if (!N) @@ -518,7 +527,7 @@ void NullabilityChecker::checkPreStmt(co if (Filter.CheckNullableReturnedFromNonnull && Nullness != NullConstraint::IsNotNull && TrackedNullabValue == Nullability::Nullable && - StaticNullability == Nullability::Nonnull) { + RequiredNullability == Nullability::Nonnull) { static CheckerProgramPointTag Tag(this, "NullableReturnedFromNonnull"); ExplodedNode *N = C.addTransition(State, C.getPredecessor(), &Tag); reportBugIfPreconditionHolds(ErrorKind::NullableReturnedToNonnull, N, @@ -526,9 +535,10 @@ void NullabilityChecker::checkPreStmt(co } return; } - if (StaticNullability == Nullability::Nullable) { + if (RequiredNullability == Nullability::Nullable) { State = State->set<NullabilityMap>(Region, - NullabilityState(StaticNullability, S)); + NullabilityState(RequiredNullability, + S)); C.addTransition(State); } } @@ -564,13 +574,14 @@ void NullabilityChecker::checkPreCall(co NullConstraint Nullness = getNullConstraint(*ArgSVal, State); - Nullability ParamNullability = getNullabilityAnnotation(Param->getType()); - Nullability ArgStaticNullability = + Nullability RequiredNullability = + getNullabilityAnnotation(Param->getType()); + Nullability ArgExprTypeLevelNullability = getNullabilityAnnotation(ArgExpr->getType()); if (Filter.CheckNullPassedToNonnull && Nullness == NullConstraint::IsNull && - ArgStaticNullability != Nullability::Nonnull && - ParamNullability == Nullability::Nonnull) { + ArgExprTypeLevelNullability != Nullability::Nonnull && + RequiredNullability == Nullability::Nonnull) { ExplodedNode *N = C.generateErrorNode(State); if (!N) return; @@ -592,7 +603,7 @@ void NullabilityChecker::checkPreCall(co continue; if (Filter.CheckNullablePassedToNonnull && - ParamNullability == Nullability::Nonnull) { + RequiredNullability == Nullability::Nonnull) { ExplodedNode *N = C.addTransition(State); reportBugIfPreconditionHolds(ErrorKind::NullablePassedToNonnull, N, Region, C, ArgExpr, /*SuppressPath=*/true); @@ -608,10 +619,10 @@ void NullabilityChecker::checkPreCall(co continue; } // No tracked nullability yet. - if (ArgStaticNullability != Nullability::Nullable) + if (ArgExprTypeLevelNullability != Nullability::Nullable) continue; State = State->set<NullabilityMap>( - Region, NullabilityState(ArgStaticNullability, ArgExpr)); + Region, NullabilityState(ArgExprTypeLevelNullability, ArgExpr)); } if (State != OrigState) C.addTransition(State); Modified: cfe/trunk/test/Analysis/nullability.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullability.mm?rev=256567&r1=256566&r2=256567&view=diff ============================================================================== --- cfe/trunk/test/Analysis/nullability.mm (original) +++ cfe/trunk/test/Analysis/nullability.mm Tue Dec 29 11:40:49 2015 @@ -169,9 +169,33 @@ void testObjCMessageResultNullability() } } -void testCast() { +Dummy * _Nonnull testDirectCastNullableToNonnull() { + Dummy *p = returnsNullable(); + takesNonnull((Dummy * _Nonnull)p); // no-warning + return (Dummy * _Nonnull)p; // no-warning +} + +Dummy * _Nonnull testIndirectCastNullableToNonnull() { Dummy *p = (Dummy * _Nonnull)returnsNullable(); - takesNonnull(p); + takesNonnull(p); // no-warning + return p; // no-warning +} + +Dummy * _Nonnull testDirectCastNilToNonnull() { + takesNonnull((Dummy * _Nonnull)0); // no-warning + return (Dummy * _Nonnull)0; // no-warning +} + +void testIndirectCastNilToNonnullAndPass() { + Dummy *p = (Dummy * _Nonnull)0; + // FIXME: Ideally the cast above would suppress this warning. + takesNonnull(p); // expected-warning {{Null passed to a callee that requires a non-null argument}} +} + +Dummy * _Nonnull testIndirectCastNilToNonnullAndReturn() { + Dummy *p = (Dummy * _Nonnull)0; + // FIXME: Ideally the cast above would suppress this warning. + return p; // expected-warning {{Null is returned from a function that is expected to return a non-null value}} } void testInvalidPropagation() { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits