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

[via cfe-commits]

https://github.com/tripleCC updated 
https://github.com/llvm/llvm-project/pull/67563

>From 0783db5bcaed863cd0cd5c5a2e0d8cc3355e7f51 Mon Sep 17 00:00:00 2001
From: tripleCC <tripl...@gmail.com>
Date: Wed, 27 Sep 2023 22:05:19 +0800
Subject: [PATCH] [analyzer] Fix false negative when accessing a nonnull
 property from a nullable object

---
 .../lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp | 10 +++++++++-
 clang/test/Analysis/nullability.mm                     |  7 +++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

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;
   }
 }
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits