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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.12.2025 16:15, Grygorii Strashko wrote:
> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> 
> Extend coverage support on .init and lib code.
> 
> To enable coverage support on .init code the gcc special
> section should stay in memory after Xen boot when COVERAGE=y.

What is "the gcc special section" here, and ...

> So, add hidden Kconfig option RELAX_INIT_CHECK "Relax strict check for
> .init sections only in %.init.o files", which is selected when COVERAGE=y
> and allows to bypass strict check for .init sections only in %.init.o
> files.

... why / how would this mechanism get in the way?

> --- 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 special section should stay in memory
> +# after Xen boot

Nit: Comment style (./CODING_STYLE applies to Makefile-s equally in this
regard).

> +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)
> @@ -146,10 +150,9 @@ endif
>  $(call 
> cc-option-add,cov-cflags-$(CONFIG_COVERAGE),CC,-fprofile-update=atomic)
>  
>  # Reset cov-cflags-y in cases where an objects has another one as 
> prerequisite
> -$(nocov-y) $(filter %.init.o, $(obj-y) $(obj-bin-y) $(extra-y)): \
> -    cov-cflags-y :=
> +$(nocov-y) $(extra-y): cov-cflags-y :=
>  
> -$(non-init-objects): _c_flags += $(cov-cflags-y)
> +$(obj-y) $(obj-bin-y) $(extra-y) $(lib-y): _c_flags += $(cov-cflags-y)
>  
>  ifeq ($(CONFIG_UBSAN),y)
>  # Any -fno-sanitize= options need to come after any -fsanitize= options
> @@ -259,8 +262,8 @@ $(obj)/%.o: $(src)/%.S FORCE
>       $(call if_changed_dep,cc_o_S)
>  
>  
> -quiet_cmd_obj_init_o = INIT_O  $@
> -define cmd_obj_init_o
> +quiet_cmd_obj_init_check = INIT_C  $@
> +define cmd_obj_init_check
>      $(OBJDUMP) -h $< | while read idx name sz rest; do \
>          case "$$name" in \
>          .*.local) ;; \
> @@ -269,12 +272,17 @@ define cmd_obj_init_o
>              echo "Error: size of $<:$$name is 0x$$sz" >&2; \
>              exit $$(expr $$idx + 1);; \
>          esac; \
> -    done || exit $$?; \
> +    done || exit $$?
> +endef

The " || exit $$?" isn't needed anymore when this is the last command?

> +quiet_cmd_obj_init_objcopy = INIT_O  $@
> +define cmd_obj_init_objcopy
>      $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section 
> .$(s)=.init.$(s)) $< $@

What use is this step when $(SPECIAL_DATA_SECTIONS) isn't set at all?

>  endef
>  
>  $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): $(obj)/%.init.o: 
> $(obj)/%.o FORCE
> -     $(call if_changed,obj_init_o)
> +     $(if $(filter y,$(CONFIG_RELAX_INIT_CHECK)),,$(call 
> if_changed,obj_init_check))
> +     $(call if_changed,obj_init_objcopy)

Feels pretty fragile to do the modifications (as per above - if there were any
in the first place) without first having checked that doing them is actually
(going to be) okay.

Furthermore - why RELAX_INIT_CHECK when you don't really "relax" it, but skip
it altogether?

Jan