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

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

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.
:(

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.

--
Best regards,
-grygorii