[Bug c/49975] New: warn on malformed __attribute((...))__

[Bug c/49975] New: warn on malformed __attribute((...))__

[ext at sidvind dot com]

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

           Summary: warn on malformed __attribute((...))__
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: ext@sidvind.com


Recently I ran into a linker error "multiple definition of `__'" which was
caused by struct with a IMO malformed attribute. Consider the following struct
in a header:

struct foo {
  int bar;
} __attribute((unused))__;

At a first glance it seems like there is nothing wrong here but since the "__"
is placed after "))" it happily creates an uninitialized global called "__".
This global will then be created in multiple files and when linking will lead
to the "multiple definition" error.

First of all, I think whitespace should be required after "))" to make it
really clear what is going happen. Secondly I don't think "__attribute(("
should work at all since the documentation states that "__attribute__" is the
keyword.

I've tried this with many different versions of gcc (including older versions
like gcc-3.3) and they all have this behaviour.

[Bug c/49975] warn on malformed __attribute((...))__

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-08-04 13:26:29 UTC ---
The above is a perfectly valid program, so we shouldn't reject it IMNSHO.
__attribute((...)) is a supported alternate spelling of __attribute__((...)).

[Bug c/49975] warn on malformed __attribute((...))__

[ext at sidvind dot com]

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

--- Comment #2 from David Sveningsson <ext at sidvind dot com> 2011-08-04 13:33:21 UTC ---
Might be valid, but confusing. If whitespace were required after "))" it would
make more sense as it is clear that a global is created.

As the documentation states that __attribute__ is the keyword, I think a
warning would be appropriate even if it is retained for compatibility.

[Bug c/49975] warn on malformed __attribute((...))__

[schwab@linux-m68k.org]

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

--- Comment #3 from Andreas Schwab <schwab@linux-m68k.org> 2011-08-04 13:45:13 UTC ---
It doesn't make sense to require whitespace after a paren, that will likely
break a lot of programs.

[Bug c/49975] warn on malformed __attribute((...))__

[ext at sidvind dot com]

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

--- Comment #4 from David Sveningsson <ext at sidvind dot com> 2011-08-04 13:48:55 UTC ---
(In reply to comment #3)
> It doesn't make sense to require whitespace after a paren, that will likely
> break a lot of programs.

In this quite specific case? "__attribute((..))foo" makes no sense to me and I
am surprised it is even legal code.

[Bug c/49975] warn on malformed __attribute((...))__

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-08-04 13:49:22 UTC ---
And similarly warning for __attribute(()) form, e.g. <X11/Xfuncproto.h> header
uses it.

[Bug c/49975] warn on malformed __attribute((...))__

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> 2011-08-04 13:56:46 UTC ---
Google codesearch shows __attribute is used widely in the android kernel and
many other places.

Whitespace is not significant in C, it would be strange for "))foo" to be
different to ")) foo", and of course "))," and "));" must be allowed.