From b111da14ada1c8bba8a1ce4ed903ffc66627646c Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Fri, 17 Aug 2018 22:18:08 +0000 Subject: [PATCH] [ObjC] Error out when using forward-declared protocol in a @protocol expression Clang emits invalid protocol metadata when a @protocol expression is used with a forward-declared protocol. The protocol metadata is missing protocol conformance list of the protocol since we don't have access to the definition of it in the compiled translation unit. The linker then might end up picking the invalid metadata when linking which will lead to incorrect runtime protocol conformance checks. This commit makes sure that Clang fails to compile code that uses a @protocol expression with a forward-declared protocol. This ensures that Clang does not emit invalid protocol metadata. I added an extra assert in CodeGen to ensure that this kind of issue won't happen in other places. rdar://32787811 Differential Revision: https://reviews.llvm.org/D49462 llvm-svn: 340102 --- clang/include/clang/Basic/DiagnosticGroups.td | 3 ++- .../clang/Basic/DiagnosticSemaKinds.td | 4 ++-- clang/lib/CodeGen/CGObjCMac.cpp | 5 +++-- clang/lib/Sema/SemaExpr.cpp | 10 ---------- clang/lib/Sema/SemaExprObjC.cpp | 6 +++++- .../forward-declare-protocol-gnu.m | 10 ++++++---- .../forward-protocol-metadata-symbols.m | 6 +++--- clang/test/CodeGenObjC/hidden-visibility.m | 2 +- clang/test/CodeGenObjC/link-errors.m | 2 +- clang/test/CodeGenObjC/protocol-comdat.m | 4 ++-- clang/test/CodeGenObjC/protocols-lazy.m | 19 ++++++++++++++----- clang/test/CodeGenObjC/protocols.m | 3 ++- clang/test/PCH/objc_exprs.h | 4 +++- .../Parser/objc-cxx-keyword-identifiers.mm | 4 +++- clang/test/SemaObjC/protocol-expr-neg-1.m | 6 +++--- 15 files changed, 50 insertions(+), 38 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index c0128f56a59e..25c12445f711 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -620,7 +620,8 @@ def DeallocInCategory:DiagGroup<"dealloc-in-category">; def SelectorTypeMismatch : DiagGroup<"selector-type-mismatch">; def Selector : DiagGroup<"selector", [SelectorTypeMismatch]>; def Protocol : DiagGroup<"protocol">; -def AtProtocol : DiagGroup<"at-protocol">; +// No longer in use, preserve for backwards compatibility. +def : DiagGroup<"at-protocol">; def PropertyAccessDotSyntax: DiagGroup<"property-access-dot-syntax">; def PropertyAttr : DiagGroup<"property-attribute-mismatch">; def SuperSubClassMismatch : DiagGroup<"super-class-method-mismatch">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index dc192aafe38c..853bc5eddc4d 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -867,8 +867,8 @@ def err_protocol_has_circular_dependency : Error< "protocol has circular dependency">; def err_undeclared_protocol : Error<"cannot find protocol declaration for %0">; def warn_undef_protocolref : Warning<"cannot find protocol definition for %0">; -def warn_atprotocol_protocol : Warning< - "@protocol is using a forward protocol declaration of %0">, InGroup; +def err_atprotocol_protocol : Error< + "@protocol is using a forward protocol declaration of %0">; def warn_readonly_property : Warning< "attribute 'readonly' of property %0 restricts attribute " "'readwrite' of property inherited from %1">, diff --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp index 92e442d33124..2b6d895f93ca 100644 --- a/clang/lib/CodeGen/CGObjCMac.cpp +++ b/clang/lib/CodeGen/CGObjCMac.cpp @@ -6838,8 +6838,9 @@ llvm::Constant *CGObjCNonFragileABIMac::GetOrEmitProtocol( return Entry; // Use the protocol definition, if there is one. - if (const ObjCProtocolDecl *Def = PD->getDefinition()) - PD = Def; + assert(PD->hasDefinition() && + "emitting protocol metadata without definition"); + PD = PD->getDefinition(); auto methodLists = ProtocolMethodLists::get(PD); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 1259d25629b2..73e717f605fd 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -8095,16 +8095,6 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS, if (RHS.isInvalid()) return Incompatible; } - - Expr *PRE = RHS.get()->IgnoreParenCasts(); - if (Diagnose && isa(PRE)) { - ObjCProtocolDecl *PDecl = cast(PRE)->getProtocol(); - if (PDecl && !PDecl->hasDefinition()) { - Diag(PRE->getExprLoc(), diag::warn_atprotocol_protocol) << PDecl; - Diag(PDecl->getLocation(), diag::note_entity_declared_at) << PDecl; - } - } - CastKind Kind; Sema::AssignConvertType result = CheckAssignmentConstraints(LHSType, RHS, Kind, ConvertRHS); diff --git a/clang/lib/Sema/SemaExprObjC.cpp b/clang/lib/Sema/SemaExprObjC.cpp index bbbe343db0ac..13a009090f9e 100644 --- a/clang/lib/Sema/SemaExprObjC.cpp +++ b/clang/lib/Sema/SemaExprObjC.cpp @@ -1227,8 +1227,12 @@ ExprResult Sema::ParseObjCProtocolExpression(IdentifierInfo *ProtocolId, Diag(ProtoLoc, diag::err_undeclared_protocol) << ProtocolId; return true; } - if (PDecl->hasDefinition()) + if (!PDecl->hasDefinition()) { + Diag(ProtoLoc, diag::err_atprotocol_protocol) << PDecl; + Diag(PDecl->getLocation(), diag::note_entity_declared_at) << PDecl; + } else { PDecl = PDecl->getDefinition(); + } QualType Ty = Context.getObjCProtoType(); if (Ty.isNull()) diff --git a/clang/test/CodeGenObjC/forward-declare-protocol-gnu.m b/clang/test/CodeGenObjC/forward-declare-protocol-gnu.m index b57a4a48d4a0..3731fb078eea 100644 --- a/clang/test/CodeGenObjC/forward-declare-protocol-gnu.m +++ b/clang/test/CodeGenObjC/forward-declare-protocol-gnu.m @@ -3,9 +3,11 @@ // Regression test: check that we don't crash when referencing a forward-declared protocol. @protocol P; -Protocol *getProtocol(void) -{ - return @protocol(P); -} +@interface I

+@end + +@implementation I + +@end // CHECK: @.objc_protocol diff --git a/clang/test/CodeGenObjC/forward-protocol-metadata-symbols.m b/clang/test/CodeGenObjC/forward-protocol-metadata-symbols.m index 16d33ec15d82..76afc9c6c760 100644 --- a/clang/test/CodeGenObjC/forward-protocol-metadata-symbols.m +++ b/clang/test/CodeGenObjC/forward-protocol-metadata-symbols.m @@ -3,7 +3,7 @@ @interface NSObject @end -@protocol P0; +@protocol P0 @end @interface A : NSObject +(Class) getClass; @@ -19,8 +19,8 @@ int main() { } // CHECK: @"\01l_OBJC_PROTOCOL_$_P0" = weak hidden global -// CHECK: @"\01l_OBJC_CLASS_PROTOCOLS_$_A" = private global // CHECK: @"\01l_OBJC_LABEL_PROTOCOL_$_P0" = weak hidden global +// CHECK: @"\01l_OBJC_CLASS_PROTOCOLS_$_A" = private global // CHECK: @"\01l_OBJC_PROTOCOL_REFERENCE_$_P0" = weak hidden global // CHECK: llvm.used = appending global [3 x i8*] @@ -33,7 +33,7 @@ int main() { // CHECK-SAME: OBJC_METH_VAR_NAME_ // CHECK-SAME: OBJC_METH_VAR_TYPE_ // CHECK-SAME: "\01l_OBJC_$_CLASS_METHODS_A" -// CHECK-SAME: "\01l_OBJC_CLASS_PROTOCOLS_$_A" // CHECK-SAME: OBJC_CLASS_NAME_.1 +// CHECK-SAME: "\01l_OBJC_CLASS_PROTOCOLS_$_A" // CHECK-SAME: "OBJC_LABEL_CLASS_$" // CHECK-SAME: section "llvm.metadata" diff --git a/clang/test/CodeGenObjC/hidden-visibility.m b/clang/test/CodeGenObjC/hidden-visibility.m index cb23ca18f81f..03290c29bbdb 100644 --- a/clang/test/CodeGenObjC/hidden-visibility.m +++ b/clang/test/CodeGenObjC/hidden-visibility.m @@ -16,7 +16,7 @@ @end -@protocol Prot0; +@protocol Prot0 @end id f0() { return @protocol(Prot0); diff --git a/clang/test/CodeGenObjC/link-errors.m b/clang/test/CodeGenObjC/link-errors.m index f2d9ddba883b..44797d9e8d1d 100644 --- a/clang/test/CodeGenObjC/link-errors.m +++ b/clang/test/CodeGenObjC/link-errors.m @@ -10,7 +10,7 @@ -(id) init; @end -@protocol P; +@protocol P @end @interface A : Root @end diff --git a/clang/test/CodeGenObjC/protocol-comdat.m b/clang/test/CodeGenObjC/protocol-comdat.m index a19ba8cf35dc..ddf8e5a77d4c 100644 --- a/clang/test/CodeGenObjC/protocol-comdat.m +++ b/clang/test/CodeGenObjC/protocol-comdat.m @@ -4,8 +4,8 @@ - (void) method; @end -@protocol Q; -@protocol R; +@protocol Q @end +@protocol R @end @interface I

@end diff --git a/clang/test/CodeGenObjC/protocols-lazy.m b/clang/test/CodeGenObjC/protocols-lazy.m index fba7454b9549..a2dfc106d643 100644 --- a/clang/test/CodeGenObjC/protocols-lazy.m +++ b/clang/test/CodeGenObjC/protocols-lazy.m @@ -18,7 +18,10 @@ void f0() { id x = @protocol(P2); } // RUN: grep OBJC_PROTOCOL_P3 %t | count 3 // RUN: not grep OBJC_PROTOCOL_INSTANCE_METHODS_P3 %t @protocol P3; -void f1() { id x = @protocol(P3); } +@interface UserP3 +@end +@implementation UserP3 +@end // Definition triggered by class reference. // RUN: grep OBJC_PROTOCOL_P4 %t | count 3 @@ -31,10 +34,16 @@ void f1() { id x = @protocol(P3); } // RUN: grep OBJC_PROTOCOL_P5 %t | count 3 // RUN: grep OBJC_PROTOCOL_INSTANCE_METHODS_P5 %t | count 3 @protocol P5; -void f2() { id x = @protocol(P5); } // This generates a forward - // reference, which has to be - // updated on the next line. -@protocol P5 -im1; @end +@interface UserP5 // This generates a forward + // reference, which has to be + // updated on the next line. +@end +@protocol P5 -im1; @end +@implementation UserP5 + +- im1 { } + +@end // Protocol reference following definition. // RUN: grep OBJC_PROTOCOL_P6 %t | count 4 diff --git a/clang/test/CodeGenObjC/protocols.m b/clang/test/CodeGenObjC/protocols.m index 6dadb11273dc..31965e171189 100644 --- a/clang/test/CodeGenObjC/protocols.m +++ b/clang/test/CodeGenObjC/protocols.m @@ -7,7 +7,8 @@ void p(const char*, ...); -(int) conformsTo: (id) x; @end -@protocol P0; +@protocol P0 +@end @protocol P1 +(void) classMethodReq0; diff --git a/clang/test/PCH/objc_exprs.h b/clang/test/PCH/objc_exprs.h index 807304c20f86..e4952f5929bf 100644 --- a/clang/test/PCH/objc_exprs.h +++ b/clang/test/PCH/objc_exprs.h @@ -1,11 +1,13 @@ @protocol foo; +@protocol foo2 +@end @class itf; // Expressions typedef typeof(@"foo" "bar") objc_string; typedef typeof(@encode(int)) objc_encode; -typedef typeof(@protocol(foo)) objc_protocol; +typedef typeof(@protocol(foo2)) objc_protocol; typedef typeof(@selector(noArgs)) objc_selector_noArgs; typedef typeof(@selector(oneArg:)) objc_selector_oneArg; typedef typeof(@selector(foo:bar:)) objc_selector_twoArg; diff --git a/clang/test/Parser/objc-cxx-keyword-identifiers.mm b/clang/test/Parser/objc-cxx-keyword-identifiers.mm index 6791f0d3732a..cff38c554371 100644 --- a/clang/test/Parser/objc-cxx-keyword-identifiers.mm +++ b/clang/test/Parser/objc-cxx-keyword-identifiers.mm @@ -18,7 +18,9 @@ struct S { @protocol new // expected-error {{expected identifier; 'new' is a keyword in Objective-C++}} @end -@protocol P2, delete; // expected-error {{expected identifier; 'delete' is a keyword in Objective-C++}} +@protocol P2; +@protocol delete // expected-error {{expected identifier; 'delete' is a keyword in Objective-C++}} +@end @class Foo, try; // expected-error {{expected identifier; 'try' is a keyword in Objective-C++}} diff --git a/clang/test/SemaObjC/protocol-expr-neg-1.m b/clang/test/SemaObjC/protocol-expr-neg-1.m index 26cdac70ae8d..f659b30046e1 100644 --- a/clang/test/SemaObjC/protocol-expr-neg-1.m +++ b/clang/test/SemaObjC/protocol-expr-neg-1.m @@ -12,7 +12,7 @@ int main() { Protocol *proto = @protocol(p1); - Protocol *fproto = @protocol(fproto); // expected-warning {{@protocol is using a forward protocol declaration of 'fproto'}} + Protocol *fproto = @protocol(fproto); // expected-error {{@protocol is using a forward protocol declaration of 'fproto'}} Protocol *pp = @protocol(i); // expected-error {{cannot find protocol declaration for 'i'}} Protocol *p1p = @protocol(cl); // expected-error {{cannot find protocol declaration for 'cl'}} } @@ -26,9 +26,9 @@ int main() @end int doesConform(id foo) { - return [foo conformsToProtocol:@protocol(TestProtocol)]; // expected-warning {{@protocol is using a forward protocol declaration of 'TestProtocol'}} + return [foo conformsToProtocol:@protocol(TestProtocol)]; // expected-error {{@protocol is using a forward protocol declaration of 'TestProtocol'}} } int doesConformSuper(id foo) { - return [foo conformsToProtocol:@protocol(SuperProtocol)]; // expected-warning {{@protocol is using a forward protocol declaration of 'SuperProtocol'}} + return [foo conformsToProtocol:@protocol(SuperProtocol)]; // expected-error {{@protocol is using a forward protocol declaration of 'SuperProtocol'}} }