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 <[email protected]>
>>
>> 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
