Re: [PATCH] XEN: enable MC/DC coverage for Clang

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/11/2025 12:31 pm, Saman Dehghan wrote:
> On Mon, Nov 24, 2025 at 5:15 AM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 
> wrote:
>> On 24/11/2025 2:18 am, Saman Dehghan wrote:
>>> @@ -127,16 +131,25 @@ extern uint64_t __stop___llvm_prf_cnts[];
>>>  #define END_NAMES       ((const void *)__stop___llvm_prf_names)
>>>  #define START_COUNTERS  ((void *)__start___llvm_prf_cnts)
>>>  #define END_COUNTERS    ((void *)__stop___llvm_prf_cnts)
>>> +#define START_BITMAP    ((void *)__start___llvm_prf_bits)
>>> +#define END_BITMAP      ((void *)__stop___llvm_prf_bits)
>>>
>>>  static void cf_check reset_counters(void)
>>>  {
>>>      memset(START_COUNTERS, 0, END_COUNTERS - START_COUNTERS);
>>> +#ifdef CONFIG_CONDITION_COVERAGE
>>> +    memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
>>> +#endif
>> ... this:
>>
>>     if ( IS_ENABLED(CONFIG_CONDITION_COVERAGE) )
>>         memset(START_BITMAP, 0, END_BITMAP - START_BITMAP);
>>
>>>  }
> Thanks Andrew.
>
> IS_ENABLED(CONFIG_CONDITION_COVERAGE) is not the same as #ifdef
> CONFIG_CONDITION_COVERAGE.
> When the option is completely undefined, IS_ENABLED() returns 1 (enabled).
> So even with no CONFIG_CONDITION_COVERAGE defined, the code takes the
> "enabled" path, which is not what we want here.

What makes you think this?  (No - that's not how IS_ENABLED() works.)

IS_ENABLED() exists for the purpose given here, to turn preprocessor
conditionals into something visible to the compiler.  Notably it allows
for syntax checking even in the disabled code, which is why it's
preferred wherever possible.

~Andrew