[Clang] Support typedef with btf_decl_tag attributes

[yonghong-song]

Summary

Previously, btf_del_tag attribute supports record, field, global variable,
function and function parameter ([1], [2]). This patch added support for typedef.
The main reason is for typedef of an anonymous struct/union, we can only apply
btf_decl_tag attribute to the anonymous struct/union like below:

typedef struct { ... } __btf_decl_tag target_type

In this case, the __btf_decl_tag attribute applies to anonymous struct,
which increases downstream implementation complexity. But if
typedef with btf_decl_tag attribute is supported, we can have

typedef struct { ... } target_type __btf_decl_tag

which applies __btf_decl_tag to typedef "target_type" which make it
easier to directly associate btf_decl_tag with a named type.
This patch permitted btf_decl_tag with typedef types with this reason.

[yonghong-song]

The corresponding clang codegen and llvm implementation: https://reviews.llvm.org/D110129

[aaron.ballman]

First, to build linux kernel with btf_tag annotated user pointer, we have

#define user attribute__((btf_tag("user")))
and the linux kernel contains code like below ([2])

typedef signalfn_t user *__sighandler_t;
and the user attribute applies to typedef type sighandler_t.
So clang needs to support btf_tag attribute with typedef type
to avoid compilation error.

I want to make sure we're clear on the semantics here. That defines __sighandler_t to be the type __signalfn_t __user *, so you expect this to annotate the type, not the declaration of the typedef. So this is about allowing btf_tag on types in addition to declarations?

typedef struct { ... } target_type __btf_tag

Similar here -- this applies the __btf_tag to the type, not to the declaration of the typedef, right?

I'm asking because this raises other questions. For example:

void func(int i);
void func(int __attribute__((btf_tag("__user"))) i);

Is the second a valid redeclaration of the first? Additionally:

__attribute__((overloadable)) void func(int i) { puts("one"); }
__attribute__((overloadable)) void func(int __attribute__((btf_tag("__user"))) i) { puts("two"); }

Is this a valid overload set because of the type attribute? Along the same lines, does adding this attribute to the type cause any ABI differences in how the type is passed?

[dblaikie]

Given this is about being preserved into debug info - I imagine it'll have the same behavior as using a typedef in a function return type - whenever that currently shows up in the DWARF, this attribute would. Where it doesn't, this doesn't.

So I wouldn't expect this feature to do any checking/require the typedef to be used consistently - but fair questions to consider.

[yonghong-song]

In D110127#3012215, @aaron.ballman wrote:

First, to build linux kernel with btf_tag annotated user pointer, we have

#define user attribute__((btf_tag("user")))
and the linux kernel contains code like below ([2])

typedef signalfn_t user *__sighandler_t;
and the user attribute applies to typedef type sighandler_t.
So clang needs to support btf_tag attribute with typedef type
to avoid compilation error.

I want to make sure we're clear on the semantics here. That defines __sighandler_t to be the type __signalfn_t __user *, so you expect this to annotate the type, not the declaration of the typedef. So this is about allowing btf_tag on types in addition to declarations?

This is a good question and below are some examples:

[$ ~/work/tests/llvm/btf_tag] cat t.c
#define __tag1 __attribute__((btf_tag("tag1")))
typedef __tag1 unsigned * __u;
__u u;
[$ ~/work/tests/llvm/btf_tag] clang -Xclang -ast-dump -c t.c
...
|-TypedefDecl 0x8279c00 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list 'struct __va_list_tag [1]'
| `-ConstantArrayType 0x8234e30 'struct __va_list_tag [1]' 1 
|   `-RecordType 0x8234c70 'struct __va_list_tag'
|     `-Record 0x8234be8 '__va_list_tag'
|-TypedefDecl 0x8279d00 <t.c:2:1, col:27> col:27 referenced __u 'unsigned int *'
| |-PointerType 0x8279cc0 'unsigned int *'
| | `-BuiltinType 0x8234040 'unsigned int'
| `-BTFTagAttr 0x8279d58 <line:1:31, col:45> "tag1"
`-VarDecl 0x8279df0 <line:3:1, col:5> col:5 u '__u':'unsigned int *'

[$ ~/work/tests/llvm/btf_tag] cat t.c
#define __tag1 __attribute__((btf_tag("tag1")))
typedef unsigned __tag1 * __u;
__u u;
[$ ~/work/tests/llvm/btf_tag] clang -Xclang -ast-dump -c t.c
...
|-TypedefDecl 0x8279c00 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list 'struct __va_list_tag [1]'
| `-ConstantArrayType 0x8234e30 'struct __va_list_tag [1]' 1 
|   `-RecordType 0x8234c70 'struct __va_list_tag'
|     `-Record 0x8234be8 '__va_list_tag'
|-TypedefDecl 0x8279d00 <t.c:2:1, col:27> col:27 referenced __u 'unsigned int *'
| |-PointerType 0x8279cc0 'unsigned int *'
| | `-BuiltinType 0x8234040 'unsigned int'
| `-BTFTagAttr 0x8279d58 <line:1:31, col:45> "tag1"
`-VarDecl 0x8279df0 <line:3:1, col:5> col:5 u '__u':'unsigned int *'

[$ ~/work/tests/llvm/btf_tag] cat t.c
#define __tag1 __attribute__((btf_tag("tag1")))
typedef unsigned * __tag1 __u;
__u u;
[$ ~/work/tests/llvm/btf_tag] clang -Xclang -ast-dump -c t.c
...
|-TypedefDecl 0x8279c00 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list 'struct __va_list_tag [1]'
| `-ConstantArrayType 0x8234e30 'struct __va_list_tag [1]' 1 
|   `-RecordType 0x8234c70 'struct __va_list_tag'
|     `-Record 0x8234be8 '__va_list_tag'
|-TypedefDecl 0x8279d00 <t.c:2:1, col:27> col:27 referenced __u 'unsigned int *'
| |-PointerType 0x8279cc0 'unsigned int *'
| | `-BuiltinType 0x8234040 'unsigned int'
| `-BTFTagAttr 0x8279d58 <line:1:31, col:45> "tag1"
`-VarDecl 0x8279df0 <line:3:1, col:5> col:5 u '__u':'unsigned int *'

[$ ~/work/tests/llvm/btf_tag] cat t.c
#define __tag1 __attribute__((btf_tag("tag1")))
typedef unsigned * __u __tag1;
__u u;
[$ ~/work/tests/llvm/btf_tag] clang -Xclang -ast-dump -c t.c
...
|-TypedefDecl 0x8279c00 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list 'struct __va_list_tag [1]'
| `-ConstantArrayType 0x8234e30 'struct __va_list_tag [1]' 1 
|   `-RecordType 0x8234c70 'struct __va_list_tag'
|     `-Record 0x8234be8 '__va_list_tag'
|-TypedefDecl 0x8279d00 <t.c:2:1, col:20> col:20 referenced __u 'unsigned int *'
| |-PointerType 0x8279cc0 'unsigned int *'
| | `-BuiltinType 0x8234040 'unsigned int'
| `-BTFTagAttr 0x8279d58 <line:1:31, col:45> "tag1"
`-VarDecl 0x8279df0 <line:3:1, col:5> col:5 u '__u':'unsigned int *'

You can see that it doesn't matter where the attribute is placed, the attribute is always attached to the typedef.
I think the reason is for declarations, we only allow the btf_tag attribute to be placed for record, field, var, func, typedef,
and "unsigned *" does not qualify, so the attribute is applied to typedef.

See below example for structure, the attribute can be applied to either structure or to typedef.

typedef struct { ... } target_type __btf_tag

Similar here -- this applies the __btf_tag to the type, not to the declaration of the typedef, right?

In the above case, it actually applied to the typedef. See below example.

[$ ~/work/tests/llvm/btf_tag] cat t.c
#define __tag1 __attribute__((btf_tag("tag1")))
typedef __tag1 struct { int a; } __s;
__s a;
[$ ~/work/tests/llvm/btf_tag] clang -Xclang -ast-dump -c t.c
...
|-RecordDecl 0x8279cb8 <t.c:2:16, col:32> col:16 struct definition
| `-FieldDecl 0x8279d78 <col:25, col:29> col:29 a 'int'
|-TypedefDecl 0x8279e28 <col:1, col:34> col:34 referenced __s 'struct __s':'__s'
| |-ElaboratedType 0x8279dd0 'struct __s' sugar
| | `-RecordType 0x8279d40 '__s'
| |   `-Record 0x8279cb8 ''
| `-BTFTagAttr 0x8279e80 <line:1:31, col:45> "tag1"
`-VarDecl 0x8279f30 <line:3:1, col:5> col:5 a '__s':'__s'

the attribute is attached to typedef.

[$ ~/work/tests/llvm/btf_tag] cat t.c
#define __tag1 __attribute__((btf_tag("tag1")))
typedef struct __tag1 { int a; } __s;
__s a;
[$ ~/work/tests/llvm/btf_tag] clang -Xclang -ast-dump -c t.c
...
|-RecordDecl 0x8279cb8 <t.c:2:9, col:32> col:9 struct definition
| |-BTFTagAttr 0x8279d60 <line:1:31, col:45> "tag1"
| `-FieldDecl 0x8279de0 <line:2:25, col:29> col:29 a 'int'
|-TypedefDecl 0x8279e88 <col:1, col:34> col:34 referenced __s 'struct __s':'__s'
| `-ElaboratedType 0x8279e30 'struct __s' sugar
|   `-RecordType 0x8279d40 '__s'
|     `-Record 0x8279cb8 ''
`-VarDecl 0x8279f30 <line:3:1, col:5> col:5 a '__s':'__s'

The attribute is attached to the structure.

[$ ~/work/tests/llvm/btf_tag] cat t.c
#define __tag1 __attribute__((btf_tag("tag1")))
typedef struct { int a; } __tag1 __s;
__s a;
[$ ~/work/tests/llvm/btf_tag] clang -Xclang -ast-dump -c t.c
...
|-RecordDecl 0x8279c58 <t.c:2:9, col:25> col:9 struct definition
| |-BTFTagAttr 0x8279dc8 <line:1:31, col:45> "tag1"
| `-FieldDecl 0x8279d18 <line:2:18, col:22> col:22 a 'int'
|-TypedefDecl 0x8279e88 <col:1, col:34> col:34 referenced __s 'struct __s':'__s'
| `-ElaboratedType 0x8279e30 'struct __s' sugar
|   `-RecordType 0x8279ce0 '__s'
|     `-Record 0x8279c58 ''
`-VarDecl 0x8279f30 <line:3:1, col:5> col:5 a '__s':'__s'

the attribute is attached to the structure.

[$ ~/work/tests/llvm/btf_tag] cat t.c
#define __tag1 __attribute__((btf_tag("tag1")))
typedef struct { int a; } __s __tag1;
__s a;
[$ ~/work/tests/llvm/btf_tag] clang -Xclang -ast-dump -c t.c
...
|-RecordDecl 0x8279c58 <t.c:2:9, col:25> col:9 struct definition
| `-FieldDecl 0x8279d18 <col:18, col:22> col:22 a 'int'
|-TypedefDecl 0x8279e28 <col:1, col:27> col:27 referenced __s 'struct __s':'__s'
| |-ElaboratedType 0x8279dd0 'struct __s' sugar
| | `-RecordType 0x8279ce0 '__s'
| |   `-Record 0x8279c58 ''
| `-BTFTagAttr 0x8279e80 <line:1:31, col:45> "tag1"
`-VarDecl 0x8279f30 <line:3:1, col:5> col:5 a '__s':'__s'

The attribute is attached to typedef.

I'm asking because this raises other questions. For example:

void func(int i);
void func(int __attribute__((btf_tag("__user"))) i);

Is the second a valid redeclaration of the first? Additionally:

This should be okay as btf_tag is accumulative attribute.

__attribute__((overloadable)) void func(int i) { puts("one"); }
__attribute__((overloadable)) void func(int __attribute__((btf_tag("__user"))) i) { puts("two"); }

Is this a valid overload set because of the type attribute? Along the same lines, does adding this attribute to the type cause any ABI differences in how the type is passed?

btf_tag is for C only so overload function case won't happen.

[aaron.ballman]

In D110127#3013391, @dblaikie wrote:

Given this is about being preserved into debug info - I imagine it'll have the same behavior as using a typedef in a function return type - whenever that currently shows up in the DWARF, this attribute would. Where it doesn't, this doesn't.

So I wouldn't expect this feature to do any checking/require the typedef to be used consistently - but fair questions to consider.

In that case, I'd assume this is a declaration attribute that appertains to the declaration of the typedef, and not to the underlying type. Basically, it sounds like this models more closely to the nodebug attribute than the address_space attribute: https://godbolt.org/z/zoo1xnG4n

(This matters because of the [[clang::btf_tag(...)]] spelling that's used in C2x.)

[aaron.ballman]

In D110127#3013876, @yonghong-song wrote:

You can see that it doesn't matter where the attribute is placed, the attribute is always attached to the typedef.
I think the reason is for declarations, we only allow the btf_tag attribute to be placed for record, field, var, func, typedef,
and "unsigned *" does not qualify, so the attribute is applied to typedef.

Agreed. If this attribute appertains to the type, then it goes through SemaType.cpp and may require extra work to encode the information into the type system. If it appertains to the declaration, then what's in SemaDeclAttr.cpp is fine, but then I have questions about the use of the attribute with cast notation from D110116 where this is being used definitely as a type attribute.

One confounding factor here is that __attribute__ will slide around to whatever makes sense. You should try your examples with [[]] in C2x mode as well -- the placement of the attribute syntax is strongly tied to what the attribute applies to.

I'm asking because this raises other questions. For example:

void func(int i);
void func(int __attribute__((btf_tag("__user"))) i);

Is the second a valid redeclaration of the first? Additionally:

This should be okay as btf_tag is accumulative attribute.

Okay, this is sounding more and more like a declaration attribute.

__attribute__((overloadable)) void func(int i) { puts("one"); }
__attribute__((overloadable)) void func(int __attribute__((btf_tag("__user"))) i) { puts("two"); }

Is this a valid overload set because of the type attribute? Along the same lines, does adding this attribute to the type cause any ABI differences in how the type is passed?

btf_tag is for C only so overload function case won't happen.

That's incorrect -- Clang supports attribute overloadable in C: https://godbolt.org/z/eThKsn3zM

[yonghong-song]

In D110127#3016371, @aaron.ballman wrote:

In D110127#3013876, @yonghong-song wrote:

You can see that it doesn't matter where the attribute is placed, the attribute is always attached to the typedef.
I think the reason is for declarations, we only allow the btf_tag attribute to be placed for record, field, var, func, typedef,
and "unsigned *" does not qualify, so the attribute is applied to typedef.

Agreed. If this attribute appertains to the type, then it goes through SemaType.cpp and may require extra work to encode the information into the type system. If it appertains to the declaration, then what's in SemaDeclAttr.cpp is fine, but then I have questions about the use of the attribute with cast notation from D110116 where this is being used definitely as a type attribute.

For typedef use case, it intends to be used as a decl attribute.

One confounding factor here is that __attribute__ will slide around to whatever makes sense. You should try your examples with [[]] in C2x mode as well -- the placement of the attribute syntax is strongly tied to what the attribute applies to.

I tried [[]] syntax, it does seem better than otherwise, e.g.,

[$ ~/work/tests/llvm/btf_tag] cat t1.c
#define __tag1 [[clang::btf_tag("tag1")]]
typedef unsigned __tag1 * __u;
__u u;
[$~/work/tests/llvm/btf_tag] clang -c -std=c2x t1.c
t1.c:2:18: error: 'btf_tag' attribute cannot be applied to types
typedef unsigned __tag1 * __u;
                 ^
t1.c:1:18: note: expanded from macro '__tag1'
#define __tag1 [[clang::btf_tag("tag1")]]
                 ^
1 error generated.
[$ ~/work/tests/llvm/btf_tag]

and

[$ ~/work/tests/llvm/btf_tag] cat t1.c
#define __tag1 [[clang::btf_tag("tag1")]]
typedef unsigned * __u __tag1;
__u u;
[$ ~/work/tests/llvm/btf_tag] clang -c -std=c2x t1.c
[$ ~/work/tests/llvm/btf_tag]

If using old __attribute__ syntax, both above examples will compile successfully.

I will change my test to use -std=c2x.

I'm asking because this raises other questions. For example:

void func(int i);
void func(int __attribute__((btf_tag("__user"))) i);

Is the second a valid redeclaration of the first? Additionally:

This should be okay as btf_tag is accumulative attribute.

Okay, this is sounding more and more like a declaration attribute.

__attribute__((overloadable)) void func(int i) { puts("one"); }
__attribute__((overloadable)) void func(int __attribute__((btf_tag("__user"))) i) { puts("two"); }

Is this a valid overload set because of the type attribute? Along the same lines, does adding this attribute to the type cause any ABI differences in how the type is passed?

btf_tag is for C only so overload function case won't happen.

That's incorrect -- Clang supports attribute overloadable in C: https://godbolt.org/z/eThKsn3zM

In this case, the overload is not valid and the attribute does not have an impact on overloading decision.

[$ ~/work/tests/llvm/btf_tag] cat t.c
#include <stdio.h>

__attribute__((overloadable)) void func(int i) { puts("int"); }
__attribute__((overloadable)) void func(int i __attribute__((btf_tag("tag2")))) { puts("float"); }

int main(void) {
  func(1);
}
[$ ~/work/tests/llvm/btf_tag] clang -c t.c
t.c:4:36: error: redefinition of 'func'
__attribute__((overloadable)) void func(int i __attribute__((btf_tag("tag2")))) { puts("float"); }
                                   ^
t.c:3:36: note: previous definition is here
__attribute__((overloadable)) void func(int i) { puts("int"); }
                                   ^
1 error generated.
[$ ~/work/tests/llvm/btf_tag]

So overloading is rejected. This is similar to below example with aligned attribute,

[$ ~/work/tests/llvm/btf_tag] cat t.c
#include <stdio.h>

__attribute__((overloadable)) void func(int i) { puts("int"); }
__attribute__((overloadable)) void func(int i __attribute__((aligned(8)))) { puts("float"); }

int main(void) {
  func(1);
}
[$ ~/work/tests/llvm/btf_tag] clang -c t.c
t.c:4:36: error: redefinition of 'func'
__attribute__((overloadable)) void func(int i __attribute__((aligned(8)))) { puts("float"); }
                                   ^
t.c:3:36: note: previous definition is here
__attribute__((overloadable)) void func(int i) { puts("int"); }
                                   ^
1 error generated.
[$ ~/work/tests/llvm/btf_tag]

[yonghong-song]

• use -std=c2x attribute syntax [[]] in test to more precisely specify the location of the attribute.

[yonghong-song]

@aaron.ballman ping. Hi, Aaron, did you get time looking at this patch again? I made some adjustments in test and explained overloadable attribute use case in comments. thanks!

[aaron.ballman]

clang/include/clang/Basic/Attr.td

1838–1843

What worries me about this is that this is now a decl *or* type attribute and one of the subjects is a typedef name which can be either a type or a decl. However, this attribute has no type semantics currently -- the only reason it's allowed on a type at all is because the Linux kernel seems to use it for cast operations, but we don't support that (we just parse it and ignore it). But then what are the semantics when used on a typedef if we want to actually support that casting case in the future?

(This is one of the reasons why GNU attributes are so incredibly difficult to use in practice -- their ability to "slide" around to whatever makes the most sense based on the name of the attribute means you run into these situations and have to work hard to tease out the desired semantics.)

[yonghong-song]

Thanks @aaron.ballman Let me think about how to deal with the typedef situation you mentioned and get back to you once I got some possible idea for discussion.

[yonghong-song]

@aaron.ballman We discussed internally and I think your concern of mixing type attribute and decl attribute is valid. In the long run, having two separate attributes, one for type (btf_type_tag) and another for decl (btf_decl_tag) seems clean and better. I have crafted one POC patch https://reviews.llvm.org/D111199, could you take a look at the approach? Also, the D111199 has hack about storing btf_type_tag string in the AST, I would really appreciate if you can give some advice how to deal with it. Thanks!

[yonghong-song]

@aaron.ballman ping. To address your concerns, I intend to create another type attribute btf_type_tag (https://reviews.llvm.org/D111199), could you comment on this new approach?

[yonghong-song]

• rebase based on new btf_decl_tag (https://reviews.llvm.org/D111588)

[yonghong-song]

@aaron.ballman ping. This patch just implemented a btf_decl_tag for typedef. There is no type attribute involved here. Could you take a look?

[aaron.ballman]

LGTM!