Re: [PATCH v3] Support LLVM raw profile versions 5, 6, 7, 8, 9, and 10

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 25/10/2025 12:38 am, Saman Dehghan wrote:
>>> +        .padding_bytes_after_counters = 0,
>>> +#if __clang_major__ >= 18
>>> +        .num_bitmap_bytes = 0,
>>> +        .padding_bytes_after_bitmap_bytes = 0,
>>> +#endif
>>>          .names_size = END_NAMES - START_NAMES,
>>> +#if __clang_major__ >= 14
>>> +        .counters_delta = START_COUNTERS - START_DATA,
>>> +#else
>>>          .counters_delta = (uintptr_t)START_COUNTERS,
>>> +#endif
>>> +
>>> +#if __clang_major__ >= 18
>>> +        .bitmap_delta = 0,
>>> +#endif
>>>          .names_delta = (uintptr_t)START_NAMES,
>>> +#if __clang_major__ >= 19 && __clang_major__ <= 20
>>> +        .num_vtables = 0,
>>> +        .vnames_size = 0,
>>> +#endif
>> Because this is a structure initialiser, everything set explicitly to 0
>> can be omitted.  This removes all #ifdef-ary except the .counters_delta
>> I believe, as well as the .padding_byte_* fields.
>>
> Is it undefined behaviour to leave struct members uninitialized for
> local variables?

They are well defined as 0 when using a structure initialiser.

>
>> The resulting diff is far smaller.
>>
>>>          .value_kind_last = LLVM_PROFILE_NUM_KINDS - 1,
>>>      };
>>>      unsigned int off = 0;
>>> diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
>>> index b126dfe887..42550a85a2 100644
>>> --- a/xen/include/xen/xen.lds.h
>>> +++ b/xen/include/xen/xen.lds.h
>>> @@ -81,6 +81,24 @@
>>>    .stab.index 0 : { *(.stab.index) }         \
>>>    .stab.indexstr 0 : { *(.stab.indexstr) }
>>>
>>> +#if defined(CONFIG_COVERAGE) && defined(CONFIG_CC_IS_CLANG)
>>> +
>>> +#define LLVM_COV_RW_DATA                                   \
>>> +    DECL_SECTION(__llvm_prf_cnts) { *(__llvm_prf_cnts) }   \
>>> +    DECL_SECTION(__llvm_prf_data) { *(__llvm_prf_data) }   \
>>> +    DECL_SECTION(__llvm_prf_bits) { *(__llvm_prf_bits) }
>>> +
>>> +#define LLVM_COV_RO_DATA                                   \
>>> +    DECL_SECTION(__llvm_prf_names) { *(__llvm_prf_names) }
>>> +
>>> +#define LLVM_COV_DEBUG                                     \
>>> +    DECL_DEBUG(__llvm_covfun, 8)                           \
>>> +    DECL_DEBUG(__llvm_covmap, 8)
>>> +#else
>>> +#define LLVM_COV_RW_DATA
>>> +#define LLVM_COV_RO_DATA
>>> +#define LLVM_COV_DEBUG
>>> +#endif
>> Newline here.
>>
>> But, there's no problem stating sections which are unused.  I think the
>> outer #if/#else can be dropped.
>>
>> Otherwise, Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>
>> I can fix these all up on commit, seeing as it's release acked for 4.21
> Thank you for offering to fix them up. Let me know how I can help or
> if I need to send another version.

This is the version with the fixups I suggested, run through CI:

https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/2118913271
https://gitlab.com/xen-project/hardware/xen-staging/-/commit/c06dadb80a1e4058c7cdef4144a7e4c4799a38a7

However, one further thing I noticed.  On v2, Roger asked you to express
struct llvm_profile_{header,data} in terms of LLVM_PROFILE_VERSION,
rather than __clang_major__.

If you want to start from the version I cleaned up, and then submit a v4
addressing Roger's feedback, please feel free.  This will save needing
to fix it up later.

~Andrew