Support clang coverage profile 10, 9, 8 #1

enum-class · 2025-09-22T00:22:38Z

Added clang coverage profile 10, 9, 8.
initialize llvm_profile_header for all versions based on llvm source code in compiler-rt/include/profile/InstrProfData.inc for each version.
On X86 I test Xen hypervisor code coverage using Clang instrumentation on a Debian 12 (Bookworm) system. Xen was compiled from source with coverage enabled, installed, and booted in Dom0. Coverage data was extracted using xencov and compared against expected metrics from gcov
On ARM I test it using qemu and follow this instructions: in here

Example of coverage output on ARM:
cov.zip

Right now clang versions 14-19 with profile version 8,9,10 are added. Clean master branch in x86 platform is not linkable with any version of clang. just compilable. Clean master branch in ARM platform is not compilable with any version of clang. Only in clang-19 if we disable this flag `-mgeneral-regs-only`it will succesfuly compile and link. We tested compilability of our patch in above senario and compiled with clang 14-19 in x86 and in ARM it compiled and linked with clang-19 in the described situation.

xen/Rules.mk

xen/common/coverage/llvm.c

xen/Rules.mk

whentojump · 2025-09-29T23:00:06Z

xen/common/coverage/llvm.c

+ */
+int __llvm_profile_runtime;
+
+extern char __start___llvm_prf_data[];


should char be const struct llvm_profile_data?

These are linker-defined symbols that mark section boundaries. The linker doesn't understand C types like struct llvm_profile_data, it only places symbols at memory addresses.
Using char* (which is 1 byte) makes pointer arithmetic straightforward since we are really just working with addresses.

The tricky thing here is it is an array not a pointer. So it sort of "allures" people to use __start___llvm_prf_data[i] even though our patch currently does not.

In fact, in the Linux counterpart, we calculate "the number of elements" using

static inline unsigned long __llvm_prf_data_count(void) { return __llvm_prf_data_size() / sizeof(__llvm_prf_data_start[0]); }

I still think we should revert to the right type. Similar thing for cnts which is 64 bit wide.

xen/common/coverage/llvm.c

whentojump · 2025-09-29T23:09:34Z

xen/include/xen/types.h

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



Why this signed type is needed?

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

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.

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

whentojump · 2025-09-29T23:51:15Z

xen/common/coverage/llvm.c

+extern char __stop___llvm_prf_cnts[];
+
+#define START_DATA      ((const char *)__start___llvm_prf_data)
+#define END_DATA        ((const char *)__stop___llvm_prf_data)


The existing code uses void * and maybe let's not touch that.
I guess this is one of reasons why git diff gets confused and bloated

I tried void* but in calculation of sections I faced issue so I could not get output with llvm-profdata merge.

xen/common/coverage/llvm.c

whentojump · 2025-09-30T00:45:41Z

xen/common/coverage/llvm.c

+    uint64_t binary_ids_size;
+    uint64_t data_size;
+    uint64_t padding_bytes_before_counters;
+    uint64_t counter_size;


Before this field gets renamed to "num_counters" it has always been called "counter*s*_size"? llvm/llvm-project@a6f33ad

In new version renamed to num_{counter, num_data}

I FIXIT this #FIXME :))
https://github.com/llvm/llvm-project/blob/6009708b4367171ccdbf4b5905cb6a803753fe18/compiler-rt/include/profile/InstrProfData.inc#L131

whentojump

Thanks, Saman. This version looks great. Please see my new comments. (I will follow up in some of old conversations in a bit)

whentojump · 2025-10-01T02:24:00Z

xen/common/coverage/llvm.c

+#elif __clang_major__ == 18
+#define LLVM_PROFILE_VERSION    9
 #define LLVM_PROFILE_NUM_KINDS  2
+#elif __clang_major__ >= 14 && __clang_major__ <= 17


nit:

I think && __clang_major__ <= 17 is not needed

three lines above: if the rest of code is using >= 18 let's maybe be consistent here

whentojump · 2025-10-01T02:26:26Z

xen/common/coverage/llvm.c

 * __llvm_profile_runtime must be defined according to the docs at:
 *
- * https://clang.llvm.org/docs/SourceBasedCodeCoverage.html 
+ * https://clang.llvm.org/docs/SourceBasedCodeCoverage.html


please avoid whitespace changes (even though we should blame upstream for inappropriate coding style and lack of code linting :))

whentojump · 2025-10-01T02:35:15Z

xen/common/coverage/llvm.c

+                - (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,


can you also sprinkle ifdef's to this initialization list

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?

can you also sprinkle ifdef's to this initialization

could you please explain more

won't work?

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

whentojump · 2025-10-01T03:38:51Z

Followed up in old conversations as well.

Saman can you give @matthew-l-weber and me write access to this repo so we can also resolve/reopen conversations? Thanks

enum-class · 2025-10-01T15:22:06Z

Followed up in old conversations as well.

Saman can you give @matthew-l-weber and me write access to this repo so we can also resolve/reopen conversations? Thanks

I tried to fix what you have suggested. But maybe I couldn't see some comments (it become messy :))
I don't know how can I give you access to reopen conversations ? in the settings I could not find

matthew-l-weber · 2025-10-06T13:27:38Z

xen/common/coverage/llvm.c

 #endif

-#define LLVM_PROFILE_VERSION    4
+#if __clang_major__ >= 19


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

matthew-l-weber · 2025-10-06T13:29:00Z

xen/common/coverage/llvm.c

+#define LLVM_PROFILE_VERSION    8
+#define LLVM_PROFILE_NUM_KINDS  2
+#else
+#error "Unsupported Clang version"


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

Definitely. Here are some links to consider:

LLVM documentation on the format: https://llvm.org/docs/InstrProfileFormat.html

The changes of header fields (addition, rename, semantic changes) in LLVM source code: https://github.com/llvm/llvm-project/blame/llvmorg-22-init/compiler-rt/include/profile/InstrProfData.inc#L154

Our post asking for opinions about supporting multiple versions from LLVM side: https://discourse.llvm.org/t/supporting-multiple-llvm-profile-formats-at-once/88473

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

matthew-l-weber · 2025-10-06T13:32:00Z

If we're close, it would be nice to see the squashed commits into the single commit for the patch so we can review the commit description and additional details to add under a ---

whentojump · 2025-10-06T22:17:48Z

@matthew-l-weber Thanks for your suggestions!

Saman is refining and testing his v2, focusing on removing unnecessary type changes as pointed out both internally and by Andrew Cooper.

We are breaking down the patches into these parts:

General llvm-cov support with newer toolchains (Header definition changes; initializer changes; dumper changes?)
MC/DC (Makefile changes; dumper changes)
Docs etc

We will let you know when the squashed commit with proper message becomes ready. Thanks!
(I believe Saman has already given you write access to the repo, so feel free to resolve/reopen discussions.)

enum-class added 3 commits September 18, 2025 08:24

add clang-20, add required initializations. add mcdc flags to makefile

9b219ad

condition check on clang version for mcdc support

9073788

matthew-l-weber reviewed Sep 23, 2025

View reviewed changes

xen/Rules.mk Show resolved Hide resolved

matthew-l-weber reviewed Sep 23, 2025

View reviewed changes

xen/common/coverage/llvm.c Show resolved Hide resolved

get_header

935b856

whentojump reviewed Sep 29, 2025

View reviewed changes

xen/common/coverage/llvm.c Outdated Show resolved Hide resolved

whentojump reviewed Sep 30, 2025

View reviewed changes

make minimal changes/ remove mcdc

85fcc16

enum-class requested review from matthew-l-weber and whentojump October 1, 2025 02:14

whentojump reviewed Oct 1, 2025

View reviewed changes

Address concerns.

b55862f

matthew-l-weber requested changes Oct 6, 2025

View reviewed changes

Support clang coverage profile 10, 9, 8 #1

Are you sure you want to change the base?

Support clang coverage profile 10, 9, 8 #1

Uh oh!

Conversation

enum-class commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

whentojump Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

whentojump Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

whentojump Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

whentojump left a comment

Choose a reason for hiding this comment

Uh oh!

whentojump Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

whentojump commented Oct 1, 2025

Uh oh!

enum-class commented Oct 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

whentojump Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthew-l-weber commented Oct 6, 2025

Uh oh!

whentojump commented Oct 6, 2025

Uh oh!

Reviewers

Assignees

Labels

enum-class commented Sep 22, 2025 •

edited

Loading

whentojump Sep 29, 2025 •

edited

Loading

whentojump Sep 29, 2025 •

edited

Loading

whentojump Sep 30, 2025 •

edited

Loading

whentojump Oct 1, 2025 •

edited

Loading

whentojump Oct 6, 2025 •

edited

Loading