Skip to content

Commit

Permalink
[analyzer] Fix false negative when pass implicit cast nil to nonnull
Browse files Browse the repository at this point in the history
We should look through implicit casts before determining the type of the arguments, and only allow explicit cast to _Nonnull to suppress warning

```
void foo(NSString *_Nonnull);

foo((NSString * _Nonnull)nil); // no-warning
id obj = nil;
foo(obj); // should warning here (implicit cast id to NSString *_Nonnull)

```

Reviewed By: xazax.hun, steakhal

Differential Revision: https://reviews.llvm.org/D154221
  • Loading branch information
songruiwang committed Jul 6, 2023
1 parent ab49d30 commit b22a5d4
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 12 deletions.
2 changes: 1 addition & 1 deletion clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
Nullability RequiredNullability =
getNullabilityAnnotation(Param->getType());
Nullability ArgExprTypeLevelNullability =
getNullabilityAnnotation(ArgExpr->getType());
getNullabilityAnnotation(lookThroughImplicitCasts(ArgExpr)->getType());

unsigned ParamIdx = Param->getFunctionScopeIndex() + 1;

Expand Down
13 changes: 2 additions & 11 deletions clang/test/Analysis/nullability-arc.mm
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,nullability\
// RUN: -analyzer-output=text -verify %s -fobjc-arc

#if !__has_feature(objc_arc)
// expected-no-diagnostics
#endif


#define nil ((id)0)
Expand All @@ -24,16 +21,10 @@ @implementation Derived
- (void)foo:(Param *)param {
// FIXME: Why do we not emit the warning under ARC?
[super foo:param];
#if __has_feature(objc_arc)
// expected-warning@-2{{nil passed to a callee that requires a non-null 1st parameter}}
// expected-note@-3 {{nil passed to a callee that requires a non-null 1st parameter}}
#endif

[self foo:nil];
#if __has_feature(objc_arc)
// expected-note@-2{{Calling 'foo:'}}
// expected-note@-3{{Passing nil object reference via 1st parameter 'param'}}
#endif
// expected-warning@-1{{nil passed to a callee that requires a non-null 1st parameter}}
// expected-note@-2 {{nil passed to a callee that requires a non-null 1st parameter}}
}
@end

12 changes: 12 additions & 0 deletions clang/test/Analysis/nullability.mm
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ - (void)takesUnspecified:(int *)p;
void takesNonnull(Dummy *_Nonnull);
void takesUnspecified(Dummy *);
void takesNonnullBlock(void (^ _Nonnull)(void));
void takesNonnullObject(NSObject *_Nonnull);

Dummy *_Nullable returnsNullable();
Dummy *_Nonnull returnsNonnull();
Expand Down Expand Up @@ -275,6 +276,17 @@ void testObjCPropertyReadNullability() {
return (Dummy * _Nonnull)0; // no-warning
}

void testImplicitCastNilToNonnull() {
id obj = nil;
takesNonnullObject(obj); // expected-warning {{nil passed to a callee that requires a non-null 1st parameter}}
}

void testImplicitCastNullableArgToNonnull(TestObject *_Nullable obj) {
if (!obj) {
takesNonnullObject(obj); // expected-warning {{nil passed to a callee that requires a non-null 1st parameter}}
}
}

void testIndirectCastNilToNonnullAndPass() {
Dummy *p = (Dummy * _Nonnull)0;
// FIXME: Ideally the cast above would suppress this warning.
Expand Down

0 comments on commit b22a5d4

Please sign in to comment.