[PATCH] D143967: [DebugInfo][BPF] Add 'btf:type_tag' annotation in DWARF

Wed Apr 19 10:46:08 PDT 2023

dfaust added a comment.

In D143967#4233742 <https://reviews.llvm.org/D143967#4233742>, @eddyz87 wrote:

> Moving type tags past typedefs would also make C code reconstruction from BTF incomplete. Such reconstruction is used now by e.g. bpftool to create a vmlinux.h header with all kernel type definitions. So, I think type tags should not be moved past typedefs.

Yes I agree. Tags should not be moved past typedefs.
In the GCC implementation we are not moving tags past typedefs.

> It uses `btf_type_is_modifier()` utility function, which treats typedef as a modifier.
>
> So, in theory the transformation moving tags past typedef is necessary. On the other hand, such transformation is not applied now, and this does not cause issues. Which suggests that there are no cases in practice where type tag follows typedef (and thus, this is not required for backwards compatibility).

It seems to me then that the appropriate fix is to remove the expectation of this transformation from the verifier (for the reasons above).

If it is not important for backwards compatibility, then I guess it should not be problematic to change in the kernel? (I say, as not a kernel hacker...)

I find it odd that `BTF_KIND_TYPEDEF` is included in `btf_type_is_modifier()`, seems like the expectation of this transformation is almost an unintended side effect of that.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143967/new/

https://reviews.llvm.org/D143967