[PATCH] D59327: [Sema] Fix a use-after-free of a _Nonnull ParsedAttr

[Erik Pilkington via Phabricator via cfe-commits]

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

We were allocating the implicit attribute in the declarator's attribute pool, but putting into the declaration specifier's `ParsedAttributesView`. If there are multiple declarators, then we'll use the attribute from the declaration specifier after clearing out the declarators attribute pool. Fix this by allocating the attribute in the declaration specifier's pool. This problem was creating some nonsensical diagnostics and crashes on the testcase (only in NDEBUG, though).

rdar://48529718

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D59327

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaObjC/nonnull.m


Index: clang/test/SemaObjC/nonnull.m
===================================================================
--- clang/test/SemaObjC/nonnull.m
+++ clang/test/SemaObjC/nonnull.m
@@ -125,3 +125,9 @@
 }
 
 void (^PR23117)(int *) = ^(int *p1) __attribute__((nonnull(1))) {};
+
+typedef int *intptr;
+#pragma clang assume_nonnull begin
+intptr a, b;
+intptr c, (*d)();
+#pragma clang assume_nonnull end
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4221,7 +4221,7 @@
   auto inferPointerNullability =
       [&](SimplePointerKind pointerKind, SourceLocation pointerLoc,
           SourceLocation pointerEndLoc,
-          ParsedAttributesView &attrs) -> ParsedAttr * {
+          ParsedAttributesView &attrs, AttributePool &Pool) -> ParsedAttr * {
     // We've seen a pointer.
     if (NumPointersRemaining > 0)
       --NumPointersRemaining;
@@ -4235,11 +4235,9 @@
       ParsedAttr::Syntax syntax = inferNullabilityCS
                                       ? ParsedAttr::AS_ContextSensitiveKeyword
                                       : ParsedAttr::AS_Keyword;
-      ParsedAttr *nullabilityAttr =
-          state.getDeclarator().getAttributePool().create(
-              S.getNullabilityKeyword(*inferNullability),
-              SourceRange(pointerLoc), nullptr, SourceLocation(), nullptr, 0,
-              syntax);
+      ParsedAttr *nullabilityAttr = Pool.create(
+          S.getNullabilityKeyword(*inferNullability), SourceRange(pointerLoc),
+          nullptr, SourceLocation(), nullptr, 0, syntax);
 
       attrs.addAtEnd(nullabilityAttr);
 
@@ -4298,7 +4296,8 @@
         if (auto *attr = inferPointerNullability(
                 pointerKind, D.getDeclSpec().getTypeSpecTypeLoc(),
                 D.getDeclSpec().getEndLoc(),
-                D.getMutableDeclSpec().getAttributes())) {
+                D.getMutableDeclSpec().getAttributes(),
+                D.getMutableDeclSpec().getAttributePool())) {
           T = state.getAttributedType(
               createNullabilityAttr(Context, *attr, *inferNullability), T, T);
         }
@@ -4338,7 +4337,8 @@
 
       // Handle pointer nullability.
       inferPointerNullability(SimplePointerKind::BlockPointer, DeclType.Loc,
-                              DeclType.EndLoc, DeclType.getAttrs());
+                              DeclType.EndLoc, DeclType.getAttrs(),
+                              state.getDeclarator().getAttributePool());
 
       T = S.BuildBlockPointerType(T, D.getIdentifierLoc(), Name);
       if (DeclType.Cls.TypeQuals || LangOpts.OpenCL) {
@@ -4360,7 +4360,8 @@
 
       // Handle pointer nullability
       inferPointerNullability(SimplePointerKind::Pointer, DeclType.Loc,
-                              DeclType.EndLoc, DeclType.getAttrs());
+                              DeclType.EndLoc, DeclType.getAttrs(),
+                              state.getDeclarator().getAttributePool());
 
       if (LangOpts.ObjC && T->getAs<ObjCObjectType>()) {
         T = Context.getObjCObjectPointerType(T);
@@ -4892,7 +4893,8 @@
 
       // Handle pointer nullability.
       inferPointerNullability(SimplePointerKind::MemberPointer, DeclType.Loc,
-                              DeclType.EndLoc, DeclType.getAttrs());
+                              DeclType.EndLoc, DeclType.getAttrs(),
+                              state.getDeclarator().getAttributePool());
 
       if (SS.isInvalid()) {
         // Avoid emitting extra errors if we already errored on the scope.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59327.190489.patch
Type: text/x-patch
Size: 3568 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190313/e9e0184d/attachment.bin>