r340102 - [ObjC] Error out when using forward-declared protocol in a @protocol

Alex Lorenz via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 17 15:18:08 PDT 2018


Author: arphaman
Date: Fri Aug 17 15:18:08 2018
New Revision: 340102

URL: http://llvm.org/viewvc/llvm-project?rev=340102&view=rev
Log:
[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

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticGroups.td
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/CodeGen/CGObjCMac.cpp
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/lib/Sema/SemaExprObjC.cpp
    cfe/trunk/test/CodeGenObjC/forward-declare-protocol-gnu.m
    cfe/trunk/test/CodeGenObjC/forward-protocol-metadata-symbols.m
    cfe/trunk/test/CodeGenObjC/hidden-visibility.m
    cfe/trunk/test/CodeGenObjC/link-errors.m
    cfe/trunk/test/CodeGenObjC/protocol-comdat.m
    cfe/trunk/test/CodeGenObjC/protocols-lazy.m
    cfe/trunk/test/CodeGenObjC/protocols.m
    cfe/trunk/test/PCH/objc_exprs.h
    cfe/trunk/test/Parser/objc-cxx-keyword-identifiers.mm
    cfe/trunk/test/SemaObjC/protocol-expr-neg-1.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Aug 17 15:18:08 2018
@@ -620,7 +620,8 @@ def DeallocInCategory:DiagGroup<"dealloc
 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">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Aug 17 15:18:08 2018
@@ -867,8 +867,8 @@ def err_protocol_has_circular_dependency
   "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">,

Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Fri Aug 17 15:18:08 2018
@@ -6838,8 +6838,9 @@ llvm::Constant *CGObjCNonFragileABIMac::
     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);
 

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Aug 17 15:18:08 2018
@@ -8095,16 +8095,6 @@ Sema::CheckSingleAssignmentConstraints(Q
     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);

Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Fri Aug 17 15:18:08 2018
@@ -1227,8 +1227,12 @@ ExprResult Sema::ParseObjCProtocolExpres
     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())

Modified: cfe/trunk/test/CodeGenObjC/forward-declare-protocol-gnu.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/forward-declare-protocol-gnu.m?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/forward-declare-protocol-gnu.m (original)
+++ cfe/trunk/test/CodeGenObjC/forward-declare-protocol-gnu.m Fri Aug 17 15:18:08 2018
@@ -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);
-}
+ at interface I <P>
+ at end
+
+ at implementation I
+
+ at end
 
 // CHECK: @.objc_protocol

Modified: cfe/trunk/test/CodeGenObjC/forward-protocol-metadata-symbols.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/forward-protocol-metadata-symbols.m?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/forward-protocol-metadata-symbols.m (original)
+++ cfe/trunk/test/CodeGenObjC/forward-protocol-metadata-symbols.m Fri Aug 17 15:18:08 2018
@@ -3,7 +3,7 @@
 
 @interface NSObject @end
 
- at protocol P0;
+ at 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"

Modified: cfe/trunk/test/CodeGenObjC/hidden-visibility.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/hidden-visibility.m?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/hidden-visibility.m (original)
+++ cfe/trunk/test/CodeGenObjC/hidden-visibility.m Fri Aug 17 15:18:08 2018
@@ -16,7 +16,7 @@
 @end
 
 
- at protocol Prot0;
+ at protocol Prot0 @end
 
 id f0() {
   return @protocol(Prot0);

Modified: cfe/trunk/test/CodeGenObjC/link-errors.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/link-errors.m?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/link-errors.m (original)
+++ cfe/trunk/test/CodeGenObjC/link-errors.m Fri Aug 17 15:18:08 2018
@@ -10,7 +10,7 @@
 -(id) init;
 @end
 
- at protocol P;
+ at protocol P @end
 
 @interface A : Root
 @end

Modified: cfe/trunk/test/CodeGenObjC/protocol-comdat.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/protocol-comdat.m?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/protocol-comdat.m (original)
+++ cfe/trunk/test/CodeGenObjC/protocol-comdat.m Fri Aug 17 15:18:08 2018
@@ -4,8 +4,8 @@
 - (void) method;
 @end
 
- at protocol Q;
- at protocol R;
+ at protocol Q @end
+ at protocol R @end
 
 @interface I<P>
 @end

Modified: cfe/trunk/test/CodeGenObjC/protocols-lazy.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/protocols-lazy.m?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/protocols-lazy.m (original)
+++ cfe/trunk/test/CodeGenObjC/protocols-lazy.m Fri Aug 17 15:18:08 2018
@@ -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); }
+ at interface UserP3<P3>
+ at end
+ at implementation UserP3
+ at 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.
- at protocol P5 -im1; @end               
+ at interface UserP5<P5> // This generates a forward
+                      // reference, which has to be
+                      // updated on the next line.
+ at end
+ at protocol P5 -im1; @end
+ at implementation UserP5
+
+- im1 { }
+
+ at end
 
 // Protocol reference following definition.
 // RUN: grep OBJC_PROTOCOL_P6 %t | count 4

Modified: cfe/trunk/test/CodeGenObjC/protocols.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/protocols.m?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/protocols.m (original)
+++ cfe/trunk/test/CodeGenObjC/protocols.m Fri Aug 17 15:18:08 2018
@@ -7,7 +7,8 @@ void p(const char*, ...);
 -(int) conformsTo: (id) x;
 @end
 
- at protocol P0;
+ at protocol P0
+ at end
 
 @protocol P1
 +(void) classMethodReq0;

Modified: cfe/trunk/test/PCH/objc_exprs.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/objc_exprs.h?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/test/PCH/objc_exprs.h (original)
+++ cfe/trunk/test/PCH/objc_exprs.h Fri Aug 17 15:18:08 2018
@@ -1,11 +1,13 @@
 
 @protocol foo;
+ at protocol foo2
+ at 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;

Modified: cfe/trunk/test/Parser/objc-cxx-keyword-identifiers.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/objc-cxx-keyword-identifiers.mm?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/test/Parser/objc-cxx-keyword-identifiers.mm (original)
+++ cfe/trunk/test/Parser/objc-cxx-keyword-identifiers.mm Fri Aug 17 15:18:08 2018
@@ -18,7 +18,9 @@ struct S {
 @protocol new // expected-error {{expected identifier; 'new' is a keyword in Objective-C++}}
 @end
 
- at protocol P2, delete; // expected-error {{expected identifier; 'delete' is a keyword in Objective-C++}}
+ at protocol P2;
+ at protocol delete // expected-error {{expected identifier; 'delete' is a keyword in Objective-C++}}
+ at end
 
 @class Foo, try; // expected-error {{expected identifier; 'try' is a keyword in Objective-C++}}
 

Modified: cfe/trunk/test/SemaObjC/protocol-expr-neg-1.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/protocol-expr-neg-1.m?rev=340102&r1=340101&r2=340102&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/protocol-expr-neg-1.m (original)
+++ cfe/trunk/test/SemaObjC/protocol-expr-neg-1.m Fri Aug 17 15:18:08 2018
@@ -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'}}
 }




More information about the cfe-commits mailing list