On Thu, Jul 5, 2018 at 8:54 AM, Keane, Erich <erich.ke...@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.ball...@gmail.com [mailto:aaron.ball...@gmail.com] On Behalf Of > Aaron Ballman > Sent: Tuesday, July 3, 2018 1:43 PM > To: Keane, Erich <erich.ke...@intel.com> > Cc: cfe-commits <cfe-commits@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@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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits