Attributes order, it is meaningful and clang loses it.

[llvmbot]

Bugzilla Link	3268
Version	unspecified
OS	All
Reporter	LLVM Bugzilla Contributor
CC	@asl,@efriedma-quic

Extended Description

It is critical in presence of asm attribute, the asm attribute must be first.
Instead in this code:
---->
typedef unsigned long size_t;
extern int strerror_r(int __errnum, char *__buf, size_t __buflen)
asm ("" "__xpg_strerror_r")
attribute ((nothrow))
attribute ((nonnull (2)));
----<
The asm attribute comes out last.

[efriedma-quic]

I don't follow... what's the visible issue you're seeing?

[llvmbot]

The issue is in the clang's AST.
The attributes order in the int strerror() declaration is:
no_throw
non_null([2])
asm_label(__xpg_strerror_r)

instead of
asm_label(__xpg_strerror_r)
no_throw
non_null([2])

[llvmbot]

Can someone please comment on this?

If clang has good reasons to reorder the attributes, please let us know. In this case we will write code to reorder the attributes so as to make the sequence legal. But it would be a pity to do such a hack if the right solution is simply to stop clang from reordering them.

[lattner]

I don't understand how one is semantically different than the other. Why do you care of asm is first or last?

[llvmbot]

Because asm is legal only if it is first. Since in my application I have (among other things) to warn the programmer about the fact he/she has got the ordering wrong, the fact that clang reorders the attributes makes this task difficult/impossible without reparsing.

More generally, the attribute mechanism is something relatively fluid (new attributes can and will be added): it is possible there will be other reasons not to shuffle the attributes.

[lattner]

Ok, so it doesn't break any existing clients.

[llvmbot]

Another thing to consider is that, for gcc, asm("label") is not an attribute and clang's choice to represent it as an attribute is questionable. For example, attributes can occur in function definitions, but asm("label") is only valid in function declarations.

Moreover, clang merges attributes for the same function found in different declarations and in the definition, and subsequent declarations/definition contain also the attributes present in the preceding ones. This result in asm("label") being applied to illegal contexts.

[llvmbot]

Are we actually going to fix this?

[llvmbot]

We definitely hope so. Has comment #​7 been given proper consideration?

[llvmbot]

Clang's attributes are just away to attach information to a decl "on the side", without taking space in every declaration. Nothing necessarily implies that what clang models this way has to be an actual attribute. I don't see any problem with modeling asm this way.

I do agree there are merging considerations of attributes that clang loses (or should be more careful about). However, that isn't really what this bug is about.

[bwendling]

FWIW, the -ast-dump of the original code outputs this:

`-FunctionDecl 0x11b9c720 <line:2:1, line:5:29> line:2:12 strerror_r 'int (int, char *, size_t)' extern
  |-ParmVarDecl 0x11b9c440 <col:23, col:27> col:27 __errnum 'int'
  |-ParmVarDecl 0x11b9c4c0 <col:37, col:43> col:43 __buf 'char *'
  |-ParmVarDecl 0x11b9c560 <col:50, col:57> col:57 __buflen 'size_t':'unsigned long'
  |-AsmLabelAttr 0x11b9c7c0 <line:3:6> "__xpg_strerror_r" IsLiteralLabel
  |-NoThrowAttr 0x11b9c850 <line:4:17>
  `-NonNullAttr 0x11b9c878 <line:5:17, col:27> 2

[erichkeane]

Since this has been filed, the 'ordering' of attributes has been made consistent. This happened a few years ago.

THOUGH note that 'nothrow' is actually a 'type attribute' in C++ mode, so it ends up being put into the FunctionDecl's type itself.

I think this is a 'fixed' bug.