On 18/09/2024 09:41, Arnaldo Carvalho de Melo wrote:
> On Mon, Sep 16, 2024 at 02:49:46PM +0100, Alan Maguire wrote:
>> use pahole to encode BTF and pfunct to check that
>>
>> - DWARF functions that made it into BTF match signatures
>> - for functions we say we skipped, we did indeed skip them in BTF
>> encoding; and
>> - it was correct to skip these functions
>>
>> It is not possible to check everything, as pfunct does not
>> print "."-suffixed optimized representations, and it does not
>> analyze location calling conventions. As a starting point we check
>> for
>>
>> - functions encoded in BTF match signatures with DWARF
>> - functions with multiple incompatible return values
>> - functions with incompatible parameter count/type
>>
>> This test automates manual validation of encoding/skipping,
>> and as such will be useful to ensure regressions are not
>> introduced, for comparing gcc/LLVM-based DWARF representations
>> etc.
>>
>> The test takes ~5 minutes to run in all; output looks like this:
>>
>> $ vmlinux=~/kbuild/bpf-next/vmlinux bash ./btf_functions.sh
>
> I'm trying without specifying vmlinux, what we have now in the
> tests/tests should find the one for the running kernel and use it, but
> doing so I got tons of warnings about egrep usage, so I folded this:
>
I should have better explained the rationale with the env var;
the idea is that the user can specify vmlinux=/path2/vmlinux and that
would be useful for a "make test" target - the vmlinux to be used could
be specified across multiple tests; we'd just need to add the same
envvar override to the other tests that need vmlinux.
> diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
> index 45094077809c1e3e..4a60a18845115d3d 100755
> --- a/tests/btf_functions.sh
> +++ b/tests/btf_functions.sh
> @@ -69,7 +69,7 @@ const_insensitive=0
> for f in $funcs ; do
> btf=$(grep " $f(" $outdir/btf.funcs)
> # look for non-inline match first
> - dwarf=$(grep " $f(" $outdir/dwarf.funcs |egrep -v "^inline ")
> + dwarf=$(grep " $f(" $outdir/dwarf.funcs | grep -Ev "^inline ")
> if [[ "$btf" != "$dwarf" ]]; then
> # function might be declared inline in DWARF.
> inline_dwarf=$(grep " $f(" $outdir/dwarf.funcs |grep "^inline ")
>
thanks; I incorporated this change (along with some other changes to
minimize the amount of greps/pipes where possible) into the v3 series.
> Running with it now:
>
> root@x1:/home/acme/git/pahole# tests/tests
> 1: Validation of BTF encoding of functions; this may take some time...
>
> Seems stuck, so:
>
This part takes a while - for me around 3.5 minutes to walk through all
functions in BTF and compare signatures to DWARF.
> root@x1:/home/acme/git/pahole# pahole --running_kernel_vmlinux
> /usr/lib/debug/lib/modules/6.10.8-200.fc40.x86_64/vmlinux
> root@x1:/home/acme/git/pahole#
>
> Had to stop (traveling/LPC participation), so sending it now, what is
> written above should already be useful :)
>
> - Arnaldo
>
>> Validation of BTF encoding of functions; this may take some time...
>> Matched 55969 functions exactly.
>> Matched 239 functions with inlines.
>> Matched 1 functions with multiple const/non-const instances.
>> Ok
>> Validation of skipped function logic...
>> Validating skipped functions are absent from BTF...
>> Skipped encoding 748 functions in BTF.
>> Ok
>> Validating skipped functions have incompatible return values...
>> Found 8 functions with multiple incompatible return values.
>> Ok
>> Validating skipped functions have incompatible params/counts...
>> Found 116 instances with multiple instances with incompatible parameters.
>> Found 2 instances where inline functions were not inlined and had incompatible parameters.
>> Found 351 instances where the function name suggests optimizations led to inconsistent parameters.
>> Ok
>>
>> Suggested-by: Jiri Olsa <olsajiri@xxxxxxxxx>
>> Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
>> ---
>> tests/btf_functions.sh | 192 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 192 insertions(+)
>> create mode 100755 tests/btf_functions.sh
>>
>> diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
>> new file mode 100755
>> index 0000000..4509407
>> --- /dev/null
>> +++ b/tests/btf_functions.sh
>> @@ -0,0 +1,192 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# Copyright (c) 2024, Oracle and/or its affiliates.
>> +#
>> +# Examine functions - especially those for which we skipped BTF encoding -
>> +# to validate that they were indeed skipped for BTF encoding, and that they
>> +# also should have been.
>> +#
>> +
>> +outdir=
>> +
>> +fail()
>> +{
>> + # Do not remove test dir; might be useful for analysis
>> + trap - EXIT
>> + if [[ -d "$outdir" ]]; then
>> + echo "Test data is in $outdir"
>> + fi
>> + exit 1
>> +}
>> +
>> +cleanup()
>> +{
>> + rm ${outdir}/*
>> + rmdir $outdir
>> +}
>> +
>> +vmlinux=${vmlinux:-$1}
>> +
>> +if [ -z "$vmlinux" ] ; then
>> + vmlinux=$(pahole --running_kernel_vmlinux)
>> + if [ -z "$vmlinux" ] ; then
>> + echo "Please specify a vmlinux file to operate on"
>> + exit 1
>> + fi
>> +fi
>> +
>> +if [ ! -f "$vmlinux" ] ; then
>> + echo "$vmlinux file not available, please specify another"
>> + exit 1
>> +fi
>> +
>> +outdir=$(mktemp -d /tmp/btf_functions.sh.XXXXXX)
>> +
>> +trap cleanup EXIT
>> +
>> +test -n "$VERBOSE" && printf "Encoding..."
>> +
>> +pahole --btf_features=default --btf_encode_detached=$outdir/vmlinux.btf --verbose $vmlinux |grep "skipping BTF encoding of function" > ${outdir}/skipped_fns
>> +
>> +test -n "$VERBOSE" && printf "done.\n"
>> +
>> +echo "Validation of BTF encoding of functions; this may take some time..."
>> +
>> +funcs=$(pfunct --format_path=btf $outdir/vmlinux.btf |sort)
>> +
>> +# all functions from DWARF; some inline functions are not inlined so include them too
>> +pfunct --all --no_parm_names --format_path=dwarf $vmlinux | \
>> + awk '{ print $0}'|sort|uniq > $outdir/dwarf.funcs
>> +# all functions from BTF (removing bpf_kfunc prefix where found)
>> +pfunct --all --no_parm_names --format_path=btf $outdir/vmlinux.btf |\
>> + awk '{ gsub("^bpf_kfunc ",""); print $0}'|sort|uniq > $outdir/btf.funcs
>> +
>> +exact=0
>> +inline=0
>> +const_insensitive=0
>> +
>> +for f in $funcs ; do
>> + btf=$(grep " $f(" $outdir/btf.funcs)
>> + # look for non-inline match first
>> + dwarf=$(grep " $f(" $outdir/dwarf.funcs |egrep -v "^inline ")
>> + if [[ "$btf" != "$dwarf" ]]; then
>> + # function might be declared inline in DWARF.
>> + inline_dwarf=$(grep " $f(" $outdir/dwarf.funcs |grep "^inline ")
>> + if [[ "inline $btf" != "$inline_dwarf" ]]; then
>> + # some functions have multiple instances in DWARF where one has
>> + # const param(s) and another does not (see errpos()). We do not
>> + # mark these functions inconsistent as though they technically
>> + # have different prototypes, the data itself is not different.
>> + btf_noconst=$(echo $btf | awk '{gsub("const ",""); print $0 }')
>> + dwarf_noconst=$(echo $dwarf | awk '{gsub("const ",""); print $0 }')
>> + if [[ "$dwarf_noconst" =~ "$btf_noconst" ]]; then
>> + const_insensitive=$((const_insensitive+1))
>> + else
>> + echo "ERROR: mismatch for '$f()' : BTF '$btf' not found; DWARF '$dwarf'"
>> + fail
>> + fi
>> + else
>> + inline=$((inline+1))
>> + fi
>> + else
>> + exact=$((exact+1))
>> + fi
>> +done
>> +
>> +echo "Matched $exact functions exactly."
>> +echo "Matched $inline functions with inlines."
>> +echo "Matched $const_insensitive functions with multiple const/non-const instances."
>> +echo "Ok"
>> +
>> +echo "Validation of skipped function logic..."
>> +
>> +skipped_cnt=$(wc -l ${outdir}/skipped_fns | awk '{ print $1}')
>> +
>> +if [[ "$skipped_cnt" == "0" ]]; then
>> + echo "No skipped functions. Done."
>> + exit 0
>> +fi
>> +
>> +echo "Validating skipped functions are absent from BTF..."
>> +
>> +skipped_fns=$(awk '{print $1}' $outdir/skipped_fns)
>> +for s in $skipped_fns ; do
>> + # Ensure the skipped function are not in BTF
>> + inbtf=$(grep " $s(" $outdir/btf.funcs)
>> + if [[ -n "$inbtf" ]]; then
>> + echo "ERROR: '${s}()' was added incorrectly to BTF: '$inbtf'"
>> + fail
>> + fi
>> +done
>> +
>> +echo "Skipped encoding $skipped_cnt functions in BTF."
>> +echo "Ok"
>> +
>> +echo "Validating skipped functions have incompatible return values..."
>> +
>> +return_mismatches=$(awk '/return type mismatch/ { print $1 }' $outdir/skipped_fns)
>> +return_count=0
>> +
>> +for r in $return_mismatches ; do
>> + # Ensure there are multiple instances with incompatible return values
>> + grep " $r(" $outdir/dwarf.funcs | \
>> + awk -v FN=$r '{i = index($0, FN); if (i>0) print substr($0, 0, i-1) }' \
>> + | uniq > ${outdir}/retvals.$r
>> + test -n "$VERBOSE" && echo "'${r}()' has return values:"
>> + test -n "$VERBOSE" && cat ${outdir}/retvals.$r
>> + cnt=$(wc -l ${outdir}/retvals.$r | awk '{ print $1 }')
>> + if [[ $cnt -lt 2 ]]; then
>> + echo "ERROR: '${r}()' has only one return value; it should not be reported as having incompatible return values"
>> + fail
>> + fi
>> + return_count=$((return_count+1))
>> +done
>> +
>> +echo "Found $return_count functions with multiple incompatible return values."
>> +echo "Ok"
>> +
>> +echo "Validating skipped functions have incompatible params/counts..."
>> +
>> +param_mismatches=$(awk '/due to param / { print $1 }' $outdir/skipped_fns)
>> +
>> +multiple=0
>> +multiple_inline=0
>> +optimized=0
>> +warnings=0
>> +
>> +for p in $param_mismatches ; do
>> + skipmsg=$(awk -v FN=$p '{ if ($1 == FN) print $0 }' $outdir/skipped_fns)
>> + altname=$(echo $skipmsg | awk '{ i=index($2,")"); print substr($2,2,i-2); }')
>> + if [[ "$altname" != "$p" ]]; then
>> + test -n "$VERBOSE" && echo "skipping optimized function $p ($altname); pfunct may not reflect late optimizations."
>> + optimized=$((optimized+1))
>> + continue
>> + fi
>> + # Ensure there are multiple instances with incompatible params
>> + grep " $p(" $outdir/dwarf.funcs | uniq > ${outdir}/protos.$p
>> + test -n "$VERBOSE" && echo "'${p}()' has prototypes:"
>> + test -n "$VERBOSE" && cat ${outdir}/protos.$p
>> + cnt=$(wc -l ${outdir}/protos.$p | awk '{ print $1 }')
>> + if [[ $cnt -lt 2 ]]; then
>> + # function may be inlined in multiple sites with different protos
>> + inlined=$(grep inline ${outdir}/protos.$p)
>> + if [[ -n "$inlined" ]]; then
>> + multiple_inline=$((multiple_inline+1))
>> + else
>> + echo "WARN: '${p}()' has only one prototype; if it was subject to late optimization, pfunct may not reflect inconsistencies pahole found."
>> + echo "Full skip message from pahole: $skipmsg"
>> + warnings=$((warnings+1))
>> + fi
>> + else
>> + multiple=$((multiple+1))
>> + fi
>> +done
>> +
>> +echo "Found $multiple instances with multiple instances with incompatible parameters."
>> +echo "Found $multiple_inline instances where inline functions were not inlined and had incompatible parameters."
>> +echo "Found $optimized instances where the function name suggests optimizations led to inconsistent parameters."
>> +echo "Found $warnings instances where pfunct did not notice inconsistencies."
>> +echo "Ok"
>> +
>> +exit 0
>> --
>> 2.43.5
