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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.12.2025 15:21, Grygorii Strashko wrote:
> 
> 
> On 09.12.25 15:19, Andrew Cooper wrote:
>> On 08/12/2025 6:49 pm, Grygorii Strashko wrote:
>>> Hi Andrew,
>>>
>>> On 06.12.25 16:21, Andrew Cooper wrote:
>>>> On 06/12/2025 2:15 pm, 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.
>>>>>>
>>>>>> 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.
>>>>
>>>> we can't target.
>>>
>>> I've come up with below diff - seems it's working without
>>> DO_NOT_FREE_INIT_MEMORY.
>>> Is this what you have in mind?
>>>
>>> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
>>> index 8fc201d12c2c..16b1a82db46e 100644
>>> --- a/xen/Kconfig.debug
>>> +++ b/xen/Kconfig.debug
>>> @@ -40,7 +40,6 @@ config COVERAGE
>>>          depends on SYSCTL && !LIVEPATCH
>>>          select SUPPRESS_DUPLICATE_SYMBOL_WARNINGS if
>>> !ENFORCE_UNIQUE_SYMBOLS
>>>          select RELAX_INIT_CHECK
>>> -       select DO_NOT_FREE_INIT_MEMORY
>>>          help
>>>            Enable code coverage support.
>>>   
>>> diff --git a/xen/Rules.mk b/xen/Rules.mk
>>> index 8c4861a427e6..47fdcc1d23b5 100644
>>> --- a/xen/Rules.mk
>>> +++ b/xen/Rules.mk
>>> @@ -33,11 +33,15 @@ cov-cflags-y :=
>>>   nocov-y :=
>>>   noubsan-y :=
>>>   
>>> +# when coverage is enabled the gcc internal section should stay in
>>> memory
>>> +# after Xen boot
>>> +ifneq ($(CONFIG_COVERAGE),y)
>>>   SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \
>>>                                               $(foreach w,1 2 4, \
>>>                                                          
>>> rodata.str$(w).$(a)) \
>>>                                               rodata.cst$(a)) \
>>>                            $(foreach r,rel rel.ro,data.$(r).local)
>>> +endif
>>>   
>>>   # The filename build.mk has precedence over Makefile
>>>   include $(firstword $(wildcard $(srcdir)/build.mk) $(srcdir)/Makefile)
>>> diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
>>> index 60b3ae40728f..8180c78f1510 100644
>>> --- a/xen/common/libelf/Makefile
>>> +++ b/xen/common/libelf/Makefile
>>> @@ -1,8 +1,10 @@
>>>   obj-bin-y := libelf.o
>>>   libelf-objs := libelf-tools.o libelf-loader.o libelf-dominfo.o
>>>   
>>> +ifneq ($(CONFIG_COVERAGE),y)
>>>   SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
>>>   OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section
>>> .$(s)=.init.$(s))
>>> +endif
>>>   
>>>   CFLAGS-y += -Wno-pointer-sign
>>>   
>>> diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile
>>> index ae0f69c01373..fb26e5bff0fd 100644
>>> --- a/xen/common/libfdt/Makefile
>>> +++ b/xen/common/libfdt/Makefile
>>> @@ -4,7 +4,9 @@ SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
>>>   
>>>   # For CONFIG_OVERLAY_DTB, libfdt functionalities will be needed
>>> during runtime.
>>>   ifneq ($(CONFIG_OVERLAY_DTB),y)
>>> -OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section
>>> .$(s)=.init.$(s))
>>> +       ifneq ($(CONFIG_COVERAGE),y)
>>> +               OBJCOPYFLAGS := $(foreach
>>> s,$(SECTIONS),--rename-section .$(s)=.init.$(s))
>>> +       endif
>>>   endif
>>
>> This is the (aforementioned) non-standard way of doing .init.o, which is
>> why it doesn't play nicely.
>>
>> I suggest that we first convert libelf and libfdt to the standard way of
>> doing .init.
> 
> I assume the rest is ok.
> 
>>
>> For libelf this means we need regular __init annotations, but #undef'd
>> outside of __XEN__ (when we're doing the userspace build).
>>
> 
> Need clarification here - this are imported libraries and changing their code
> directly was not welcome before. Therefore there is Xen specific magic in 
> Makefiles.
> :(

I can't and won't speak for libfdt, but for libelf I think we should really
consider this ours (not imported) the latest as of the re-work for XSA-55.

Jan

> Just an idea1, may be ".init" handling can be just dropped from libelf and 
> libfdt
> Makefiles with comment added instead (kinda "TODO") - they will be built-in.
> It doesn't work with CONFIG_CC_SPLIT_SECTIONS any way now.
> 
> Just an idea2, drop libelf and libfdt changes from this patch.
> - they will be not in coverage report (nocov-y += *.obj)
> - will be resolved in the future.
> 
> Trying to avoid blocking on external dependencies :(
> 
>> For libfdt, this will need some init_or_$FOO things (matching
>> init_or_livepatch).
>>
>> Once the custom init has been made standard, this code becomes easier to
>> move into lib, and we no longer have special cases when trying to extend
>> coverage.
> 
>