On Tue, Feb 15, 2022 at 4:12 PM Delyan Kratunov <delyank@xxxxxx> wrote: > > When emitting type declarations in skeletons, bpftool will now also emit > static assertions on the size of the data/bss/rodata/etc fields. This > ensures that in situations where userspace and kernel types have the same > name but differ in size we do not silently produce incorrect results but > instead break the build. > > This was reported in [1] and as expected the repro in [2] fails to build > on the new size assert after this change. > > [1]: Closes: https://github.com/libbpf/libbpf/issues/433 > [2]: https://github.com/fuweid/iovisor-bcc-pr-3777 > > Signed-off-by: Delyan Kratunov <delyank@xxxxxx> > --- LGTM with a trivial styling nits. But this doesn't apply cleanly to bpf-next (see [0]). Can you please rebase and resend. Also for single-patch submissions we don't require cover letter, to please just put all the description into one patch without cover letter. [0] https://github.com/kernel-patches/bpf/pull/2563#issuecomment-1040929960 > tools/bpf/bpftool/gen.c | 134 +++++++++++++++++++++++++++++++++------- > 1 file changed, 112 insertions(+), 22 deletions(-) [...] > + > + bpf_object__for_each_map(map, obj) { > + if (!bpf_map__is_internal(map)) > + continue; > + if (!(bpf_map__map_flags(map) & BPF_F_MMAPABLE)) > + continue; > + if (!get_map_ident(map, map_ident, sizeof(map_ident))) > + continue; > + > + sec = find_type_for_map(obj, map_ident); > + nit: unnecessary empty line between assignment and "error checking" > + if (!sec) { > + /* best effort, couldn't find the type for this map */ > + continue; > + } > + > + sec_var = btf_var_secinfos(sec); > + vlen = btf_vlen(sec); > + > + for (i = 0; i < vlen; i++, sec_var++) { > + const struct btf_type *var = btf__type_by_id(btf, sec_var->type); > + const char *var_name = btf__name_by_offset(btf, var->name_off); > + __u32 var_type_id = var->type; > + __s64 var_size = btf__resolve_size(btf, var_type_id); > + > + if (var_size < 0) > + continue; > + > + /* static variables are not exposed through BPF skeleton */ > + if (btf_var(var)->linkage == BTF_VAR_STATIC) > + continue; > + > + var_ident[0] = '\0'; > + strncat(var_ident, var_name, sizeof(var_ident) - 1); > + sanitize_identifier(var_ident); > + > + printf("\t_Static_assert("); > + printf("sizeof(s->%1$s->%2$s) == %3$lld, ", > + map_ident, var_ident, var_size); > + printf("\"unexpected size of '%1$s'\");\n", var_ident); nit: I'd keep this as one printf, it makes it a bit easier to follow. > + } > + } [...]
