r336225 - Fix allocation of Nullability attribute.

[Keane, Erich via cfe-commits]

Unfortunately I'm not sure of a good way to validate this.  The only way I was able to even discover this was with manual instrumentation of D48788.  There is a future opportunity to better instrument the source to find these things in the future that'll catch more issues like this one however.

-----Original Message-----
From: aaron.ballman at gmail.com [mailto:aaron.ballman at gmail.com] On Behalf Of Aaron Ballman
Sent: Tuesday, July 3, 2018 1:43 PM
To: Keane, Erich <erich.keane at intel.com>
Cc: cfe-commits <cfe-commits at lists.llvm.org>
Subject: Re: r336225 - Fix allocation of Nullability attribute.

On Tue, Jul 3, 2018 at 4:30 PM, Erich Keane via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> Author: erichkeane
> Date: Tue Jul  3 13:30:34 2018
> New Revision: 336225
>
> URL: http://llvm.org/viewvc/llvm-project?rev=336225&view=rev
> Log:
> Fix allocation of Nullability attribute.
>
> Existing code always allocates for on the declarator's attribute pool, 
> but sometimes adds it to the declspec.  This patch ensures that the 
> correct pool is used.
>
>
> Discovered while testing: https://reviews.llvm.org/D48788

Can you devise a testcase for this change, or is that hard to get the original behavior to fail in a consistent way?

~Aaron

>
> Modified:
>     cfe/trunk/lib/Parse/ParseObjc.cpp
>
> Modified: cfe/trunk/lib/Parse/ParseObjc.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseObjc.cpp?
> rev=336225&r1=336224&r2=336225&view=diff
> ======================================================================
> ========
> --- cfe/trunk/lib/Parse/ParseObjc.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseObjc.cpp Tue Jul  3 13:30:34 2018
> @@ -381,25 +381,23 @@ static void addContextSensitiveTypeNulla
>                                                 SourceLocation nullabilityLoc,
>                                                 bool &addedToDeclSpec) {
>    // Create the attribute.
> -  auto getNullabilityAttr = [&]() -> AttributeList * {
> -    return D.getAttributePool().create(
> -             P.getNullabilityKeyword(nullability),
> -             SourceRange(nullabilityLoc),
> -             nullptr, SourceLocation(),
> -             nullptr, 0,
> -             AttributeList::AS_ContextSensitiveKeyword);
> +  auto getNullabilityAttr = [&](AttributePool &Pool) -> AttributeList * {
> +    return Pool.create(P.getNullabilityKeyword(nullability),
> +                       SourceRange(nullabilityLoc), nullptr, SourceLocation(),
> +                       nullptr, 0, 
> + AttributeList::AS_ContextSensitiveKeyword);
>    };
>
>    if (D.getNumTypeObjects() > 0) {
>      // Add the attribute to the declarator chunk nearest the declarator.
> -    auto nullabilityAttr = getNullabilityAttr();
> +    auto nullabilityAttr = getNullabilityAttr(D.getAttributePool());
>      DeclaratorChunk &chunk = D.getTypeObject(0);
>      nullabilityAttr->setNext(chunk.getAttrListRef());
>      chunk.getAttrListRef() = nullabilityAttr;
>    } else if (!addedToDeclSpec) {
>      // Otherwise, just put it on the declaration specifiers (if one
>      // isn't there already).
> -    D.getMutableDeclSpec().addAttributes(getNullabilityAttr());
> +    D.getMutableDeclSpec().addAttributes(
> +        getNullabilityAttr(D.getDeclSpec().getAttributePool()));
>      addedToDeclSpec = true;
>    }
>  }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits