[analyzer] Fix false negative when accessing a nonnull property from a nullable object

[tripleCC]

Summary

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 receiver and nullability of MethodDecl together. The RetType in NullabilityChecker should all be replaced with M.getOriginExpr()->getType().

@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 .

[steakhal]

For new revisions, prefer creating a PR at GitHub.

clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

906–907

I'm not sure if getOriginExpr is always non-null. How do you know?

[tripleCC]

adjust return type after checking getOriginExpr is non-null

[tripleCC]

clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

906–907

I'm not sure if getOriginExpr is always non-null. How do you know?

The getOriginExpr my be null , I optimized this logic to only perform the operation if getOriginExpr is not null.

[tripleCC]

In D159549#4651290, @steakhal wrote:

For new revisions, prefer creating a PR at GitHub.

ok, I will create a github pull request for the patch

[tripleCC]

merged in https://github.com/llvm/llvm-project/pull/67563#event-10544755217

clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

if (llvm::sys::path::filename(FilePath).startswith("CG")) {
     State = State->set<NullabilityMap>(Region, Nullability::Contradicted);
     C.addTransition(State);
     return;
   }
 
   const NullabilityState *TrackedNullability =
       State->get<NullabilityMap>(Region);
 
+  if (const Expr *E = Call.getOriginExpr())
+    ReturnType = E->getType();
+
   if (!TrackedNullability &&
       getNullabilityAnnotation(ReturnType) == Nullability::Nullable) {
     State = State->set<NullabilityMap>(Region, Nullability::Nullable);
     C.addTransition(State);
   }
 }
 
 static Nullability getReceiverNullability(const ObjCMethodCall &M,
                                           ProgramStateRef State) {
   if (M.isReceiverSelfOrSuper()) {
     // For super and super class receivers we assume that the receiver is
if (ComputedNullab != RetValTracked &&
       State = State->set<NullabilityMap>(
           ReturnRegion, NullabilityState(ComputedNullab, NullabilitySource));
       C.addTransition(State);
     }
     return;
   }
 
   // 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
   // this:
   //
   //     if (foo.prop) {    // ok, it's nonnull here...
   //         [bar doStuffWithNonnullVal:foo.prop];     // ...but what about
   //         here?

clang/test/Analysis/nullability.mm

 - (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;
++ (nullable TestObject *)getNullableObject;
 @end
 
 TestObject * getUnspecifiedTestObject();
 TestObject *_Nonnull getNonnullTestObject();
 TestObject *_Nullable getNullableTestObject();
 
 int getRandom();
 
void testObjCPropertyReadNullability() {
   case 7:
     // Assertions must be effective at suppressing warnings
     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;
+  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;
   }
 }
 
 Dummy * _Nonnull testDirectCastNullableToNonnull() {
   Dummy *p = returnsNullable();
   takesNonnull((Dummy * _Nonnull)p);  // no-warning
   return (Dummy * _Nonnull)p;         // no-warning
 }