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: 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 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. 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@lists.llvm.org > >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
