Re: [PATCH v3 3/3] xen: debug: gcov: add condition coverage support

[Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>]

Hi Anthony,

"Anthony PERARD" <anthony.perard@vates.tech> writes:

> On Sat, Apr 05, 2025 at 03:30:49AM +0000, Volodymyr Babchuk wrote:
>> --- a/xen/Rules.mk
>> +++ b/xen/Rules.mk
>> @@ -133,18 +133,19 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += -DINIT_SECTIONS
>>  
>>  non-init-objects = $(filter-out %.init.o, $(obj-y) $(obj-bin-y) $(extra-y))
>>  
>> -ifeq ($(CONFIG_COVERAGE),y)
>
> This removes an "ifeq ()", so you probably need to remove and "endif"
> somewhere else, which doesn't appear in this snippet.

Yes, I'm sorry, it just didn't got into the snippet. I wanted to discuss
approach only, so this is not the final version.

>>  ifeq ($(CONFIG_CC_IS_CLANG),y)
>> -    COV_FLAGS := -fprofile-instr-generate -fcoverage-mapping
>> +    cov-flags-$(CONFIG_COVERAGE) := -fprofile-instr-generate -fcoverage-mapping
>
> If you do this assignment like that, it would be better to make sure
> $(cov-flags-y) is initialised properly, that is have a:
>
>   cov-flags-y :=
>
> before the first conditional assignment, then have all conditional
> assignment be +=.

Sure.

>
>>  else
>> -    COV_FLAGS := -fprofile-arcs -ftest-coverage
>> +    cov-flags-$(CONFIG_COVERAGE) := -fprofile-arcs -ftest-coverage
>> +    cov-flags-$(CONFIG_CONDITION_COVERAGE) += -fcondition-coverage
>
> What happen if CONFIG_CONDITION_COVERAGE=y but CONFIG_COVERAGE=n ?

Kconfig ensures that this is impossible:

config CONDITION_COVERAGE
	bool "Condition coverage support"
	depends on COVERAGE && CC_HAS_MCDC

I believe, this is enough, and we don't need a separate check on
Makefile level.

-- 
WBR, Volodymyr