r249721 - 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.
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 9 07:37:25 PDT 2015
On Fri, Oct 9, 2015 at 8:53 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Thu, Oct 8, 2015 at 6:10 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>> On Thu, Oct 8, 2015 at 2:59 PM, Adrian Zgorzalek <adek at 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? ;)
>
> I'll look into that one. :-)
I've corrected the issue, but am not overly enamored with the
diagnostic wording. If you can think of something better, please let
me know.
~Aaron
>
> ~Aaron
>
>>
>> Try this one:
>>
>> __attribute__((ownership_takes(__, 1))) void f(void*);
>>>
>>> On Oct 8, 2015, at 2:52 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>>
>>> On Thu, Oct 8, 2015 at 2:45 PM, Adrian Zgorzalek <adek at fb.com> wrote:
>>>>
>>>>
>>>> On Oct 8, 2015, at 2:17 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>>>
>>>> On Thu, Oct 8, 2015 at 2:10 PM, Adrian Zgorzalek via cfe-commits
>>>> <cfe-commits at 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 <aaron at aaronballman.com>
>>>>> > wrote:
>>>>> >
>>>>> > On Thu, Oct 8, 2015 at 4:49 PM, Richard Smith <richard at metafoo.co.uk>
>>>>> > wrote:
>>>>> >> On Thu, Oct 8, 2015 at 1:21 PM, Aaron Ballman
>>>>> >> <aaron at aaronballman.com>
>>>>> >> wrote:
>>>>> >>>
>>>>> >>> On Thu, Oct 8, 2015 at 4:16 PM, Richard Smith
>>>>> >>> <richard at metafoo.co.uk>
>>>>> >>> wrote:
>>>>> >>>> On Thu, Oct 8, 2015 at 12:24 PM, Aaron Ballman via cfe-commits
>>>>> >>>> <cfe-commits at 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 at 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 at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>
>>>>
>>>
>>>
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: attr_proto.patch
Type: application/octet-stream
Size: 5654 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151009/5b39f9b3/attachment.obj>
More information about the cfe-commits
mailing list