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.
Adrian Zgorzalek via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 8 13:39:21 PDT 2015
Hey!
I think you are right. This will not regress any of the cases, because previous behaviour either checked str.size() >= 4 OR didn’t check it at all, but then called .substr(2, str.size() - 4), which would crash. I think, though, that explicitly checking str.size() >= 4 makes sense. See comments below.
I will have a patch to fix it up. See comments below.
Adrian
> On 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=I7JuuiH4eEYBv1meYlgoiyDdl9yvM1r3kGhI0NE1f2g%3D%0A&s=4eae7d09ce0f4330138bcb73d7c5b39ae009250829081e6ef41033dc71e8bfc4
>>> 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=I7JuuiH4eEYBv1meYlgoiyDdl9yvM1r3kGhI0NE1f2g%3D%0A&s=d7e208b0f19fa5d62b635aab96f22ed209d3ffb85762228e4cd2f87ff1cf1983
>>>
>>> ==============================================================================
>>> --- 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("__")) {
I will make it:
+ if (AttrName.size() >= 4 && 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
> 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) {
Checks size, so is fine.
>>> - 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);
- 4, so it would crash if shorter than 4 chars
>>
>>> + 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);
>>>
- 4, so it would crash if shorter than 4 chars
>>> 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=I7JuuiH4eEYBv1meYlgoiyDdl9yvM1r3kGhI0NE1f2g%3D%0A&s=05439cdca48d2211d77529dfccfb6dc62c357b71314bb08784cd50fbdbedabe1
>>>
>>> ==============================================================================
>>> --- 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=I7JuuiH4eEYBv1meYlgoiyDdl9yvM1r3kGhI0NE1f2g%3D%0A&s=22aeed2be3ecdaf531240f222bc8fca0e439bbea1d5ad3d8602d1ada60780451
>>>
>>> ==============================================================================
>>> --- 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=I7JuuiH4eEYBv1meYlgoiyDdl9yvM1r3kGhI0NE1f2g%3D%0A&s=1e420d6424a37b153567dc5a4f5c2ffaa2d7e1ecfabc1ceea65e4c8b79a41218
>>>
>>> ==============================================================================
>>> --- 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=I7JuuiH4eEYBv1meYlgoiyDdl9yvM1r3kGhI0NE1f2g%3D%0A&s=a5ca94f7c36c4d43f7d7b0dcb2908f1d43a794f6c3fb6ddebe040b4f1d1c1273
>>
>>
More information about the cfe-commits
mailing list