diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h index 55fd4b8880b54017ebdbb08f8d85a35bf5428f0a..7e6a3b9faafb77e02de7986a72831fe512e423a4 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -957,6 +957,11 @@ public: llvm_unreachable("Unknown message kind"); } + // Returns the property accessed by this method, either explicitly via + // property syntax or implicitly via a getter or setter method. Returns + // nullptr if the call is not a prooperty access. + const ObjCPropertyDecl *getAccessedProperty() const; + RuntimeDefinition getRuntimeDefinition() const override; bool argumentsMayEscape() const override; diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp index 59b90b5ce9876f9e7f629edf895ae518e8c85cb7..2c7b5302dd6f729ede82f8d26b9efb6bdbcbfa60 100644 --- a/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -678,9 +678,26 @@ ArrayRef<ParmVarDecl*> ObjCMethodCall::parameters() const { return D->parameters(); } -void -ObjCMethodCall::getExtraInvalidatedValues(ValueList &Values, - RegionAndSymbolInvalidationTraits *ETraits) const { +void ObjCMethodCall::getExtraInvalidatedValues( + ValueList &Values, RegionAndSymbolInvalidationTraits *ETraits) const { + + // If the method call is a setter for property known to be backed by + // an instance variable, don't invalidate the entire receiver, just + // the storage for that instance variable. + if (const ObjCPropertyDecl *PropDecl = getAccessedProperty()) { + if (const ObjCIvarDecl *PropIvar = PropDecl->getPropertyIvarDecl()) { + SVal IvarLVal = getState()->getLValue(PropIvar, getReceiverSVal()); + const MemRegion *IvarRegion = IvarLVal.getAsRegion(); + ETraits->setTrait( + IvarRegion, + RegionAndSymbolInvalidationTraits::TK_DoNotInvalidateSuperRegion); + ETraits->setTrait(IvarRegion, + RegionAndSymbolInvalidationTraits::TK_SuppressEscape); + Values.push_back(IvarLVal); + return; + } + } + Values.push_back(getReceiverSVal()); } @@ -740,6 +757,18 @@ const PseudoObjectExpr *ObjCMethodCall::getContainingPseudoObjectExpr() const { return ObjCMessageDataTy::getFromOpaqueValue(Data).getPointer(); } +static const Expr * +getSyntacticFromForPseudoObjectExpr(const PseudoObjectExpr *POE) { + const Expr *Syntactic = POE->getSyntacticForm(); + + // This handles the funny case of assigning to the result of a getter. + // This can happen if the getter returns a non-const reference. + if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(Syntactic)) + Syntactic = BO->getLHS(); + + return Syntactic; +} + ObjCMessageKind ObjCMethodCall::getMessageKind() const { if (!Data) { @@ -749,12 +778,7 @@ ObjCMessageKind ObjCMethodCall::getMessageKind() const { // Check if parent is a PseudoObjectExpr. if (const PseudoObjectExpr *POE = dyn_cast_or_null<PseudoObjectExpr>(S)) { - const Expr *Syntactic = POE->getSyntacticForm(); - - // This handles the funny case of assigning to the result of a getter. - // This can happen if the getter returns a non-const reference. - if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(Syntactic)) - Syntactic = BO->getLHS(); + const Expr *Syntactic = getSyntacticFromForPseudoObjectExpr(POE); ObjCMessageKind K; switch (Syntactic->getStmtClass()) { @@ -790,6 +814,27 @@ ObjCMessageKind ObjCMethodCall::getMessageKind() const { return static_cast<ObjCMessageKind>(Info.getInt()); } +const ObjCPropertyDecl *ObjCMethodCall::getAccessedProperty() const { + // Look for properties accessed with property syntax (foo.bar = ...) + if ( getMessageKind() == OCM_PropertyAccess) { + const PseudoObjectExpr *POE = getContainingPseudoObjectExpr(); + assert(POE && "Property access without PseudoObjectExpr?"); + + const Expr *Syntactic = getSyntacticFromForPseudoObjectExpr(POE); + auto *RefExpr = cast<ObjCPropertyRefExpr>(Syntactic); + + if (RefExpr->isExplicitProperty()) + return RefExpr->getExplicitProperty(); + } + + // Look for properties accessed with method syntax ([foo setBar:...]). + const ObjCMethodDecl *MD = getDecl(); + if (!MD || !MD->isPropertyAccessor()) + return nullptr; + + // Note: This is potentially quite slow. + return MD->findPropertyDecl(); +} bool ObjCMethodCall::canBeOverridenInSubclass(ObjCInterfaceDecl *IDecl, Selector Sel) const { diff --git a/test/Analysis/properties.m b/test/Analysis/properties.m index 4fdbb69d87a19c1f6e53505cc2bd509a3b432817..d92f1a1a7e8d6ff4c3b0b56ec4352fae3c0b6e58 100644 --- a/test/Analysis/properties.m +++ b/test/Analysis/properties.m @@ -238,6 +238,75 @@ void testConsistencyAssign(Person *p) { } @end +//------ +// Setter ivar invalidation. +//------ + +@interface ClassWithSetters +// Note: These properties have implicit @synthesize implementations to be +// backed with ivars. +@property (assign) int propWithIvar1; +@property (assign) int propWithIvar2; + +@property (retain) NSNumber *retainedProperty; + +@end + +@interface ClassWithSetters (InOtherTranslationUnit) +// The implementation of this property is in another translation unit. +// We don't know whether it is backed by an ivar or not. +@property (assign) int propInOther; +@end + +@implementation ClassWithSetters +- (void) testSettingPropWithIvarInvalidatesExactlyThatIvar; { + _propWithIvar1 = 1; + _propWithIvar2 = 2; + self.propWithIvar1 = 66; + + // Calling the setter of a property backed by the instance variable + // should invalidate the storage for the instance variable but not + // the rest of the receiver. Ideally we would model the setter completely + // but doing so would cause the new value to escape when it is bound + // to the ivar. This would cause bad false negatives in the retain count + // checker. (There is a test for this scenario in + // testWriteRetainedValueToRetainedProperty below). + clang_analyzer_eval(_propWithIvar1 == 66); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(_propWithIvar2 == 2); // expected-warning{{TRUE}} + + _propWithIvar1 = 1; + [self setPropWithIvar1:66]; + + clang_analyzer_eval(_propWithIvar1 == 66); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(_propWithIvar2 == 2); // expected-warning{{TRUE}} +} + +- (void) testSettingPropWithoutIvarInvalidatesEntireInstance; { + _propWithIvar1 = 1; + _propWithIvar2 = 2; + self.propInOther = 66; + + clang_analyzer_eval(_propWithIvar1 == 66); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(_propWithIvar2 == 2); // expected-warning{{UNKNOWN}} + + _propWithIvar1 = 1; + _propWithIvar2 = 2; + [self setPropInOther:66]; + + clang_analyzer_eval(_propWithIvar1 == 66); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(_propWithIvar2 == 2); // expected-warning{{UNKNOWN}} +} + +#if !__has_feature(objc_arc) +- (void) testWriteRetainedValueToRetainedProperty; { + NSNumber *number = [[NSNumber alloc] initWithInteger:5]; // expected-warning {{Potential leak of an object stored into 'number'}} + + // Make sure we catch this leak. + self.retainedProperty = number; +} +#endif +@end + #if !__has_feature(objc_arc) void testOverrelease(Person *p, int coin) { switch (coin) {