On Thu, Oct 8, 2015 at 2:45 PM, Adrian Zgorzalek <a...@fb.com> wrote:
>
> On Oct 8, 2015, at 2:17 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
>
> On Thu, Oct 8, 2015 at 2:10 PM, Adrian Zgorzalek via cfe-commits <
> 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>
>> wrote:
>> >
>> > On Thu, Oct 8, 2015 at 4:49 PM, Richard Smith <rich...@metafoo.co.uk>
>> wrote:
>> >> On Thu, Oct 8, 2015 at 1:21 PM, Aaron Ballman <aa...@aaronballman.com>
>> >> wrote:
>> >>>
>> >>> On Thu, Oct 8, 2015 at 4:16 PM, Richard Smith <rich...@metafoo.co.uk>
>> >>> wrote:
>> >>>> On Thu, Oct 8, 2015 at 12:24 PM, Aaron Ballman via cfe-commits
>> >>>> <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
>> >>>>>
>> 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
>> 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>
>
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
