[PATCH] PR23175 (fix) - Infinite loop iterating Objective-C method declarations in categories when the AST was deserialized from an .ast file

[Tom Honermann]

Hi klimek,

This patch corrects several errors that result in incorrect Objective-C method declaration enumeration that occurs when an AST is deserialized from an .ast file.

There were two issues:

1) In lib/AST/DeclObjC.cpp, in ObjCImplDecl::setClassInterface(), the base class version (ObjCImplDecl) of (non-virtual) getIdentifier() was being called, but the derived class version (ObjCCategoryImplDecl) is what is required.  See this comment in include/clang/AST/DeclObjC.h:
     class ObjCCategoryImplDecl : public ObjCImplDecl {
     ...
     /// getIdentifier - Get the identifier that names the category
     /// interface associated with this implementation.
     /// FIXME: This is a bad API, we are hiding NamedDecl::getIdentifier()
     /// with a different meaning. For example:
     /// ((NamedDecl *)SomeCategoryImplDecl)->getIdentifier()
     /// returns the class interface name, whereas
     /// ((ObjCCategoryImplDecl *)SomeCategoryImplDecl)->getIdentifier()
     /// returns the category name.
     IdentifierInfo *getIdentifier() const {
       return Id;
     }
   I did not change the interface, but clearly, it would be a good idea to do so.  The patch uses the existing downcasted pointer to call the correct version.

2) With the above change, when deserializing a ObjCCategoryImplDecl instance, ObjCImplDecl::setClassInterface() was being called by ASTDeclReader::VisitObjCImplDecl() before the category name was deserialized in ASTDeclReader::VisitObjCCategoryImplDecl().  The patch delays the call to ObjCImplDecl::setClassInterface() until after the category name is deserialized and set.

REPOSITORY
  rL LLVM

http://reviews.llvm.org/D9127

Files:
  lib/AST/DeclObjC.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp

Index: lib/AST/DeclObjC.cpp
===================================================================
--- lib/AST/DeclObjC.cpp
+++ lib/AST/DeclObjC.cpp
@@ -1741,8 +1741,10 @@
 
   } else if (ObjCCategoryImplDecl *ImplD =
              dyn_cast_or_null<ObjCCategoryImplDecl>(this)) {
-    if (ObjCCategoryDecl *CD = IFace->FindCategoryDeclaration(getIdentifier()))
+    if (ObjCCategoryDecl *CD =
+        IFace->FindCategoryDeclaration(ImplD->getIdentifier())) {
       Ctx.setObjCImplementation(CD, ImplD);
+    }
   }
 
   ClassInterface = IFace;
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -981,17 +981,20 @@
 
 void ASTDeclReader::VisitObjCImplDecl(ObjCImplDecl *D) {
   VisitObjCContainerDecl(D);
-  D->setClassInterface(ReadDeclAs<ObjCInterfaceDecl>(Record, Idx));
+  // For ObjCCategoryImplDecl, setClassInterface must not be called until the
+  // category identifier has been deserialized and set.  See PR23175.
 }
 
 void ASTDeclReader::VisitObjCCategoryImplDecl(ObjCCategoryImplDecl *D) {
   VisitObjCImplDecl(D);
   D->setIdentifier(Reader.GetIdentifierInfo(F, Record, Idx));
   D->CategoryNameLoc = ReadSourceLocation(Record, Idx);
+  D->setClassInterface(ReadDeclAs<ObjCInterfaceDecl>(Record, Idx));
 }
 
 void ASTDeclReader::VisitObjCImplementationDecl(ObjCImplementationDecl *D) {
   VisitObjCImplDecl(D);
+  D->setClassInterface(ReadDeclAs<ObjCInterfaceDecl>(Record, Idx));
   D->setSuperClass(ReadDeclAs<ObjCInterfaceDecl>(Record, Idx));
   D->SuperLoc = ReadSourceLocation(Record, Idx);
   D->setIvarLBraceLoc(ReadSourceLocation(Record, Idx));
Index: lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -695,19 +695,20 @@
 
 void ASTDeclWriter::VisitObjCImplDecl(ObjCImplDecl *D) {
   VisitObjCContainerDecl(D);
-  Writer.AddDeclRef(D->getClassInterface(), Record);
   // Abstract class (no need to define a stable serialization::DECL code).
 }
 
 void ASTDeclWriter::VisitObjCCategoryImplDecl(ObjCCategoryImplDecl *D) {
   VisitObjCImplDecl(D);
   Writer.AddIdentifierRef(D->getIdentifier(), Record);
   Writer.AddSourceLocation(D->getCategoryNameLoc(), Record);
+  Writer.AddDeclRef(D->getClassInterface(), Record);
   Code = serialization::DECL_OBJC_CATEGORY_IMPL;
 }
 
 void ASTDeclWriter::VisitObjCImplementationDecl(ObjCImplementationDecl *D) {
   VisitObjCImplDecl(D);
+  Writer.AddDeclRef(D->getClassInterface(), Record);
   Writer.AddDeclRef(D->getSuperClass(), Record);
   Writer.AddSourceLocation(D->getSuperClassLoc(), Record);
   Writer.AddSourceLocation(D->getIvarLBraceLoc(), Record);

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D9127.24061.patch
Type: text/x-patch
Size: 2798 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150420/2cdbef7e/attachment.bin>