Skip to content

Commit

Permalink
[StaticAnalyzer] Fix block pointer type nullability check
Browse files Browse the repository at this point in the history
This patch fixes a false negative when the property type
is an objective-c block pointer.

Patch By tripleCC!

Differential Revision: https://reviews.llvm.org/D151651
  • Loading branch information
tripleCC authored and steakhal committed May 30, 2023
1 parent 9c561e8 commit 993060e
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 7 deletions.
18 changes: 11 additions & 7 deletions clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,10 @@ static NullConstraint getNullConstraint(DefinedOrUnknownSVal Val,
return NullConstraint::Unknown;
}

static bool isValidPointerType(QualType T) {
return T->isAnyPointerType() || T->isBlockPointerType();
}

const SymbolicRegion *
NullabilityChecker::getTrackRegion(SVal Val, bool CheckSuperRegion) const {
if (!NeedTracking)
Expand Down Expand Up @@ -621,7 +625,7 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
if (!RetExpr)
return;

if (!RetExpr->getType()->isAnyPointerType())
if (!isValidPointerType(RetExpr->getType()))
return;

ProgramStateRef State = C.getState();
Expand Down Expand Up @@ -754,7 +758,7 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
if (!ArgSVal)
continue;

if (!Param->getType()->isAnyPointerType() &&
if (!isValidPointerType(Param->getType()) &&
!Param->getType()->isReferenceType())
continue;

Expand Down Expand Up @@ -841,7 +845,7 @@ void NullabilityChecker::checkPostCall(const CallEvent &Call,
if (!FuncType)
return;
QualType ReturnType = FuncType->getReturnType();
if (!ReturnType->isAnyPointerType())
if (!isValidPointerType(ReturnType))
return;
ProgramStateRef State = C.getState();
if (State->get<InvariantViolated>())
Expand Down Expand Up @@ -935,7 +939,7 @@ void NullabilityChecker::checkPostObjCMessage(const ObjCMethodCall &M,
if (!Decl)
return;
QualType RetType = Decl->getReturnType();
if (!RetType->isAnyPointerType())
if (!isValidPointerType(RetType))
return;

ProgramStateRef State = C.getState();
Expand Down Expand Up @@ -1089,9 +1093,9 @@ void NullabilityChecker::checkPostStmt(const ExplicitCastExpr *CE,
CheckerContext &C) const {
QualType OriginType = CE->getSubExpr()->getType();
QualType DestType = CE->getType();
if (!OriginType->isAnyPointerType())
if (!isValidPointerType(OriginType))
return;
if (!DestType->isAnyPointerType())
if (!isValidPointerType(DestType))
return;

ProgramStateRef State = C.getState();
Expand Down Expand Up @@ -1215,7 +1219,7 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S,
return;

QualType LocType = TVR->getValueType();
if (!LocType->isAnyPointerType())
if (!isValidPointerType(LocType))
return;

ProgramStateRef State = C.getState();
Expand Down
13 changes: 13 additions & 0 deletions clang/test/Analysis/nullability.mm
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@ - (int *_Nonnull)returnsNonnull;
- (int *_Nullable)returnsNullable;
- (int *)returnsUnspecified;
- (void)takesNonnull:(int *_Nonnull)p;
- (void)takesNonnullBlock:(void (^ _Nonnull)(void))block;
- (void)takesNullable:(int *_Nullable)p;
- (void)takesUnspecified:(int *)p;
@property(readonly, strong) NSString *stuff;
@property(readonly, nonnull) int *propReturnsNonnull;
@property(readonly, nonnull) void (^propReturnsNonnullBlock)(void);
@property(readonly, nullable) void (^propReturnsNullableBlock)(void);
@property(readonly, nullable) int *propReturnsNullable;
@property(readonly) int *propReturnsUnspecified;
@end
Expand All @@ -65,6 +68,7 @@ - (void)takesUnspecified:(int *)p;
void takesNullable(Dummy *_Nullable);
void takesNonnull(Dummy *_Nonnull);
void takesUnspecified(Dummy *);
void takesNonnullBlock(void (^ _Nonnull)(void));

Dummy *_Nullable returnsNullable();
Dummy *_Nonnull returnsNonnull();
Expand Down Expand Up @@ -197,6 +201,7 @@ void testObjCPropertyReadNullability() {
switch (getRandom()) {
case 0:
[o takesNonnull:o.propReturnsNonnull]; // no-warning
[o takesNonnullBlock:o.propReturnsNonnullBlock]; // no-warning
break;
case 1:
[o takesNonnull:o.propReturnsUnspecified]; // no-warning
Expand Down Expand Up @@ -236,6 +241,9 @@ void testObjCPropertyReadNullability() {
assert(o.propReturnsNullable);
[o takesNonnull:o.propReturnsNullable]; // no-warning
break;
case 8:
[o takesNonnullBlock:o.propReturnsNullableBlock]; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
break;
}
}

Expand Down Expand Up @@ -308,6 +316,11 @@ void testIndirectNilPassToNonnull() {
takesNonnull(p); // expected-warning {{Null passed to a callee that requires a non-null 1st parameter}}
}

void testBlockIndirectNilPassToNonnull() {
void (^p)(void) = nil;
takesNonnullBlock(p); // expected-warning {{Null passed to a callee that requires a non-null 1st parameter}}
}

void testConditionalNilPassToNonnull(Dummy *p) {
if (!p) {
takesNonnull(p); // expected-warning {{Null passed to a callee that requires a non-null 1st parameter}}
Expand Down

0 comments on commit 993060e

Please sign in to comment.