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

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, Jan 31, 2023 at 09:33:49AM -0300, Arnaldo Carvalho de Melo wrote:
> 
> 
> On January 31, 2023 9:14:05 AM GMT-03:00, Alan Maguire <alan.maguire@oracle.com> wrote:
> >On 31/01/2023 01:04, Arnaldo Carvalho de Melo wrote:
> >> Em Mon, Jan 30, 2023 at 09:25:17PM -0300, Arnaldo Carvalho de Melo escreveu:
> >>> Em Mon, Jan 30, 2023 at 10:37:56PM +0000, Alan Maguire escreveu:
> >>>> On 30/01/2023 20:23, Arnaldo Carvalho de Melo wrote:
> >>>>> Em Mon, Jan 30, 2023 at 05:10:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> >>>>>> +++ b/dwarves.h
> >>>>>> @@ -262,6 +262,7 @@ struct cu {
> >>>>>>  	uint8_t		 has_addr_info:1;
> >>>>>>  	uint8_t		 uses_global_strings:1;
> >>>>>>  	uint8_t		 little_endian:1;
> >>>>>> +	uint8_t		 nr_register_params;
> >>>>>>  	uint16_t	 language;
> >>>>>>  	unsigned long	 nr_inline_expansions;
> >>>>>>  	size_t		 size_inline_expansions;
> >>>>>
> >>>  
> >>>> Thanks for this, never thought of cross-builds to be honest!
> >>>
> >>>> Tested just now on x86_64 and aarch64 at my end, just ran
> >>>> into one small thing on one system; turns out EM_RISCV isn't
> >>>> defined if using a very old elf.h; below works around this
> >>>> (dwarves otherwise builds fine on this system).
> >>>
> >>> Ok, will add it and will test with containers for older distros too.
> >> 
> >> Its on the 'next' branch, so that it gets tested in the libbpf github
> >> repo at:
> >> 
> >> https://github.com/libbpf/libbpf/actions/workflows/pahole.yml
> >> 
> >> It failed yesterday and today due to problems with the installation of
> >> llvm, probably tomorrow it'll be back working as I saw some
> >> notifications floating by.
> >> 
> >> I added the conditional EM_RISCV definition as well as removed the dup
> >> iterator that Jiri noticed.
> >>
> >
> >Thanks again Arnaldo! I've hit an issue with this series in
> >BTF encoding of kfuncs; specifically we see some kfuncs missing
> >from the BTF representation, and as a result:
> >
> >WARN: resolve_btfids: unresolved symbol bpf_xdp_metadata_rx_hash
> >WARN: resolve_btfids: unresolved symbol bpf_task_kptr_get
> >WARN: resolve_btfids: unresolved symbol bpf_ct_change_status
> >
> >Not sure why I didn't notice this previously.
> >
> >The problem is the DWARF - and therefore BTF - generated for a function like
> >
> >int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32 *hash)
> >{
> >        return -EOPNOTSUPP;
> >}
> >
> >looks like this:
> >
> >   <8af83a2>   DW_AT_external    : 1
> >    <8af83a2>   DW_AT_name        : (indirect string, offset: 0x358bdc): bpf_xdp_metadata_rx_hash
> >    <8af83a6>   DW_AT_decl_file   : 5
> >    <8af83a7>   DW_AT_decl_line   : 737
> >    <8af83a9>   DW_AT_decl_column : 5
> >    <8af83aa>   DW_AT_prototyped  : 1
> >    <8af83aa>   DW_AT_type        : <0x8ad8547>
> >    <8af83ae>   DW_AT_sibling     : <0x8af83cd>
> > <2><8af83b2>: Abbrev Number: 38 (DW_TAG_formal_parameter)
> >    <8af83b3>   DW_AT_name        : ctx
> >    <8af83b7>   DW_AT_decl_file   : 5
> >    <8af83b8>   DW_AT_decl_line   : 737
> >    <8af83ba>   DW_AT_decl_column : 51
> >    <8af83bb>   DW_AT_type        : <0x8af421d>
> > <2><8af83bf>: Abbrev Number: 35 (DW_TAG_formal_parameter)
> >    <8af83c0>   DW_AT_name        : (indirect string, offset: 0x27f6a2): hash
> >    <8af83c4>   DW_AT_decl_file   : 5
> >    <8af83c5>   DW_AT_decl_line   : 737
> >    <8af83c7>   DW_AT_decl_column : 61
> >    <8af83c8>   DW_AT_type        : <0x8adc424>
> >
> >...and because there are no further abstract origin references
> >with location information either, we classify it as lacking 
> >locations for (some of) the parameters, and as a result
> >we skip BTF encoding. We can work around that by doing this:
> >
> >__attribute__ ((optimize("O0"))) int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32 *hash)
> >{
> >	return -EOPNOTSUPP;
> >}
> >
> >Should we #define some kind of "kfunc" prefix equivalent to the
> >above to handle these cases in include/linux/bpf.h perhaps?
> >If that makes sense, I'll send bpf-next patches to cover the
> >set of kfuncs.
> 
> Jiri?

hum I wonder what's the point of the kfunc if it returns -EOPNOTSUPP,
at least I can't see any other version of it.. maybe some temporary
stuff like for bpf_task_kptr_get

but I think it's good idea to make sure it does not get optimized out,
so some kfunc macro seems like good idea.. or maybe we could use
also declaration tag for kfuncs

David already send some patchset for BPF_KFUNC macro, could be part of
that

https://lore.kernel.org/bpf/20230123171506.71995-1-void@manifault.com/

jirka

> 
> >The other thing we might want to do is bump the libbpf version
> >for dwarves 1.25, what do you think? I've tested with libbpf 1.1
> >and aside from the above issue all looks good (there's a few dedup
> >improvements that this version will give us). I can send a patch for
> >the libbpf update if that makes sense.
> 
> 
> Please send it, then we give it some more days of wider testing,
> 
> Yonghong, Andrii, comments on updating libbpf in the pahole submodule?
> 
> - Arnaldo