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

[Jiri Olsa <olsajiri@gmail.com>]

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

> 
>  btf_encoder.c     | 67 +++++++++++++++++++++++++------------------------------
>  dwarves.h         |  6 +++++
>  dwarves_fprintf.c | 48 ++++++++++++++++++++++++++-------------
>  3 files changed, 70 insertions(+), 51 deletions(-)
> 
> -- 
> 1.8.3.1
>