Skip to content

Support clang coverage profile 10, 9, 8 #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Support clang coverage profile 10, 9, 8 #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ac8ffd8
Add support for llvm coverage.
enum-class Sep 18, 2025
9b219ad
add clang-20, add required initializations. add mcdc flags to makefile
enum-class Sep 20, 2025
9073788
condition check on clang version for mcdc support
enum-class Sep 20, 2025
935b856
get_header
enum-class Sep 29, 2025
85fcc16
make minimal changes/ remove mcdc
enum-class Oct 1, 2025
b55862f
Address concerns.
enum-class Oct 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 56 additions & 22 deletions xen/common/coverage/llvm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to update the clang version conversation in the docs? https://github.com/enum-class/xen/blob/saman/docs/hypervisor-guide/code-coverage.rst#compiling-xen

#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"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a subsection talking about the profile and challenges with keeping it matching newer tool revisions? Maybe links to LLVM/clang profile definition to understand what to update in Xen and then a link in the docs to this file?

https://github.com/enum-class/xen/blob/saman/docs/hypervisor-guide/code-coverage.rst#clang-coverage

Copy link
Collaborator

@whentojump whentojump Oct 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely. Here are some links to consider:

We will also put up some general guideline text on how to update it in the future.

#endif

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;
};

Expand All @@ -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)

static void cf_check reset_counters(void)
{
Expand All @@ -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),
.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),
.counters_delta = (uintptr_t)START_COUNTERS - (uintptr_t)START_DATA,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also sprinkle ifdef's to this initialization list

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The num calculation is a bit defensive to me. Do you have evidence

        .num_data = (END_DATA - START_DATA) / sizeof(struct llvm_profile_data),
        .num_counters = (END_COUNTERS - START_COUNTERS) / sizeof(uint64_t),

won't work?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also sprinkle ifdef's to this initialization

could you please explain more

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't work?

The linker script does not guarantee the data section boundaries align to sizeof(struct llvm_profile_data)

.names_delta = (uintptr_t)START_NAMES,
.value_kind_last = LLVM_PROFILE_NUM_KINDS - 1,
};
Expand Down
1 change: 1 addition & 0 deletions xen/include/xen/types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ typedef signed long ssize_t;

typedef __PTRDIFF_TYPE__ ptrdiff_t;
typedef __UINTPTR_TYPE__ uintptr_t;
typedef __INTPTR_TYPE__ intptr_t;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this signed type is needed?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signed arithmetic avoids undefined or unexpected behavior
If uintptr_t were used, the subtraction could result in an unsigned underflow

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give an example how the underflow could happen?

My concern is this touches some file that many other components depend and can block our patch specifically for coverage. But if you can justify it well, go for it.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also we need that for llvm_profile_data
https://github.com/llvm/llvm-project/blob/e21dc4bd5474d04b8e62d7331362edcc5648d7e5/compiler-rt/include/profile/InstrProfData.inc#L80

/*
* Users of this macro are expected to pass a positive value.
Expand Down