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

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

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
 
 obj-y += libfdt.o
--
Best regards,
-grygorii