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 <[email protected]>

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 <[email protected]>
---
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


Reply via email to