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.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 8 13:49:59 PDT 2015
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: 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 at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151008/a7dd4782/attachment-0001.html>
More information about the cfe-commits
mailing list