[libclang] Fix use-after-free bug when handling attributes indexing info.

When indexing a property with a getter/setter with attributes, the allocated memory
for AttrListInfo could get released before its destructor is run.

Fixes rdar://11113442.

llvm-svn: 153792

clang/test/Index/index-attrs.m

@@ -3,5 +3,15 @@
 @property (retain) __attribute__((iboutletcollection(Foo))) Foo *prop;
 @end
 
+@interface I
+-(id)prop __attribute__((annotate("anno")));
+-(void)setProp:(id)p __attribute__((annotate("anno")));
+@property (assign) id prop __attribute__((annotate("anno")));
+@end
+
 // RUN: c-index-test -index-file %s | FileCheck %s
 // CHECK:      <attribute>: attribute(iboutletcollection)= [IBOutletCollection=ObjCInterface]
+
+// CHECK: <attribute>: attribute(annotate)=anno
+// CHECK: <getter>: kind: objc-instance-method | name: prop | {{.*}} <attribute>: attribute(annotate)=anno
+// CHECK: <setter>: kind: objc-instance-method | name: setProp: | {{.*}} <attribute>: attribute(annotate)=anno

clang/tools/libclang/IndexingContext.cpp

@@ -61,9 +61,9 @@ IBOutletCollectionInfo::IBOutletCollectionInfo(
     IBCollInfo.objcClass = 0;
 }
 
-AttrListInfo::AttrListInfo(const Decl *D,
-                           IndexingContext &IdxCtx,
-                           ScratchAlloc &SA) : ref_cnt(0) {
+AttrListInfo::AttrListInfo(const Decl *D, IndexingContext &IdxCtx)
+  : SA(IdxCtx), ref_cnt(0) {
+
   if (!D->hasAttrs())
     return;
 
@@ -113,19 +113,11 @@ AttrListInfo::AttrListInfo(const Decl *D,
     CXAttrs.push_back(&Attrs[i]);
 }
 
-AttrListInfo::AttrListInfo(const AttrListInfo &other) {
-  assert(other.ref_cnt == 0 &&
-         "Should not copy an AttrListInfo that is ref-counted");
-  ref_cnt = 0;
-
-  Attrs = other.Attrs;
-  IBCollAttrs = other.IBCollAttrs;
-
-  for (unsigned i = 0, e = IBCollAttrs.size(); i != e; ++i)
-    CXAttrs.push_back(&IBCollAttrs[i]);
-
-  for (unsigned i = 0, e = Attrs.size(); i != e; ++i)
-    CXAttrs.push_back(&Attrs[i]);
+IntrusiveRefCntPtr<AttrListInfo>
+AttrListInfo::create(const Decl *D, IndexingContext &IdxCtx) {
+  ScratchAlloc SA(IdxCtx);
+  AttrListInfo *attrs = SA.allocate<AttrListInfo>();
+  return new (attrs) AttrListInfo(D, IdxCtx);
 }
 
 IndexingContext::CXXBasesListInfo::CXXBasesListInfo(const CXXRecordDecl *D,
@@ -281,9 +273,8 @@ bool IndexingContext::handleDecl(const NamedDecl *D,
   DInfo.loc = getIndexLoc(Loc);
   DInfo.isImplicit = D->isImplicit();
 
-  AttrListInfo AttrList(D, *this, SA);
-  DInfo.attributes = AttrList.getAttrs();
-  DInfo.numAttributes = AttrList.getNumAttrs();
+  DInfo.attributes = DInfo.EntInfo.attributes;
+  DInfo.numAttributes = DInfo.EntInfo.numAttributes;
 
   getContainerInfo(D->getDeclContext(), DInfo.SemanticContainer);
   DInfo.semanticContainer = &DInfo.SemanticContainer;
@@ -443,9 +434,10 @@ bool IndexingContext::handleObjCProtocol(const ObjCProtocolDecl *D) {
 }
 
 bool IndexingContext::handleObjCCategory(const ObjCCategoryDecl *D) {
+  ScratchAlloc SA(*this);
+
   ObjCCategoryDeclInfo CatDInfo(/*isImplementation=*/false);
   EntityInfo ClassEntity;
-  ScratchAlloc SA(*this);
   const ObjCInterfaceDecl *IFaceD = D->getClassInterface();
   SourceLocation ClassLoc = D->getLocation();
   SourceLocation CategoryLoc = D->IsClassExtension() ? ClassLoc
@@ -474,10 +466,11 @@ bool IndexingContext::handleObjCCategory(const ObjCCategoryDecl *D) {
 }
 
 bool IndexingContext::handleObjCCategoryImpl(const ObjCCategoryImplDecl *D) {
+  ScratchAlloc SA(*this);
+
   const ObjCCategoryDecl *CatD = D->getCategoryDecl();
   ObjCCategoryDeclInfo CatDInfo(/*isImplementation=*/true);
   EntityInfo ClassEntity;
-  ScratchAlloc SA(*this);
   const ObjCInterfaceDecl *IFaceD = CatD->getClassInterface();
   SourceLocation ClassLoc = D->getLocation();
   SourceLocation CategoryLoc = D->getCategoryNameLoc();
@@ -522,10 +515,11 @@ bool IndexingContext::handleSynthesizedObjCMethod(const ObjCMethodDecl *D,
 }
 
 bool IndexingContext::handleObjCProperty(const ObjCPropertyDecl *D) {
+  ScratchAlloc SA(*this);
+
   ObjCPropertyDeclInfo DInfo;
   EntityInfo GetterEntity;
   EntityInfo SetterEntity;
-  ScratchAlloc SA(*this);
 
   DInfo.ObjCPropDeclInfo.declInfo = &DInfo;
 
@@ -846,11 +840,9 @@ void IndexingContext::getEntityInfo(const NamedDecl *D,
   EntityInfo.lang = CXIdxEntityLang_C;
 
   if (D->hasAttrs()) {
-    AttrListInfo *attrs = SA.allocate<AttrListInfo>();
-    new (attrs) AttrListInfo(D, *this, SA);
-    EntityInfo.AttrList = attrs;
-    EntityInfo.attributes = attrs->getAttrs();
-    EntityInfo.numAttributes = attrs->getNumAttrs();
+    EntityInfo.AttrList = AttrListInfo::create(D, *this);
+    EntityInfo.attributes = EntityInfo.AttrList->getAttrs();
+    EntityInfo.numAttributes = EntityInfo.AttrList->getNumAttrs();
   }
 
   if (const TagDecl *TD = dyn_cast<TagDecl>(D)) {

clang/tools/libclang/IndexingContext.h

@@ -25,9 +25,24 @@ namespace clang {
 
 namespace cxindex {
   class IndexingContext;
-  class ScratchAlloc;
   class AttrListInfo;
 
+class ScratchAlloc {
+  IndexingContext &IdxCtx;
+
+public:
+  explicit ScratchAlloc(IndexingContext &indexCtx);
+  ScratchAlloc(const ScratchAlloc &SA);
+
+  ~ScratchAlloc();
+
+  const char *toCStr(StringRef Str);
+  const char *copyCStr(StringRef Str);
+
+  template <typename T>
+  T *allocate();
+};
+
 struct EntityInfo : public CXIdxEntityInfo {
   const NamedDecl *Dcl;
   IndexingContext *IndexCtx;
@@ -229,16 +244,20 @@ struct IBOutletCollectionInfo : public AttrInfo {
 };
 
 class AttrListInfo {
+  ScratchAlloc SA;
+
   SmallVector<AttrInfo, 2> Attrs;
   SmallVector<IBOutletCollectionInfo, 2> IBCollAttrs;
   SmallVector<CXIdxAttrInfo *, 2> CXAttrs;
   unsigned ref_cnt;
 
+  AttrListInfo(const AttrListInfo&); // DO NOT IMPLEMENT
+  void operator=(const AttrListInfo&); // DO NOT IMPLEMENT
 public:
-  AttrListInfo(const Decl *D,
-               IndexingContext &IdxCtx,
-               ScratchAlloc &SA);
-  AttrListInfo(const AttrListInfo &other);
+  AttrListInfo(const Decl *D, IndexingContext &IdxCtx);
+
+  static IntrusiveRefCntPtr<AttrListInfo> create(const Decl *D,
+                                                 IndexingContext &IdxCtx);
 
   const CXIdxAttrInfo *const *getAttrs() const {
     if (CXAttrs.empty())
@@ -488,28 +507,23 @@ private:
   static bool shouldIgnoreIfImplicit(const Decl *D);
 };
 
-class ScratchAlloc {
-  IndexingContext &IdxCtx;
+inline ScratchAlloc::ScratchAlloc(IndexingContext &idxCtx) : IdxCtx(idxCtx) {
+  ++IdxCtx.StrAdapterCount;
+}
+inline ScratchAlloc::ScratchAlloc(const ScratchAlloc &SA) : IdxCtx(SA.IdxCtx) {
+  ++IdxCtx.StrAdapterCount;
+}
 
-public:
-  explicit ScratchAlloc(IndexingContext &indexCtx) : IdxCtx(indexCtx) {
-    ++IdxCtx.StrAdapterCount;
-  }
+inline ScratchAlloc::~ScratchAlloc() {
+  --IdxCtx.StrAdapterCount;
+  if (IdxCtx.StrAdapterCount == 0)
+    IdxCtx.StrScratch.Reset();
+}
 
-  ~ScratchAlloc() {
-    --IdxCtx.StrAdapterCount;
-    if (IdxCtx.StrAdapterCount == 0)
-      IdxCtx.StrScratch.Reset();
-  }
-
-  const char *toCStr(StringRef Str);
-  const char *copyCStr(StringRef Str);
-
-  template <typename T>
-  T *allocate() {
-    return IdxCtx.StrScratch.Allocate<T>();
-  }
-};
+template <typename T>
+inline T *ScratchAlloc::allocate() {
+  return IdxCtx.StrScratch.Allocate<T>();
+}
 
 }} // end clang::cxindex