[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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@340102 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index c0128f56a59ec5bbadb8ba34b2b38958ec8912c7..25c12445f7115a7aad7cceeb1f4a368e27d63ac7 100644 (file)
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/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/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index dc192aafe38c0e37e2e644e856f50a356bf17522..853bc5eddc4d25002b426d4a68ecf744c6c33432 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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<AtProtocol>;
+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/lib/CodeGen/CGObjCMac.cpp b/lib/CodeGen/CGObjCMac.cpp
index 92e442d331249b0233b76e80edf88a2979a7e33f..2b6d895f93ca69a040a1dbb0e7ab47921d1dd460 100644 (file)
--- a/lib/CodeGen/CGObjCMac.cpp
+++ b/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/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 1259d25629b2338a391234fd56ee2458e1bb43bc..73e717f605fd738251ed7727c09ed84e04cafdfe 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/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<ObjCProtocolExpr>(PRE)) {
-    ObjCProtocolDecl *PDecl = cast<ObjCProtocolExpr>(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/lib/Sema/SemaExprObjC.cpp b/lib/Sema/SemaExprObjC.cpp
index bbbe343db0ac0b660d0d4b5447a1d11c0083d648..13a009090f9eeb13263f91e0632d029c8902a120 100644 (file)
--- a/lib/Sema/SemaExprObjC.cpp
+++ b/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/test/CodeGenObjC/forward-declare-protocol-gnu.m b/test/CodeGenObjC/forward-declare-protocol-gnu.m
index b57a4a48d4a00ca907d4ffd2d29885484a6b591e..3731fb078eea93e281a796ca4df9aa8eeddc305c 100644 (file)
--- a/test/CodeGenObjC/forward-declare-protocol-gnu.m
+++ b/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 <P>
+@end
+
+@implementation I
+
+@end
 
 // CHECK: @.objc_protocol

diff --git a/test/CodeGenObjC/forward-protocol-metadata-symbols.m b/test/CodeGenObjC/forward-protocol-metadata-symbols.m
index 16d33ec15d82ca1bf86aeb53c3bb4d445486725b..76afc9c6c7609878a4f4978f75bc9f0828ab6233 100644 (file)
--- a/test/CodeGenObjC/forward-protocol-metadata-symbols.m
+++ b/test/CodeGenObjC/forward-protocol-metadata-symbols.m
@@ -3,7 +3,7 @@
 
 @interface NSObject @end
 
-@protocol P0;
+@protocol P0 @end
 
 @interface A : NSObject <P0>
 +(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/test/CodeGenObjC/hidden-visibility.m b/test/CodeGenObjC/hidden-visibility.m
index cb23ca18f81fc810db1453258d772c2d11fe626f..03290c29bbdb1f724e7c87fa30bc5c70b6be8185 100644 (file)
--- a/test/CodeGenObjC/hidden-visibility.m
+++ b/test/CodeGenObjC/hidden-visibility.m
@@ -16,7 +16,7 @@
 @end
 
 
-@protocol Prot0;
+@protocol Prot0 @end
 
 id f0() {
   return @protocol(Prot0);

diff --git a/test/CodeGenObjC/link-errors.m b/test/CodeGenObjC/link-errors.m
index f2d9ddba883ba311310cd00ff6d963c8841507d7..44797d9e8d1dc30731c6be2da36135d13293b5c2 100644 (file)
--- a/test/CodeGenObjC/link-errors.m
+++ b/test/CodeGenObjC/link-errors.m
@@ -10,7 +10,7 @@
 -(id) init;
 @end
 
-@protocol P;
+@protocol P @end
 
 @interface A : Root
 @end

diff --git a/test/CodeGenObjC/protocol-comdat.m b/test/CodeGenObjC/protocol-comdat.m
index a19ba8cf35dcfe256644225ba831dd4f6fa404b8..ddf8e5a77d4c4d5cf8fcec597ccfb34a83df5a88 100644 (file)
--- a/test/CodeGenObjC/protocol-comdat.m
+++ b/test/CodeGenObjC/protocol-comdat.m
@@ -4,8 +4,8 @@
 - (void) method;
 @end
 
-@protocol Q;
-@protocol R;
+@protocol Q @end
+@protocol R @end
 
 @interface I<P>
 @end

diff --git a/test/CodeGenObjC/protocols-lazy.m b/test/CodeGenObjC/protocols-lazy.m
index fba7454b95498a3b77be31e0163ec03ad7db7061..a2dfc106d6433460b99f6b2ecfeeb089e200fc4b 100644 (file)
--- a/test/CodeGenObjC/protocols-lazy.m
+++ b/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<P3>
+@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<P5> // 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/test/CodeGenObjC/protocols.m b/test/CodeGenObjC/protocols.m
index 6dadb11273dca3a06ce883bc99ef751a6c2659e7..31965e17118997eb588096d8c43046adb0fa5739 100644 (file)
--- a/test/CodeGenObjC/protocols.m
+++ b/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/test/PCH/objc_exprs.h b/test/PCH/objc_exprs.h
index 807304c20f86609fa08adad5a9f80ddb7570c3b3..e4952f5929bf4053ffdd3f4a88f68afb672d1c47 100644 (file)
--- a/test/PCH/objc_exprs.h
+++ b/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/test/Parser/objc-cxx-keyword-identifiers.mm b/test/Parser/objc-cxx-keyword-identifiers.mm
index 6791f0d3732a20d7eedc0582c4fae856e98d8a5d..cff38c5543713059427cf3abb7155c8d0f6255e8 100644 (file)
--- a/test/Parser/objc-cxx-keyword-identifiers.mm
+++ b/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/test/SemaObjC/protocol-expr-neg-1.m b/test/SemaObjC/protocol-expr-neg-1.m
index 26cdac70ae8db8f796e33638ebaa5db7985ad843..f659b30046e130d6273168270adaf09c2e5d7d9b 100644 (file)
--- a/test/SemaObjC/protocol-expr-neg-1.m
+++ b/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'}}
 }