On 10/2/25 11:53 AM, Andrew Cooper wrote:
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.

In this case, I agree that it should be considered for 4.21:
 Release-Acked-By: Oleksii Kurochko<[email protected]>

Thanks.

~ Oleksii


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.

Reply via email to