Author: Artem Dergachev Date: 2019-11-21T18:59:46-08:00 New Revision: 0b58b80edb81bf8fb401f8a6e057ca9d50abc0f7
URL: https://github.com/llvm/llvm-project/commit/0b58b80edb81bf8fb401f8a6e057ca9d50abc0f7 DIFF: https://github.com/llvm/llvm-project/commit/0b58b80edb81bf8fb401f8a6e057ca9d50abc0f7.diff LOG: [analyzer] Fix Objective-C accessor body farms after 2073dd2d. Fix a canonicalization problem for the newly added property accessor stubs that was causing a wrong decl to be used for 'self' in the accessor's body farm. Fix a crash when constructing a body farm for accessors of a property that is declared and @synthesize'd in different (but related) interfaces. Differential Revision: https://reviews.llvm.org/D70158 Added: Modified: clang/lib/Analysis/BodyFarm.cpp clang/lib/StaticAnalyzer/Core/CallEvent.cpp clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist clang/test/Analysis/nullability-notes.m clang/test/Analysis/properties.m Removed: ################################################################################ diff --git a/clang/lib/Analysis/BodyFarm.cpp b/clang/lib/Analysis/BodyFarm.cpp index be065ed9330f..694913b3ac93 100644 --- a/clang/lib/Analysis/BodyFarm.cpp +++ b/clang/lib/Analysis/BodyFarm.cpp @@ -737,50 +737,65 @@ static const ObjCIvarDecl *findBackingIvar(const ObjCPropertyDecl *Prop) { } static Stmt *createObjCPropertyGetter(ASTContext &Ctx, - const ObjCPropertyDecl *Prop) { - // First, find the backing ivar. - const ObjCIvarDecl *IVar = findBackingIvar(Prop); - if (!IVar) - return nullptr; + const ObjCMethodDecl *MD) { + // First, find the backing ivar. + const ObjCIvarDecl *IVar = nullptr; + + // Property accessor stubs sometimes do not correspond to any property. + if (MD->isSynthesizedAccessorStub()) { + const ObjCInterfaceDecl *IntD = MD->getClassInterface(); + const ObjCImplementationDecl *ImpD = IntD->getImplementation(); + for (const auto *V: ImpD->ivars()) { + if (V->getName() == MD->getSelector().getNameForSlot(0)) + IVar = V; + } + } - // Ignore weak variables, which have special behavior. - if (Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak) - return nullptr; + if (!IVar) { + const ObjCPropertyDecl *Prop = MD->findPropertyDecl(); + IVar = findBackingIvar(Prop); + if (!IVar) + return nullptr; + + // Ignore weak variables, which have special behavior. + if (Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak) + return nullptr; - // Look to see if Sema has synthesized a body for us. This happens in - // Objective-C++ because the return value may be a C++ class type with a - // non-trivial copy constructor. We can only do this if we can find the - // @synthesize for this property, though (or if we know it's been auto- - // synthesized). - const ObjCImplementationDecl *ImplDecl = - IVar->getContainingInterface()->getImplementation(); - if (ImplDecl) { - for (const auto *I : ImplDecl->property_impls()) { - if (I->getPropertyDecl() != Prop) - continue; - - if (I->getGetterCXXConstructor()) { - ASTMaker M(Ctx); - return M.makeReturn(I->getGetterCXXConstructor()); + // Look to see if Sema has synthesized a body for us. This happens in + // Objective-C++ because the return value may be a C++ class type with a + // non-trivial copy constructor. We can only do this if we can find the + // @synthesize for this property, though (or if we know it's been auto- + // synthesized). + const ObjCImplementationDecl *ImplDecl = + IVar->getContainingInterface()->getImplementation(); + if (ImplDecl) { + for (const auto *I : ImplDecl->property_impls()) { + if (I->getPropertyDecl() != Prop) + continue; + + if (I->getGetterCXXConstructor()) { + ASTMaker M(Ctx); + return M.makeReturn(I->getGetterCXXConstructor()); + } } } - } - // Sanity check that the property is the same type as the ivar, or a - // reference to it, and that it is either an object pointer or trivially - // copyable. - if (!Ctx.hasSameUnqualifiedType(IVar->getType(), - Prop->getType().getNonReferenceType())) - return nullptr; - if (!IVar->getType()->isObjCLifetimeType() && - !IVar->getType().isTriviallyCopyableType(Ctx)) - return nullptr; + // Sanity check that the property is the same type as the ivar, or a + // reference to it, and that it is either an object pointer or trivially + // copyable. + if (!Ctx.hasSameUnqualifiedType(IVar->getType(), + Prop->getType().getNonReferenceType())) + return nullptr; + if (!IVar->getType()->isObjCLifetimeType() && + !IVar->getType().isTriviallyCopyableType(Ctx)) + return nullptr; + } // Generate our body: // return self->_ivar; ASTMaker M(Ctx); - const VarDecl *selfVar = Prop->getGetterMethodDecl()->getSelfDecl(); + const VarDecl *selfVar = MD->getSelfDecl(); if (!selfVar) return nullptr; @@ -791,7 +806,7 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx, selfVar->getType()), IVar); - if (!Prop->getType()->isReferenceType()) + if (!MD->getReturnType()->isReferenceType()) loadedIVar = M.makeLvalueToRvalue(loadedIVar, IVar->getType()); return M.makeReturn(loadedIVar); @@ -814,10 +829,6 @@ Stmt *BodyFarm::getBody(const ObjCMethodDecl *D) { return Val.getValue(); Val = nullptr; - const ObjCPropertyDecl *Prop = D->findPropertyDecl(); - if (!Prop) - return nullptr; - // For now, we only synthesize getters. // Synthesizing setters would cause false negatives in the // RetainCountChecker because the method body would bind the parameter @@ -840,7 +851,7 @@ Stmt *BodyFarm::getBody(const ObjCMethodDecl *D) { return nullptr; } - Val = createObjCPropertyGetter(C, Prop); + Val = createObjCPropertyGetter(C, D); return Val.getValue(); } diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp index e7408b805fa8..b89bbe3f54c5 100644 --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -1309,6 +1309,8 @@ RuntimeDefinition ObjCMethodCall::getRuntimeDefinition() const { } const ObjCMethodDecl *MD = Val.getValue(); + if (MD && !MD->hasBody()) + MD = MD->getCanonicalDecl(); if (CanBeSubClassed) return RuntimeDefinition(MD, Receiver); else diff --git a/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist b/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist index 5e915d63ffde..88cf2fa882ad 100644 --- a/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist +++ b/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist @@ -16,12 +16,46 @@ <key>start</key> <array> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>31</integer> <key>col</key><integer>3</integer> <key>file</key><integer>0</integer> </dict> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>31</integer> + <key>col</key><integer>33</integer> + <key>file</key><integer>0</integer> + </dict> + </array> + <key>end</key> + <array> + <dict> + <key>line</key><integer>33</integer> + <key>col</key><integer>3</integer> + <key>file</key><integer>0</integer> + </dict> + <dict> + <key>line</key><integer>33</integer> + <key>col</key><integer>10</integer> + <key>file</key><integer>0</integer> + </dict> + </array> + </dict> + </array> + </dict> + <dict> + <key>kind</key><string>control</string> + <key>edges</key> + <array> + <dict> + <key>start</key> + <array> + <dict> + <key>line</key><integer>33</integer> + <key>col</key><integer>3</integer> + <key>file</key><integer>0</integer> + </dict> + <dict> + <key>line</key><integer>33</integer> <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> @@ -29,12 +63,12 @@ <key>end</key> <array> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>33</integer> <key>col</key><integer>22</integer> <key>file</key><integer>0</integer> </dict> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>33</integer> <key>col</key><integer>22</integer> <key>file</key><integer>0</integer> </dict> @@ -46,7 +80,7 @@ <key>kind</key><string>event</string> <key>location</key> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>33</integer> <key>col</key><integer>22</integer> <key>file</key><integer>0</integer> </dict> @@ -54,12 +88,12 @@ <array> <array> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>33</integer> <key>col</key><integer>22</integer> <key>file</key><integer>0</integer> </dict> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>33</integer> <key>col</key><integer>22</integer> <key>file</key><integer>0</integer> </dict> @@ -79,12 +113,12 @@ <key>start</key> <array> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>33</integer> <key>col</key><integer>22</integer> <key>file</key><integer>0</integer> </dict> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>33</integer> <key>col</key><integer>22</integer> <key>file</key><integer>0</integer> </dict> @@ -92,12 +126,12 @@ <key>end</key> <array> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>33</integer> <key>col</key><integer>3</integer> <key>file</key><integer>0</integer> </dict> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>33</integer> <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> @@ -113,12 +147,12 @@ <key>start</key> <array> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>33</integer> <key>col</key><integer>3</integer> <key>file</key><integer>0</integer> </dict> <dict> - <key>line</key><integer>16</integer> + <key>line</key><integer>33</integer> <key>col</key><integer>10</integer> <key>file</key><integer>0</integer> </dict> @@ -126,12 +160,12 @@ <key>end</key> <array> <dict> - <key>line</key><integer>17</integer> + <key>line</key><integer>36</integer> <key>col</key><integer>3</integer> <key>file</key><integer>0</integer> </dict> <dict> - <key>line</key><integer>17</integer> + <key>line</key><integer>36</integer> <key>col</key><integer>14</integer> <key>file</key><integer>0</integer> </dict> @@ -143,7 +177,7 @@ <key>kind</key><string>event</string> <key>location</key> <dict> - <key>line</key><integer>17</integer> + <key>line</key><integer>36</integer> <key>col</key><integer>3</integer> <key>file</key><integer>0</integer> </dict> @@ -151,12 +185,12 @@ <array> <array> <dict> - <key>line</key><integer>17</integer> + <key>line</key><integer>36</integer> <key>col</key><integer>16</integer> <key>file</key><integer>0</integer> </dict> <dict> - <key>line</key><integer>17</integer> + <key>line</key><integer>36</integer> <key>col</key><integer>16</integer> <key>file</key><integer>0</integer> </dict> @@ -177,10 +211,10 @@ <key>issue_hash_content_of_line_in_context</key><string>ff735bea0eb12d4d172b139143c32365</string> <key>issue_context_kind</key><string>Objective-C method</string> <key>issue_context</key><string>method</string> - <key>issue_hash_function_offset</key><string>3</string> + <key>issue_hash_function_offset</key><string>6</string> <key>location</key> <dict> - <key>line</key><integer>17</integer> + <key>line</key><integer>36</integer> <key>col</key><integer>3</integer> <key>file</key><integer>0</integer> </dict> @@ -188,9 +222,11 @@ <dict> <key>0</key> <array> - <integer>14</integer> - <integer>16</integer> - <integer>17</integer> + <integer>26</integer> + <integer>30</integer> + <integer>31</integer> + <integer>33</integer> + <integer>36</integer> </array> </dict> </dict> diff --git a/clang/test/Analysis/nullability-notes.m b/clang/test/Analysis/nullability-notes.m index 39dbb8fd7222..e1b4e8f3d5ce 100644 --- a/clang/test/Analysis/nullability-notes.m +++ b/clang/test/Analysis/nullability-notes.m @@ -1,6 +1,22 @@ -// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced -analyzer-output=text -verify %s -// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced -analyzer-output=plist -o %t.plist %s -// RUN: %normalize_plist <%t.plist | diff -ub %S/Inputs/expected-plists/nullability-notes.m.plist - +// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core \ +// RUN: -analyzer-checker=nullability.NullPassedToNonnull \ +// RUN: -analyzer-checker=nullability.NullReturnedFromNonnull \ +// RUN: -analyzer-checker=nullability.NullablePassedToNonnull \ +// RUN: -analyzer-checker=nullability.NullableReturnedFromNonnull \ +// RUN: -analyzer-checker=nullability.NullableDereferenced \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-output=text -verify %s +// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core \ +// RUN: -analyzer-checker=nullability.NullPassedToNonnull \ +// RUN: -analyzer-checker=nullability.NullReturnedFromNonnull \ +// RUN: -analyzer-checker=nullability.NullablePassedToNonnull \ +// RUN: -analyzer-checker=nullability.NullableReturnedFromNonnull \ +// RUN: -analyzer-checker=nullability.NullableDereferenced \ +// RUN: -analyzer-output=plist -o %t.plist %s +// RUN: %normalize_plist <%t.plist \ +// RUN: | diff -ub %S/Inputs/expected-plists/nullability-notes.m.plist - + +void clang_analyzer_warnOnDeadSymbol(id); #include "Inputs/system-header-simulator-for-nullability.h" @@ -12,8 +28,11 @@ -(void) method; @end; @implementation ClassWithProperties -(void) method { + clang_analyzer_warnOnDeadSymbol(self); // no-crash NSObject *x = self.x; // expected-note{{Nullability 'nullable' is inferred}} + // expected-warning@-1{{SYMBOL DEAD}} + // expected-note@-2 {{SYMBOL DEAD}} takesNonnull(x); // expected-warning{{Nullable pointer is passed to a callee that requires a non-null 1st parameter}} // expected-note@-1{{Nullable pointer is passed to a callee that requires a non-null 1st parameter}} } diff --git a/clang/test/Analysis/properties.m b/clang/test/Analysis/properties.m index 17b156035a34..2f427f275182 100644 --- a/clang/test/Analysis/properties.m +++ b/clang/test/Analysis/properties.m @@ -3,6 +3,8 @@ void clang_analyzer_eval(int); +#define nil ((id)0) + typedef const void * CFTypeRef; extern CFTypeRef CFRetain(CFTypeRef cf); void CFRelease(CFTypeRef cf); @@ -1040,3 +1042,48 @@ - (void)foo { clang_analyzer_eval(self.still_no_custom_accessor == self.still_no_custom_accessor); // expected-warning{{TRUE}} } @end + +@interface Shadowed +@property (assign) NSObject *o; +- (NSObject *)getShadowedIvar; +- (void)clearShadowedIvar; +- (NSObject *)getShadowedProp; +- (void)clearShadowedProp; +@end + +@implementation Shadowed +- (NSObject *)getShadowedIvar { + return self->_o; +} +- (void)clearShadowedIvar { + self->_o = nil; +} +- (NSObject *)getShadowedProp { + return self.o; +} +- (void)clearShadowedProp { + self.o = nil; +} +@end + +@interface Shadowing : Shadowed +@end + +@implementation Shadowing +// Property 'o' is declared in the superclass but synthesized here. +// This creates a separate ivar that shadows the superclass's ivar, +// but the old ivar is still accessible from the methods of the superclass. +// The old property, however, is not accessible with the property syntax +// even from the superclass methods. +@synthesize o; + +-(void)testPropertyShadowing { + NSObject *oo = self.o; + clang_analyzer_eval(self.o == oo); // expected-warning{{TRUE}} + clang_analyzer_eval([self getShadowedIvar] == oo); // expected-warning{{UNKNOWN}} + [self clearShadowedIvar]; + clang_analyzer_eval(self.o == oo); // expected-warning{{TRUE}} + clang_analyzer_eval([self getShadowedIvar] == oo); // expected-warning{{UNKNOWN}} + clang_analyzer_eval([self getShadowedIvar] == nil); // expected-warning{{TRUE}} +} +@end _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
