Skip to content

Commit 0afeef8

Browse files
vsapsaivsapsai
authored
Merge pull request #3478 from apple/eng/PR-82854574-objcinterfacedecl-cherry-pick
Cherry-pick ObjCInterfaceDecl miscompilation fix and related clean-up.
2 parents ea8afba + 2c0aca6 commit 0afeef8

File tree

12 files changed

+222
-28
lines changed

12 files changed

+222
-28
lines changed

‎clang/include/clang/AST/DeclObjC.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1965,6 +1965,13 @@ class ObjCIvarDecl : public FieldDecl {
19651965
const ObjCIvarDecl *getNextIvar() const { return NextIvar; }
19661966
void setNextIvar(ObjCIvarDecl *ivar) { NextIvar = ivar; }
19671967

1968+
ObjCIvarDecl *getCanonicalDecl() override {
1969+
return cast<ObjCIvarDecl>(FieldDecl::getCanonicalDecl());
1970+
}
1971+
const ObjCIvarDecl *getCanonicalDecl() const {
1972+
return const_cast<ObjCIvarDecl *>(this)->getCanonicalDecl();
1973+
}
1974+
19681975
void setAccessControl(AccessControl ac) { DeclAccess = ac; }
19691976

19701977
AccessControl getAccessControl() const { return AccessControl(DeclAccess); }

‎clang/include/clang/AST/Type.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6162,18 +6162,17 @@ inline ObjCProtocolDecl **ObjCTypeParamType::getProtocolStorageImpl() {
61626162
class ObjCInterfaceType : public ObjCObjectType {
61636163
friend class ASTContext; // ASTContext creates these.
61646164
friend class ASTReader;
6165-
friend class ObjCInterfaceDecl;
61666165
template <class T> friend class serialization::AbstractTypeReader;
61676166

6168-
mutable ObjCInterfaceDecl *Decl;
6167+
ObjCInterfaceDecl *Decl;
61696168

61706169
ObjCInterfaceType(const ObjCInterfaceDecl *D)
61716170
: ObjCObjectType(Nonce_ObjCInterface),
61726171
Decl(const_cast<ObjCInterfaceDecl*>(D)) {}
61736172

61746173
public:
61756174
/// Get the declaration of this interface.
6176-
ObjCInterfaceDecl *getDecl() const { return Decl; }
6175+
ObjCInterfaceDecl *getDecl() const;
61776176

61786177
bool isSugared() const { return false; }
61796178
QualType desugar() const { return QualType(this, 0); }

‎clang/lib/AST/DeclObjC.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -603,10 +603,6 @@ void ObjCInterfaceDecl::allocateDefinitionData() {
603603
assert(!hasDefinition() && "ObjC class already has a definition");
604604
Data.setPointer(new (getASTContext()) DefinitionData());
605605
Data.getPointer()->Definition = this;
606-
607-
// Make the type point at the definition, now that we have one.
608-
if (TypeForDecl)
609-
cast<ObjCInterfaceType>(TypeForDecl)->Decl = this;
610606
}
611607

612608
void ObjCInterfaceDecl::startDefinition() {

‎clang/lib/AST/RecordLayoutBuilder.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3383,6 +3383,7 @@ uint64_t ASTContext::getFieldOffset(const ValueDecl *VD) const {
33833383
uint64_t ASTContext::lookupFieldBitOffset(const ObjCInterfaceDecl *OID,
33843384
const ObjCImplementationDecl *ID,
33853385
const ObjCIvarDecl *Ivar) const {
3386+
Ivar = Ivar->getCanonicalDecl();
33863387
const ObjCInterfaceDecl *Container = Ivar->getContainingInterface();
33873388

33883389
// FIXME: We should eliminate the need to have ObjCImplementationDecl passed

‎clang/lib/AST/Type.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -821,6 +821,13 @@ QualType ObjCObjectType::stripObjCKindOfTypeAndQuals(
821821
/*isKindOf=*/false);
822822
}
823823

824+
ObjCInterfaceDecl *ObjCInterfaceType::getDecl() const {
825+
ObjCInterfaceDecl *Canon = Decl->getCanonicalDecl();
826+
if (ObjCInterfaceDecl *Def = Canon->getDefinition())
827+
return Def;
828+
return Canon;
829+
}
830+
824831
const ObjCObjectPointerType *ObjCObjectPointerType::stripObjCKindOfTypeAndQuals(
825832
const ASTContext &ctx) const {
826833
if (!isKindOfType() && qual_empty())

‎clang/lib/Sema/SemaLookup.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5325,11 +5325,8 @@ static NamedDecl *getDefinitionToImport(NamedDecl *D) {
53255325
return FD->getDefinition();
53265326
if (TagDecl *TD = dyn_cast<TagDecl>(D))
53275327
return TD->getDefinition();
5328-
// The first definition for this ObjCInterfaceDecl might be in the TU
5329-
// and not associated with any module. Use the one we know to be complete
5330-
// and have just seen in a module.
53315328
if (ObjCInterfaceDecl *ID = dyn_cast<ObjCInterfaceDecl>(D))
5332-
return ID;
5329+
return ID->getDefinition();
53335330
if (ObjCProtocolDecl *PD = dyn_cast<ObjCProtocolDecl>(D))
53345331
return PD->getDefinition();
53355332
if (TemplateDecl *TD = dyn_cast<TemplateDecl>(D))

‎clang/lib/Serialization/ASTReaderDecl.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1149,6 +1149,13 @@ void ASTDeclReader::ReadObjCDefinitionData(
11491149

11501150
void ASTDeclReader::MergeDefinitionData(ObjCInterfaceDecl *D,
11511151
struct ObjCInterfaceDecl::DefinitionData &&NewDD) {
1152+
struct ObjCInterfaceDecl::DefinitionData &DD = D->data();
1153+
if (DD.Definition != NewDD.Definition) {
1154+
Reader.MergedDeclContexts.insert(
1155+
std::make_pair(NewDD.Definition, DD.Definition));
1156+
Reader.mergeDefinitionVisibility(DD.Definition, NewDD.Definition);
1157+
}
1158+
11521159
// FIXME: odr checking?
11531160
}
11541161

@@ -2647,7 +2654,7 @@ static bool allowODRLikeMergeInC(NamedDecl *ND) {
26472654
if (!ND)
26482655
return false;
26492656
// TODO: implement merge for other necessary decls.
2650-
if (isa<EnumConstantDecl>(ND))
2657+
if (isa<EnumConstantDecl, FieldDecl, IndirectFieldDecl>(ND))
26512658
return true;
26522659
return false;
26532660
}
@@ -3323,6 +3330,9 @@ DeclContext *ASTDeclReader::getPrimaryContextForMerging(ASTReader &Reader,
33233330
return ED->getASTContext().getLangOpts().CPlusPlus? ED->getDefinition()
33243331
: nullptr;
33253332

3333+
if (auto *OID = dyn_cast<ObjCInterfaceDecl>(DC))
3334+
return OID->getDefinition();
3335+
33263336
// We can see the TU here only if we have no Sema object. In that case,
33273337
// there's no TU scope to look in, so using the DC alone is sufficient.
33283338
if (auto *TU = dyn_cast<TranslationUnitDecl>(DC))

‎clang/test/Modules/decldef.mm

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
// RUN: rm -rf %t
2-
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_1 -DUSE_2 -DUSE_3 -DUSE_4
3-
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_2 -DUSE_3 -DUSE_4
4-
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_3 -DUSE_4
5-
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_4
2+
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_1 -DUSE_2 -DUSE_3 -DUSE_4 -DUSE_5
3+
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_2 -DUSE_3 -DUSE_4 -DUSE_5
4+
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_3 -DUSE_4 -DUSE_5
5+
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_4 -DUSE_5
6+
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_5
67

78
// expected-note@Inputs/def.h:5 0-1{{here}}
9+
// expected-note@Inputs/def.h:11 0-1{{here}}
810
// expected-note@Inputs/def.h:16 0-1{{here}}
911
// expected-note@Inputs/def-include.h:11 0-1{{here}}
1012

@@ -51,11 +53,17 @@ void testB() {
5153
}
5254

5355
void testDef() {
56+
#ifdef USE_4
5457
[def defMethod];
58+
#ifndef USED
59+
// expected-error@-2{{definition of 'Def' must be imported from module 'decldef.Def' before it is required}}
60+
#define USED
61+
#endif
62+
#endif
5563
}
5664

5765
void testDef2() {
58-
#ifdef USE_4
66+
#ifdef USE_5
5967
def2->func();
6068
def3->func();
6169
#ifndef USED

‎clang/test/Modules/interface-diagnose-missing-import.m

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
// RUN: rm -rf %t
22
// RUN: %clang_cc1 %s -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F%S/Inputs/interface-diagnose-missing-import -verify
3+
// expected-no-diagnostics
34
@interface Buggy
45
@end
56

67
@import Foo.Bar;
78

8-
@interface Buggy (MyExt) // expected-error {{definition of 'Buggy' must be imported from module 'Foo' before it is required}}
9+
// No diagnostic for inaccessible 'Buggy' definition because we have another definition right in this file.
10+
@interface Buggy (MyExt)
911
@end
10-
11-
// expected-note@Foo/RandoPriv.h:3{{definition here is not reachable}}

‎clang/test/Modules/merge-objc-interface-visibility.m

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -F%t/Frameworks %t/test.m -DHIDDEN_FIRST=1 \
4+
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
5+
// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -F%t/Frameworks %t/test.m -DHIDDEN_FIRST=0 \
6+
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
7+
8+
// Test a case when Objective-C interface is imported both as hidden and as visible.
9+
10+
//--- Frameworks/Foundation.framework/Headers/Foundation.h
11+
@interface NSObject
12+
@end
13+
14+
//--- Frameworks/Foundation.framework/Modules/module.modulemap
15+
framework module Foundation {
16+
header "Foundation.h"
17+
export *
18+
}
19+
20+
//--- Frameworks/Regular.framework/Headers/Regular.h
21+
#import <Foundation/Foundation.h>
22+
@interface Regular : NSObject
23+
@end
24+
25+
//--- Frameworks/Regular.framework/Modules/module.modulemap
26+
framework module Regular {
27+
header "Regular.h"
28+
export *
29+
}
30+
31+
//--- Frameworks/RegularHider.framework/Headers/Visible.h
32+
// Empty, file required to create a module.
33+
34+
//--- Frameworks/RegularHider.framework/Headers/Hidden.h
35+
#import <Foundation/Foundation.h>
36+
@interface Regular : NSObject
37+
@end
38+
39+
//--- Frameworks/RegularHider.framework/Modules/module.modulemap
40+
framework module RegularHider {
41+
header "Visible.h"
42+
export *
43+
44+
explicit module Hidden {
45+
header "Hidden.h"
46+
export *
47+
}
48+
}
49+
50+
//--- test.m
51+
52+
#if HIDDEN_FIRST
53+
#import <RegularHider/Visible.h>
54+
#import <Regular/Regular.h>
55+
#else
56+
#import <Regular/Regular.h>
57+
#import <RegularHider/Visible.h>
58+
#endif
59+
60+
@interface SubClass : Regular
61+
@end

‎clang/test/Modules/merge-objc-interface.m

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -F%t/Frameworks %t/test.m \
4+
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
5+
// RUN: %clang_cc1 -emit-llvm -o %t/test.bc -F%t/Frameworks %t/test-functions.m \
6+
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
7+
8+
// Test a case when Objective-C interface ivars are present in two different modules.
9+
10+
//--- Frameworks/Foundation.framework/Headers/Foundation.h
11+
@interface NSObject
12+
@end
13+
14+
//--- Frameworks/Foundation.framework/Modules/module.modulemap
15+
framework module Foundation {
16+
header "Foundation.h"
17+
export *
18+
}
19+
20+
//--- Frameworks/ObjCInterface.framework/Headers/ObjCInterface.h
21+
#import <Foundation/Foundation.h>
22+
@interface ObjCInterface : NSObject {
23+
@public
24+
id _item;
25+
}
26+
@end
27+
28+
@interface WithBitFields : NSObject {
29+
@public
30+
int x: 3;
31+
int y: 4;
32+
}
33+
@end
34+
35+
//--- Frameworks/ObjCInterface.framework/Modules/module.modulemap
36+
framework module ObjCInterface {
37+
header "ObjCInterface.h"
38+
export *
39+
}
40+
41+
//--- Frameworks/ObjCInterfaceCopy.framework/Headers/ObjCInterfaceCopy.h
42+
#import <Foundation/Foundation.h>
43+
@interface ObjCInterface : NSObject {
44+
@public
45+
id _item;
46+
}
47+
@end
48+
49+
@interface WithBitFields : NSObject {
50+
@public
51+
int x: 3;
52+
int y: 4;
53+
}
54+
@end
55+
56+
// Inlined function present only in Copy.framework to make sure it uses decls from Copy module.
57+
__attribute__((always_inline)) void inlinedIVarAccessor(ObjCInterface *obj, WithBitFields *bitFields) {
58+
obj->_item = 0;
59+
bitFields->x = 0;
60+
}
61+
62+
//--- Frameworks/ObjCInterfaceCopy.framework/Modules/module.modulemap
63+
framework module ObjCInterfaceCopy {
64+
header "ObjCInterfaceCopy.h"
65+
export *
66+
}
67+
68+
//--- test.m
69+
#import <ObjCInterface/ObjCInterface.h>
70+
#import <ObjCInterfaceCopy/ObjCInterfaceCopy.h>
71+
72+
@implementation ObjCInterface
73+
- (void)test:(id)item {
74+
_item = item;
75+
}
76+
@end
77+
78+
@implementation WithBitFields
79+
- (void)reset {
80+
x = 0;
81+
y = 0;
82+
}
83+
@end
84+
85+
//--- test-functions.m
86+
#import <ObjCInterface/ObjCInterface.h>
87+
88+
void testAccessIVar(ObjCInterface *obj, id item) {
89+
obj->_item = item;
90+
}
91+
void testAccessBitField(WithBitFields *obj) {
92+
obj->x = 0;
93+
}
94+
95+
#import <ObjCInterfaceCopy/ObjCInterfaceCopy.h>
96+
97+
void testAccessIVarLater(ObjCInterface *obj, id item) {
98+
obj->_item = item;
99+
}
100+
void testAccessBitFieldLater(WithBitFields *obj) {
101+
obj->y = 0;
102+
}
103+
void testInlinedFunction(ObjCInterface *obj, WithBitFields *bitFields) {
104+
inlinedIVarAccessor(obj, bitFields);
105+
}

‎clang/test/Modules/odr_hash.mm

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -241,12 +241,12 @@ @interface Interface4 <T : I1 *> {
241241
@end
242242
@interface Interface5 <T : I1 *> {
243243
@public
244-
T x; // FIXME: align with upstream (rdar://43906928).
244+
T y; // FIXME: align with upstream (rdar://43906928).
245245
}
246246
@end
247247
@interface Interface6 <T1 : I1 *, T2 : I2 *> {
248248
@public
249-
T1 x;
249+
T1 z;
250250
}
251251
@end
252252
#elif defined(SECOND)
@@ -257,14 +257,17 @@ @interface Interface4 <T : I1 *> {
257257
@end
258258
@interface Interface5 <T : I1 *> {
259259
@public
260-
T x; // FIXME: align with upstream (rdar://43906928).
260+
T y; // FIXME: align with upstream (rdar://43906928).
261261
}
262262
@end
263263
@interface Interface6 <T1 : I1 *, T2 : I2 *> {
264264
@public
265-
T2 x;
265+
T2 z;
266266
}
267267
@end
268+
#else
269+
// [email protected]:* {{'Interface6::z' from module 'FirstModule' is not present in definition of 'Interface6' in module 'SecondModule'}}
270+
// [email protected]:* {{declaration of 'z' does not match}}
268271
#endif
269272

270273
namespace Types {
@@ -276,18 +279,18 @@ @interface Interface6 <T1 : I1 *, T2 : I2 *> {
276279
};
277280
struct Invalid2 {
278281
Interface5 *I;
279-
decltype(I->x) x;
282+
decltype(I->y) y;
280283
};
281284
struct Invalid3 {
282285
Interface6 *I;
283-
decltype(I->x) x;
286+
decltype(I->z) z;
284287
};
285288
#else
286289
Invalid1 i1;
287290
Invalid2 i2;
288291
Invalid3 i3;
289-
// [email protected]:* {{'Types::ObjCTypeParam::Invalid3::x' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid3' in module 'SecondModule'}}
290-
// [email protected]:* {{declaration of 'x' does not match}}
292+
// [email protected]:* {{'Types::ObjCTypeParam::Invalid3::z' from module 'FirstModule' is not present in definition of 'Types::ObjCTypeParam::Invalid3' in module 'SecondModule'}}
293+
// [email protected]:* {{declaration of 'z' does not match}}
291294
#endif
292295

293296
} // namespace ObjCTypeParam

0 commit comments

Comments
 (0)