On 01/10/2025 11:09 pm, Saman Dehghan wrote: > This change enables compatibility for measuring code coverage > with Clang versions 14 through 20 by supporting their > respective raw profile formats. > > 1- Add support for LLVM raw profile versions 8, 9, and 10 > 2- Initialized llvm_profile_header for all versions based on llvm source code > in > `compiler-rt/include/profile/InstrProfData.inc` for each version. > 3- We tested this patch for all clang versions from 14 through 20 on both ARM > and X86 platform > > Signed-off-by: Saman Dehghan <[email protected]>
CC-ing Oleksii. This should be considered for 4.21 at this point. Coverage is an optional feature, off-by-default, but Xen does support GCC and Clang (older Clang at least), and right now newer Clang simply malfunctions. I guess I should update https://xenbits.xen.org/docs/latest/hypervisor-guide/code-coverage.html given the new toolchain baselines. > --- > xen/common/coverage/llvm.c | 78 +++++++++++++++++++++++++++----------- > xen/include/xen/types.h | 1 + > 2 files changed, 57 insertions(+), 22 deletions(-) > > diff --git a/xen/common/coverage/llvm.c b/xen/common/coverage/llvm.c > index 517b2aa8c2..f92f10654c 100644 > --- a/xen/common/coverage/llvm.c > +++ b/xen/common/coverage/llvm.c > @@ -44,27 +44,55 @@ > ((uint64_t)'f' << 16) | ((uint64_t)'R' << 8) | ((uint64_t)129) > #endif > > -#define LLVM_PROFILE_VERSION 4 > +#if __clang_major__ >= 19 > +#define LLVM_PROFILE_VERSION 10 > +#define LLVM_PROFILE_NUM_KINDS 3 > +#elif __clang_major__ == 18 > +#define LLVM_PROFILE_VERSION 9 > #define LLVM_PROFILE_NUM_KINDS 2 > +#elif __clang_major__ >= 14 > +#define LLVM_PROFILE_VERSION 8 > +#define LLVM_PROFILE_NUM_KINDS 2 > +#else > +#error "Unsupported Clang version" > +#endif Does this exclude Clang 13? Our baseline is 11 or later. How hard would it be to support those too? > > struct llvm_profile_data { > uint64_t name_ref; > uint64_t function_hash; > - void *counter; > - void *function; > - void *values; > + intptr_t *relative_counter; > +#if __clang_major__ >= 18 > + intptr_t *relative_bitmap; > +#endif > + intptr_t *function; > + intptr_t *values; > uint32_t nr_counters; > uint16_t nr_value_sites[LLVM_PROFILE_NUM_KINDS]; > +#if __clang_major__ >= 18 > + uint32_t numbitmap_bytes; > +#endif > }; > > struct llvm_profile_header { > uint64_t magic; > uint64_t version; > - uint64_t data_size; > - uint64_t counters_size; > + uint64_t binary_ids_size; > + uint64_t num_data; > + uint64_t padding_bytes_before_counters; > + uint64_t num_counters; > + uint64_t padding_bytes_after_counters; > + uint64_t num_bitmap_bytes; > + uint64_t padding_bytes_after_bitmap_bytes; > uint64_t names_size; > +#if __clang_major__ >= 18 > uint64_t counters_delta; > + uint64_t bitmap_delta; > +#endif > uint64_t names_delta; > +#if __clang_major__ >= 19 > + uint64_t num_vtables; > + uint64_t vnames_size; > +#endif > uint64_t value_kind_last; > }; > > @@ -76,19 +104,20 @@ struct llvm_profile_header { > */ > int __llvm_profile_runtime; > > -extern const struct llvm_profile_data __start___llvm_prf_data[]; > -extern const struct llvm_profile_data __stop___llvm_prf_data[]; > -extern const char __start___llvm_prf_names[]; > -extern const char __stop___llvm_prf_names[]; > -extern uint64_t __start___llvm_prf_cnts[]; > -extern uint64_t __stop___llvm_prf_cnts[]; > +extern char __start___llvm_prf_data[]; > +extern char __stop___llvm_prf_data[]; > +extern char __start___llvm_prf_names[]; > +extern char __stop___llvm_prf_names[]; > +extern char __start___llvm_prf_cnts[]; > +extern char __stop___llvm_prf_cnts[]; > + > +#define START_DATA ((const char *)__start___llvm_prf_data) > +#define END_DATA ((const char *)__stop___llvm_prf_data) > +#define START_NAMES ((const char *)__start___llvm_prf_names) > +#define END_NAMES ((const char *)__stop___llvm_prf_names) > +#define START_COUNTERS ((char *)__start___llvm_prf_cnts) > +#define END_COUNTERS ((char *)__stop___llvm_prf_cnts) > > -#define START_DATA ((const void *)__start___llvm_prf_data) > -#define END_DATA ((const void *)__stop___llvm_prf_data) > -#define START_NAMES ((const void *)__start___llvm_prf_names) > -#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) Why change these from char to void ? > > static void cf_check reset_counters(void) > { > @@ -107,10 +136,15 @@ static int cf_check dump( > struct llvm_profile_header header = { > .magic = LLVM_PROFILE_MAGIC, > .version = LLVM_PROFILE_VERSION, > - .data_size = (END_DATA - START_DATA) / sizeof(struct > llvm_profile_data), > - .counters_size = (END_COUNTERS - START_COUNTERS) / sizeof(uint64_t), > - .names_size = END_NAMES - START_NAMES, > - .counters_delta = (uintptr_t)START_COUNTERS, > + .binary_ids_size = 0, > + .num_data = (((intptr_t)END_DATA + sizeof(struct llvm_profile_data) > - 1) > + - (intptr_t)START_DATA) / sizeof(struct llvm_profile_data), I can see why we might want to round-down END, but this doesn't need casting to intptr_t irrespective of char vs void. > + .padding_bytes_before_counters = 0, > + .num_counters = (((intptr_t)END_COUNTERS + sizeof(uint64_t) - 1) > + - (intptr_t)START_COUNTERS) / sizeof(uint64_t), > + .padding_bytes_after_counters = 0, > + .names_size = (END_NAMES - START_NAMES) * sizeof(char), sizeof(char) is by definition 1. > + .counters_delta = (uintptr_t)START_COUNTERS - (uintptr_t)START_DATA, > .names_delta = (uintptr_t)START_NAMES, > .value_kind_last = LLVM_PROFILE_NUM_KINDS - 1, > }; > diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h > index 73ddccbbd5..799bfe0b95 100644 > --- a/xen/include/xen/types.h > +++ b/xen/include/xen/types.h > @@ -18,6 +18,7 @@ typedef signed long ssize_t; > > typedef __PTRDIFF_TYPE__ ptrdiff_t; > typedef __UINTPTR_TYPE__ uintptr_t; > +typedef __INTPTR_TYPE__ intptr_t; > > /* > * Users of this macro are expected to pass a positive value.
