Re: [RFC dwarves 0/4] dwarves: change BTF encoding skip logic for functions

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Fri, Feb 17, 2023 at 11:10:29PM +0000, Alan Maguire escreveu:
> It has been observed [1] that the recent dwarves changes
> that skip BTF encoding for functions that have optimized-out
> parameters are too aggressive, leading to missing kfuncs
> which generate warnings and a BPF selftest failure.
> 
> Here a different approach is used; we observe that
> just because a function does not _use_ a parameter,
> it does not mean it was not passed to it.  What we
> are really keen to detect are cases where the calling
> conventions are violated such that a function will
> not have parameter values in the expected registers.
> In such cases, tracing and kfunc behaviour will be
> unstable.  We are not worried about parameters being
> optimized out, provided that optimization does not
> lead to other parameters being passed via
> unexpected registers.
> 
> So this series attempts to detect such cases by
> examining register (DW_OP_regX) values for
> parameters where available; if these match
> expectations, the function is deemed safe to add to
> BTF, even if parameters are optimized out.
> 
> Using this approach, the only functions that
> BTF generation is suppressed for are
> 
> 1. those with parameters that violate calling
>    conventions where present; and
> 2. those which have multiple inconsistent prototypes.

This sounds sensible at first sight, I've applied the patches so that it
can go thru further testing on the libbpf CI, for now its just on the
'next' branch, the testing I did so far:

make allmodconfig + enablig BTF on bpf-next reverting that revert so
that we use the new options,

Now I'm building and booting a kernel with a fedora-ish config without
using the new options and then with them (reverting the revert) to
compare the tools/testing/selftests/bpf/ ./test-progs output
before/after.

Its an extended holiday down here, so I'll be spotty but want to get
this moving forward,

Thanks!

- Arnaldo
 
> With these changes, running pahole on a gcc-built
> vmlinux skips
> 
> - 1164 functions due to multiple inconsistent function
>   prototypes.  Most of these are "."-suffixed optimized
>   fuctions.
> - 331 functions due to unexpected register usage
> 
> For a clang-built kernel, the numbers are
> 
> - 539 functions with inconsistent prototypes are skipped
> - 209 functions with unexpected register usage are skipped
> 
> One complication is that functions that are passed
> structs (or typedef structs) can use multiple registers
> to pass those structures.  Examples include
> bpf_lsm_socket_getpeersec_stream() (passing a typedef
> struct sockptr_t) and the bpf_testmod_test_struct_arg_1
> function in bpf_testmod.  Because multiple registers
> are used to represent the structure, this throws
> off expectations for any subsequent parameter->register
> mappings.  To handle this, simply exempt functions
> that have struct (or typedef struct) parameters from
> our register checks.
> 
> Note to test this series on bpf-next, the following
> commit should be reverted (reverting the revert
> so that the flags are added to BTF encoding when
> using pahole v1.25):
> 
> commit 1f5dfcc78ab4 ("Revert "bpf: Add --skip_encoding_btf_inconsistent_proto, --btf_gen_optimized to pahole flags for v1.25"")
> 
> With these changes we also see tracing_struct now pass:
> 
> $ sudo ./test_progs -t tracing_struct
> #233     tracing_struct:OK
> Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
> 
> Further testing is needed - along with support for additional
> parameter index -> DWARF reg for more platforms.
> 
> Future work could also add annotations for optimized-out
> parameters via BTF tags to help guide tracing.
> 
> [1] https://lore.kernel.org/bpf/CAADnVQ+hfQ9LEmEFXneB7hm17NvRniXSShrHLaM-1BrguLjLQw@mail.gmail.com/
> 
> Alan Maguire (4):
>   dwarf_loader: mark functions that do not use expected registers for
>     params
>   btf_encoder: exclude functions with unexpected param register use not
>     optimizations
>   pahole: update descriptions for btf_gen_optimized,
>     skip_encoding_btf_inconsistent_proto
>   pahole: update man page for options also
> 
>  btf_encoder.c      |  24 +++++++---
>  dwarf_loader.c     | 109 ++++++++++++++++++++++++++++++++++++++++++---
>  dwarves.h          |   5 +++
>  man-pages/pahole.1 |   4 +-
>  pahole.c           |   4 +-
>  5 files changed, 129 insertions(+), 17 deletions(-)
> 
> -- 
> 2.31.1
> 

-- 

- Arnaldo