Re: [PATCH v2 dwarves 1/5] dwarves: help dwarf loader spot functions with optimized-out parameters

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

Em Wed, Feb 01, 2023 at 05:18:29PM +0000, Alan Maguire escreveu:
> On 01/02/2023 17:01, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Feb 01, 2023 at 08:49:07AM -0800, Alexei Starovoitov escreveu:
> >> It feels fixed pahole should be done under some flag
> >> otherwise when people update the pahole the existing and older
> >> kernels might stop building with warns:
> >> WARN: resolve_btfids: unresolved symbol bpf_xdp_metadata_rx_hash
> >> WARN: resolve_btfids: unresolved symbol bpf_task_kptr_get
> >> ...
 
> Good point, something like
 
> --skip_inconsistent_proto	Skip functions that have multiple inconsistent
> 				function prototypes sharing the same name, or
> 				have optimized-out parameters.

We have:

⬢[acme@toolbox pahole]$ grep '"skip_encoding.*' pahole.c
		.name = "skip_encoding_btf_vars",
		.name = "skip_encoding_btf_decl_tag",
		.name = "skip_encoding_btf_type_tag",
		.name = "skip_encoding_btf_enum64",
⬢[acme@toolbox pahole]$

Perhaps, even being long, we should be consistent and name it:

	--skip_encoding_btf_inconsistent_proto

?
 
> ? Implementation needs a bit of thought though because we're
> not really doing the same thing that we were before. Previously we
> were adding the first instance of a function in the CU we came across.
> Probably safest to resurrect that behaviour for the legacy
> non-skip-inconsistent-proto case I think. The final patch handling

Consider getting what I have now in my next branch, that has the fixups
I made while reviewing, as discussed in this thread:

⬢[acme@toolbox pahole]$ git log --oneline -6
b1576cf15106efd7 (HEAD -> master) pahole: Sync with libbpf-1.1
e9db5622d97395b7 btf_encoder: Delay function addition to check for function prototype inconsistencies
74675488e8ed5718 btf_encoder: Represent "."-suffixed functions (".isra.0") in BTF
be470fa5757e5915 btf_encoder: Rework btf_encoders__*() API to allow traversal of encoders
d6e0778f6b5912da btf_encoder: Refactor function addition into dedicated btf_encoder__add_func
f77b5ae93844b5c4 dwarf_loader: Help spotting functions with optimized-out parameters
⬢[acme@toolbox pahole]$

And at the point where you change the behaviour you introduce the
option, so that we don't have to remove it and then ressurect.

- Arnaldo

> inconsistent function prototypes will need to be reworked a bit to 
> support this, since we tossed this approach and used saving/merging 
> multiple instances in the tree instead.  Once I've built bpf trees I'll
> have a go at getting this working.
> 
> >> Arnaldo, could you check what warns do you see with this fixed pahole
> >> in bpf tree ?
> > 
> > Sure.
> > 
> 
> I can collect this for x86_64/aarch64 too; might take a few hours
> before I have the results.
> 
> >> If there are only few warns then we can manually add __used noinline
> >> to these places, push to bpf tree and push to stable.
> >>
> >> Then in bpf-next we can clean up everything with __bpf_kfunc.
> > 

-- 

- Arnaldo