r336225 - Fix allocation of Nullability attribute.

[Aaron Ballman via cfe-commits]

On Thu, Jul 5, 2018 at 8:54 AM, Keane, Erich <erich.keane at intel.com> wrote:
> 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.

I kind of thought that might be the case. Thank you for verifying (and the fix)!

~Aaron

>
> -----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