| Message ID | 20181214233427.2667602-1-yhs@fb.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: btf: fix struct/union/fwd types with kind_flag | expand |
On Fri, Dec 14, 2018 at 03:34:27PM -0800, Yonghong Song wrote: > This patch fixed two issues with BTF. One is related to > struct/union bitfield encoding and the other is related to > forward type. > > Issue #1 and solution: > ====================== > > Current btf encoding of bitfield follows what pahole generates. > For each bitfield, pahole will duplicate the type chain and > put the bitfield size at the final int or enum type. > Since the BTF enum type cannot encode bit size, > pahole workarounds the issue by generating > an int type whenever the enum bit size is not 32. > > For example, > -bash-4.4$ cat t.c > typedef int ___int; > enum A { A1, A2, A3 }; > struct t { > int a[5]; > ___int b:4; > volatile enum A c:4; > } g; > -bash-4.4$ gcc -c -O2 -g t.c > The current kernel supports the following BTF encoding: > $ pahole -JV t.o > [1] TYPEDEF ___int type_id=2 > [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > [3] ENUM A size=4 vlen=3 > A1 val=0 > A2 val=1 > A3 val=2 > [4] STRUCT t size=24 vlen=3 > a type_id=5 bits_offset=0 > b type_id=9 bits_offset=160 > c type_id=11 bits_offset=164 > [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 > [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) > [7] VOLATILE (anon) type_id=3 > [8] INT int size=1 bit_offset=0 nr_bits=4 encoding=(none) > [9] TYPEDEF ___int type_id=8 > [10] INT (anon) size=1 bit_offset=0 nr_bits=4 encoding=SIGNED > [11] VOLATILE (anon) type_id=10 > > Two issues are in the above: > . by changing enum type to int, we lost the original > type information and this will not be ideal later > when we try to convert BTF to a header file. > . the type duplication for bitfields will cause > BTF bloat. Duplicated types cannot be deduplicated > later if the bitfield size is different. > > To fix this issue, this patch implemented a compatible > change for BTF struct type encoding: > . the bit 31 of struct_type->info, previously reserved, > now is used to indicate whether bitfield_size is > encoded in btf_member or not. > . if bit 31 of struct_type->info is set, > btf_member->offset will encode like: > bit 0 - 23: bit offset > bit 24 - 31: bitfield size > if bit 31 is not set, the old behavior is preserved: > bit 0 - 31: bit offset > > So if the struct contains a bit field, the maximum bit offset > will be reduced to (2^24 - 1) instead of MAX_UINT. The maximum > bitfield size will be 256 which is enough for today as maximum > bitfield in compiler can be 128 where int128 type is supported. > > This kernel patch intends to support the new BTF encoding: > $ pahole -JV t.o > [1] TYPEDEF ___int type_id=2 > [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > [3] ENUM A size=4 vlen=3 > A1 val=0 > A2 val=1 > A3 val=2 > [4] STRUCT t kind_flag=1 size=24 vlen=3 > a type_id=5 bitfield_size=0 bits_offset=0 > b type_id=1 bitfield_size=4 bits_offset=160 > c type_id=7 bitfield_size=4 bits_offset=164 > [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 > [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) > [7] VOLATILE (anon) type_id=3 > > Issue #2 and solution: > ====================== > > Current forward type in BTF does not specify whether the original > type is struct or union. This will not work for type pretty print > and BTF-to-header-file conversion as struct/union must be specified. > $ cat tt.c > struct t; > union u; > int foo(struct t *t, union u *u) { return 0; } > $ gcc -c -g -O2 tt.c > $ pahole -JV tt.o > [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > [2] FWD t type_id=0 > [3] PTR (anon) type_id=2 > [4] FWD u type_id=0 > [5] PTR (anon) type_id=4 > > To fix this issue, similar to issue #1, type->info bit 31 > is used. If the bit is set, it is union type. Otherwise, it is > a struct type. > > $ pahole -JV tt.o > [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > [2] FWD t kind_flag=0 type_id=0 > [3] PTR (anon) kind_flag=0 type_id=2 > [4] FWD u kind_flag=1 type_id=0 > [5] PTR (anon) kind_flag=0 type_id=4 > > Pahole/LLVM change: > =================== > > The new kind_flag functionality has been implemented in pahole > and llvm: > https://github.com/yonghong-song/pahole/tree/bitfield > https://github.com/yonghong-song/llvm/tree/bitfield > > Note that pahole hasn't implemented func/func_proto kind > and .BTF.ext. So to print function signature with bpftool, > the llvm compiler should be used. > > Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)") > Signed-off-by: Martin KaFai Lau <kafai@fb.com> > Signed-off-by: Yonghong Song <yhs@fb.com> > --- > include/uapi/linux/btf.h | 15 ++- > kernel/bpf/btf.c | 274 ++++++++++++++++++++++++++++++++++++--- > 2 files changed, 267 insertions(+), 22 deletions(-) > > diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h > index 14f66948fc95..34aba40ed926 100644 > --- a/include/uapi/linux/btf.h > +++ b/include/uapi/linux/btf.h > @@ -34,7 +34,9 @@ struct btf_type { > * bits 0-15: vlen (e.g. # of struct's members) > * bits 16-23: unused > * bits 24-27: kind (e.g. int, ptr, array...etc) > - * bits 28-31: unused > + * bits 28-30: unused > + * bit 31: kind_flag, currently used by > + * struct, union and fwd > */ > __u32 info; > /* "size" is used by INT, ENUM, STRUCT and UNION. > @@ -52,6 +54,7 @@ struct btf_type { > > #define BTF_INFO_KIND(info) (((info) >> 24) & 0x0f) > #define BTF_INFO_VLEN(info) ((info) & 0xffff) > +#define BTF_INFO_KFLAG(info) ((info) >> 31) > > #define BTF_KIND_UNKN 0 /* Unknown */ > #define BTF_KIND_INT 1 /* Integer */ > @@ -110,9 +113,17 @@ struct btf_array { > struct btf_member { > __u32 name_off; > __u32 type; > - __u32 offset; /* offset in bits */ > + __u32 offset; /* [bitfield_size and] offset in bits */ > }; > > +/* If the type info kind_flag set, the btf_member.offset > + * contains both member bit offset and bitfield size, and > + * bitfield size will set for struct/union bitfield members. > + * Otherwise, it contains only bit offset. > + */ nit. It may be better to move this comment to the btf_member.offset above. > +#define BTF_MEMBER_BITFIELD_SIZE(val) ((val) >> 24) > +#define BTF_MEMBER_BIT_OFFSET(val) ((val) & 0xffffff) After re-thinking this setup again, I still think having these macros in btf.h to also do the kflag checking would be nice. Unlike BTF_INFO_KIND() and BTF_INT_ENCODING() which don't depend on other facts, the btf.h raw user must check kflag anyway before calling BTF_MEMBER_BIT*(). Forcing a kflag check before the user can access these convenient 0xfffff and >>24 conversions may enforce this kflag check to some extend. Since it is in uapi, it will not be easy to change later. The above concern could be overkill ;), just want to ensure it has been thought through a bit more here. It could be as easy as moving the new btf_member_bit*() from btf.c to here and remove these two macros (or move them back to btf.c). > + > /* BTF_KIND_FUNC_PROTO is followed by multiple "struct btf_param". > * The exact number of btf_param is stored in the vlen (of the > * info in "struct btf_type"). > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index 72caa799e82f..ec3f80d3bef6 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -164,7 +164,7 @@ > #define BITS_ROUNDUP_BYTES(bits) \ > (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits)) > > -#define BTF_INFO_MASK 0x0f00ffff > +#define BTF_INFO_MASK 0x8f00ffff > #define BTF_INT_MASK 0x0fffffff > #define BTF_TYPE_ID_VALID(type_id) ((type_id) <= BTF_MAX_TYPE) > #define BTF_STR_OFFSET_VALID(name_off) ((name_off) <= BTF_MAX_NAME_OFFSET) > @@ -274,6 +274,10 @@ struct btf_kind_operations { > const struct btf_type *struct_type, > const struct btf_member *member, > const struct btf_type *member_type); > + int (*check_kflag_member)(struct btf_verifier_env *env, > + const struct btf_type *struct_type, > + const struct btf_member *member, > + const struct btf_type *member_type); > void (*log_details)(struct btf_verifier_env *env, > const struct btf_type *t); > void (*seq_show)(const struct btf *btf, const struct btf_type *t, > @@ -419,6 +423,25 @@ static u16 btf_type_vlen(const struct btf_type *t) > return BTF_INFO_VLEN(t->info); > } > [ ... ] > +static int btf_enum_check_kflag_member(struct btf_verifier_env *env, > + const struct btf_type *struct_type, > + const struct btf_member *member, > + const struct btf_type *member_type) > +{ > + u32 struct_bits_off, nr_bits, bytes_end, struct_size; > + u32 int_bitsize = sizeof(int) * BITS_PER_BYTE; > + > + struct_bits_off = BTF_MEMBER_BIT_OFFSET(member->offset); > + nr_bits = BTF_MEMBER_BITFIELD_SIZE(member->offset); > + if (!nr_bits) { > + nr_bits = int_bitsize; For non bitfield enum (i.e. !nr_bits), does it have to check for byte alignment also? i.e. BITS_PER_BYTE_MASKED(struct_bits_off). Others look good. > + } else if (nr_bits > int_bitsize) { > + btf_verifier_log_member(env, struct_type, member, > + "Invalid member bitfield_size"); > + return -EINVAL; > + } > + > + struct_size = struct_type->size; > + bytes_end = BITS_ROUNDUP_BYTES(struct_bits_off + nr_bits); > + if (struct_size < bytes_end) { > + btf_verifier_log_member(env, struct_type, member, > + "Member exceeds struct_size"); > + return -EINVAL; > + } > + > + return 0; > +} > +
On 12/15/18 8:37 AM, Martin Lau wrote: > On Fri, Dec 14, 2018 at 03:34:27PM -0800, Yonghong Song wrote: >> This patch fixed two issues with BTF. One is related to >> struct/union bitfield encoding and the other is related to >> forward type. >> >> Issue #1 and solution: >> ====================== >> >> Current btf encoding of bitfield follows what pahole generates. >> For each bitfield, pahole will duplicate the type chain and >> put the bitfield size at the final int or enum type. >> Since the BTF enum type cannot encode bit size, >> pahole workarounds the issue by generating >> an int type whenever the enum bit size is not 32. >> >> For example, >> -bash-4.4$ cat t.c >> typedef int ___int; >> enum A { A1, A2, A3 }; >> struct t { >> int a[5]; >> ___int b:4; >> volatile enum A c:4; >> } g; >> -bash-4.4$ gcc -c -O2 -g t.c >> The current kernel supports the following BTF encoding: >> $ pahole -JV t.o >> [1] TYPEDEF ___int type_id=2 >> [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >> [3] ENUM A size=4 vlen=3 >> A1 val=0 >> A2 val=1 >> A3 val=2 >> [4] STRUCT t size=24 vlen=3 >> a type_id=5 bits_offset=0 >> b type_id=9 bits_offset=160 >> c type_id=11 bits_offset=164 >> [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 >> [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) >> [7] VOLATILE (anon) type_id=3 >> [8] INT int size=1 bit_offset=0 nr_bits=4 encoding=(none) >> [9] TYPEDEF ___int type_id=8 >> [10] INT (anon) size=1 bit_offset=0 nr_bits=4 encoding=SIGNED >> [11] VOLATILE (anon) type_id=10 >> >> Two issues are in the above: >> . by changing enum type to int, we lost the original >> type information and this will not be ideal later >> when we try to convert BTF to a header file. >> . the type duplication for bitfields will cause >> BTF bloat. Duplicated types cannot be deduplicated >> later if the bitfield size is different. >> >> To fix this issue, this patch implemented a compatible >> change for BTF struct type encoding: >> . the bit 31 of struct_type->info, previously reserved, >> now is used to indicate whether bitfield_size is >> encoded in btf_member or not. >> . if bit 31 of struct_type->info is set, >> btf_member->offset will encode like: >> bit 0 - 23: bit offset >> bit 24 - 31: bitfield size >> if bit 31 is not set, the old behavior is preserved: >> bit 0 - 31: bit offset >> >> So if the struct contains a bit field, the maximum bit offset >> will be reduced to (2^24 - 1) instead of MAX_UINT. The maximum >> bitfield size will be 256 which is enough for today as maximum >> bitfield in compiler can be 128 where int128 type is supported. >> >> This kernel patch intends to support the new BTF encoding: >> $ pahole -JV t.o >> [1] TYPEDEF ___int type_id=2 >> [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >> [3] ENUM A size=4 vlen=3 >> A1 val=0 >> A2 val=1 >> A3 val=2 >> [4] STRUCT t kind_flag=1 size=24 vlen=3 >> a type_id=5 bitfield_size=0 bits_offset=0 >> b type_id=1 bitfield_size=4 bits_offset=160 >> c type_id=7 bitfield_size=4 bits_offset=164 >> [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 >> [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) >> [7] VOLATILE (anon) type_id=3 >> >> Issue #2 and solution: >> ====================== >> >> Current forward type in BTF does not specify whether the original >> type is struct or union. This will not work for type pretty print >> and BTF-to-header-file conversion as struct/union must be specified. >> $ cat tt.c >> struct t; >> union u; >> int foo(struct t *t, union u *u) { return 0; } >> $ gcc -c -g -O2 tt.c >> $ pahole -JV tt.o >> [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >> [2] FWD t type_id=0 >> [3] PTR (anon) type_id=2 >> [4] FWD u type_id=0 >> [5] PTR (anon) type_id=4 >> >> To fix this issue, similar to issue #1, type->info bit 31 >> is used. If the bit is set, it is union type. Otherwise, it is >> a struct type. >> >> $ pahole -JV tt.o >> [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >> [2] FWD t kind_flag=0 type_id=0 >> [3] PTR (anon) kind_flag=0 type_id=2 >> [4] FWD u kind_flag=1 type_id=0 >> [5] PTR (anon) kind_flag=0 type_id=4 >> >> Pahole/LLVM change: >> =================== >> >> The new kind_flag functionality has been implemented in pahole >> and llvm: >> https://github.com/yonghong-song/pahole/tree/bitfield >> https://github.com/yonghong-song/llvm/tree/bitfield >> >> Note that pahole hasn't implemented func/func_proto kind >> and .BTF.ext. So to print function signature with bpftool, >> the llvm compiler should be used. >> >> Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)") >> Signed-off-by: Martin KaFai Lau <kafai@fb.com> >> Signed-off-by: Yonghong Song <yhs@fb.com> >> --- >> include/uapi/linux/btf.h | 15 ++- >> kernel/bpf/btf.c | 274 ++++++++++++++++++++++++++++++++++++--- >> 2 files changed, 267 insertions(+), 22 deletions(-) >> >> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h >> index 14f66948fc95..34aba40ed926 100644 >> --- a/include/uapi/linux/btf.h >> +++ b/include/uapi/linux/btf.h >> @@ -34,7 +34,9 @@ struct btf_type { >> * bits 0-15: vlen (e.g. # of struct's members) >> * bits 16-23: unused >> * bits 24-27: kind (e.g. int, ptr, array...etc) >> - * bits 28-31: unused >> + * bits 28-30: unused >> + * bit 31: kind_flag, currently used by >> + * struct, union and fwd >> */ >> __u32 info; >> /* "size" is used by INT, ENUM, STRUCT and UNION. >> @@ -52,6 +54,7 @@ struct btf_type { >> >> #define BTF_INFO_KIND(info) (((info) >> 24) & 0x0f) >> #define BTF_INFO_VLEN(info) ((info) & 0xffff) >> +#define BTF_INFO_KFLAG(info) ((info) >> 31) >> >> #define BTF_KIND_UNKN 0 /* Unknown */ >> #define BTF_KIND_INT 1 /* Integer */ >> @@ -110,9 +113,17 @@ struct btf_array { >> struct btf_member { >> __u32 name_off; >> __u32 type; >> - __u32 offset; /* offset in bits */ >> + __u32 offset; /* [bitfield_size and] offset in bits */ >> }; >> >> +/* If the type info kind_flag set, the btf_member.offset >> + * contains both member bit offset and bitfield size, and >> + * bitfield size will set for struct/union bitfield members. >> + * Otherwise, it contains only bit offset. >> + */ > nit. It may be better to move this comment to the btf_member.offset > above. Make sense. Will have smaller comments as well before the below two macros to indicate kind_flag needs to be set for the below two macros make sense. > >> +#define BTF_MEMBER_BITFIELD_SIZE(val) ((val) >> 24) >> +#define BTF_MEMBER_BIT_OFFSET(val) ((val) & 0xffffff) > After re-thinking this setup again, I still think > having these macros in btf.h to also do the kflag checking > would be nice. > > Unlike BTF_INFO_KIND() and BTF_INT_ENCODING() which don't > depend on other facts, the btf.h raw user must check kflag > anyway before calling BTF_MEMBER_BIT*(). > Forcing a kflag check before the user can access these convenient > 0xfffff and >>24 conversions may enforce this kflag check to > some extend. > > Since it is in uapi, it will not be easy to change later. > The above concern could be overkill ;), just want to ensure > it has been thought through a bit more here. > > It could be as easy as moving the new btf_member_bit*() from > btf.c to here and remove these two macros (or move them back to btf.c). Not an expert in uapi design. I did not put static inline function (check kflag and get bitfield_size/offset) there with following reasons. (1). The user/kernel may want to check kflag either to simplify their code or with certain guaranteed properties, e.g., extra offset/regular_int checking. So putting macros in the header file will be good and necessary as we do not want end user (kernel/usr) to have "shift" or "and" in their codes. (2). putting static inline functions there feels a little bit redundant and could be misleading here if used blindly. First, we have macros here which you can already get information. Second, if user just uses btf_member_bitfield_size() and assumes that bitfield_size = 0, that will be wrong since bitfield_size = 0 only if kflag is set, user needs to trace to base int to find real bitfield size if kflag = 0. So user has to be aware of kflag even if he calls btf_member_bitfield_size(). > >> + >> /* BTF_KIND_FUNC_PROTO is followed by multiple "struct btf_param". >> * The exact number of btf_param is stored in the vlen (of the >> * info in "struct btf_type"). >> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c >> index 72caa799e82f..ec3f80d3bef6 100644 >> --- a/kernel/bpf/btf.c >> +++ b/kernel/bpf/btf.c >> @@ -164,7 +164,7 @@ >> #define BITS_ROUNDUP_BYTES(bits) \ >> (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits)) >> >> -#define BTF_INFO_MASK 0x0f00ffff >> +#define BTF_INFO_MASK 0x8f00ffff >> #define BTF_INT_MASK 0x0fffffff >> #define BTF_TYPE_ID_VALID(type_id) ((type_id) <= BTF_MAX_TYPE) >> #define BTF_STR_OFFSET_VALID(name_off) ((name_off) <= BTF_MAX_NAME_OFFSET) >> @@ -274,6 +274,10 @@ struct btf_kind_operations { >> const struct btf_type *struct_type, >> const struct btf_member *member, >> const struct btf_type *member_type); >> + int (*check_kflag_member)(struct btf_verifier_env *env, >> + const struct btf_type *struct_type, >> + const struct btf_member *member, >> + const struct btf_type *member_type); >> void (*log_details)(struct btf_verifier_env *env, >> const struct btf_type *t); >> void (*seq_show)(const struct btf *btf, const struct btf_type *t, >> @@ -419,6 +423,25 @@ static u16 btf_type_vlen(const struct btf_type *t) >> return BTF_INFO_VLEN(t->info); >> } >> > > [ ... ] > >> +static int btf_enum_check_kflag_member(struct btf_verifier_env *env, >> + const struct btf_type *struct_type, >> + const struct btf_member *member, >> + const struct btf_type *member_type) >> +{ >> + u32 struct_bits_off, nr_bits, bytes_end, struct_size; >> + u32 int_bitsize = sizeof(int) * BITS_PER_BYTE; >> + >> + struct_bits_off = BTF_MEMBER_BIT_OFFSET(member->offset); >> + nr_bits = BTF_MEMBER_BITFIELD_SIZE(member->offset); >> + if (!nr_bits) { >> + nr_bits = int_bitsize; > For non bitfield enum (i.e. !nr_bits), does it have to check for > byte alignment also? i.e. BITS_PER_BYTE_MASKED(struct_bits_off). Good catch. Will add this in next revision. > > Others look good. > >> + } else if (nr_bits > int_bitsize) { >> + btf_verifier_log_member(env, struct_type, member, >> + "Invalid member bitfield_size"); >> + return -EINVAL; >> + } >> + >> + struct_size = struct_type->size; >> + bytes_end = BITS_ROUNDUP_BYTES(struct_bits_off + nr_bits); >> + if (struct_size < bytes_end) { >> + btf_verifier_log_member(env, struct_type, member, >> + "Member exceeds struct_size"); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> +
On Sat, Dec 15, 2018 at 04:37:06PM +0000, Martin Lau wrote: > On Fri, Dec 14, 2018 at 03:34:27PM -0800, Yonghong Song wrote: > > This patch fixed two issues with BTF. One is related to > > struct/union bitfield encoding and the other is related to > > forward type. > > > > Issue #1 and solution: > > ====================== > > > > Current btf encoding of bitfield follows what pahole generates. > > For each bitfield, pahole will duplicate the type chain and > > put the bitfield size at the final int or enum type. > > Since the BTF enum type cannot encode bit size, > > pahole workarounds the issue by generating > > an int type whenever the enum bit size is not 32. > > > > For example, > > -bash-4.4$ cat t.c > > typedef int ___int; > > enum A { A1, A2, A3 }; > > struct t { > > int a[5]; > > ___int b:4; > > volatile enum A c:4; > > } g; > > -bash-4.4$ gcc -c -O2 -g t.c > > The current kernel supports the following BTF encoding: > > $ pahole -JV t.o > > [1] TYPEDEF ___int type_id=2 > > [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > > [3] ENUM A size=4 vlen=3 > > A1 val=0 > > A2 val=1 > > A3 val=2 > > [4] STRUCT t size=24 vlen=3 > > a type_id=5 bits_offset=0 > > b type_id=9 bits_offset=160 > > c type_id=11 bits_offset=164 > > [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 > > [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) > > [7] VOLATILE (anon) type_id=3 > > [8] INT int size=1 bit_offset=0 nr_bits=4 encoding=(none) > > [9] TYPEDEF ___int type_id=8 > > [10] INT (anon) size=1 bit_offset=0 nr_bits=4 encoding=SIGNED > > [11] VOLATILE (anon) type_id=10 > > > > Two issues are in the above: > > . by changing enum type to int, we lost the original > > type information and this will not be ideal later > > when we try to convert BTF to a header file. > > . the type duplication for bitfields will cause > > BTF bloat. Duplicated types cannot be deduplicated > > later if the bitfield size is different. > > > > To fix this issue, this patch implemented a compatible > > change for BTF struct type encoding: > > . the bit 31 of struct_type->info, previously reserved, > > now is used to indicate whether bitfield_size is > > encoded in btf_member or not. > > . if bit 31 of struct_type->info is set, > > btf_member->offset will encode like: > > bit 0 - 23: bit offset > > bit 24 - 31: bitfield size > > if bit 31 is not set, the old behavior is preserved: > > bit 0 - 31: bit offset > > > > So if the struct contains a bit field, the maximum bit offset > > will be reduced to (2^24 - 1) instead of MAX_UINT. The maximum > > bitfield size will be 256 which is enough for today as maximum > > bitfield in compiler can be 128 where int128 type is supported. > > > > This kernel patch intends to support the new BTF encoding: > > $ pahole -JV t.o > > [1] TYPEDEF ___int type_id=2 > > [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > > [3] ENUM A size=4 vlen=3 > > A1 val=0 > > A2 val=1 > > A3 val=2 > > [4] STRUCT t kind_flag=1 size=24 vlen=3 > > a type_id=5 bitfield_size=0 bits_offset=0 > > b type_id=1 bitfield_size=4 bits_offset=160 > > c type_id=7 bitfield_size=4 bits_offset=164 > > [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 > > [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) > > [7] VOLATILE (anon) type_id=3 > > > > Issue #2 and solution: > > ====================== > > > > Current forward type in BTF does not specify whether the original > > type is struct or union. This will not work for type pretty print > > and BTF-to-header-file conversion as struct/union must be specified. > > $ cat tt.c > > struct t; > > union u; > > int foo(struct t *t, union u *u) { return 0; } > > $ gcc -c -g -O2 tt.c > > $ pahole -JV tt.o > > [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > > [2] FWD t type_id=0 > > [3] PTR (anon) type_id=2 > > [4] FWD u type_id=0 > > [5] PTR (anon) type_id=4 > > > > To fix this issue, similar to issue #1, type->info bit 31 > > is used. If the bit is set, it is union type. Otherwise, it is > > a struct type. > > > > $ pahole -JV tt.o > > [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > > [2] FWD t kind_flag=0 type_id=0 > > [3] PTR (anon) kind_flag=0 type_id=2 > > [4] FWD u kind_flag=1 type_id=0 > > [5] PTR (anon) kind_flag=0 type_id=4 > > > > Pahole/LLVM change: > > =================== > > > > The new kind_flag functionality has been implemented in pahole > > and llvm: > > https://github.com/yonghong-song/pahole/tree/bitfield > > https://github.com/yonghong-song/llvm/tree/bitfield > > > > Note that pahole hasn't implemented func/func_proto kind > > and .BTF.ext. So to print function signature with bpftool, > > the llvm compiler should be used. > > > > Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)") > > Signed-off-by: Martin KaFai Lau <kafai@fb.com> > > Signed-off-by: Yonghong Song <yhs@fb.com> > > --- > > include/uapi/linux/btf.h | 15 ++- > > kernel/bpf/btf.c | 274 ++++++++++++++++++++++++++++++++++++--- > > 2 files changed, 267 insertions(+), 22 deletions(-) > > > > diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h > > index 14f66948fc95..34aba40ed926 100644 > > --- a/include/uapi/linux/btf.h > > +++ b/include/uapi/linux/btf.h > > @@ -34,7 +34,9 @@ struct btf_type { > > * bits 0-15: vlen (e.g. # of struct's members) > > * bits 16-23: unused > > * bits 24-27: kind (e.g. int, ptr, array...etc) > > - * bits 28-31: unused > > + * bits 28-30: unused > > + * bit 31: kind_flag, currently used by > > + * struct, union and fwd > > */ > > __u32 info; > > /* "size" is used by INT, ENUM, STRUCT and UNION. > > @@ -52,6 +54,7 @@ struct btf_type { > > > > #define BTF_INFO_KIND(info) (((info) >> 24) & 0x0f) > > #define BTF_INFO_VLEN(info) ((info) & 0xffff) > > +#define BTF_INFO_KFLAG(info) ((info) >> 31) > > > > #define BTF_KIND_UNKN 0 /* Unknown */ > > #define BTF_KIND_INT 1 /* Integer */ > > @@ -110,9 +113,17 @@ struct btf_array { > > struct btf_member { > > __u32 name_off; > > __u32 type; > > - __u32 offset; /* offset in bits */ > > + __u32 offset; /* [bitfield_size and] offset in bits */ > > }; > > > > +/* If the type info kind_flag set, the btf_member.offset > > + * contains both member bit offset and bitfield size, and > > + * bitfield size will set for struct/union bitfield members. > > + * Otherwise, it contains only bit offset. > > + */ > nit. It may be better to move this comment to the btf_member.offset > above. > > > +#define BTF_MEMBER_BITFIELD_SIZE(val) ((val) >> 24) > > +#define BTF_MEMBER_BIT_OFFSET(val) ((val) & 0xffffff) > After re-thinking this setup again, I still think > having these macros in btf.h to also do the kflag checking > would be nice. > > Unlike BTF_INFO_KIND() and BTF_INT_ENCODING() which don't > depend on other facts, the btf.h raw user must check kflag > anyway before calling BTF_MEMBER_BIT*(). > Forcing a kflag check before the user can access these convenient > 0xfffff and >>24 conversions may enforce this kflag check to > some extend. > > Since it is in uapi, it will not be easy to change later. > The above concern could be overkill ;), just want to ensure > it has been thought through a bit more here. > > It could be as easy as moving the new btf_member_bit*() from > btf.c to here and remove these two macros (or move them back to btf.c). I think moving: +static u32 btf_member_bitfield_size(const struct btf_type *struct_type, + const struct btf_member *member) +{ + return btf_type_kflag(struct_type) ? BTF_MEMBER_BITFIELD_SIZE(member->offset) + : 0; +} into uapi/btf.h may or may not be useful for btf uapi users. What are the chances that these static inline helpers will be reused by BTF logic in libbpf or other libs? At this point we don't know. So I would keep btf.h minimal. I agree that BTF_MEMBER_BIT_OFFSET() shouldn't be reused blindly. The users have to do BTF_INFO_KFLAG() check first. But this is the case for pretty much all of BTF data structures. In that sense BTF_MEMBER_BITFIELD_SIZE/BTF_MEMBER_BIT_OFFSET serve as a documentation and fit the style of btf.h header. imo having these two macros in uapi/btf.h is better than replacing them with comment only. After this set the whole BTF really needs to be documented.
On Sat, Dec 15, 2018 at 09:44:44AM -0800, Alexei Starovoitov wrote: > On Sat, Dec 15, 2018 at 04:37:06PM +0000, Martin Lau wrote: > > On Fri, Dec 14, 2018 at 03:34:27PM -0800, Yonghong Song wrote: > > > This patch fixed two issues with BTF. One is related to > > > struct/union bitfield encoding and the other is related to > > > forward type. > > > > > > Issue #1 and solution: > > > ====================== > > > > > > Current btf encoding of bitfield follows what pahole generates. > > > For each bitfield, pahole will duplicate the type chain and > > > put the bitfield size at the final int or enum type. > > > Since the BTF enum type cannot encode bit size, > > > pahole workarounds the issue by generating > > > an int type whenever the enum bit size is not 32. > > > > > > For example, > > > -bash-4.4$ cat t.c > > > typedef int ___int; > > > enum A { A1, A2, A3 }; > > > struct t { > > > int a[5]; > > > ___int b:4; > > > volatile enum A c:4; > > > } g; > > > -bash-4.4$ gcc -c -O2 -g t.c > > > The current kernel supports the following BTF encoding: > > > $ pahole -JV t.o > > > [1] TYPEDEF ___int type_id=2 > > > [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > > > [3] ENUM A size=4 vlen=3 > > > A1 val=0 > > > A2 val=1 > > > A3 val=2 > > > [4] STRUCT t size=24 vlen=3 > > > a type_id=5 bits_offset=0 > > > b type_id=9 bits_offset=160 > > > c type_id=11 bits_offset=164 > > > [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 > > > [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) > > > [7] VOLATILE (anon) type_id=3 > > > [8] INT int size=1 bit_offset=0 nr_bits=4 encoding=(none) > > > [9] TYPEDEF ___int type_id=8 > > > [10] INT (anon) size=1 bit_offset=0 nr_bits=4 encoding=SIGNED > > > [11] VOLATILE (anon) type_id=10 > > > > > > Two issues are in the above: > > > . by changing enum type to int, we lost the original > > > type information and this will not be ideal later > > > when we try to convert BTF to a header file. > > > . the type duplication for bitfields will cause > > > BTF bloat. Duplicated types cannot be deduplicated > > > later if the bitfield size is different. > > > > > > To fix this issue, this patch implemented a compatible > > > change for BTF struct type encoding: > > > . the bit 31 of struct_type->info, previously reserved, > > > now is used to indicate whether bitfield_size is > > > encoded in btf_member or not. > > > . if bit 31 of struct_type->info is set, > > > btf_member->offset will encode like: > > > bit 0 - 23: bit offset > > > bit 24 - 31: bitfield size > > > if bit 31 is not set, the old behavior is preserved: > > > bit 0 - 31: bit offset > > > > > > So if the struct contains a bit field, the maximum bit offset > > > will be reduced to (2^24 - 1) instead of MAX_UINT. The maximum > > > bitfield size will be 256 which is enough for today as maximum > > > bitfield in compiler can be 128 where int128 type is supported. > > > > > > This kernel patch intends to support the new BTF encoding: > > > $ pahole -JV t.o > > > [1] TYPEDEF ___int type_id=2 > > > [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > > > [3] ENUM A size=4 vlen=3 > > > A1 val=0 > > > A2 val=1 > > > A3 val=2 > > > [4] STRUCT t kind_flag=1 size=24 vlen=3 > > > a type_id=5 bitfield_size=0 bits_offset=0 > > > b type_id=1 bitfield_size=4 bits_offset=160 > > > c type_id=7 bitfield_size=4 bits_offset=164 > > > [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 > > > [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) > > > [7] VOLATILE (anon) type_id=3 > > > > > > Issue #2 and solution: > > > ====================== > > > > > > Current forward type in BTF does not specify whether the original > > > type is struct or union. This will not work for type pretty print > > > and BTF-to-header-file conversion as struct/union must be specified. > > > $ cat tt.c > > > struct t; > > > union u; > > > int foo(struct t *t, union u *u) { return 0; } > > > $ gcc -c -g -O2 tt.c > > > $ pahole -JV tt.o > > > [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > > > [2] FWD t type_id=0 > > > [3] PTR (anon) type_id=2 > > > [4] FWD u type_id=0 > > > [5] PTR (anon) type_id=4 > > > > > > To fix this issue, similar to issue #1, type->info bit 31 > > > is used. If the bit is set, it is union type. Otherwise, it is > > > a struct type. > > > > > > $ pahole -JV tt.o > > > [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > > > [2] FWD t kind_flag=0 type_id=0 > > > [3] PTR (anon) kind_flag=0 type_id=2 > > > [4] FWD u kind_flag=1 type_id=0 > > > [5] PTR (anon) kind_flag=0 type_id=4 > > > > > > Pahole/LLVM change: > > > =================== > > > > > > The new kind_flag functionality has been implemented in pahole > > > and llvm: > > > https://github.com/yonghong-song/pahole/tree/bitfield > > > https://github.com/yonghong-song/llvm/tree/bitfield > > > > > > Note that pahole hasn't implemented func/func_proto kind > > > and .BTF.ext. So to print function signature with bpftool, > > > the llvm compiler should be used. > > > > > > Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)") > > > Signed-off-by: Martin KaFai Lau <kafai@fb.com> > > > Signed-off-by: Yonghong Song <yhs@fb.com> > > > --- > > > include/uapi/linux/btf.h | 15 ++- > > > kernel/bpf/btf.c | 274 ++++++++++++++++++++++++++++++++++++--- > > > 2 files changed, 267 insertions(+), 22 deletions(-) > > > > > > diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h > > > index 14f66948fc95..34aba40ed926 100644 > > > --- a/include/uapi/linux/btf.h > > > +++ b/include/uapi/linux/btf.h > > > @@ -34,7 +34,9 @@ struct btf_type { > > > * bits 0-15: vlen (e.g. # of struct's members) > > > * bits 16-23: unused > > > * bits 24-27: kind (e.g. int, ptr, array...etc) > > > - * bits 28-31: unused > > > + * bits 28-30: unused > > > + * bit 31: kind_flag, currently used by > > > + * struct, union and fwd > > > */ > > > __u32 info; > > > /* "size" is used by INT, ENUM, STRUCT and UNION. > > > @@ -52,6 +54,7 @@ struct btf_type { > > > > > > #define BTF_INFO_KIND(info) (((info) >> 24) & 0x0f) > > > #define BTF_INFO_VLEN(info) ((info) & 0xffff) > > > +#define BTF_INFO_KFLAG(info) ((info) >> 31) > > > > > > #define BTF_KIND_UNKN 0 /* Unknown */ > > > #define BTF_KIND_INT 1 /* Integer */ > > > @@ -110,9 +113,17 @@ struct btf_array { > > > struct btf_member { > > > __u32 name_off; > > > __u32 type; > > > - __u32 offset; /* offset in bits */ > > > + __u32 offset; /* [bitfield_size and] offset in bits */ > > > }; > > > > > > +/* If the type info kind_flag set, the btf_member.offset > > > + * contains both member bit offset and bitfield size, and > > > + * bitfield size will set for struct/union bitfield members. > > > + * Otherwise, it contains only bit offset. > > > + */ > > nit. It may be better to move this comment to the btf_member.offset > > above. > > > > > +#define BTF_MEMBER_BITFIELD_SIZE(val) ((val) >> 24) > > > +#define BTF_MEMBER_BIT_OFFSET(val) ((val) & 0xffffff) > > After re-thinking this setup again, I still think > > having these macros in btf.h to also do the kflag checking > > would be nice. > > > > Unlike BTF_INFO_KIND() and BTF_INT_ENCODING() which don't > > depend on other facts, the btf.h raw user must check kflag > > anyway before calling BTF_MEMBER_BIT*(). > > Forcing a kflag check before the user can access these convenient > > 0xfffff and >>24 conversions may enforce this kflag check to > > some extend. > > > > Since it is in uapi, it will not be easy to change later. > > The above concern could be overkill ;), just want to ensure > > it has been thought through a bit more here. > > > > It could be as easy as moving the new btf_member_bit*() from > > btf.c to here and remove these two macros (or move them back to btf.c). > > I think moving: > +static u32 btf_member_bitfield_size(const struct btf_type *struct_type, > + const struct btf_member *member) > +{ > + return btf_type_kflag(struct_type) ? BTF_MEMBER_BITFIELD_SIZE(member->offset) > + : 0; > +} > > into uapi/btf.h may or may not be useful for btf uapi users. > What are the chances that these static inline helpers will be > reused by BTF logic in libbpf or other libs? > At this point we don't know. > So I would keep btf.h minimal. ok. Make sense > I agree that BTF_MEMBER_BIT_OFFSET() shouldn't be reused blindly. > The users have to do BTF_INFO_KFLAG() check first. > But this is the case for pretty much all of BTF data structures. Other similar situation in btf.h (i.e. a single u32 field can be interpreted differently) has at least an union as an indication (e.g. size and type in btf_type) Here we cannot add the union (bitfield_offset:24 and bitfield_size:8) and we cannot change the name "offset" also. I am worry about m->offset will directly be used without checking the BTF_INFO_KFLAG(). may be a "union { __u32 offset; __u32 bitsize_offset; };"......
On 12/15/18 2:10 PM, Martin Lau wrote: > On Sat, Dec 15, 2018 at 09:44:44AM -0800, Alexei Starovoitov wrote: >> On Sat, Dec 15, 2018 at 04:37:06PM +0000, Martin Lau wrote: >>> On Fri, Dec 14, 2018 at 03:34:27PM -0800, Yonghong Song wrote: >>>> This patch fixed two issues with BTF. One is related to >>>> struct/union bitfield encoding and the other is related to >>>> forward type. >>>> >>>> Issue #1 and solution: >>>> ====================== >>>> >>>> Current btf encoding of bitfield follows what pahole generates. >>>> For each bitfield, pahole will duplicate the type chain and >>>> put the bitfield size at the final int or enum type. >>>> Since the BTF enum type cannot encode bit size, >>>> pahole workarounds the issue by generating >>>> an int type whenever the enum bit size is not 32. >>>> >>>> For example, >>>> -bash-4.4$ cat t.c >>>> typedef int ___int; >>>> enum A { A1, A2, A3 }; >>>> struct t { >>>> int a[5]; >>>> ___int b:4; >>>> volatile enum A c:4; >>>> } g; >>>> -bash-4.4$ gcc -c -O2 -g t.c >>>> The current kernel supports the following BTF encoding: >>>> $ pahole -JV t.o >>>> [1] TYPEDEF ___int type_id=2 >>>> [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >>>> [3] ENUM A size=4 vlen=3 >>>> A1 val=0 >>>> A2 val=1 >>>> A3 val=2 >>>> [4] STRUCT t size=24 vlen=3 >>>> a type_id=5 bits_offset=0 >>>> b type_id=9 bits_offset=160 >>>> c type_id=11 bits_offset=164 >>>> [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 >>>> [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) >>>> [7] VOLATILE (anon) type_id=3 >>>> [8] INT int size=1 bit_offset=0 nr_bits=4 encoding=(none) >>>> [9] TYPEDEF ___int type_id=8 >>>> [10] INT (anon) size=1 bit_offset=0 nr_bits=4 encoding=SIGNED >>>> [11] VOLATILE (anon) type_id=10 >>>> >>>> Two issues are in the above: >>>> . by changing enum type to int, we lost the original >>>> type information and this will not be ideal later >>>> when we try to convert BTF to a header file. >>>> . the type duplication for bitfields will cause >>>> BTF bloat. Duplicated types cannot be deduplicated >>>> later if the bitfield size is different. >>>> >>>> To fix this issue, this patch implemented a compatible >>>> change for BTF struct type encoding: >>>> . the bit 31 of struct_type->info, previously reserved, >>>> now is used to indicate whether bitfield_size is >>>> encoded in btf_member or not. >>>> . if bit 31 of struct_type->info is set, >>>> btf_member->offset will encode like: >>>> bit 0 - 23: bit offset >>>> bit 24 - 31: bitfield size >>>> if bit 31 is not set, the old behavior is preserved: >>>> bit 0 - 31: bit offset >>>> >>>> So if the struct contains a bit field, the maximum bit offset >>>> will be reduced to (2^24 - 1) instead of MAX_UINT. The maximum >>>> bitfield size will be 256 which is enough for today as maximum >>>> bitfield in compiler can be 128 where int128 type is supported. >>>> >>>> This kernel patch intends to support the new BTF encoding: >>>> $ pahole -JV t.o >>>> [1] TYPEDEF ___int type_id=2 >>>> [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >>>> [3] ENUM A size=4 vlen=3 >>>> A1 val=0 >>>> A2 val=1 >>>> A3 val=2 >>>> [4] STRUCT t kind_flag=1 size=24 vlen=3 >>>> a type_id=5 bitfield_size=0 bits_offset=0 >>>> b type_id=1 bitfield_size=4 bits_offset=160 >>>> c type_id=7 bitfield_size=4 bits_offset=164 >>>> [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 >>>> [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) >>>> [7] VOLATILE (anon) type_id=3 >>>> >>>> Issue #2 and solution: >>>> ====================== >>>> >>>> Current forward type in BTF does not specify whether the original >>>> type is struct or union. This will not work for type pretty print >>>> and BTF-to-header-file conversion as struct/union must be specified. >>>> $ cat tt.c >>>> struct t; >>>> union u; >>>> int foo(struct t *t, union u *u) { return 0; } >>>> $ gcc -c -g -O2 tt.c >>>> $ pahole -JV tt.o >>>> [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >>>> [2] FWD t type_id=0 >>>> [3] PTR (anon) type_id=2 >>>> [4] FWD u type_id=0 >>>> [5] PTR (anon) type_id=4 >>>> >>>> To fix this issue, similar to issue #1, type->info bit 31 >>>> is used. If the bit is set, it is union type. Otherwise, it is >>>> a struct type. >>>> >>>> $ pahole -JV tt.o >>>> [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >>>> [2] FWD t kind_flag=0 type_id=0 >>>> [3] PTR (anon) kind_flag=0 type_id=2 >>>> [4] FWD u kind_flag=1 type_id=0 >>>> [5] PTR (anon) kind_flag=0 type_id=4 >>>> >>>> Pahole/LLVM change: >>>> =================== >>>> >>>> The new kind_flag functionality has been implemented in pahole >>>> and llvm: >>>> https://github.com/yonghong-song/pahole/tree/bitfield >>>> https://github.com/yonghong-song/llvm/tree/bitfield >>>> >>>> Note that pahole hasn't implemented func/func_proto kind >>>> and .BTF.ext. So to print function signature with bpftool, >>>> the llvm compiler should be used. >>>> >>>> Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)") >>>> Signed-off-by: Martin KaFai Lau <kafai@fb.com> >>>> Signed-off-by: Yonghong Song <yhs@fb.com> >>>> --- >>>> include/uapi/linux/btf.h | 15 ++- >>>> kernel/bpf/btf.c | 274 ++++++++++++++++++++++++++++++++++++--- >>>> 2 files changed, 267 insertions(+), 22 deletions(-) >>>> >>>> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h >>>> index 14f66948fc95..34aba40ed926 100644 >>>> --- a/include/uapi/linux/btf.h >>>> +++ b/include/uapi/linux/btf.h >>>> @@ -34,7 +34,9 @@ struct btf_type { >>>> * bits 0-15: vlen (e.g. # of struct's members) >>>> * bits 16-23: unused >>>> * bits 24-27: kind (e.g. int, ptr, array...etc) >>>> - * bits 28-31: unused >>>> + * bits 28-30: unused >>>> + * bit 31: kind_flag, currently used by >>>> + * struct, union and fwd >>>> */ >>>> __u32 info; >>>> /* "size" is used by INT, ENUM, STRUCT and UNION. >>>> @@ -52,6 +54,7 @@ struct btf_type { >>>> >>>> #define BTF_INFO_KIND(info) (((info) >> 24) & 0x0f) >>>> #define BTF_INFO_VLEN(info) ((info) & 0xffff) >>>> +#define BTF_INFO_KFLAG(info) ((info) >> 31) >>>> >>>> #define BTF_KIND_UNKN 0 /* Unknown */ >>>> #define BTF_KIND_INT 1 /* Integer */ >>>> @@ -110,9 +113,17 @@ struct btf_array { >>>> struct btf_member { >>>> __u32 name_off; >>>> __u32 type; >>>> - __u32 offset; /* offset in bits */ >>>> + __u32 offset; /* [bitfield_size and] offset in bits */ >>>> }; >>>> >>>> +/* If the type info kind_flag set, the btf_member.offset >>>> + * contains both member bit offset and bitfield size, and >>>> + * bitfield size will set for struct/union bitfield members. >>>> + * Otherwise, it contains only bit offset. >>>> + */ >>> nit. It may be better to move this comment to the btf_member.offset >>> above. >>> >>>> +#define BTF_MEMBER_BITFIELD_SIZE(val) ((val) >> 24) >>>> +#define BTF_MEMBER_BIT_OFFSET(val) ((val) & 0xffffff) >>> After re-thinking this setup again, I still think >>> having these macros in btf.h to also do the kflag checking >>> would be nice. >>> >>> Unlike BTF_INFO_KIND() and BTF_INT_ENCODING() which don't >>> depend on other facts, the btf.h raw user must check kflag >>> anyway before calling BTF_MEMBER_BIT*(). >>> Forcing a kflag check before the user can access these convenient >>> 0xfffff and >>24 conversions may enforce this kflag check to >>> some extend. >>> >>> Since it is in uapi, it will not be easy to change later. >>> The above concern could be overkill ;), just want to ensure >>> it has been thought through a bit more here. >>> >>> It could be as easy as moving the new btf_member_bit*() from >>> btf.c to here and remove these two macros (or move them back to btf.c). >> >> I think moving: >> +static u32 btf_member_bitfield_size(const struct btf_type *struct_type, >> + const struct btf_member *member) >> +{ >> + return btf_type_kflag(struct_type) ? BTF_MEMBER_BITFIELD_SIZE(member->offset) >> + : 0; >> +} >> >> into uapi/btf.h may or may not be useful for btf uapi users. >> What are the chances that these static inline helpers will be >> reused by BTF logic in libbpf or other libs? >> At this point we don't know. > >> So I would keep btf.h minimal. > ok. Make sense > >> I agree that BTF_MEMBER_BIT_OFFSET() shouldn't be reused blindly. >> The users have to do BTF_INFO_KFLAG() check first. >> But this is the case for pretty much all of BTF data structures. > Other similar situation in btf.h (i.e. a single u32 field can be > interpreted differently) has at least an union as an indication > (e.g. size and type in btf_type) > > Here we cannot add the union (bitfield_offset:24 and bitfield_size:8) > and we cannot change the name "offset" also. I am worry about > m->offset will directly be used without checking the BTF_INFO_KFLAG(). > > may be a "union { __u32 offset; __u32 bitsize_offset; };"...... The union with two __u32 is great idea. Maybe the bitsize_offset becomes "bitfield_size_offset" to reflect its real intention? >
On 12/15/2018 12:34 AM, Yonghong Song wrote: > This patch fixed two issues with BTF. One is related to > struct/union bitfield encoding and the other is related to > forward type. > > Issue #1 and solution: > ====================== > > Current btf encoding of bitfield follows what pahole generates. > For each bitfield, pahole will duplicate the type chain and > put the bitfield size at the final int or enum type. > Since the BTF enum type cannot encode bit size, > pahole workarounds the issue by generating > an int type whenever the enum bit size is not 32. > > For example, > -bash-4.4$ cat t.c > typedef int ___int; > enum A { A1, A2, A3 }; > struct t { > int a[5]; > ___int b:4; > volatile enum A c:4; > } g; > -bash-4.4$ gcc -c -O2 -g t.c > The current kernel supports the following BTF encoding: > $ pahole -JV t.o > [1] TYPEDEF ___int type_id=2 > [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > [3] ENUM A size=4 vlen=3 > A1 val=0 > A2 val=1 > A3 val=2 > [4] STRUCT t size=24 vlen=3 > a type_id=5 bits_offset=0 > b type_id=9 bits_offset=160 > c type_id=11 bits_offset=164 > [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 > [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) > [7] VOLATILE (anon) type_id=3 > [8] INT int size=1 bit_offset=0 nr_bits=4 encoding=(none) > [9] TYPEDEF ___int type_id=8 > [10] INT (anon) size=1 bit_offset=0 nr_bits=4 encoding=SIGNED > [11] VOLATILE (anon) type_id=10 > > Two issues are in the above: > . by changing enum type to int, we lost the original > type information and this will not be ideal later > when we try to convert BTF to a header file. > . the type duplication for bitfields will cause > BTF bloat. Duplicated types cannot be deduplicated > later if the bitfield size is different. > > To fix this issue, this patch implemented a compatible > change for BTF struct type encoding: > . the bit 31 of struct_type->info, previously reserved, > now is used to indicate whether bitfield_size is > encoded in btf_member or not. > . if bit 31 of struct_type->info is set, > btf_member->offset will encode like: > bit 0 - 23: bit offset > bit 24 - 31: bitfield size > if bit 31 is not set, the old behavior is preserved: > bit 0 - 31: bit offset > > So if the struct contains a bit field, the maximum bit offset > will be reduced to (2^24 - 1) instead of MAX_UINT. The maximum > bitfield size will be 256 which is enough for today as maximum > bitfield in compiler can be 128 where int128 type is supported. Looks good in general, just small nit below. > This kernel patch intends to support the new BTF encoding: > $ pahole -JV t.o > [1] TYPEDEF ___int type_id=2 > [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > [3] ENUM A size=4 vlen=3 > A1 val=0 > A2 val=1 > A3 val=2 > [4] STRUCT t kind_flag=1 size=24 vlen=3 > a type_id=5 bitfield_size=0 bits_offset=0 > b type_id=1 bitfield_size=4 bits_offset=160 > c type_id=7 bitfield_size=4 bits_offset=164 > [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 > [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) > [7] VOLATILE (anon) type_id=3 [...] > +static int btf_int_check_kflag_member(struct btf_verifier_env *env, > + const struct btf_type *struct_type, > + const struct btf_member *member, > + const struct btf_type *member_type) > +{ > + u32 struct_bits_off, nr_bits, nr_int_data_bits, bytes_offset; > + u32 int_data = btf_type_int(member_type); > + u32 struct_size = struct_type->size; > + u32 nr_copy_bits; > + > + /* a regular int type is required for the kflag int member */ > + if (!btf_type_int_is_regular(member_type)) { > + btf_verifier_log_member(env, struct_type, member, > + "Invalid member base type"); > + return -EINVAL; > + } > + > + /* check sanity of bitfield size */ > + nr_bits = BTF_MEMBER_BITFIELD_SIZE(member->offset); > + struct_bits_off = BTF_MEMBER_BIT_OFFSET(member->offset); > + nr_int_data_bits = BTF_INT_BITS(int_data); > + if (!nr_bits) { > + /* Not a bitfield member, member offset must be at byte > + * boundary. > + */ > + if (BITS_PER_BYTE_MASKED(struct_bits_off)) { > + btf_verifier_log_member(env, struct_type, member, > + "Invalid member offset"); > + return -EINVAL; > + } > + > + nr_bits = nr_int_data_bits; > + } else if (nr_bits > nr_int_data_bits) { Should the test here not include the bit offset as well aka nr_copy_bits? Thus test would be e.g. (nr_copy_bits > nr_int_data_bits || nr_copy_bits > BITS_PER_U64) ... Wrt to future 256 bit support, doesn't UAPI on BTF_INT_BITS() only support up to max 255 bits? > + btf_verifier_log_member(env, struct_type, member, > + "Invalid member bitfield_size"); > + return -EINVAL; > + } > + > + bytes_offset = BITS_ROUNDDOWN_BYTES(struct_bits_off); > + nr_copy_bits = nr_bits + BITS_PER_BYTE_MASKED(struct_bits_off); > + if (nr_copy_bits > BITS_PER_U64) { > + btf_verifier_log_member(env, struct_type, member, > + "nr_copy_bits exceeds 64"); > + return -EINVAL; > + } > + > + if (struct_size < bytes_offset || > + struct_size - bytes_offset < BITS_ROUNDUP_BYTES(nr_copy_bits)) { > + btf_verifier_log_member(env, struct_type, member, > + "Member exceeds struct_size"); > + return -EINVAL; > + } > + > + return 0; > +}
On Sat, Dec 15, 2018 at 02:26:44PM -0800, Yonghong Song wrote: > > > On 12/15/18 2:10 PM, Martin Lau wrote: > > On Sat, Dec 15, 2018 at 09:44:44AM -0800, Alexei Starovoitov wrote: > >> On Sat, Dec 15, 2018 at 04:37:06PM +0000, Martin Lau wrote: > >>> On Fri, Dec 14, 2018 at 03:34:27PM -0800, Yonghong Song wrote: > >>>> This patch fixed two issues with BTF. One is related to > >>>> struct/union bitfield encoding and the other is related to > >>>> forward type. > >>>> > >>>> Issue #1 and solution: > >>>> ====================== > >>>> > >>>> Current btf encoding of bitfield follows what pahole generates. > >>>> For each bitfield, pahole will duplicate the type chain and > >>>> put the bitfield size at the final int or enum type. > >>>> Since the BTF enum type cannot encode bit size, > >>>> pahole workarounds the issue by generating > >>>> an int type whenever the enum bit size is not 32. > >>>> > >>>> For example, > >>>> -bash-4.4$ cat t.c > >>>> typedef int ___int; > >>>> enum A { A1, A2, A3 }; > >>>> struct t { > >>>> int a[5]; > >>>> ___int b:4; > >>>> volatile enum A c:4; > >>>> } g; > >>>> -bash-4.4$ gcc -c -O2 -g t.c > >>>> The current kernel supports the following BTF encoding: > >>>> $ pahole -JV t.o > >>>> [1] TYPEDEF ___int type_id=2 > >>>> [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > >>>> [3] ENUM A size=4 vlen=3 > >>>> A1 val=0 > >>>> A2 val=1 > >>>> A3 val=2 > >>>> [4] STRUCT t size=24 vlen=3 > >>>> a type_id=5 bits_offset=0 > >>>> b type_id=9 bits_offset=160 > >>>> c type_id=11 bits_offset=164 > >>>> [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 > >>>> [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) > >>>> [7] VOLATILE (anon) type_id=3 > >>>> [8] INT int size=1 bit_offset=0 nr_bits=4 encoding=(none) > >>>> [9] TYPEDEF ___int type_id=8 > >>>> [10] INT (anon) size=1 bit_offset=0 nr_bits=4 encoding=SIGNED > >>>> [11] VOLATILE (anon) type_id=10 > >>>> > >>>> Two issues are in the above: > >>>> . by changing enum type to int, we lost the original > >>>> type information and this will not be ideal later > >>>> when we try to convert BTF to a header file. > >>>> . the type duplication for bitfields will cause > >>>> BTF bloat. Duplicated types cannot be deduplicated > >>>> later if the bitfield size is different. > >>>> > >>>> To fix this issue, this patch implemented a compatible > >>>> change for BTF struct type encoding: > >>>> . the bit 31 of struct_type->info, previously reserved, > >>>> now is used to indicate whether bitfield_size is > >>>> encoded in btf_member or not. > >>>> . if bit 31 of struct_type->info is set, > >>>> btf_member->offset will encode like: > >>>> bit 0 - 23: bit offset > >>>> bit 24 - 31: bitfield size > >>>> if bit 31 is not set, the old behavior is preserved: > >>>> bit 0 - 31: bit offset > >>>> > >>>> So if the struct contains a bit field, the maximum bit offset > >>>> will be reduced to (2^24 - 1) instead of MAX_UINT. The maximum > >>>> bitfield size will be 256 which is enough for today as maximum > >>>> bitfield in compiler can be 128 where int128 type is supported. > >>>> > >>>> This kernel patch intends to support the new BTF encoding: > >>>> $ pahole -JV t.o > >>>> [1] TYPEDEF ___int type_id=2 > >>>> [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > >>>> [3] ENUM A size=4 vlen=3 > >>>> A1 val=0 > >>>> A2 val=1 > >>>> A3 val=2 > >>>> [4] STRUCT t kind_flag=1 size=24 vlen=3 > >>>> a type_id=5 bitfield_size=0 bits_offset=0 > >>>> b type_id=1 bitfield_size=4 bits_offset=160 > >>>> c type_id=7 bitfield_size=4 bits_offset=164 > >>>> [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 > >>>> [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) > >>>> [7] VOLATILE (anon) type_id=3 > >>>> > >>>> Issue #2 and solution: > >>>> ====================== > >>>> > >>>> Current forward type in BTF does not specify whether the original > >>>> type is struct or union. This will not work for type pretty print > >>>> and BTF-to-header-file conversion as struct/union must be specified. > >>>> $ cat tt.c > >>>> struct t; > >>>> union u; > >>>> int foo(struct t *t, union u *u) { return 0; } > >>>> $ gcc -c -g -O2 tt.c > >>>> $ pahole -JV tt.o > >>>> [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > >>>> [2] FWD t type_id=0 > >>>> [3] PTR (anon) type_id=2 > >>>> [4] FWD u type_id=0 > >>>> [5] PTR (anon) type_id=4 > >>>> > >>>> To fix this issue, similar to issue #1, type->info bit 31 > >>>> is used. If the bit is set, it is union type. Otherwise, it is > >>>> a struct type. > >>>> > >>>> $ pahole -JV tt.o > >>>> [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED > >>>> [2] FWD t kind_flag=0 type_id=0 > >>>> [3] PTR (anon) kind_flag=0 type_id=2 > >>>> [4] FWD u kind_flag=1 type_id=0 > >>>> [5] PTR (anon) kind_flag=0 type_id=4 > >>>> > >>>> Pahole/LLVM change: > >>>> =================== > >>>> > >>>> The new kind_flag functionality has been implemented in pahole > >>>> and llvm: > >>>> https://github.com/yonghong-song/pahole/tree/bitfield > >>>> https://github.com/yonghong-song/llvm/tree/bitfield > >>>> > >>>> Note that pahole hasn't implemented func/func_proto kind > >>>> and .BTF.ext. So to print function signature with bpftool, > >>>> the llvm compiler should be used. > >>>> > >>>> Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)") > >>>> Signed-off-by: Martin KaFai Lau <kafai@fb.com> > >>>> Signed-off-by: Yonghong Song <yhs@fb.com> > >>>> --- > >>>> include/uapi/linux/btf.h | 15 ++- > >>>> kernel/bpf/btf.c | 274 ++++++++++++++++++++++++++++++++++++--- > >>>> 2 files changed, 267 insertions(+), 22 deletions(-) > >>>> > >>>> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h > >>>> index 14f66948fc95..34aba40ed926 100644 > >>>> --- a/include/uapi/linux/btf.h > >>>> +++ b/include/uapi/linux/btf.h > >>>> @@ -34,7 +34,9 @@ struct btf_type { > >>>> * bits 0-15: vlen (e.g. # of struct's members) > >>>> * bits 16-23: unused > >>>> * bits 24-27: kind (e.g. int, ptr, array...etc) > >>>> - * bits 28-31: unused > >>>> + * bits 28-30: unused > >>>> + * bit 31: kind_flag, currently used by > >>>> + * struct, union and fwd > >>>> */ > >>>> __u32 info; > >>>> /* "size" is used by INT, ENUM, STRUCT and UNION. > >>>> @@ -52,6 +54,7 @@ struct btf_type { > >>>> > >>>> #define BTF_INFO_KIND(info) (((info) >> 24) & 0x0f) > >>>> #define BTF_INFO_VLEN(info) ((info) & 0xffff) > >>>> +#define BTF_INFO_KFLAG(info) ((info) >> 31) > >>>> > >>>> #define BTF_KIND_UNKN 0 /* Unknown */ > >>>> #define BTF_KIND_INT 1 /* Integer */ > >>>> @@ -110,9 +113,17 @@ struct btf_array { > >>>> struct btf_member { > >>>> __u32 name_off; > >>>> __u32 type; > >>>> - __u32 offset; /* offset in bits */ > >>>> + __u32 offset; /* [bitfield_size and] offset in bits */ > >>>> }; > >>>> > >>>> +/* If the type info kind_flag set, the btf_member.offset > >>>> + * contains both member bit offset and bitfield size, and > >>>> + * bitfield size will set for struct/union bitfield members. > >>>> + * Otherwise, it contains only bit offset. > >>>> + */ > >>> nit. It may be better to move this comment to the btf_member.offset > >>> above. > >>> > >>>> +#define BTF_MEMBER_BITFIELD_SIZE(val) ((val) >> 24) > >>>> +#define BTF_MEMBER_BIT_OFFSET(val) ((val) & 0xffffff) > >>> After re-thinking this setup again, I still think > >>> having these macros in btf.h to also do the kflag checking > >>> would be nice. > >>> > >>> Unlike BTF_INFO_KIND() and BTF_INT_ENCODING() which don't > >>> depend on other facts, the btf.h raw user must check kflag > >>> anyway before calling BTF_MEMBER_BIT*(). > >>> Forcing a kflag check before the user can access these convenient > >>> 0xfffff and >>24 conversions may enforce this kflag check to > >>> some extend. > >>> > >>> Since it is in uapi, it will not be easy to change later. > >>> The above concern could be overkill ;), just want to ensure > >>> it has been thought through a bit more here. > >>> > >>> It could be as easy as moving the new btf_member_bit*() from > >>> btf.c to here and remove these two macros (or move them back to btf.c). > >> > >> I think moving: > >> +static u32 btf_member_bitfield_size(const struct btf_type *struct_type, > >> + const struct btf_member *member) > >> +{ > >> + return btf_type_kflag(struct_type) ? BTF_MEMBER_BITFIELD_SIZE(member->offset) > >> + : 0; > >> +} > >> > >> into uapi/btf.h may or may not be useful for btf uapi users. > >> What are the chances that these static inline helpers will be > >> reused by BTF logic in libbpf or other libs? > >> At this point we don't know. > > > >> So I would keep btf.h minimal. > > ok. Make sense > > > >> I agree that BTF_MEMBER_BIT_OFFSET() shouldn't be reused blindly. > >> The users have to do BTF_INFO_KFLAG() check first. > >> But this is the case for pretty much all of BTF data structures. > > Other similar situation in btf.h (i.e. a single u32 field can be > > interpreted differently) has at least an union as an indication > > (e.g. size and type in btf_type) > > > > Here we cannot add the union (bitfield_offset:24 and bitfield_size:8) > > and we cannot change the name "offset" also. I am worry about > > m->offset will directly be used without checking the BTF_INFO_KFLAG(). > > > > may be a "union { __u32 offset; __u32 bitsize_offset; };"...... > > The union with two __u32 is great idea. Maybe the > bitsize_offset becomes "bitfield_size_offset" to reflect > its real intention? SGTM. Probably also spell out when to use "offset" and when to use "bitfield_size_offset" like the union in "struct btf_type". The BTF_MEMBER_BIT*() macro may also need to adjust to access the bitfield_size_offset instead to make the case clearer.
On 12/15/18 2:37 PM, Daniel Borkmann wrote: > On 12/15/2018 12:34 AM, Yonghong Song wrote: >> This patch fixed two issues with BTF. One is related to >> struct/union bitfield encoding and the other is related to >> forward type. >> >> Issue #1 and solution: >> ====================== >> >> Current btf encoding of bitfield follows what pahole generates. >> For each bitfield, pahole will duplicate the type chain and >> put the bitfield size at the final int or enum type. >> Since the BTF enum type cannot encode bit size, >> pahole workarounds the issue by generating >> an int type whenever the enum bit size is not 32. >> >> For example, >> -bash-4.4$ cat t.c >> typedef int ___int; >> enum A { A1, A2, A3 }; >> struct t { >> int a[5]; >> ___int b:4; >> volatile enum A c:4; >> } g; >> -bash-4.4$ gcc -c -O2 -g t.c >> The current kernel supports the following BTF encoding: >> $ pahole -JV t.o >> [1] TYPEDEF ___int type_id=2 >> [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >> [3] ENUM A size=4 vlen=3 >> A1 val=0 >> A2 val=1 >> A3 val=2 >> [4] STRUCT t size=24 vlen=3 >> a type_id=5 bits_offset=0 >> b type_id=9 bits_offset=160 >> c type_id=11 bits_offset=164 >> [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 >> [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) >> [7] VOLATILE (anon) type_id=3 >> [8] INT int size=1 bit_offset=0 nr_bits=4 encoding=(none) >> [9] TYPEDEF ___int type_id=8 >> [10] INT (anon) size=1 bit_offset=0 nr_bits=4 encoding=SIGNED >> [11] VOLATILE (anon) type_id=10 >> >> Two issues are in the above: >> . by changing enum type to int, we lost the original >> type information and this will not be ideal later >> when we try to convert BTF to a header file. >> . the type duplication for bitfields will cause >> BTF bloat. Duplicated types cannot be deduplicated >> later if the bitfield size is different. >> >> To fix this issue, this patch implemented a compatible >> change for BTF struct type encoding: >> . the bit 31 of struct_type->info, previously reserved, >> now is used to indicate whether bitfield_size is >> encoded in btf_member or not. >> . if bit 31 of struct_type->info is set, >> btf_member->offset will encode like: >> bit 0 - 23: bit offset >> bit 24 - 31: bitfield size >> if bit 31 is not set, the old behavior is preserved: >> bit 0 - 31: bit offset >> >> So if the struct contains a bit field, the maximum bit offset >> will be reduced to (2^24 - 1) instead of MAX_UINT. The maximum >> bitfield size will be 256 which is enough for today as maximum >> bitfield in compiler can be 128 where int128 type is supported. > > Looks good in general, just small nit below. > >> This kernel patch intends to support the new BTF encoding: >> $ pahole -JV t.o >> [1] TYPEDEF ___int type_id=2 >> [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >> [3] ENUM A size=4 vlen=3 >> A1 val=0 >> A2 val=1 >> A3 val=2 >> [4] STRUCT t kind_flag=1 size=24 vlen=3 >> a type_id=5 bitfield_size=0 bits_offset=0 >> b type_id=1 bitfield_size=4 bits_offset=160 >> c type_id=7 bitfield_size=4 bits_offset=164 >> [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 >> [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) >> [7] VOLATILE (anon) type_id=3 > [...] >> +static int btf_int_check_kflag_member(struct btf_verifier_env *env, >> + const struct btf_type *struct_type, >> + const struct btf_member *member, >> + const struct btf_type *member_type) >> +{ >> + u32 struct_bits_off, nr_bits, nr_int_data_bits, bytes_offset; >> + u32 int_data = btf_type_int(member_type); >> + u32 struct_size = struct_type->size; >> + u32 nr_copy_bits; >> + >> + /* a regular int type is required for the kflag int member */ >> + if (!btf_type_int_is_regular(member_type)) { >> + btf_verifier_log_member(env, struct_type, member, >> + "Invalid member base type"); >> + return -EINVAL; >> + } >> + >> + /* check sanity of bitfield size */ >> + nr_bits = BTF_MEMBER_BITFIELD_SIZE(member->offset); >> + struct_bits_off = BTF_MEMBER_BIT_OFFSET(member->offset); >> + nr_int_data_bits = BTF_INT_BITS(int_data); >> + if (!nr_bits) { >> + /* Not a bitfield member, member offset must be at byte >> + * boundary. >> + */ >> + if (BITS_PER_BYTE_MASKED(struct_bits_off)) { >> + btf_verifier_log_member(env, struct_type, member, >> + "Invalid member offset"); >> + return -EINVAL; >> + } >> + >> + nr_bits = nr_int_data_bits; >> + } else if (nr_bits > nr_int_data_bits) { > > Should the test here not include the bit offset as well aka nr_copy_bits? > Thus test would be e.g. (nr_copy_bits > nr_int_data_bits || nr_copy_bits > > BITS_PER_U64) ... The test here is strictly checking whether the bitfield size is covered by underlying int type. struct t { int a:1; char b:8; ... } In this case, for member "b", bitsize = 8, nr_copy_bits = 9, nr_int_data_bits = 8. nr_copy_bits > nr_int_data_bits, but it is legal. > Wrt to future 256 bit support, doesn't UAPI on BTF_INT_BITS() only support > up to max 255 bits? This is a good question. BTF_INT_BITS() can be extended to maximum 0xffff. There are bits available there. When the kind_flag is set, the bitfield size can range from 1 to 255, the same as today BTF_INT_BITS range. If user writes struct t { ... int256 m:256; ... }; The bitfield size information will not get encoded and the member "m" will just refer to a base type int256, the same as struct t { ... int256 m; ... }; Did you any problem with this? > >> + btf_verifier_log_member(env, struct_type, member, >> + "Invalid member bitfield_size"); >> + return -EINVAL; >> + } >> + >> + bytes_offset = BITS_ROUNDDOWN_BYTES(struct_bits_off); >> + nr_copy_bits = nr_bits + BITS_PER_BYTE_MASKED(struct_bits_off); >> + if (nr_copy_bits > BITS_PER_U64) { >> + btf_verifier_log_member(env, struct_type, member, >> + "nr_copy_bits exceeds 64"); >> + return -EINVAL; >> + } >> + >> + if (struct_size < bytes_offset || >> + struct_size - bytes_offset < BITS_ROUNDUP_BYTES(nr_copy_bits)) { >> + btf_verifier_log_member(env, struct_type, member, >> + "Member exceeds struct_size"); >> + return -EINVAL; >> + } >> + >> + return 0; >> +}
On 12/15/18 3:04 PM, Martin Lau wrote: > On Sat, Dec 15, 2018 at 02:26:44PM -0800, Yonghong Song wrote: >> >> >> On 12/15/18 2:10 PM, Martin Lau wrote: >>> On Sat, Dec 15, 2018 at 09:44:44AM -0800, Alexei Starovoitov wrote: >>>> On Sat, Dec 15, 2018 at 04:37:06PM +0000, Martin Lau wrote: >>>>> On Fri, Dec 14, 2018 at 03:34:27PM -0800, Yonghong Song wrote: >>>>>> This patch fixed two issues with BTF. One is related to >>>>>> struct/union bitfield encoding and the other is related to >>>>>> forward type. >>>>>> >>>>>> Issue #1 and solution: >>>>>> ====================== >>>>>> >>>>>> Current btf encoding of bitfield follows what pahole generates. >>>>>> For each bitfield, pahole will duplicate the type chain and >>>>>> put the bitfield size at the final int or enum type. >>>>>> Since the BTF enum type cannot encode bit size, >>>>>> pahole workarounds the issue by generating >>>>>> an int type whenever the enum bit size is not 32. >>>>>> >>>>>> For example, >>>>>> -bash-4.4$ cat t.c >>>>>> typedef int ___int; >>>>>> enum A { A1, A2, A3 }; >>>>>> struct t { >>>>>> int a[5]; >>>>>> ___int b:4; >>>>>> volatile enum A c:4; >>>>>> } g; >>>>>> -bash-4.4$ gcc -c -O2 -g t.c >>>>>> The current kernel supports the following BTF encoding: >>>>>> $ pahole -JV t.o >>>>>> [1] TYPEDEF ___int type_id=2 >>>>>> [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >>>>>> [3] ENUM A size=4 vlen=3 >>>>>> A1 val=0 >>>>>> A2 val=1 >>>>>> A3 val=2 >>>>>> [4] STRUCT t size=24 vlen=3 >>>>>> a type_id=5 bits_offset=0 >>>>>> b type_id=9 bits_offset=160 >>>>>> c type_id=11 bits_offset=164 >>>>>> [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 >>>>>> [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) >>>>>> [7] VOLATILE (anon) type_id=3 >>>>>> [8] INT int size=1 bit_offset=0 nr_bits=4 encoding=(none) >>>>>> [9] TYPEDEF ___int type_id=8 >>>>>> [10] INT (anon) size=1 bit_offset=0 nr_bits=4 encoding=SIGNED >>>>>> [11] VOLATILE (anon) type_id=10 >>>>>> >>>>>> Two issues are in the above: >>>>>> . by changing enum type to int, we lost the original >>>>>> type information and this will not be ideal later >>>>>> when we try to convert BTF to a header file. >>>>>> . the type duplication for bitfields will cause >>>>>> BTF bloat. Duplicated types cannot be deduplicated >>>>>> later if the bitfield size is different. >>>>>> >>>>>> To fix this issue, this patch implemented a compatible >>>>>> change for BTF struct type encoding: >>>>>> . the bit 31 of struct_type->info, previously reserved, >>>>>> now is used to indicate whether bitfield_size is >>>>>> encoded in btf_member or not. >>>>>> . if bit 31 of struct_type->info is set, >>>>>> btf_member->offset will encode like: >>>>>> bit 0 - 23: bit offset >>>>>> bit 24 - 31: bitfield size >>>>>> if bit 31 is not set, the old behavior is preserved: >>>>>> bit 0 - 31: bit offset >>>>>> >>>>>> So if the struct contains a bit field, the maximum bit offset >>>>>> will be reduced to (2^24 - 1) instead of MAX_UINT. The maximum >>>>>> bitfield size will be 256 which is enough for today as maximum >>>>>> bitfield in compiler can be 128 where int128 type is supported. >>>>>> >>>>>> This kernel patch intends to support the new BTF encoding: >>>>>> $ pahole -JV t.o >>>>>> [1] TYPEDEF ___int type_id=2 >>>>>> [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >>>>>> [3] ENUM A size=4 vlen=3 >>>>>> A1 val=0 >>>>>> A2 val=1 >>>>>> A3 val=2 >>>>>> [4] STRUCT t kind_flag=1 size=24 vlen=3 >>>>>> a type_id=5 bitfield_size=0 bits_offset=0 >>>>>> b type_id=1 bitfield_size=4 bits_offset=160 >>>>>> c type_id=7 bitfield_size=4 bits_offset=164 >>>>>> [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 >>>>>> [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) >>>>>> [7] VOLATILE (anon) type_id=3 >>>>>> >>>>>> Issue #2 and solution: >>>>>> ====================== >>>>>> >>>>>> Current forward type in BTF does not specify whether the original >>>>>> type is struct or union. This will not work for type pretty print >>>>>> and BTF-to-header-file conversion as struct/union must be specified. >>>>>> $ cat tt.c >>>>>> struct t; >>>>>> union u; >>>>>> int foo(struct t *t, union u *u) { return 0; } >>>>>> $ gcc -c -g -O2 tt.c >>>>>> $ pahole -JV tt.o >>>>>> [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >>>>>> [2] FWD t type_id=0 >>>>>> [3] PTR (anon) type_id=2 >>>>>> [4] FWD u type_id=0 >>>>>> [5] PTR (anon) type_id=4 >>>>>> >>>>>> To fix this issue, similar to issue #1, type->info bit 31 >>>>>> is used. If the bit is set, it is union type. Otherwise, it is >>>>>> a struct type. >>>>>> >>>>>> $ pahole -JV tt.o >>>>>> [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >>>>>> [2] FWD t kind_flag=0 type_id=0 >>>>>> [3] PTR (anon) kind_flag=0 type_id=2 >>>>>> [4] FWD u kind_flag=1 type_id=0 >>>>>> [5] PTR (anon) kind_flag=0 type_id=4 >>>>>> >>>>>> Pahole/LLVM change: >>>>>> =================== >>>>>> >>>>>> The new kind_flag functionality has been implemented in pahole >>>>>> and llvm: >>>>>> https://github.com/yonghong-song/pahole/tree/bitfield >>>>>> https://github.com/yonghong-song/llvm/tree/bitfield >>>>>> >>>>>> Note that pahole hasn't implemented func/func_proto kind >>>>>> and .BTF.ext. So to print function signature with bpftool, >>>>>> the llvm compiler should be used. >>>>>> >>>>>> Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)") >>>>>> Signed-off-by: Martin KaFai Lau <kafai@fb.com> >>>>>> Signed-off-by: Yonghong Song <yhs@fb.com> >>>>>> --- >>>>>> include/uapi/linux/btf.h | 15 ++- >>>>>> kernel/bpf/btf.c | 274 ++++++++++++++++++++++++++++++++++++--- >>>>>> 2 files changed, 267 insertions(+), 22 deletions(-) >>>>>> >>>>>> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h >>>>>> index 14f66948fc95..34aba40ed926 100644 >>>>>> --- a/include/uapi/linux/btf.h >>>>>> +++ b/include/uapi/linux/btf.h >>>>>> @@ -34,7 +34,9 @@ struct btf_type { >>>>>> * bits 0-15: vlen (e.g. # of struct's members) >>>>>> * bits 16-23: unused >>>>>> * bits 24-27: kind (e.g. int, ptr, array...etc) >>>>>> - * bits 28-31: unused >>>>>> + * bits 28-30: unused >>>>>> + * bit 31: kind_flag, currently used by >>>>>> + * struct, union and fwd >>>>>> */ >>>>>> __u32 info; >>>>>> /* "size" is used by INT, ENUM, STRUCT and UNION. >>>>>> @@ -52,6 +54,7 @@ struct btf_type { >>>>>> >>>>>> #define BTF_INFO_KIND(info) (((info) >> 24) & 0x0f) >>>>>> #define BTF_INFO_VLEN(info) ((info) & 0xffff) >>>>>> +#define BTF_INFO_KFLAG(info) ((info) >> 31) >>>>>> >>>>>> #define BTF_KIND_UNKN 0 /* Unknown */ >>>>>> #define BTF_KIND_INT 1 /* Integer */ >>>>>> @@ -110,9 +113,17 @@ struct btf_array { >>>>>> struct btf_member { >>>>>> __u32 name_off; >>>>>> __u32 type; >>>>>> - __u32 offset; /* offset in bits */ >>>>>> + __u32 offset; /* [bitfield_size and] offset in bits */ >>>>>> }; >>>>>> >>>>>> +/* If the type info kind_flag set, the btf_member.offset >>>>>> + * contains both member bit offset and bitfield size, and >>>>>> + * bitfield size will set for struct/union bitfield members. >>>>>> + * Otherwise, it contains only bit offset. >>>>>> + */ >>>>> nit. It may be better to move this comment to the btf_member.offset >>>>> above. >>>>> >>>>>> +#define BTF_MEMBER_BITFIELD_SIZE(val) ((val) >> 24) >>>>>> +#define BTF_MEMBER_BIT_OFFSET(val) ((val) & 0xffffff) >>>>> After re-thinking this setup again, I still think >>>>> having these macros in btf.h to also do the kflag checking >>>>> would be nice. >>>>> >>>>> Unlike BTF_INFO_KIND() and BTF_INT_ENCODING() which don't >>>>> depend on other facts, the btf.h raw user must check kflag >>>>> anyway before calling BTF_MEMBER_BIT*(). >>>>> Forcing a kflag check before the user can access these convenient >>>>> 0xfffff and >>24 conversions may enforce this kflag check to >>>>> some extend. >>>>> >>>>> Since it is in uapi, it will not be easy to change later. >>>>> The above concern could be overkill ;), just want to ensure >>>>> it has been thought through a bit more here. >>>>> >>>>> It could be as easy as moving the new btf_member_bit*() from >>>>> btf.c to here and remove these two macros (or move them back to btf.c). >>>> >>>> I think moving: >>>> +static u32 btf_member_bitfield_size(const struct btf_type *struct_type, >>>> + const struct btf_member *member) >>>> +{ >>>> + return btf_type_kflag(struct_type) ? BTF_MEMBER_BITFIELD_SIZE(member->offset) >>>> + : 0; >>>> +} >>>> >>>> into uapi/btf.h may or may not be useful for btf uapi users. >>>> What are the chances that these static inline helpers will be >>>> reused by BTF logic in libbpf or other libs? >>>> At this point we don't know. >>> >>>> So I would keep btf.h minimal. >>> ok. Make sense >>> >>>> I agree that BTF_MEMBER_BIT_OFFSET() shouldn't be reused blindly. >>>> The users have to do BTF_INFO_KFLAG() check first. >>>> But this is the case for pretty much all of BTF data structures. >>> Other similar situation in btf.h (i.e. a single u32 field can be >>> interpreted differently) has at least an union as an indication >>> (e.g. size and type in btf_type) >>> >>> Here we cannot add the union (bitfield_offset:24 and bitfield_size:8) >>> and we cannot change the name "offset" also. I am worry about >>> m->offset will directly be used without checking the BTF_INFO_KFLAG(). >>> >>> may be a "union { __u32 offset; __u32 bitsize_offset; };"...... >> >> The union with two __u32 is great idea. Maybe the >> bitsize_offset becomes "bitfield_size_offset" to reflect >> its real intention? > SGTM. Probably also spell out when to use "offset" > and when to use "bitfield_size_offset" like the > union in "struct btf_type". The BTF_MEMBER_BIT*() macro > may also need to adjust to access the bitfield_size_offset > instead to make the case clearer. Sounds good. This is my plan to do as well.
On 12/15/18 3:13 PM, Yonghong Song wrote: >> may be a "union { __u32 offset; __u32 bitsize_offset; };"...... > The union with two __u32 is great idea. Maybe the > bitsize_offset becomes "bitfield_size_offset" to reflect > its real intention? I don't think union and verbose name will help. imo it's add confusion. I prefer to keep it as-is with simple 'offset' name.
On 12/15/18 9:44 AM, Alexei Starovoitov wrote: > On Sat, Dec 15, 2018 at 04:37:06PM +0000, Martin Lau wrote: >> On Fri, Dec 14, 2018 at 03:34:27PM -0800, Yonghong Song wrote: >>> This patch fixed two issues with BTF. One is related to >>> struct/union bitfield encoding and the other is related to >>> forward type. >>> >>> Issue #1 and solution: >>> ====================== >>> >>> Current btf encoding of bitfield follows what pahole generates. >>> For each bitfield, pahole will duplicate the type chain and >>> put the bitfield size at the final int or enum type. >>> Since the BTF enum type cannot encode bit size, >>> pahole workarounds the issue by generating >>> an int type whenever the enum bit size is not 32. >>> >>> For example, >>> -bash-4.4$ cat t.c >>> typedef int ___int; >>> enum A { A1, A2, A3 }; >>> struct t { >>> int a[5]; >>> ___int b:4; >>> volatile enum A c:4; >>> } g; >>> -bash-4.4$ gcc -c -O2 -g t.c >>> The current kernel supports the following BTF encoding: >>> $ pahole -JV t.o >>> [1] TYPEDEF ___int type_id=2 >>> [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >>> [3] ENUM A size=4 vlen=3 >>> A1 val=0 >>> A2 val=1 >>> A3 val=2 >>> [4] STRUCT t size=24 vlen=3 >>> a type_id=5 bits_offset=0 >>> b type_id=9 bits_offset=160 >>> c type_id=11 bits_offset=164 >>> [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 >>> [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) >>> [7] VOLATILE (anon) type_id=3 >>> [8] INT int size=1 bit_offset=0 nr_bits=4 encoding=(none) >>> [9] TYPEDEF ___int type_id=8 >>> [10] INT (anon) size=1 bit_offset=0 nr_bits=4 encoding=SIGNED >>> [11] VOLATILE (anon) type_id=10 >>> >>> Two issues are in the above: >>> . by changing enum type to int, we lost the original >>> type information and this will not be ideal later >>> when we try to convert BTF to a header file. >>> . the type duplication for bitfields will cause >>> BTF bloat. Duplicated types cannot be deduplicated >>> later if the bitfield size is different. >>> >>> To fix this issue, this patch implemented a compatible >>> change for BTF struct type encoding: >>> . the bit 31 of struct_type->info, previously reserved, >>> now is used to indicate whether bitfield_size is >>> encoded in btf_member or not. >>> . if bit 31 of struct_type->info is set, >>> btf_member->offset will encode like: >>> bit 0 - 23: bit offset >>> bit 24 - 31: bitfield size >>> if bit 31 is not set, the old behavior is preserved: >>> bit 0 - 31: bit offset >>> >>> So if the struct contains a bit field, the maximum bit offset >>> will be reduced to (2^24 - 1) instead of MAX_UINT. The maximum >>> bitfield size will be 256 which is enough for today as maximum >>> bitfield in compiler can be 128 where int128 type is supported. >>> >>> This kernel patch intends to support the new BTF encoding: >>> $ pahole -JV t.o >>> [1] TYPEDEF ___int type_id=2 >>> [2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >>> [3] ENUM A size=4 vlen=3 >>> A1 val=0 >>> A2 val=1 >>> A3 val=2 >>> [4] STRUCT t kind_flag=1 size=24 vlen=3 >>> a type_id=5 bitfield_size=0 bits_offset=0 >>> b type_id=1 bitfield_size=4 bits_offset=160 >>> c type_id=7 bitfield_size=4 bits_offset=164 >>> [5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5 >>> [6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none) >>> [7] VOLATILE (anon) type_id=3 >>> >>> Issue #2 and solution: >>> ====================== >>> >>> Current forward type in BTF does not specify whether the original >>> type is struct or union. This will not work for type pretty print >>> and BTF-to-header-file conversion as struct/union must be specified. >>> $ cat tt.c >>> struct t; >>> union u; >>> int foo(struct t *t, union u *u) { return 0; } >>> $ gcc -c -g -O2 tt.c >>> $ pahole -JV tt.o >>> [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >>> [2] FWD t type_id=0 >>> [3] PTR (anon) type_id=2 >>> [4] FWD u type_id=0 >>> [5] PTR (anon) type_id=4 >>> >>> To fix this issue, similar to issue #1, type->info bit 31 >>> is used. If the bit is set, it is union type. Otherwise, it is >>> a struct type. >>> >>> $ pahole -JV tt.o >>> [1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED >>> [2] FWD t kind_flag=0 type_id=0 >>> [3] PTR (anon) kind_flag=0 type_id=2 >>> [4] FWD u kind_flag=1 type_id=0 >>> [5] PTR (anon) kind_flag=0 type_id=4 >>> >>> Pahole/LLVM change: >>> =================== >>> >>> The new kind_flag functionality has been implemented in pahole >>> and llvm: >>> https://github.com/yonghong-song/pahole/tree/bitfield >>> https://github.com/yonghong-song/llvm/tree/bitfield >>> >>> Note that pahole hasn't implemented func/func_proto kind >>> and .BTF.ext. So to print function signature with bpftool, >>> the llvm compiler should be used. >>> >>> Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)") >>> Signed-off-by: Martin KaFai Lau <kafai@fb.com> >>> Signed-off-by: Yonghong Song <yhs@fb.com> >>> --- >>> include/uapi/linux/btf.h | 15 ++- >>> kernel/bpf/btf.c | 274 ++++++++++++++++++++++++++++++++++++--- >>> 2 files changed, 267 insertions(+), 22 deletions(-) >>> >>> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h >>> index 14f66948fc95..34aba40ed926 100644 >>> --- a/include/uapi/linux/btf.h >>> +++ b/include/uapi/linux/btf.h >>> @@ -34,7 +34,9 @@ struct btf_type { >>> * bits 0-15: vlen (e.g. # of struct's members) >>> * bits 16-23: unused >>> * bits 24-27: kind (e.g. int, ptr, array...etc) >>> - * bits 28-31: unused >>> + * bits 28-30: unused >>> + * bit 31: kind_flag, currently used by >>> + * struct, union and fwd >>> */ >>> __u32 info; >>> /* "size" is used by INT, ENUM, STRUCT and UNION. >>> @@ -52,6 +54,7 @@ struct btf_type { >>> >>> #define BTF_INFO_KIND(info) (((info) >> 24) & 0x0f) >>> #define BTF_INFO_VLEN(info) ((info) & 0xffff) >>> +#define BTF_INFO_KFLAG(info) ((info) >> 31) >>> >>> #define BTF_KIND_UNKN 0 /* Unknown */ >>> #define BTF_KIND_INT 1 /* Integer */ >>> @@ -110,9 +113,17 @@ struct btf_array { >>> struct btf_member { >>> __u32 name_off; >>> __u32 type; >>> - __u32 offset; /* offset in bits */ >>> + __u32 offset; /* [bitfield_size and] offset in bits */ >>> }; >>> >>> +/* If the type info kind_flag set, the btf_member.offset >>> + * contains both member bit offset and bitfield size, and >>> + * bitfield size will set for struct/union bitfield members. >>> + * Otherwise, it contains only bit offset. >>> + */ >> nit. It may be better to move this comment to the btf_member.offset >> above. >> >>> +#define BTF_MEMBER_BITFIELD_SIZE(val) ((val) >> 24) >>> +#define BTF_MEMBER_BIT_OFFSET(val) ((val) & 0xffffff) >> After re-thinking this setup again, I still think >> having these macros in btf.h to also do the kflag checking >> would be nice. >> >> Unlike BTF_INFO_KIND() and BTF_INT_ENCODING() which don't >> depend on other facts, the btf.h raw user must check kflag >> anyway before calling BTF_MEMBER_BIT*(). >> Forcing a kflag check before the user can access these convenient >> 0xfffff and >>24 conversions may enforce this kflag check to >> some extend. >> >> Since it is in uapi, it will not be easy to change later. >> The above concern could be overkill ;), just want to ensure >> it has been thought through a bit more here. >> >> It could be as easy as moving the new btf_member_bit*() from >> btf.c to here and remove these two macros (or move them back to btf.c). > > I think moving: > +static u32 btf_member_bitfield_size(const struct btf_type *struct_type, > + const struct btf_member *member) > +{ > + return btf_type_kflag(struct_type) ? BTF_MEMBER_BITFIELD_SIZE(member->offset) > + : 0; > +} > > into uapi/btf.h may or may not be useful for btf uapi users. > What are the chances that these static inline helpers will be > reused by BTF logic in libbpf or other libs? > At this point we don't know. > So I would keep btf.h minimal. > I agree that BTF_MEMBER_BIT_OFFSET() shouldn't be reused blindly. > The users have to do BTF_INFO_KFLAG() check first. > But this is the case for pretty much all of BTF data structures. > In that sense BTF_MEMBER_BITFIELD_SIZE/BTF_MEMBER_BIT_OFFSET > serve as a documentation and fit the style of btf.h header. > imo having these two macros in uapi/btf.h is better than > replacing them with comment only. > > After this set the whole BTF really needs to be documented. Yes，Martin and I will work on the documentation very soon.
======================
Current btf encoding of bitfield follows what pahole generates.
For each bitfield, pahole will duplicate the type chain and
put the bitfield size at the final int or enum type.
Since the BTF enum type cannot encode bit size,
pahole workarounds the issue by generating
an int type whenever the enum bit size is not 32.
For example,
-bash-4.4$ cat t.c
typedef int ___int;
enum A { A1, A2, A3 };
struct t {
int a[5];
___int b:4;
volatile enum A c:4;
} g;
-bash-4.4$ gcc -c -O2 -g t.c
The current kernel supports the following BTF encoding:
$ pahole -JV t.o
[1] TYPEDEF ___int type_id=2
[2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
[3] ENUM A size=4 vlen=3
A1 val=0
A2 val=1
A3 val=2
[4] STRUCT t size=24 vlen=3
a type_id=5 bits_offset=0
b type_id=9 bits_offset=160
c type_id=11 bits_offset=164
[5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5
[6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none)
[7] VOLATILE (anon) type_id=3
[8] INT int size=1 bit_offset=0 nr_bits=4 encoding=(none)
[9] TYPEDEF ___int type_id=8
[10] INT (anon) size=1 bit_offset=0 nr_bits=4 encoding=SIGNED
[11] VOLATILE (anon) type_id=10
Two issues are in the above:
. by changing enum type to int, we lost the original
type information and this will not be ideal later
when we try to convert BTF to a header file.
. the type duplication for bitfields will cause
BTF bloat. Duplicated types cannot be deduplicated
later if the bitfield size is different.
To fix this issue, this patch implemented a compatible
change for BTF struct type encoding:
. the bit 31 of struct_type->info, previously reserved,
now is used to indicate whether bitfield_size is
encoded in btf_member or not.
. if bit 31 of struct_type->info is set,
btf_member->offset will encode like:
bit 0 - 23: bit offset
bit 24 - 31: bitfield size
if bit 31 is not set, the old behavior is preserved:
bit 0 - 31: bit offset
So if the struct contains a bit field, the maximum bit offset
will be reduced to (2^24 - 1) instead of MAX_UINT. The maximum
bitfield size will be 256 which is enough for today as maximum
bitfield in compiler can be 128 where int128 type is supported.
This kernel patch intends to support the new BTF encoding:
$ pahole -JV t.o
[1] TYPEDEF ___int type_id=2
[2] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
[3] ENUM A size=4 vlen=3
A1 val=0
A2 val=1
A3 val=2
[4] STRUCT t kind_flag=1 size=24 vlen=3
a type_id=5 bitfield_size=0 bits_offset=0
b type_id=1 bitfield_size=4 bits_offset=160
c type_id=7 bitfield_size=4 bits_offset=164
[5] ARRAY (anon) type_id=2 index_type_id=2 nr_elems=5
[6] INT sizetype size=8 bit_offset=0 nr_bits=64 encoding=(none)
[7] VOLATILE (anon) type_id=3
Issue #2 and solution:
======================
Current forward type in BTF does not specify whether the original
type is struct or union. This will not work for type pretty print
and BTF-to-header-file conversion as struct/union must be specified.
$ cat tt.c
struct t;
union u;
int foo(struct t *t, union u *u) { return 0; }
$ gcc -c -g -O2 tt.c
$ pahole -JV tt.o
[1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
[2] FWD t type_id=0
[3] PTR (anon) type_id=2
[4] FWD u type_id=0
[5] PTR (anon) type_id=4
To fix this issue, similar to issue #1, type->info bit 31
is used. If the bit is set, it is union type. Otherwise, it is
a struct type.
$ pahole -JV tt.o
[1] INT int size=4 bit_offset=0 nr_bits=32 encoding=SIGNED
[2] FWD t kind_flag=0 type_id=0
[3] PTR (anon) kind_flag=0 type_id=2
[4] FWD u kind_flag=1 type_id=0
[5] PTR (anon) kind_flag=0 type_id=4
Pahole/LLVM change:
===================
The new kind_flag functionality has been implemented in pahole
and llvm:
https://github.com/yonghong-song/pahole/tree/bitfield
https://github.com/yonghong-song/llvm/tree/bitfield
Note that pahole hasn't implemented func/func_proto kind
and .BTF.ext. So to print function signature with bpftool,
the llvm compiler should be used.
Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)")
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/uapi/linux/btf.h | 15 ++-
kernel/bpf/btf.c | 274 ++++++++++++++++++++++++++++++++++++---
2 files changed, 267 insertions(+), 22 deletions(-)
diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index 14f66948fc95..34aba40ed926 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -34,7 +34,9 @@ struct btf_type {
* bits 0-15: vlen (e.g. # of struct's members)
* bits 16-23: unused
* bits 24-27: kind (e.g. int, ptr, array...etc)
- * bits 28-31: unused
+ * bits 28-30: unused
+ * bit 31: kind_flag, currently used by
+ * struct, union and fwd
*/
__u32 info;
/* "size" is used by INT, ENUM, STRUCT and UNION.
@@ -52,6 +54,7 @@ struct btf_type {
#define BTF_INFO_KIND(info) (((info) >> 24) & 0x0f)
#define BTF_INFO_VLEN(info) ((info) & 0xffff)
+#define BTF_INFO_KFLAG(info) ((info) >> 31)
#define BTF_KIND_UNKN 0 /* Unknown */
#define BTF_KIND_INT 1 /* Integer */
@@ -110,9 +113,17 @@ struct btf_array {
struct btf_member {
__u32 name_off;
__u32 type;
- __u32 offset; /* offset in bits */
+ __u32 offset; /* [bitfield_size and] offset in bits */
};
+/* If the type info kind_flag set, the btf_member.offset
+ * contains both member bit offset and bitfield size, and
+ * bitfield size will set for struct/union bitfield members.
+ * Otherwise, it contains only bit offset.
+ */
+#define BTF_MEMBER_BITFIELD_SIZE(val) ((val) >> 24)
+#define BTF_MEMBER_BIT_OFFSET(val) ((val) & 0xffffff)
+
/* BTF_KIND_FUNC_PROTO is followed by multiple "struct btf_param".
* The exact number of btf_param is stored in the vlen (of the
* info in "struct btf_type").
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 72caa799e82f..ec3f80d3bef6 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -164,7 +164,7 @@
#define BITS_ROUNDUP_BYTES(bits) \
(BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits))
-#define BTF_INFO_MASK 0x0f00ffff
+#define BTF_INFO_MASK 0x8f00ffff
#define BTF_INT_MASK 0x0fffffff
#define BTF_TYPE_ID_VALID(type_id) ((type_id) <= BTF_MAX_TYPE)
#define BTF_STR_OFFSET_VALID(name_off) ((name_off) <= BTF_MAX_NAME_OFFSET)
@@ -274,6 +274,10 @@ struct btf_kind_operations {
const struct btf_type *struct_type,
const struct btf_member *member,
const struct btf_type *member_type);
+ int (*check_kflag_member)(struct btf_verifier_env *env,
+ const struct btf_type *struct_type,
+ const struct btf_member *member,
+ const struct btf_type *member_type);
void (*log_details)(struct btf_verifier_env *env,
const struct btf_type *t);
void (*seq_show)(const struct btf *btf, const struct btf_type *t,
@@ -419,6 +423,25 @@ static u16 btf_type_vlen(const struct btf_type *t)
return BTF_INFO_VLEN(t->info);
}
+static bool btf_type_kflag(const struct btf_type *t)
+{
+ return BTF_INFO_KFLAG(t->info);
+}
+
+static u32 btf_member_bit_offset(const struct btf_type *struct_type,
+ const struct btf_member *member)
+{
+ return btf_type_kflag(struct_type) ? BTF_MEMBER_BIT_OFFSET(member->offset)
+ : member->offset;
+}
+
+static u32 btf_member_bitfield_size(const struct btf_type *struct_type,
+ const struct btf_member *member)
+{
+ return btf_type_kflag(struct_type) ? BTF_MEMBER_BITFIELD_SIZE(member->offset)
+ : 0;
+}
+
static u32 btf_type_int(const struct btf_type *t)
{
return *(u32 *)(t + 1);
@@ -627,9 +650,17 @@ static void btf_verifier_log_member(struct btf_verifier_env *env,
if (env->phase != CHECK_META)
btf_verifier_log_type(env, struct_type, NULL);
- __btf_verifier_log(log, "\t%s type_id=%u bits_offset=%u",
- __btf_name_by_offset(btf, member->name_off),
- member->type, member->offset);
+ if (btf_type_kflag(struct_type))
+ __btf_verifier_log(log,
+ "\t%s type_id=%u bitfield_size=%u bits_offset=%u",
+ __btf_name_by_offset(btf, member->name_off),
+ member->type,
+ BTF_MEMBER_BITFIELD_SIZE(member->offset),
+ BTF_MEMBER_BIT_OFFSET(member->offset));
+ else
+ __btf_verifier_log(log, "\t%s type_id=%u bits_offset=%u",
+ __btf_name_by_offset(btf, member->name_off),
+ member->type, member->offset);
if (fmt && *fmt) {
__btf_verifier_log(log, " ");
@@ -945,6 +976,38 @@ static int btf_df_check_member(struct btf_verifier_env *env,
return -EINVAL;
}
+static int btf_df_check_kflag_member(struct btf_verifier_env *env,
+ const struct btf_type *struct_type,
+ const struct btf_member *member,
+ const struct btf_type *member_type)
+{
+ btf_verifier_log_basic(env, struct_type,
+ "Unsupported check_kflag_member");
+ return -EINVAL;
+}
+
+/* Used for ptr, array and struct/union type members.
+ * int, enum and modifier types have their specific callback functions.
+ */
+static int btf_generic_check_kflag_member(struct btf_verifier_env *env,
+ const struct btf_type *struct_type,
+ const struct btf_member *member,
+ const struct btf_type *member_type)
+{
+ if (BTF_MEMBER_BITFIELD_SIZE(member->offset)) {
+ btf_verifier_log_member(env, struct_type, member,
+ "Invalid member bitfield_size");
+ return -EINVAL;
+ }
+
+ /* bitfield size is 0, so member->offset represents bit offset only.
+ * It is safe to call non kflag check_member variants.
+ */
+ return btf_type_ops(member_type)->check_member(env, struct_type,
+ member,
+ member_type);
+}
+
static int btf_df_resolve(struct btf_verifier_env *env,
const struct resolve_vertex *v)
{
@@ -997,6 +1060,62 @@ static int btf_int_check_member(struct btf_verifier_env *env,
return 0;
}
+static int btf_int_check_kflag_member(struct btf_verifier_env *env,
+ const struct btf_type *struct_type,
+ const struct btf_member *member,
+ const struct btf_type *member_type)
+{
+ u32 struct_bits_off, nr_bits, nr_int_data_bits, bytes_offset;
+ u32 int_data = btf_type_int(member_type);
+ u32 struct_size = struct_type->size;
+ u32 nr_copy_bits;
+
+ /* a regular int type is required for the kflag int member */
+ if (!btf_type_int_is_regular(member_type)) {
+ btf_verifier_log_member(env, struct_type, member,
+ "Invalid member base type");
+ return -EINVAL;
+ }
+
+ /* check sanity of bitfield size */
+ nr_bits = BTF_MEMBER_BITFIELD_SIZE(member->offset);
+ struct_bits_off = BTF_MEMBER_BIT_OFFSET(member->offset);
+ nr_int_data_bits = BTF_INT_BITS(int_data);
+ if (!nr_bits) {
+ /* Not a bitfield member, member offset must be at byte
+ * boundary.
+ */
+ if (BITS_PER_BYTE_MASKED(struct_bits_off)) {
+ btf_verifier_log_member(env, struct_type, member,
+ "Invalid member offset");
+ return -EINVAL;
+ }
+
+ nr_bits = nr_int_data_bits;
+ } else if (nr_bits > nr_int_data_bits) {
+ btf_verifier_log_member(env, struct_type, member,
+ "Invalid member bitfield_size");
+ return -EINVAL;
+ }
+
+ bytes_offset = BITS_ROUNDDOWN_BYTES(struct_bits_off);
+ nr_copy_bits = nr_bits + BITS_PER_BYTE_MASKED(struct_bits_off);
+ if (nr_copy_bits > BITS_PER_U64) {
+ btf_verifier_log_member(env, struct_type, member,
+ "nr_copy_bits exceeds 64");
+ return -EINVAL;
+ }
+
+ if (struct_size < bytes_offset ||
+ struct_size - bytes_offset < BITS_ROUNDUP_BYTES(nr_copy_bits)) {
+ btf_verifier_log_member(env, struct_type, member,
+ "Member exceeds struct_size");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static s32 btf_int_check_meta(struct btf_verifier_env *env,
const struct btf_type *t,
u32 meta_left)
@@ -1016,6 +1135,11 @@ static s32 btf_int_check_meta(struct btf_verifier_env *env,
return -EINVAL;
}
+ if (btf_type_kflag(t)) {
+ btf_verifier_log_type(env, t, "Invalid btf_info kind_flag");
+ return -EINVAL;
+ }
+
int_data = btf_type_int(t);
if (int_data & ~BTF_INT_MASK) {
btf_verifier_log_basic(env, t, "Invalid int_data:%x",
@@ -1097,6 +1221,7 @@ static void btf_bitfield_seq_show(void *data, u8 bits_offset,
seq_printf(m, "0x%llx", print_num);
}
+
static void btf_int_bits_seq_show(const struct btf *btf,
const struct btf_type *t,
void *data, u8 bits_offset,
@@ -1163,6 +1288,7 @@ static const struct btf_kind_operations int_ops = {
.check_meta = btf_int_check_meta,
.resolve = btf_df_resolve,
.check_member = btf_int_check_member,
+ .check_kflag_member = btf_int_check_kflag_member,
.log_details = btf_int_log,
.seq_show = btf_int_seq_show,
};
@@ -1192,6 +1318,31 @@ static int btf_modifier_check_member(struct btf_verifier_env *env,
resolved_type);
}
+static int btf_modifier_check_kflag_member(struct btf_verifier_env *env,
+ const struct btf_type *struct_type,
+ const struct btf_member *member,
+ const struct btf_type *member_type)
+{
+ const struct btf_type *resolved_type;
+ u32 resolved_type_id = member->type;
+ struct btf_member resolved_member;
+ struct btf *btf = env->btf;
+
+ resolved_type = btf_type_id_size(btf, &resolved_type_id, NULL);
+ if (!resolved_type) {
+ btf_verifier_log_member(env, struct_type, member,
+ "Invalid member");
+ return -EINVAL;
+ }
+
+ resolved_member = *member;
+ resolved_member.type = resolved_type_id;
+
+ return btf_type_ops(resolved_type)->check_kflag_member(env, struct_type,
+ &resolved_member,
+ resolved_type);
+}
+
static int btf_ptr_check_member(struct btf_verifier_env *env,
const struct btf_type *struct_type,
const struct btf_member *member,
@@ -1227,6 +1378,11 @@ static int btf_ref_type_check_meta(struct btf_verifier_env *env,
return -EINVAL;
}
+ if (btf_type_kflag(t)) {
+ btf_verifier_log_type(env, t, "Invalid btf_info kind_flag");
+ return -EINVAL;
+ }
+
if (!BTF_TYPE_ID_VALID(t->type)) {
btf_verifier_log_type(env, t, "Invalid type_id");
return -EINVAL;
@@ -1380,6 +1536,7 @@ static struct btf_kind_operations modifier_ops = {
.check_meta = btf_ref_type_check_meta,
.resolve = btf_modifier_resolve,
.check_member = btf_modifier_check_member,
+ .check_kflag_member = btf_modifier_check_kflag_member,
.log_details = btf_ref_type_log,
.seq_show = btf_modifier_seq_show,
};
@@ -1388,6 +1545,7 @@ static struct btf_kind_operations ptr_ops = {
.check_meta = btf_ref_type_check_meta,
.resolve = btf_ptr_resolve,
.check_member = btf_ptr_check_member,
+ .check_kflag_member = btf_generic_check_kflag_member,
.log_details = btf_ref_type_log,
.seq_show = btf_ptr_seq_show,
};
@@ -1422,6 +1580,7 @@ static struct btf_kind_operations fwd_ops = {
.check_meta = btf_fwd_check_meta,
.resolve = btf_df_resolve,
.check_member = btf_df_check_member,
+ .check_kflag_member = btf_df_check_kflag_member,
.log_details = btf_ref_type_log,
.seq_show = btf_df_seq_show,
};
@@ -1480,6 +1639,11 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env,
return -EINVAL;
}
+ if (btf_type_kflag(t)) {
+ btf_verifier_log_type(env, t, "Invalid btf_info kind_flag");
+ return -EINVAL;
+ }
+
if (t->size) {
btf_verifier_log_type(env, t, "size != 0");
return -EINVAL;
@@ -1603,6 +1767,7 @@ static struct btf_kind_operations array_ops = {
.check_meta = btf_array_check_meta,
.resolve = btf_array_resolve,
.check_member = btf_array_check_member,
+ .check_kflag_member = btf_generic_check_kflag_member,
.log_details = btf_array_log,
.seq_show = btf_array_seq_show,
};
@@ -1641,6 +1806,7 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env,
u32 meta_needed, last_offset;
struct btf *btf = env->btf;
u32 struct_size = t->size;
+ u32 offset;
u16 i;
meta_needed = btf_type_vlen(t) * sizeof(*member);
@@ -1682,7 +1848,8 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env,
return -EINVAL;
}
- if (is_union && member->offset) {
+ offset = btf_member_bit_offset(t, member);
+ if (is_union && offset) {
btf_verifier_log_member(env, t, member,
"Invalid member bits_offset");
return -EINVAL;
@@ -1692,20 +1859,20 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env,
* ">" instead of ">=" because the last member could be
* "char a[0];"
*/
- if (last_offset > member->offset) {
+ if (last_offset > offset) {
btf_verifier_log_member(env, t, member,
"Invalid member bits_offset");
return -EINVAL;
}
- if (BITS_ROUNDUP_BYTES(member->offset) > struct_size) {
+ if (BITS_ROUNDUP_BYTES(offset) > struct_size) {
btf_verifier_log_member(env, t, member,
"Member bits_offset exceeds its struct size");
return -EINVAL;
}
btf_verifier_log_member(env, t, member, NULL);
- last_offset = member->offset;
+ last_offset = offset;
}
return meta_needed;
@@ -1735,9 +1902,14 @@ static int btf_struct_resolve(struct btf_verifier_env *env,
last_member_type = btf_type_by_id(env->btf,
last_member_type_id);
- err = btf_type_ops(last_member_type)->check_member(env, v->t,
- last_member,
- last_member_type);
+ if (btf_type_kflag(v->t))
+ err = btf_type_ops(last_member_type)->check_kflag_member(env, v->t,
+ last_member,
+ last_member_type);
+ else
+ err = btf_type_ops(last_member_type)->check_member(env, v->t,
+ last_member,
+ last_member_type);
if (err)
return err;
}
@@ -1759,9 +1931,14 @@ static int btf_struct_resolve(struct btf_verifier_env *env,
return env_stack_push(env, member_type, member_type_id);
}
- err = btf_type_ops(member_type)->check_member(env, v->t,
- member,
- member_type);
+ if (btf_type_kflag(v->t))
+ err = btf_type_ops(member_type)->check_kflag_member(env, v->t,
+ member,
+ member_type);
+ else
+ err = btf_type_ops(member_type)->check_member(env, v->t,
+ member,
+ member_type);
if (err)
return err;
}
@@ -1789,17 +1966,26 @@ static void btf_struct_seq_show(const struct btf *btf, const struct btf_type *t,
for_each_member(i, t, member) {
const struct btf_type *member_type = btf_type_by_id(btf,
member->type);
- u32 member_offset = member->offset;
- u32 bytes_offset = BITS_ROUNDDOWN_BYTES(member_offset);
- u8 bits8_offset = BITS_PER_BYTE_MASKED(member_offset);
const struct btf_kind_operations *ops;
+ u32 member_offset, bitfield_size;
+ u32 bytes_offset;
+ u8 bits8_offset;
if (i)
seq_puts(m, seq);
- ops = btf_type_ops(member_type);
- ops->seq_show(btf, member_type, member->type,
- data + bytes_offset, bits8_offset, m);
+ member_offset = btf_member_bit_offset(t, member);
+ bitfield_size = btf_member_bitfield_size(t, member);
+ if (bitfield_size) {
+ btf_bitfield_seq_show(data, member_offset,
+ bitfield_size, m);
+ } else {
+ bytes_offset = BITS_ROUNDDOWN_BYTES(member_offset);
+ bits8_offset = BITS_PER_BYTE_MASKED(member_offset);
+ ops = btf_type_ops(member_type);
+ ops->seq_show(btf, member_type, member->type,
+ data + bytes_offset, bits8_offset, m);
+ }
}
seq_puts(m, "}");
}
@@ -1808,6 +1994,7 @@ static struct btf_kind_operations struct_ops = {
.check_meta = btf_struct_check_meta,
.resolve = btf_struct_resolve,
.check_member = btf_struct_check_member,
+ .check_kflag_member = btf_generic_check_kflag_member,
.log_details = btf_struct_log,
.seq_show = btf_struct_seq_show,
};
@@ -1837,6 +2024,35 @@ static int btf_enum_check_member(struct btf_verifier_env *env,
return 0;
}
+static int btf_enum_check_kflag_member(struct btf_verifier_env *env,
+ const struct btf_type *struct_type,
+ const struct btf_member *member,
+ const struct btf_type *member_type)
+{
+ u32 struct_bits_off, nr_bits, bytes_end, struct_size;
+ u32 int_bitsize = sizeof(int) * BITS_PER_BYTE;
+
+ struct_bits_off = BTF_MEMBER_BIT_OFFSET(member->offset);
+ nr_bits = BTF_MEMBER_BITFIELD_SIZE(member->offset);
+ if (!nr_bits) {
+ nr_bits = int_bitsize;
+ } else if (nr_bits > int_bitsize) {
+ btf_verifier_log_member(env, struct_type, member,
+ "Invalid member bitfield_size");
+ return -EINVAL;
+ }
+
+ struct_size = struct_type->size;
+ bytes_end = BITS_ROUNDUP_BYTES(struct_bits_off + nr_bits);
+ if (struct_size < bytes_end) {
+ btf_verifier_log_member(env, struct_type, member,
+ "Member exceeds struct_size");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static s32 btf_enum_check_meta(struct btf_verifier_env *env,
const struct btf_type *t,
u32 meta_left)
@@ -1856,6 +2072,11 @@ static s32 btf_enum_check_meta(struct btf_verifier_env *env,
return -EINVAL;
}
+ if (btf_type_kflag(t)) {
+ btf_verifier_log_type(env, t, "Invalid btf_info kind_flag");
+ return -EINVAL;
+ }
+
if (t->size != sizeof(int)) {
btf_verifier_log_type(env, t, "Expected size:%zu",
sizeof(int));
@@ -1924,6 +2145,7 @@ static struct btf_kind_operations enum_ops = {
.check_meta = btf_enum_check_meta,
.resolve = btf_df_resolve,
.check_member = btf_enum_check_member,
+ .check_kflag_member = btf_enum_check_kflag_member,
.log_details = btf_enum_log,
.seq_show = btf_enum_seq_show,
};
@@ -1946,6 +2168,11 @@ static s32 btf_func_proto_check_meta(struct btf_verifier_env *env,
return -EINVAL;
}
+ if (btf_type_kflag(t)) {
+ btf_verifier_log_type(env, t, "Invalid btf_info kind_flag");
+ return -EINVAL;
+ }
+
btf_verifier_log_type(env, t, NULL);
return meta_needed;
@@ -2005,6 +2232,7 @@ static struct btf_kind_operations func_proto_ops = {
* Hence, there is no btf_func_check_member().
*/
.check_member = btf_df_check_member,
+ .check_kflag_member = btf_df_check_kflag_member,
.log_details = btf_func_proto_log,
.seq_show = btf_df_seq_show,
};
@@ -2024,6 +2252,11 @@ static s32 btf_func_check_meta(struct btf_verifier_env *env,
return -EINVAL;
}
+ if (btf_type_kflag(t)) {
+ btf_verifier_log_type(env, t, "Invalid btf_info kind_flag");
+ return -EINVAL;
+ }
+
btf_verifier_log_type(env, t, NULL);
return 0;
@@ -2033,6 +2266,7 @@ static struct btf_kind_operations func_ops = {
.check_meta = btf_func_check_meta,
.resolve = btf_df_resolve,
.check_member = btf_df_check_member,
+ .check_kflag_member = btf_df_check_kflag_member,
.log_details = btf_ref_type_log,
.seq_show = btf_df_seq_show,
};