[Bug c++/60924] New: __attribute__((no_sanitize_address)) and friends should (only?) be allowed at function definitions

[Bug c++/60924] New: __attribute__((no_sanitize_address)) and friends should (only?) be allowed at function definitions

[timurrrr at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60924

            Bug ID: 60924
           Summary: __attribute__((no_sanitize_address)) and friends
                    should (only?) be allowed at function definitions
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: timurrrr at google dot com

I believe __attribute__((no_sanitize_address)) should only be allowed at
function/method definitions.

Consider a code like this:
--- x.h ---
#define NO_ASAN __attribute__((no_sanitize_address))

class X {
  void f();
  NO_ASAN void g();
};
-----------

-- x.cpp --
#include "x.h"

NO_ASAN void X::f() {
  ...
}

void X::g() {
  ...
}
-----------

I've spent half a day analyzing a file like x.cpp before I looked at x.h and
realized a function should not be address-sanitized.
[I was getting an ASan report because the NO_ASAN macro wasn't defined due to
code misconfiguration]

I think we should disallow __attribute__((no_sanitize_address)) at declarations
because:
1) it's more consistent
2) it's much more readable, there's no "implicit attribute applied somewhere
else"
   when looking at the code of a function/method.
3) the no_sanitize_address is basically an implementation detail rather than
   a public contract, as opposed to say thiscall/noreturn/nothrow
4) applying/removing the attribute would usually result in fewer recompilations
(2000+ TUs in my case)

http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html says:
"The keyword __attribute__ allows you to specify special attributes when making
a declaration."
That probably means that attributes applied to definitions are kind of
"undocumented but seem work".

The following code indeed shows that both declaration-level attributes and
definition-level attributes work with gcc 4.8.2:
=== test.cpp ===
class X {
  void f(int *p);
  __attribute__((no_sanitize_address)) void g(int *p);
};

__attribute__((no_sanitize_address)) void X::f(int *p) {
  *p = 42;
}

void X::g(int *p) {
  *p = 42;
}
===== EOF =====
$ g++ -fsanitize=address -S -o - test.cpp  | grep asan_report
-> empty

I think GCC should explicitly allow select attributes (including
no_sanitize_address) to be applied to definitions
and deprecate some of them (including no_sanitize_address) when applied to
declarations.

[Bug c++/60924] __attribute__((no_sanitize_address)) and friends should (only?) be allowed at function definitions

[timurrrr at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60924

Timur Iskhodzhanov <timurrrr at google dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kcc at gcc dot gnu.org

--- Comment #1 from Timur Iskhodzhanov <timurrrr at google dot com> ---
[+Kostya]

[Bug c++/60924] __attribute__((no_sanitize_address)) and friends should (only?) be allowed at function definitions

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60924

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I don't see why, all other attributes are allowed on both declarations and
definitions, having some attributes that are only allowed on definitions would
be extremely confusing to users, and not really useful.

[Bug c++/60924] __attribute__((no_sanitize_address)) and friends should (only?) be allowed at function definitions

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60924

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
>I think we should disallow __attribute__((no_sanitize_address)) at declarations because:
> 1) it's more consistent

More consistent with what, it would be very inconsistent with the rest of GCC's
attributes.

>2) it's much more readable, there's no "implicit attribute applied somewhere else"
>   when looking at the code of a function/method.

This is just like where is the comment which describes what the arguments do. 
Is there is only one coding style in this world?

>4) applying/removing the attribute would usually result in fewer recompilations (2000+ TUs in my case)

This is a C++ problem rather than a GCC issue :).  Modules for the win.

[Bug c++/60924] __attribute__((no_sanitize_address)) and friends should (only?) be allowed at function definitions

[timurrrr at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60924

--- Comment #5 from Timur Iskhodzhanov <timurrrr at google dot com> ---
Jakub wrote:
> all other attributes are allowed on both declarations and definitions

Can you please point me at the documentation that says that attributes are
allowed on definitions?  The page I referenced above only mentions
declarations.

Also, I believe "all the existing/old attributes do X, so all the new ones
should do X as well" isn't an optimal long-term strategy.
It's just not flexible / generic enough.

Andrew wrote:
> More consistent with what

By "more consistent" I meant there will be fewer ways to use the attribute and
this would reduce the number of places one has to look at to find out whether a
given attribute is applied.

> Is there is only one coding style in this world?

This is indeed subjective.
Still, I think my point #3 (re: implementation detail) should be true in most
reasonable coding styles?

> Modules for the win.

True -- but they aren't used yet in most codebases AFAICT :)

[Bug c++/60924] __attribute__((no_sanitize_address)) and friends should (only?) be allowed at function definitions

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60924

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |WONTFIX
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is not going to change.  clang implements it this way too.
As mentioned GCC is consistent with itself here in that function attributes
apply for both.