On 05/08/2025 20:06, Alexis Lothoré wrote:
> Hi Alan,
>
> On Tue Aug 5, 2025 at 5:09 PM CEST, Alan Maguire wrote:
>> On 07/07/2025 15:02, Alexis Lothoré (eBPF Foundation) wrote:
>>> Add a small binary representing specific cases likely absent from
>>> standard vmlinux or kernel modules files. As a starter, the introduced
>>> binary exposes a few functions consuming structs passed by value, some
>>> passed by register, some passed on the stack:
>>>
>>> int main(void);
>>> int test_bin_func_struct_on_stack_ko(int, void *, char, short int, int, \
>>> void *, char, short int, struct test_bin_struct_packed);
>>> int test_bin_func_struct_on_stack_ok(int, void *, char, short int, int, \
>>> void *, char, short int, struct test_bin_struct);
>>> int test_bin_func_struct_ok(int, void *, char, struct test_bin_struct);
>>> int test_bin_func_ok(int, void *, char, short int);
>>>
>>> Then enrich btf_functions.sh to make it perform the following steps:
>>> - build the binary
>>> - generate BTF info and pfunct listing, both with dwarf and the
>>> generated BTF
>>> - check that any function encoded in BTF is found in DWARF
>>> - check that any function announced as skipped is indeed absent from BTF
>>> - check that any skipped function has been skipped due to uncertain
>>> parameter location
>>>
>>> Example of the new test execution:
>>> Encoding...Matched 4 functions exactly.
>>> Ok
>>> Validation of skipped function logic...
>>> Skipped encoding 1 functions in BTF.
>>> Ok
>>> Validating skipped functions have uncertain parameter location...
>>> pahole: /home/alexis/src/pahole/tests/bin/test_bin: Invalid argument
>>> Found 1 legitimately skipped function due to uncertain loc
>>> Ok
>>>
>>> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@xxxxxxxxxxx>
>>
>> Thanks for the updated changes+test. I think it'd be good to have this
>> be less verbose in successful case. Currently I see:
>>
>> 1: Validation of BTF encoding of functions; this may take some time: Ok
>> Validation of BTF encoding corner cases with test_bin functions; this
>> may take some time: make: Entering directory
>> '/home/almagui/src/github/dwarves/tests/bin'
>> gcc test_bin.c -Wall -Wextra -Werror -g -o test_bin
>> make: Leaving directory '/home/almagui/src/github/dwarves/tests/bin'
>> No skipped functions. Done.
>>
>> Ideally we just want the "Ok" for success in non-vebose mode. I'd
>> propose making the following changes in order to support that; if these
>> are okay by you there's no need to send another revision.
>
> I'm perfeclty fine with the idea, thanks for handling it. Just a
> comment/question below
>
>> diff --git a/tests/btf_functions.sh b/tests/btf_functions.sh
>> index f97bdf5..a4ab67e 100755
>> --- a/tests/btf_functions.sh
>> +++ b/tests/btf_functions.sh
>> @@ -110,7 +110,6 @@ skipped_cnt=$(wc -l ${outdir}/skipped_fns | awk '{
>> print $1}')
>>
>> if [[ "$skipped_cnt" == "0" ]]; then
>> echo "No skipped functions. Done."
>> - exit 0
>> fi
>
> Shouldn't we get rid of this whole if block then, similarly to what you
> have done with the other one below ?
>
yep, good catch, removed that too. Series applied to next branch of
https://git.kernel.org/pub/scm/devel/pahole/pahole.git
Thanks!
Alan
> Thanks,
>
> Alexis
>
