Re: [PATCH dwarves 0/3] dwarves: improve BTF encoder comparison method

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

Em Mon, Mar 13, 2023 at 10:40:53AM +0100, Jiri Olsa escreveu:
> On Fri, Mar 10, 2023 at 02:50:47PM +0000, Alan Maguire wrote:
> > Currently when looking for function prototype mismatches with a view
> > to excluding inconsistent functions, we fall back to a comparison
> > between parameter names when the name and number of parameters match.
> > This is brittle, as it is sometimes the case that a function has
> > multiple type-identical definitions which use different parameters.
> > 
> > Here the existing dwarves_fprintf functionality is re-used to instead
> > create a string representation of the function prototype - minus the
> > parameter names - to support a less brittle comparison method.
> > 
> > To support this, patch 1 generalizes function prototype print to
> > take a conf_fprintf parameter; this allows us to customize the
> > parameters we use in prototype string generation.
> > 
> > Patch 2 supports generating prototypes without modifiers such
> > as const as they can lead to false positive prototype mismatches;
> > see the patch for details.
> > 
> > Finally patch 3 replaces the logic used to compare parameter
> > names with the prototype string comparison instead.
> > 
> > Using verbose pahole output we can see some of the rejected
> > comparisons.  73 comparisons are rejected via prototype
> > comparison, 63 of which are non "."-suffixed functions.  For
> > example:
> > 
> > function mismatch for 'name_show'('name_show'): 'ssize_t ()(struct kobject *, struct kobj_attribute *, char *)' != 'ssize_t ()(struct device *, struct device_attribute *, char *)'
> > 
> > With these changes, the syscalls defined in sys_ni.c
> > that Jiri mentioned were missing [1] are present in BTF:
> > 
> > [43071] FUNC '__ia32_compat_sys_io_setup' type_id=42335 linkage=static
> > [43295] FUNC '__ia32_sys_io_setup' type_id=42335 linkage=static
> > [47536] FUNC '__x64_sys_io_setup' type_id=42335 linkage=static
> > 
> > [43290] FUNC '__ia32_sys_io_destroy' type_id=42335 linkage=static
> > [47531] FUNC '__x64_sys_io_destroy' type_id=42335 linkage=static
> > 
> > [43072] FUNC '__ia32_compat_sys_io_submit' type_id=42335 linkage=static
> > [43296] FUNC '__ia32_sys_io_submit' type_id=42335 linkage=static
> > [47537] FUNC '__x64_sys_io_submit' type_id=42335 linkage=static
> > 
> > [1] https://lore.kernel.org/bpf/ZAsBYpsBV0wvkhh0@krava/
> > 
> > Alan Maguire (3):
> >   dwarves_fprintf: generalize function prototype print to support
> >     passing conf
> >   dwarves_fprintf: support skipping modifier
> >   btf_encoder: compare functions via prototypes not parameter names
> 
> lgtm, the syscalls from sys_ni.c are there
> 
> for me the total number of syscalls increased from 249 to 432, great ;-)
> 
> Acked/Tested-by: Jiri Olsa <jolsa@kernel.org>
> 
> thanks,
> jirka

Thanks, applied and pushed to the 'next' branch for some more testing.

I really need to get 1.25 out of the door, due to other issue that crops
up with binutils 2.40 (DW_TAG_unspecified_type), and will be travelling
the last two days of this week, so any further testing is more than
welcome.

- Arnaldo