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

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

Em Fri, Mar 10, 2023 at 02:50:47PM +0000, Alan Maguire escreveu:
> 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/

I'll test this now, but b4 isn't liking the way you sent it:

⬢[acme@toolbox pahole]$ b4 am -ctsl --cc-trailers 1678459850-16140-2-git-send-email-alan.maguire@oracle.com
Grabbing thread from lore.kernel.org/all/1678459850-16140-2-git-send-email-alan.maguire%40oracle.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 2 messages in the thread
Checking attestation on all messages, may take a moment...
---
  ✓ [PATCH 1/3] dwarves_fprintf: generalize function prototype print to support passing conf
    ✓ Signed: DKIM/oracle.com
    + Link: https://lore.kernel.org/r/1678459850-16140-2-git-send-email-alan.maguire@oracle.com
    + Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
  ERROR: missing [2/3]!
  ERROR: missing [3/3]!
---
Total patches: 1
---
WARNING: Thread incomplete!
Cover: ./20230310_alan_maguire_dwarves_improve_btf_encoder_comparison_method.cover
 Link: https://lore.kernel.org/r/1678459850-16140-1-git-send-email-alan.maguire@oracle.com
 Base: applies clean to current tree
       git checkout -b 20230310_alan_maguire_oracle_com HEAD
       git am ./20230310_alan_maguire_dwarves_improve_btf_encoder_comparison_method.mbx
⬢[acme@toolbox pahole]$

I'll apply one by one
 
> 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
> 
>  btf_encoder.c     | 67 +++++++++++++++++++++++++------------------------------
>  dwarves.h         |  6 +++++
>  dwarves_fprintf.c | 48 ++++++++++++++++++++++++++-------------
>  3 files changed, 70 insertions(+), 51 deletions(-)
> 
> -- 
> 1.8.3.1
> 

-- 

- Arnaldo