Yay! It worked, thank you!
On Oct 8, 2015, at 3:10 PM, Richard Smith <rich...@metafoo.co.uk<mailto:rich...@metafoo.co.uk>> wrote: On Thu, Oct 8, 2015 at 2:59 PM, Adrian Zgorzalek <a...@fb.com<mailto:a...@fb.com>> wrote: Same story: warning: 'ownership_takes' attribute only applies to functions [-Wignored-attributes] __attribute__((ownership_takes(__))) void f(); ^ Oh, I see, you're building in C, and the diagnostic here is broken (we give the "only applies to functions" diagnostic when it's applied to a function without a prototype). =( Aaron, can I tempt you to fix that? ;) Try this one: __attribute__((ownership_takes(__, 1))) void f(void*); On Oct 8, 2015, at 2:52 PM, Richard Smith <rich...@metafoo.co.uk<mailto:rich...@metafoo.co.uk>> wrote: On Thu, Oct 8, 2015 at 2:45 PM, Adrian Zgorzalek <a...@fb.com<mailto:a...@fb.com>> wrote: On Oct 8, 2015, at 2:17 PM, Richard Smith <rich...@metafoo.co.uk<mailto:rich...@metafoo.co.uk>> wrote: On Thu, Oct 8, 2015 at 2:10 PM, Adrian Zgorzalek via cfe-commits <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote: You are right, my bad, I thought this if covers all the cases, but <foo> part could be empty. Here is the fix Please add a testcase ("__attribute__((ownership_takes(__))) void f();" maybe?). I tried different attributes but none of them triggers the assert though. They all spit out warning: warning: 'ownership_takes' attribute only applies to functions [-Wignored-attributes] __attribute__((ownership_takes(__))) f(); ^ You missed the 'void'. Do you have some other idea? The '&&' should go at the end of the previous line. Adrian > On Oct 8, 2015, at 1:52 PM, Aaron Ballman > <aa...@aaronballman.com<mailto:aa...@aaronballman.com>> wrote: > > On Thu, Oct 8, 2015 at 4:49 PM, Richard Smith > <rich...@metafoo.co.uk<mailto:rich...@metafoo.co.uk>> wrote: >> On Thu, Oct 8, 2015 at 1:21 PM, Aaron Ballman >> <aa...@aaronballman.com<mailto:aa...@aaronballman.com>> >> wrote: >>> >>> On Thu, Oct 8, 2015 at 4:16 PM, Richard Smith >>> <rich...@metafoo.co.uk<mailto:rich...@metafoo.co.uk>> >>> wrote: >>>> On Thu, Oct 8, 2015 at 12:24 PM, Aaron Ballman via cfe-commits >>>> <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote: >>>>> >>>>> Author: aaronballman >>>>> Date: Thu Oct 8 14:24:08 2015 >>>>> New Revision: 249721 >>>>> >>>>> URL: >>>>> https://urldefense.proofpoint.com/v1/url?u=http://llvm.org/viewvc/llvm-project?rev%3D249721%26view%3Drev&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=ZpIyPFH5RmN0EBF%2B6Om2Hg%3D%3D%0A&m=kMsJztGqfQof%2FUicfGEXM78GJV6jd7CTOxlypvgU10A%3D%0A&s=8862f3210cdb7661e90a0d8588334d3e1cf64e92851d05bbcd2b159daf05c7dd >>>>> Log: >>>>> When mapping no_sanitize_* attributes to no_sanitize attributes, handle >>>>> GNU-style formatting that involves prefix and suffix underscores. >>>>> Cleans up >>>>> other usages of similar functionality. >>>>> >>>>> Patch by Adrian Zgorzalek! >>>>> >>>>> Modified: >>>>> cfe/trunk/lib/Sema/SemaDeclAttr.cpp >>>>> cfe/trunk/test/SemaCXX/attr-no-sanitize-address.cpp >>>>> cfe/trunk/test/SemaCXX/attr-no-sanitize-memory.cpp >>>>> cfe/trunk/test/SemaCXX/attr-no-sanitize-thread.cpp >>>>> >>>>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp >>>>> URL: >>>>> >>>>> https://urldefense.proofpoint.com/v1/url?u=http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev%3D249721%26r1%3D249720%26r2%3D249721%26view%3Ddiff&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=ZpIyPFH5RmN0EBF%2B6Om2Hg%3D%3D%0A&m=kMsJztGqfQof%2FUicfGEXM78GJV6jd7CTOxlypvgU10A%3D%0A&s=7dae8eb5cffae4493f0a6c26033810564fd7b688297fc93106a3b980f76cdd9f >>>>> >>>>> >>>>> ============================================================================== >>>>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) >>>>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Oct 8 14:24:08 2015 >>>>> @@ -1308,6 +1308,17 @@ void Sema::AddAssumeAlignedAttr(SourceRa >>>>> AssumeAlignedAttr(AttrRange, Context, E, OE, >>>>> SpellingListIndex)); >>>>> } >>>>> >>>>> +/// Normalize the attribute, __foo__ becomes foo. >>>>> +/// Returns true if normalization was applied. >>>>> +static bool normalizeName(StringRef &AttrName) { >>>>> + if (AttrName.startswith("__") && AttrName.endswith("__")) { >>>>> + assert(AttrName.size() > 4 && "Name too short"); >>>> >>>> >>>> This assert will fire on the strings __, ___, and ____, which are valid >>>> in >>>> some of the below cases. >>> >>> That assert won't fire on anything but ____ because it's &&, not ||. >> >> >> I disagree. __ starts with __ and ends with __. The right thing to do here >> is remove the assert and put back the AttrName.size() > 4 check that the >> callers used to have. > > Hah, you are correct. I hadn't considered that point. I agree with you. :-) > > ~Aaron > >> >>> I >>> don't think these names were intended to be valid in their uses. >>> However, you are correct that this will trigger assertions instead of >>> diagnostics. Adrian, can you investigate? >>> >>>> >>>>> >>>>> + AttrName = AttrName.drop_front(2).drop_back(2); >>>>> + return true; >>>>> + } >>>>> + return false; >>>>> +} >>>>> + >>>>> static void handleOwnershipAttr(Sema &S, Decl *D, const AttributeList >>>>> &AL) { >>>>> // This attribute must be applied to a function declaration. The >>>>> first >>>>> // argument to the attribute must be an identifier, the name of the >>>>> resource, >>>>> @@ -1349,11 +1360,8 @@ static void handleOwnershipAttr(Sema &S, >>>>> >>>>> IdentifierInfo *Module = AL.getArgAsIdent(0)->Ident; >>>>> >>>>> - // Normalize the argument, __foo__ becomes foo. >>>>> StringRef ModuleName = Module->getName(); >>>>> - if (ModuleName.startswith("__") && ModuleName.endswith("__") && >>>>> - ModuleName.size() > 4) { >>>>> - ModuleName = ModuleName.drop_front(2).drop_back(2); >>>>> + if (normalizeName(ModuleName)) { >>>>> Module = &S.PP.getIdentifierTable().get(ModuleName); >>>>> } >>>>> >>>>> @@ -2648,9 +2656,7 @@ static void handleFormatAttr(Sema &S, De >>>>> IdentifierInfo *II = Attr.getArgAsIdent(0)->Ident; >>>>> StringRef Format = II->getName(); >>>>> >>>>> - // Normalize the argument, __foo__ becomes foo. >>>>> - if (Format.startswith("__") && Format.endswith("__")) { >>>>> - Format = Format.substr(2, Format.size() - 4); >>>>> + if (normalizeName(Format)) { >>>>> // If we've modified the string name, we need a new identifier for >>>>> it. >>>>> II = &S.Context.Idents.get(Format); >>>>> } >>>>> @@ -3131,9 +3137,7 @@ static void handleModeAttr(Sema &S, Decl >>>>> IdentifierInfo *Name = Attr.getArgAsIdent(0)->Ident; >>>>> StringRef Str = Name->getName(); >>>>> >>>>> - // Normalize the attribute name, __foo__ becomes foo. >>>>> - if (Str.startswith("__") && Str.endswith("__")) >>>>> - Str = Str.substr(2, Str.size() - 4); >>>>> + normalizeName(Str); >>>>> >>>>> unsigned DestWidth = 0; >>>>> bool IntegerMode = true; >>>>> @@ -4533,8 +4537,10 @@ static void handleNoSanitizeAttr(Sema &S >>>>> >>>>> static void handleNoSanitizeSpecificAttr(Sema &S, Decl *D, >>>>> const AttributeList &Attr) { >>>>> + StringRef AttrName = Attr.getName()->getName(); >>>>> + normalizeName(AttrName); >>>>> std::string SanitizerName = >>>>> - llvm::StringSwitch<std::string>(Attr.getName()->getName()) >>>>> + llvm::StringSwitch<std::string>(AttrName) >>>>> .Case("no_address_safety_analysis", "address") >>>>> .Case("no_sanitize_address", "address") >>>>> .Case("no_sanitize_thread", "thread") >>>> >>>> >>>> Is there any way we could use the spelling list index in this case >>>> rather >>>> than repeating the attribute names and __-stripping here? >>> >>> The spelling list index isn't exposed in a meaningful way (and I think >>> that would be an abuse of it; I want to someday remove that >>> implementation detail to something far more private). >>> >>> I was hoping there would be a way to use the semantic spelling, but >>> the issue is that this particular attribute doesn't have semantic >>> spellings. The NoSanitizeSpecificAttr would have one, but it has no >>> AST node to hang those off of. Since we explicitly document that we do >>> not want any additional names added to this list (see Attr.td line >>> 1500), I think this is a reasonable solution as-is. >>> >>> ~Aaron >>> >>>> >>>>> Modified: cfe/trunk/test/SemaCXX/attr-no-sanitize-address.cpp >>>>> URL: >>>>> >>>>> https://urldefense.proofpoint.com/v1/url?u=http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-no-sanitize-address.cpp?rev%3D249721%26r1%3D249720%26r2%3D249721%26view%3Ddiff&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=ZpIyPFH5RmN0EBF%2B6Om2Hg%3D%3D%0A&m=kMsJztGqfQof%2FUicfGEXM78GJV6jd7CTOxlypvgU10A%3D%0A&s=46b5171e8784900ffa959a20f8824d63377f6d80d0c92c69d5a271edb5d83930 >>>>> >>>>> >>>>> ============================================================================== >>>>> --- cfe/trunk/test/SemaCXX/attr-no-sanitize-address.cpp (original) >>>>> +++ cfe/trunk/test/SemaCXX/attr-no-sanitize-address.cpp Thu Oct 8 >>>>> 14:24:08 2015 >>>>> @@ -5,12 +5,14 @@ >>>>> #if !__has_attribute(no_sanitize_address) >>>>> #error "Should support no_sanitize_address" >>>>> #endif >>>>> - >>>>> -void noanal_fun() NO_SANITIZE_ADDRESS; >>>>> - >>>>> -void noanal_fun_args() __attribute__((no_sanitize_address(1))); // \ >>>>> - // expected-error {{'no_sanitize_address' attribute takes no >>>>> arguments}} >>>>> - >>>>> + >>>>> +void noanal_fun() NO_SANITIZE_ADDRESS; >>>>> + >>>>> +void noanal_fun_alt() __attribute__((__no_sanitize_address__)); >>>>> + >>>>> +void noanal_fun_args() __attribute__((no_sanitize_address(1))); // \ >>>>> + // expected-error {{'no_sanitize_address' attribute takes no >>>>> arguments}} >>>>> + >>>>> int noanal_testfn(int y) NO_SANITIZE_ADDRESS; >>>>> >>>>> int noanal_testfn(int y) { >>>>> >>>>> Modified: cfe/trunk/test/SemaCXX/attr-no-sanitize-memory.cpp >>>>> URL: >>>>> >>>>> https://urldefense.proofpoint.com/v1/url?u=http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-no-sanitize-memory.cpp?rev%3D249721%26r1%3D249720%26r2%3D249721%26view%3Ddiff&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=ZpIyPFH5RmN0EBF%2B6Om2Hg%3D%3D%0A&m=kMsJztGqfQof%2FUicfGEXM78GJV6jd7CTOxlypvgU10A%3D%0A&s=676ca5d3b6bef5b3c04a9d6861c547a35afb54f85454fc23551da843ce369b1c >>>>> >>>>> >>>>> ============================================================================== >>>>> --- cfe/trunk/test/SemaCXX/attr-no-sanitize-memory.cpp (original) >>>>> +++ cfe/trunk/test/SemaCXX/attr-no-sanitize-memory.cpp Thu Oct 8 >>>>> 14:24:08 >>>>> 2015 >>>>> @@ -5,12 +5,14 @@ >>>>> #if !__has_attribute(no_sanitize_memory) >>>>> #error "Should support no_sanitize_memory" >>>>> #endif >>>>> - >>>>> -void noanal_fun() NO_SANITIZE_MEMORY; >>>>> - >>>>> -void noanal_fun_args() __attribute__((no_sanitize_memory(1))); // \ >>>>> - // expected-error {{'no_sanitize_memory' attribute takes no >>>>> arguments}} >>>>> - >>>>> + >>>>> +void noanal_fun() NO_SANITIZE_MEMORY; >>>>> + >>>>> +void noanal_fun_alt() __attribute__((__no_sanitize_memory__)); >>>>> + >>>>> +void noanal_fun_args() __attribute__((no_sanitize_memory(1))); // \ >>>>> + // expected-error {{'no_sanitize_memory' attribute takes no >>>>> arguments}} >>>>> + >>>>> int noanal_testfn(int y) NO_SANITIZE_MEMORY; >>>>> >>>>> int noanal_testfn(int y) { >>>>> >>>>> Modified: cfe/trunk/test/SemaCXX/attr-no-sanitize-thread.cpp >>>>> URL: >>>>> >>>>> https://urldefense.proofpoint.com/v1/url?u=http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-no-sanitize-thread.cpp?rev%3D249721%26r1%3D249720%26r2%3D249721%26view%3Ddiff&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=ZpIyPFH5RmN0EBF%2B6Om2Hg%3D%3D%0A&m=kMsJztGqfQof%2FUicfGEXM78GJV6jd7CTOxlypvgU10A%3D%0A&s=30f400a0be1c699374d1f30504fa1f190158506f984bbeac66fe6998be9fd7b7 >>>>> >>>>> >>>>> ============================================================================== >>>>> --- cfe/trunk/test/SemaCXX/attr-no-sanitize-thread.cpp (original) >>>>> +++ cfe/trunk/test/SemaCXX/attr-no-sanitize-thread.cpp Thu Oct 8 >>>>> 14:24:08 >>>>> 2015 >>>>> @@ -5,12 +5,14 @@ >>>>> #if !__has_attribute(no_sanitize_thread) >>>>> #error "Should support no_sanitize_thread" >>>>> #endif >>>>> - >>>>> -void noanal_fun() NO_SANITIZE_THREAD; >>>>> - >>>>> -void noanal_fun_args() __attribute__((no_sanitize_thread(1))); // \ >>>>> - // expected-error {{'no_sanitize_thread' attribute takes no >>>>> arguments}} >>>>> - >>>>> + >>>>> +void noanal_fun() NO_SANITIZE_THREAD; >>>>> + >>>>> +void noanal_fun_alt() __attribute__((__no_sanitize_thread__)); >>>>> + >>>>> +void noanal_fun_args() __attribute__((no_sanitize_thread(1))); // \ >>>>> + // expected-error {{'no_sanitize_thread' attribute takes no >>>>> arguments}} >>>>> + >>>>> int noanal_testfn(int y) NO_SANITIZE_THREAD; >>>>> >>>>> int noanal_testfn(int y) { >>>>> >>>>> >>>>> _______________________________________________ >>>>> cfe-commits mailing list >>>>> cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org> >>>>> https://urldefense.proofpoint.com/v1/url?u=http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=ZpIyPFH5RmN0EBF%2B6Om2Hg%3D%3D%0A&m=kMsJztGqfQof%2FUicfGEXM78GJV6jd7CTOxlypvgU10A%3D%0A&s=76daddf3012e1755f9df59be7e195c26e1d2179d81b412635ff70771ec3f1ed5 >>>> >>>> >> >> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits<https://urldefense.proofpoint.com/v1/url?u=http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=ZpIyPFH5RmN0EBF%2B6Om2Hg%3D%3D%0A&m=WuZEZVUEJf2wFyWAtrTGXbBNpzW%2BZZw00tc0Mznrgws%3D%0A&s=58e8244698d21f8b1b08bb94af4fa2a7c7767fae61ec85c73e89c56a7de5b422>
fix_assert.patch
Description: fix_assert.patch
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
