Re: [PATCH 1/2] libtraceevent: Add loading of BTF to the tep handle
Steven Rostedt <[email protected]> Mon, 4 Aug 2025 10:41:04 -0400
| Newsgroups | org.kernel.vger.linux-trace-devel |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 4 Aug 2025 15:18:41 +0100 Douglas Raillard <[email protected]> wrote: > >>> + case BTF_KIND_INT: > >>> + encode = *(int *)((void *)t + sizeof(*t)); > >>> + /* Print unsigned ints as hex */ > >>> + if (BTF_INT_ENCODING(encode) & BTF_INT_SIGNED) > >> > >> BTF_INT_OFFSET() and BTF_INT_VAL() values should also be used to shift and mask > >> appropriately. > > > > You meant to the "arg" passed in? > > Yes, something like that: > > unsigned long long mask = (1 << BTF_INT_BITS(encode)) - 1; > unsigned long long shift = BTF_INT_OFFSET(encode); > arg = (arg >> shift) & mask; > > I suppose that how BTF_INT_OFFSET() is to be used depends on endianness, and I'm not > sure what is the endianness of the arg value. t->size would probably be involved as > well in the big endian case: https://docs.kernel.org/bpf/btf.html#btf-kind-int Hmm, I guess I'll need to update that. The endianness is stored in the tep_handle descriptor and will need to be used. Note, this needs to handle big endian machine reading little endian machine data and vice versa. As well as little and big reading their own endianness. Too bad my big endian machine died. I used that to test all the combinations. > > > > >> > >> I assume that regardless of BTF_INT_CHAR and BTF_INT_BOOL value, BTF_INT_SIGNED is > >> set appropriately. > >> > >>> + trace_seq_printf(s, "%lld", arg); > >>> + else > >>> + trace_seq_printf(s, "0x%llx", arg); > >>> + break; > >>> + case BTF_KIND_ENUM: > >> > >> Could add as well: case BTF_KIND_ENUM64: > > > > Of course then we would need to check if this is a 32 bit infrastructure. > > I'm assuming it would be treated differently. More like a struct? > > AFAIR the only point of BTF_KIND_ENUM64 is to represent enum with enumerator values that are > larger than 32 bits, since BTF_KIND_ENUM can only represent 32 bit values: > > struct btf_enum { > __u32 name_off; > __s32 val; > }; > > In contrast, BTF_KIND_ENUM64 encodes the enumerators with this: > > struct btf_enum64 { > __u32 name_off; > __u32 val_lo32; > __u32 val_hi32; > }; > > So each value can have 64bits, split over 2 fields. I assume this quirk is > just to have alignof(struct btf_enum64) == 4 so that the BTF wire format > can be mapped on that C struct easily. > > AFAIR BTF_KIND_ENUM encoding was simply an oversight in the initial BTF spec, so they corrected it > by adding another BTF_KIND_*. > > Since you make no use of the enumerators array for now, I think the handling is identical. > > > Wrt to 32 vs 64 bits, in ISO C, the enum size does not depend on the ABI: > https://port70.net/~nsz/c/c11/n1570.html#6.7.2.2p4 When I brought up 64 bit, I meant with the parameters being passed in. As this is for parsing function parameters. On 32bit an enum64 would require two registers whereas on 64bit it would only require one. -- Steve