Re: [XEN][PATCH v2] coverage: extend coverage on .init and lib code

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.12.2025 15:15, Andrew Cooper wrote:
> On 06/12/2025 9:10 am, Grygorii Strashko wrote:
>>
>>
>> On 05.12.25 22:00, Andrew Cooper wrote:
>>> On 05/12/2025 7:34 pm, Grygorii Strashko wrote:
>>>> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>>>>
>>>> Extend coverage support on .init and lib code.
>>>> Add two hidden Kconfig options:
>>>> - RELAX_INIT_CHECK "Relax strict check for .init sections only in
>>>> %.init.o
>>>> files"
>>>> - DO_NOT_FREE_INIT_MEMORY "Prevent freeing of .init sections at the
>>>> end of
>>>> Xen boot."
>>>>
>>>> Both selected selected when COVERAGE=y, as getting coverage report for
>>>> ".init" code is required:
>>>> - to bypass strict check for .init sections only in %.init.o files;
>>>> - the .init code stay in memory after Xen boot.
>>>>
>>>> RELAX_INIT_CHECK/DO_NOT_FREE_INIT_MEMORY could be used by other debug
>>>> features in the future.
>>>>
>>>> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>>>> ---
>>>> changes in v2:
>>>>   - add RELAX_INIT_CHECK and DO_NOT_FREE_INIT_MEMORY, those are two
>>>> different things,
>>>>     both potentially reusable
>>>>   - enable coverage for libfdt/libelf always
>>>>   - enable colverage for .init always
>>>
>>> This is a lot nicer (i.e. more simple).
>>>
>>> But, I still don't know why we need to avoid freeing init memory to make
>>> this work.  What explodes if we dont?
>>>
>>
>> It will just crash when coverage data is collected.
>>
>> First I made changes in make file to get .init covered
>> then I hit a crash
>> then I checked %.init.o
>> conclusion was obvious.

Maybe in that context. In the patch submission this isn't obvious at all, I
would say. Please add some explanation in such a case.

>> For example:
>> objdump -x bzimage.init.o | grep gcov
>>
>> 0000000000000010 l     O .bss    0000000000000028 __gcov0.bzimage_check
>> 0000000000000040 l     O .bss    0000000000000040
>> __gcov0.bzimage_headroom
>> 0000000000000000 l     O .bss    0000000000000008 __gcov0.output_length
>> 0000000000000080 l     O .bss    0000000000000060 __gcov0.bzimage_parse
>> 0000000000000098 l     O .init.data.rel.local    0000000000000028
>> __gcov_.bzimage_parse
>> 0000000000000070 l     O .init.data.rel.local    0000000000000028
>> __gcov_.bzimage_headroom
>> 0000000000000048 l     O .init.data.rel.local    0000000000000028
>> __gcov_.bzimage_check
>> 0000000000000020 l     O .init.data.rel.local    0000000000000028
>> __gcov_.output_length
>> 0000000000000000         *UND*    0000000000000000 __gcov_init
>> 0000000000000000         *UND*    0000000000000000 __gcov_exit
>> 0000000000000000         *UND*    0000000000000000 __gcov_merge_add
>> 0000000000000008 R_X86_64_PLT32    __gcov_init-0x0000000000000004
>> 0000000000000012 R_X86_64_PLT32    __gcov_exit-0x0000000000000004
>> 0000000000000020 R_X86_64_64       __gcov_merge_add
> 
> Aah, we should exclude the OJBCOPY too.  That's what's moving
> .data.rel.local amongst other sections we target with attributes directly.

Yet then do we still need to suppress the freeing of .init.*?

Jan