Re: [XEN PATCH v7 39/51] build: rework coverage and ubsan CFLAGS handling

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.08.2021 12:50, Anthony PERARD wrote:
> When assigning a value a target-specific variable, that also affect
> prerequisite of the target. This is mostly fine, but there is one case
> where we will not want the COV_FLAGS added to the CFLAGS.
> 
> In arch/x86/boot, we have "head.o" with "cmdline.S" as prerequisite
> and ultimately "cmdline.o", we don't want COV_FLAGS to that last one.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

I think I understand what's going on, so
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

It would seem to me though that ...

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -131,19 +131,31 @@ targets += $(targets-for-builtin)
>  
>  $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += 
> -DINIT_SECTIONS_ONLY
>  
> +non-init-objects = $(filter-out %.init.o, $(obj-y) $(obj-bin-y) $(extra-y))
> +
>  ifeq ($(CONFIG_COVERAGE),y)
>  ifeq ($(CONFIG_CC_IS_CLANG),y)
>      COV_FLAGS := -fprofile-instr-generate -fcoverage-mapping
>  else
>      COV_FLAGS := -fprofile-arcs -ftest-coverage
>  endif
> -$(filter-out %.init.o $(nocov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y 
> += $(COV_FLAGS)
> +
> +$(non-init-objects): _c_flags += $(COV_FLAGS)
> +
> +# Reset COV_FLAGS in cases where an objects as another one as prerequisite
> +$(nocov-y) $(filter %.init.o, $(obj-y) $(obj-bin-y) $(extra-y)): \
> +    COV_FLAGS :=

... pulling this and ...

>  endif
>  
>  ifeq ($(CONFIG_UBSAN),y)
>  # Any -fno-sanitize= options need to come after any -fsanitize= options
> -$(filter-out %.init.o $(noubsan-y),$(obj-y) $(obj-bin-y) $(extra-y)): \
> -CFLAGS-y += $(filter-out -fno-%,$(CFLAGS_UBSAN)) $(filter 
> -fno-%,$(CFLAGS_UBSAN))
> +UBSAN_FLAGS := $(filter-out -fno-%,$(CFLAGS_UBSAN)) $(filter 
> -fno-%,$(CFLAGS_UBSAN))
> +
> +$(non-init-objects): _c_flags += $(UBSAN_FLAGS)
> +
> +# Reset UBSAN_FLAGS in cases where an objects as another one as prerequisite
> +$(noubsan-y) $(filter %.init.o, $(obj-y) $(obj-bin-y) $(extra-y)): \
> +    UBSAN_FLAGS :=

... this up ahead of their respective _c_flags assignments would be
easier to follow, for being more logical (produce, then consume).

Also, as a nit: In the comments do you mean "... where an object has ..."?

Jan