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
Thu Oct 8 13:21:31 PDT 2015


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: http://llvm.org/viewvc/llvm-project?rev=249721&view=rev
>> 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:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=249721&r1=249720&r2=249721&view=diff
>>
>> ==============================================================================
>> --- 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
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:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-no-sanitize-address.cpp?rev=249721&r1=249720&r2=249721&view=diff
>>
>> ==============================================================================
>> --- 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:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-no-sanitize-memory.cpp?rev=249721&r1=249720&r2=249721&view=diff
>>
>> ==============================================================================
>> --- 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:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-no-sanitize-thread.cpp?rev=249721&r1=249720&r2=249721&view=diff
>>
>> ==============================================================================
>> --- 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
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>


More information about the cfe-commits mailing list