Re: [XEN][PATCH] coverage: add extended coverage capabilities

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 03/12/2025 10:24 pm, Grygorii Strashko wrote:
> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>
> Added CONFIG_COVERAGE_EXTENDED to enable coverage for .init and lib code.
> When enabled - Xen .init data will not be freed.
>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> ---
>  xen/Kconfig.debug          | 10 ++++++++++
>  xen/Rules.mk               | 13 +++++++++++++
>  xen/arch/arm/setup.c       |  2 ++
>  xen/arch/x86/setup.c       |  4 ++++
>  xen/common/libelf/Makefile |  4 +++-
>  xen/common/libfdt/Makefile |  4 +++-
>  6 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index d900d926c555..3e04e1a3992b 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -44,6 +44,16 @@ config COVERAGE
>  
>         If unsure, say N here.
>  
> +config COVERAGE_EXTENDED
> +    bool "Extended Code coverage support"
> +    depends on COVERAGE
> +    default y
> +    help
> +      Enable Extended code coverage support which include .init and libs 
> code.
> +      The .init sections are not freed in this case.
> +
> +      If unsure, say N here.

IMO this is unhelpful.  Noone wants to opt for the model we've got right
now.

Instead, I think we want:

config RELAX_INIT_CHECK
    bool

and have CONFIG_COVERAGE select it.  There's no need for any user
visible option here.


> +
>  config CONDITION_COVERAGE
>       bool "Condition coverage support"
>       depends on COVERAGE && CC_HAS_MCDC
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index 2b28d1ac3c18..6d66a2332fb0 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -260,6 +267,7 @@ $(obj)/%.o: $(src)/%.S FORCE
>  
>  
>  quiet_cmd_obj_init_o = INIT_O  $@
> +ifneq ($(CONFIG_COVERAGE_EXTENDED),y)
>  define cmd_obj_init_o
>      $(OBJDUMP) -h $< | while read idx name sz rest; do \
>          case "$$name" in \
> @@ -272,6 +280,11 @@ define cmd_obj_init_o
>      done || exit $$?; \
>      $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section 
> .$(s)=.init.$(s)) $< $@
>  endef
> +else
> +define cmd_obj_init_o
> +    $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section 
> .$(s)=.init.$(s)) $< $@
> +endef
> +endif

This wants to be something more like:

@@ -261,6 +268,7 @@ $(obj)/%.o: $(src)/%.S FORCE
 
 quiet_cmd_obj_init_o = INIT_O  $@
 define cmd_obj_init_o
+ifneq ($(CONFIG_RELAX_INIT_CHECK),y)
     $(OBJDUMP) -h $< | while read idx name sz rest; do \
         case "$$name" in \
         .*.local) ;; \
@@ -269,7 +277,8 @@ define cmd_obj_init_o
             echo "Error: size of $<:$$name is 0x$$sz" >&2; \
             exit $$(expr $$idx + 1);; \
         esac; \
-    done || exit $$?; \
+    done || exit $$?
+endif
     $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section 
.$(s)=.init.$(s)) $< $@
 endef

 
to avoid duplicating the OBJCOPY invocation.

> diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
> index 917d12b006f7..337a3a8e1b35 100644
> --- a/xen/common/libelf/Makefile
> +++ b/xen/common/libelf/Makefile
> @@ -1,5 +1,7 @@
>  obj-bin-y := libelf.o
> -nocov-y += libelf.o
> +ifneq ($(CONFIG_COVERAGE_EXTENDED),y)
> +     nocov-y += libelf.o
> +endif
>  libelf-objs := libelf-tools.o libelf-loader.o libelf-dominfo.o
>  
>  SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
> diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile
> index 6ce679f98f47..8c62563ccdbc 100644
> --- a/xen/common/libfdt/Makefile
> +++ b/xen/common/libfdt/Makefile
> @@ -5,7 +5,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))
> -nocov-y += libfdt.o
> +ifneq ($(CONFIG_COVERAGE_EXTENDED),y)
> +     nocov-y += libfdt.o
> +endif

both of these simply want to drop their nocov-ness now that coverage
does work for them.

~Andrew