[PATCH] D60101: [Sema] Fix a use-after-deallocate of a ParsedAttr

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added a reviewer: aaron.ballman.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

`moveAttrFromListToList` only makes sense when moving an attribute to a list with a pool that's either equivalent, or has a shorter lifetime. Therefore, using it to move a `ParsedAttr` from a declarator to a declaration specifier doesn't make sense, since the declaration specifier's pool outlives the declarator's. The patch adds a new function, ParsedAttributes::takeOneFrom, which transfers the attribute from one pool to another, fixing the use-after-deallocate.

rdar://49175426

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D60101

Files:
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaObjC/arc-property-decl-attrs.m


Index: clang/test/SemaObjC/arc-property-decl-attrs.m
===================================================================
--- clang/test/SemaObjC/arc-property-decl-attrs.m
+++ clang/test/SemaObjC/arc-property-decl-attrs.m
@@ -287,3 +287,5 @@
 @synthesize collision = _collision; // expected-note {{property synthesized here}}
 
 @end
+
+id i1, __weak i2, i3;
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -534,8 +534,8 @@
       // attribute from being applied multiple times and gives
       // the source-location-filler something to work with.
       state.saveDeclSpecAttrs();
-      moveAttrFromListToList(attr, declarator.getAttributes(),
-                             declarator.getMutableDeclSpec().getAttributes());
+      declarator.getMutableDeclSpec().getAttributes().takeOneFrom(
+          declarator.getAttributes(), &attr);
       return;
     }
   }
Index: clang/include/clang/Sema/ParsedAttr.h
===================================================================
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -659,6 +659,7 @@
 
 class AttributePool {
   friend class AttributeFactory;
+  friend class ParsedAttributes;
   AttributeFactory &Factory;
   llvm::TinyPtrVector<ParsedAttr *> Attrs;
 
@@ -892,6 +893,13 @@
     pool.takeAllFrom(attrs.pool);
   }
 
+  void takeOneFrom(ParsedAttributes &attrs, ParsedAttr *attr) {
+    attrs.getPool().remove(attr);
+    attrs.remove(attr);
+    getPool().add(attr);
+    addAtEnd(attr);
+  }
+
   void clear() {
     clearListOnly();
     pool.clear();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60101.193185.patch
Type: text/x-patch
Size: 1677 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190401/8a7ffc07/attachment-0001.bin>