[clang] 1dceba3 - [analyzer] Fix false negative when accessing a nonnull property from … (#67563)

[via cfe-commits]

Author: tripleCC
Date: 2023-10-04T08:38:10+08:00
New Revision: 1dceba3a3684d12394731e09a6cf3efcebf07a3a

URL: https://github.com/llvm/llvm-project/commit/1dceba3a3684d12394731e09a6cf3efcebf07a3a
DIFF: https://github.com/llvm/llvm-project/commit/1dceba3a3684d12394731e09a6cf3efcebf07a3a.diff

LOG: [analyzer] Fix false negative when accessing a nonnull property from … (#67563)

```
@interface A : NSObject
@property (nonnull, nonatomic, strong) NSString *name;
+ (nullable instancetype)shared;
@end

@[[A shared].name];
```
Consider the code above, the nullability of the name property should
depend on the result of the shared method. A warning is expected because
of adding a nullable object to array.
ObjCMessageExpr gets the actual type through
Sema::getMessageSendResultType, instead of using the return type of
MethodDecl directly. The final type is generated by considering the
nullability of receiver and MethodDecl together.
Thus, the RetType in NullabilityChecker should all be replaced with
M.getOriginExpr()->getType().

Co-authored-by: tripleCC <triplecc at gmail.com>

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
    clang/test/Analysis/nullability.mm

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 906f4e85a8e5b5b..627b51af6bd44af 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -899,6 +899,14 @@ void NullabilityChecker::checkPostCall(const CallEvent &Call,
   const NullabilityState *TrackedNullability =
       State->get<NullabilityMap>(Region);
 
+  // ObjCMessageExpr gets the actual type through
+  // Sema::getMessageSendResultType, instead of using the return type of
+  // MethodDecl directly. The final type is generated by considering the
+  // nullability of receiver and MethodDecl together. Thus, The type of
+  // ObjCMessageExpr is prefer.
+  if (const Expr *E = Call.getOriginExpr())
+    ReturnType = E->getType();
+
   if (!TrackedNullability &&
       getNullabilityAnnotation(ReturnType) == Nullability::Nullable) {
     State = State->set<NullabilityMap>(Region, Nullability::Nullable);
@@ -1053,7 +1061,7 @@ void NullabilityChecker::checkPostObjCMessage(const ObjCMethodCall &M,
   }
 
   // No tracked information. Use static type information for return value.
-  Nullability RetNullability = getNullabilityAnnotation(RetType);
+  Nullability RetNullability = getNullabilityAnnotation(Message->getType());
 
   // Properties might be computed, which means the property value could
   // theoretically change between calls even in commonly-observed cases like

diff  --git a/clang/test/Analysis/nullability.mm b/clang/test/Analysis/nullability.mm
index 06bb9912296e32f..d69116d03df7465 100644
--- a/clang/test/Analysis/nullability.mm
+++ b/clang/test/Analysis/nullability.mm
@@ -55,6 +55,7 @@ - (void)takesUnspecified:(int *)p;
 @property(readonly, nullable) void (^propReturnsNullableBlock)(void);
 @property(readonly, nullable) int *propReturnsNullable;
 @property(readonly) int *propReturnsUnspecified;
++ (nullable TestObject *)getNullableObject;
 @end
 
 TestObject * getUnspecifiedTestObject();
@@ -256,6 +257,12 @@ void testObjCPropertyReadNullability() {
   case 8:
     [o takesNonnullBlock:o.propReturnsNullableBlock]; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
     break;
+  case 9:
+    [o takesNonnull:getNullableTestObject().propReturnsNonnull]; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
+    break;
+  case 10:
+    [o takesNonnull:[TestObject getNullableObject].propReturnsNonnull]; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
+    break;
   }
 }