Skip to content

Commit 45a07a6

Browse files
[analyzer] Fix body farm for Obj-C++ properties
When property is declared in a superclass (or in a protocol), it still can be of CXXRecord type and Sema could've already generated a body for us. This patch joins two branches and two ways of acquiring IVar in order to reuse the existing code. And prevent us from generating l-value to r-value casts for C++ types. rdar://67416721 Differential Revision: https://reviews.llvm.org/D99194 (cherry picked from commit 4821c15)
1 parent 211b6a4 commit 45a07a6

File tree

2 files changed

+62
-41
lines changed

2 files changed

+62
-41
lines changed

clang/lib/Analysis/BodyFarm.cpp

Lines changed: 42 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -742,63 +742,67 @@ static const ObjCIvarDecl *findBackingIvar(const ObjCPropertyDecl *Prop) {
742742

743743
static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
744744
const ObjCMethodDecl *MD) {
745-
// First, find the backing ivar.
745+
// First, find the backing ivar.
746746
const ObjCIvarDecl *IVar = nullptr;
747+
const ObjCPropertyDecl *Prop = nullptr;
747748

748749
// Property accessor stubs sometimes do not correspond to any property decl
749750
// in the current interface (but in a superclass). They still have a
750751
// corresponding property impl decl in this case.
751752
if (MD->isSynthesizedAccessorStub()) {
752753
const ObjCInterfaceDecl *IntD = MD->getClassInterface();
753754
const ObjCImplementationDecl *ImpD = IntD->getImplementation();
754-
for (const auto *PI: ImpD->property_impls()) {
755-
if (const ObjCPropertyDecl *P = PI->getPropertyDecl()) {
756-
if (P->getGetterName() == MD->getSelector())
757-
IVar = P->getPropertyIvarDecl();
755+
for (const auto *PI : ImpD->property_impls()) {
756+
if (const ObjCPropertyDecl *Candidate = PI->getPropertyDecl()) {
757+
if (Candidate->getGetterName() == MD->getSelector()) {
758+
Prop = Candidate;
759+
IVar = Prop->getPropertyIvarDecl();
760+
}
758761
}
759762
}
760763
}
761764

762765
if (!IVar) {
763-
const ObjCPropertyDecl *Prop = MD->findPropertyDecl();
766+
Prop = MD->findPropertyDecl();
764767
IVar = findBackingIvar(Prop);
765-
if (!IVar)
766-
return nullptr;
768+
}
767769

768-
// Ignore weak variables, which have special behavior.
769-
if (Prop->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak)
770-
return nullptr;
770+
if (!IVar || !Prop)
771+
return nullptr;
772+
773+
// Ignore weak variables, which have special behavior.
774+
if (Prop->getPropertyAttributes() & ObjCPropertyAttribute::kind_weak)
775+
return nullptr;
771776

772-
// Look to see if Sema has synthesized a body for us. This happens in
773-
// Objective-C++ because the return value may be a C++ class type with a
774-
// non-trivial copy constructor. We can only do this if we can find the
775-
// @synthesize for this property, though (or if we know it's been auto-
776-
// synthesized).
777-
const ObjCImplementationDecl *ImplDecl =
777+
// Look to see if Sema has synthesized a body for us. This happens in
778+
// Objective-C++ because the return value may be a C++ class type with a
779+
// non-trivial copy constructor. We can only do this if we can find the
780+
// @synthesize for this property, though (or if we know it's been auto-
781+
// synthesized).
782+
const ObjCImplementationDecl *ImplDecl =
778783
IVar->getContainingInterface()->getImplementation();
779-
if (ImplDecl) {
780-
for (const auto *I : ImplDecl->property_impls()) {
781-
if (I->getPropertyDecl() != Prop)
782-
continue;
783-
784-
if (I->getGetterCXXConstructor()) {
785-
ASTMaker M(Ctx);
786-
return M.makeReturn(I->getGetterCXXConstructor());
787-
}
784+
if (ImplDecl) {
785+
for (const auto *I : ImplDecl->property_impls()) {
786+
if (I->getPropertyDecl() != Prop)
787+
continue;
788+
789+
if (I->getGetterCXXConstructor()) {
790+
ASTMaker M(Ctx);
791+
return M.makeReturn(I->getGetterCXXConstructor());
788792
}
789793
}
790-
791-
// Sanity check that the property is the same type as the ivar, or a
792-
// reference to it, and that it is either an object pointer or trivially
793-
// copyable.
794-
if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
795-
Prop->getType().getNonReferenceType()))
796-
return nullptr;
797-
if (!IVar->getType()->isObjCLifetimeType() &&
798-
!IVar->getType().isTriviallyCopyableType(Ctx))
799-
return nullptr;
800794
}
801795

796+
// Sanity check that the property is the same type as the ivar, or a
797+
// reference to it, and that it is either an object pointer or trivially
798+
// copyable.
799+
if (!Ctx.hasSameUnqualifiedType(IVar->getType(),
800+
Prop->getType().getNonReferenceType()))
801+
return nullptr;
802+
if (!IVar->getType()->isObjCLifetimeType() &&
803+
!IVar->getType().isTriviallyCopyableType(Ctx))
804+
return nullptr;
805+
802806
// Generate our body:
803807
// return self->_ivar;
804808
ASTMaker M(Ctx);
@@ -807,11 +811,8 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
807811
if (!selfVar)
808812
return nullptr;
809813

810-
Expr *loadedIVar =
811-
M.makeObjCIvarRef(
812-
M.makeLvalueToRvalue(
813-
M.makeDeclRefExpr(selfVar),
814-
selfVar->getType()),
814+
Expr *loadedIVar = M.makeObjCIvarRef(
815+
M.makeLvalueToRvalue(M.makeDeclRefExpr(selfVar), selfVar->getType()),
815816
IVar);
816817

817818
if (!MD->getReturnType()->isReferenceType())

clang/test/Analysis/properties.mm

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,23 @@ void testConsistencyCustomCopy(CustomCopyWrapper *w) {
7777

7878
clang_analyzer_eval(w.inner.value == 42); // expected-warning{{TRUE}}
7979
}
80+
81+
@protocol NoDirectPropertyDecl
82+
@property IntWrapperStruct inner;
83+
@end
84+
@interface NoDirectPropertyDecl <NoDirectPropertyDecl>
85+
@end
86+
@implementation NoDirectPropertyDecl
87+
@synthesize inner;
88+
@end
89+
90+
// rdar://67416721
91+
void testNoDirectPropertyDecl(NoDirectPropertyDecl *w) {
92+
clang_analyzer_eval(w.inner.value == w.inner.value); // expected-warning{{TRUE}}
93+
94+
int origValue = w.inner.value;
95+
if (origValue != 42)
96+
return;
97+
98+
clang_analyzer_eval(w.inner.value == 42); // expected-warning{{TRUE}}
99+
}

0 commit comments

Comments
 (0)