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

[erik.pilkington]

Summary

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

[aaron.ballman]

LGTM!

cfe/trunk/lib/Sema/SemaType.cpp

auto isVaList = [&S](QualType T) -> bool {
     return false;
   };
 
   // Local function that checks the nullability for a given pointer declarator.
   // Returns true if _Nonnull was inferred.
   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;
 
     // If a nullability attribute is present, there's nothing to do.
     if (hasNullabilityAttr(attrs))
       return nullptr;
 
     // If we're supposed to infer nullability, do so now.
     if (inferNullability && !inferNullabilityInnerOnlyComplete) {
       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);
 
       if (inferNullabilityCS) {
         state.getDeclarator().getMutableDeclSpec().getObjCQualifiers()
           ->setObjCDeclQualifier(ObjCDeclSpec::DQ_CSNullability);
       }
 
if (T->canHaveNullability(/*ResultIfUnknown*/false) &&
         if (T->isBlockPointerType())
           pointerKind = SimplePointerKind::BlockPointer;
         else if (T->isMemberPointerType())
           pointerKind = SimplePointerKind::MemberPointer;
 
         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);
         }
       }
     }
 
     if (complainAboutMissingNullability == CAMN_Yes &&
         T->isArrayType() && !T->getNullability(S.Context) && !isVaList(T) &&
case DeclaratorChunk::Paren:
       break;
     case DeclaratorChunk::BlockPointer:
       // If blocks are disabled, emit an error.
       if (!LangOpts.Blocks)
         S.Diag(DeclType.Loc, diag::err_blocks_disable) << LangOpts.OpenCL;
 
       // 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) {
         // OpenCL v2.0, s6.12.5 - Block variable declarations are implicitly
         // qualified with const.
         if (LangOpts.OpenCL)
           DeclType.Cls.TypeQuals |= DeclSpec::TQ_const;
         T = S.BuildQualifiedType(T, DeclType.Loc, DeclType.Cls.TypeQuals);
       }
       break;
     case DeclaratorChunk::Pointer:
       // Verify that we're not building a pointer to pointer to function with
       // exception specification.
       if (LangOpts.CPlusPlus && S.CheckDistantExceptionSpec(T)) {
         S.Diag(D.getIdentifierLoc(), diag::err_distant_exception_spec);
         D.setInvalidType(true);
         // Build the type anyway.
       }
 
       // 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);
         if (DeclType.Ptr.TypeQuals)
           T = S.BuildQualifiedType(T, DeclType.Loc, DeclType.Ptr.TypeQuals);
         break;
       }
 
for (unsigned i = 0, e = D.getNumTypeObjects(); i != e; ++i) {
     }
     case DeclaratorChunk::MemberPointer: {
       // The scope spec must refer to a class, or be dependent.
       CXXScopeSpec &SS = DeclType.Mem.Scope();
       QualType ClsType;
 
       // 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.
         D.setInvalidType(true);
       } else if (S.isDependentScopeSpecifier(SS) ||
                  dyn_cast_or_null<CXXRecordDecl>(S.computeDeclContext(SS))) {
         NestedNameSpecifier *NNS = SS.getScopeRep();
         NestedNameSpecifier *NNSPrefix = NNS->getPrefix();

cfe/trunk/test/SemaObjC/nonnull.m

 void PR18795(int (^g)(const char *h, ...) __attribute__((nonnull(1))) __attribute__((nonnull))) {
   g(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
 }
 void PR18795_helper() {
   PR18795(0); // expected-warning{{null passed to a callee that requires a non-null argument}}
 }
 
 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