On Sat, 24 Sept 2022 at 01:20, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Fri, Sep 23, 2022 at 3:32 PM Kumar Kartikeya Dwivedi > <memxor@xxxxxxxxx> wrote: > > > > Hi, > > For the following example: > > > > kkd@Legion ~/src/linux > > ; cat bpf.c > > #define tag __attribute__((btf_decl_tag("tag"))) > > > > int a tag; > > int b tag; > > > > int main() { > > return a + b; > > } > > > > -- > > > > When I compile using: > > clang -target bpf -O2 -g -c bpf.c > > > > For the BTF dump, I see: > > [1] FUNC_PROTO '(anon)' ret_type_id=2 vlen=0 > > [2] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > [3] FUNC 'main' type_id=1 linkage=global > > [4] VAR 'a' type_id=2, linkage=global > > [5] DECL_TAG 'tag' type_id=4 component_idx=-1 > > [6] VAR 'b' type_id=2, linkage=global > > [7] DECL_TAG 'tag' type_id=6 component_idx=-1 > > [8] DATASEC '.bss' size=0 vlen=2 > > type_id=4 offset=0 size=4 (VAR 'a') > > type_id=6 offset=0 size=4 (VAR 'b') > > > > There are two issues that I hit: > > > > 1. The component_idx=-1 makes it a little inconvenient to correlate > > the tag applied to a VAR in a DATASEC. In case of structs the index > > can be matched with component_idx, in case of DATASEC we have to match > > VAR's type_id. So the code has to be different. If it also had > > component_idx set it would be possible to make the code same for both > > inside the kernel's field parsing routine. > > This is expected and documented in UAPI: > > "If component_idx == -1, the tag is applied to a struct, union, > variable or function." > > Variable is an independent entity and tag applies to it. I don't > know/remember why we did it this way, but it's probably easier for > Clang to generate it this way. And it probably is easier for BPF > static linker as well. Note that you don't merge two structs together, > while static linker does merge variables between DATASECs. > > In short, DATASEC+VAR is sufficiently different from STRUCT+fields, so > is treated differently. > I see, thanks. In that case I'll add this description as a comment where the code matches the tag to the VAR. > > > > 2. The second issue is that the offset is always 0 for DATASEC VARs. > > That makes it difficult to ensure proper alignment of the variables. > > That's also expected (even if unfortunate) behavior of Clang. Good > news is that libbpf's static linker API is normalizing this. Libbpf > itself also normalizes it internally if passed .bpf.o file straight > from Clang's output. > > In short, if you run `bpftool gen object my_normalized.bpf.o > my_clang.bpf.o` you'll get offsets in my_normalized.bpf.o's BTF. > I see. It would be better if this was fixed up by clang itself, but I don't know enough about the BPF backend to comment whether that would make sense. It's unlikely people will run it through libbpf's linker when they're not statically linking more than one object (or even be using libbpf, but that's a separate thing), and it's very hard to correlate the errors to their cause when failure happens during MAP_CREATE (I guess eventually it should support a log buffer like btf load and prog load), so in the end it would be confusing for users. However we do need to ensure the offsets are correct to detect proper alignment. So adding some checks looks inevitable. I guess for the first VAR, we can assume offset==0 is fine, but from the next VAR, we should return an error if we see offset==0 again? This should catch the not normalized case in case of DATASEC vlen > 1 (== 1 should still be ok). wdyt? > > > > I would like to know if these are expected behaviors or bugs? > > Features ;) > > > Thanks > > -- > > ; clang --version > > Ubuntu clang version > > 16.0.0-++20220813052912+eaf0aa1f1fbd-1~exp1~20220813173018.344