public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "minipli at grsecurity dot net" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c/81756] type attributes silently ignored on type declarations
Date: Tue, 30 Apr 2024 19:53:14 +0000	[thread overview]
Message-ID: <bug-81756-4-7A4I3nbUEC@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-81756-4@http.gcc.gnu.org/bugzilla/>

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

Mathias Krause <minipli at grsecurity dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jsm28 at gcc dot gnu.org,
                   |                            |minipli at grsecurity dot net

--- Comment #2 from Mathias Krause <minipli at grsecurity dot net> ---
It's even worse. All GNU attributes get ignored for type declarations. Standard
C attributes, however, are taken into account and I don't see the rationale
behind this? Joseph, do you remember?

The relevant documentation regarding this is even misleading. It states the
following at https://gcc.gnu.org/onlinedocs/gcc/Type-Attributes.html:
"""
You may specify type attributes in an enum, struct or union type declaration or
definition by placing them immediately after the struct, union or enum keyword. 
"""

Please note, how declarations are explicitly mentioned, making the reader
assume this, indeed, actually works.

In the very next sentence it gets even more misleading:
"""
You can also place them just past the closing curly brace of the definition,
but this is less preferred because logically the type should be fully defined
at the closing brace.
"""

This can be read as discouraging the use in definitions, implicitly implying
declarations are the preferred way to add attributes while, in fact,
declarations silently ignore these attributes. They get parsed, still, but not
attached to the type declaration at all.

The misleading behaviour can be shown by trying to compile this C code snippet:
```
struct __attribute__((aligned(64))) foo;
struct foo { char i; };

_Static_assert(__alignof__(struct foo) == 64, "decl attribute ignored!");
```

The attribute of the declaration in line 1 should be inherited by the
definition in line 2, making the assert in line 4 pass. However, basically all
versions of gcc fail the assert, as can be seen on godbolt:
https://godbolt.org/z/ssW7T363W

Clang seems to comply to gcc's documentation much better, starting as early as
of clang 3.2. Again, here's the godbolt link: https://godbolt.org/z/MnvzsvcGG

Newer versions of gcc supporting the C23 attribute syntax do pass the test when
specifying attributes using the standard syntax, as can be seen here:
https://godbolt.org/z/8Ee4E3exP

However, I don't see a reason why it should be limited to only the C23
attribute syntax. Especially, as the GNU syntax gets *silently* ignored and
clang already unconditionally excepts both variants.


The following change fixes -- at least from my understanding -- the broken
behaviour in gcc:
diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 00f8bf4376e5..02cf46f199d7 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -4114,7 +4114,8 @@ c_parser_struct_or_union_specifier (c_parser *parser)
     c_parser_error (parser, "expected %<;%>");
   /* ??? Existing practice is that GNU attributes are ignored after
      the struct or union keyword when not defining the members.  */
-  ret = parser_xref_tag (ident_loc, code, ident, have_std_attrs, std_attrs,
+  ret = parser_xref_tag (ident_loc, code, ident, have_std_attrs || attrs,
+                        std_attrs ? chainon (std_attrs, attrs) : attrs,
                         false);
   return ret;
 }

It's far from a proper change, as there are more places that need to be adapted
to, for example, cover enums as well. However, it shows where things go wrong
and where the (still parsed!) (GNU only!) attributes get silently ignored. And
yes, the comment needs a change too.

If there's consent that gcc should follow Clang's lead and actually comply to
its own documentation, I can prepare a proper patch.

Thanks,
Mathias

       reply	other threads:[~2024-04-30 19:53 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <bug-81756-4@http.gcc.gnu.org/bugzilla/>
2024-04-30 19:53 ` minipli at grsecurity dot net [this message]
2024-04-30 21:37 ` jsm28 at gcc dot gnu.org

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-81756-4-7A4I3nbUEC@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).