[compiler-rt] Fixed __llvm_profile_write_buffer in presence of ValueProfileData. (PR #97350)

Sunil Srivastava via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 1 20:23:41 PDT 2024


https://github.com/sks75220 updated https://github.com/llvm/llvm-project/pull/97350

>From 51d551ae53985cd3bd2ecc703f9419a7b77c8d00 Mon Sep 17 00:00:00 2001
From: Sunil Srivastava <sunil.srivastava at sony.com>
Date: Mon, 1 Jul 2024 14:07:40 -0700
Subject: [PATCH 1/2] Fixed __llvm_profile_write_buffer in presence of
 ValueProfileData.

__llvm_profile_write_buffer generated invalid profile files in
presence of non empty ValueProfileData. Fixed that by
precomputing the size of ValueProfileData, adding it to the
total size, and then filling that.
---
 .../lib/profile/InstrProfilingBuffer.c        | 22 +++++++++-
 .../lib/profile/InstrProfilingInternal.h      |  7 ++++
 .../lib/profile/InstrProfilingWriter.c        | 38 +++++++++++++++++
 .../test/profile/instrprof-without-libc.c     |  6 ---
 .../test/profile/instrprof-write-buffer.c     | 42 +++++++++++++++++++
 5 files changed, 107 insertions(+), 8 deletions(-)
 create mode 100755 compiler-rt/test/profile/instrprof-write-buffer.c

diff --git a/compiler-rt/lib/profile/InstrProfilingBuffer.c b/compiler-rt/lib/profile/InstrProfilingBuffer.c
index 1c451d7ec7563..b4c3399477068 100644
--- a/compiler-rt/lib/profile/InstrProfilingBuffer.c
+++ b/compiler-rt/lib/profile/InstrProfilingBuffer.c
@@ -220,12 +220,19 @@ uint64_t __llvm_profile_get_size_for_buffer_internal(
       &PaddingBytesAfterNames, &PaddingBytesAfterVTable,
       &PaddingBytesAfterVNames);
 
+  /* Compute size of ValueProfData. */
+  int64_t VPDSize = __llvm_profile_getSizeOfValueProfData(
+      lprofGetVPDataReader(), DataBegin, DataEnd);
+  if (VPDSize < 0) {
+    /* Got error marker. We will not increase the size below. */
+    VPDSize = 0;
+  }
   return sizeof(__llvm_profile_header) + __llvm_write_binary_ids(NULL) +
          DataSize + PaddingBytesBeforeCounters + CountersSize +
          PaddingBytesAfterCounters + NumBitmapBytes +
          PaddingBytesAfterBitmapBytes + NamesSize + PaddingBytesAfterNames +
          VTableSize + PaddingBytesAfterVTable + VNameSize +
-         PaddingBytesAfterVNames;
+         PaddingBytesAfterVNames + VPDSize;
 }
 
 COMPILER_RT_VISIBILITY
@@ -237,7 +244,18 @@ void initBufferWriter(ProfDataWriter *BufferWriter, char *Buffer) {
 COMPILER_RT_VISIBILITY int __llvm_profile_write_buffer(char *Buffer) {
   ProfDataWriter BufferWriter;
   initBufferWriter(&BufferWriter, Buffer);
-  return lprofWriteData(&BufferWriter, 0, 0);
+
+  /* Pass VPDataReader hook, if it will not fail. */
+  const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
+  const __llvm_profile_data *DataEnd = __llvm_profile_end_data();
+  VPDataReaderType *Reader = lprofGetVPDataReader();
+  if (__llvm_profile_getSizeOfValueProfData(Reader,DataBegin, DataEnd) < 0) {
+    /* Attempt to use lprofGetVPDataReader will result in an error, so
+     * do not use it. Generate profile data without VPDataReader.
+    */
+    Reader = 0;
+  }
+  return lprofWriteData(&BufferWriter, Reader, 0);
 }
 
 COMPILER_RT_VISIBILITY int __llvm_profile_write_buffer_internal(
diff --git a/compiler-rt/lib/profile/InstrProfilingInternal.h b/compiler-rt/lib/profile/InstrProfilingInternal.h
index d5bd0e41fb129..0bf642b54f89f 100644
--- a/compiler-rt/lib/profile/InstrProfilingInternal.h
+++ b/compiler-rt/lib/profile/InstrProfilingInternal.h
@@ -79,6 +79,8 @@ typedef struct ProfBufferIO {
   uint32_t BufferSz;
   /* Current byte offset from the start of the buffer. */
   uint32_t CurOffset;
+  /* The number of bytes already flushed. */
+  uint64_t FlushedSize;
 } ProfBufferIO;
 
 /* The creator interface used by testing.  */
@@ -169,6 +171,11 @@ void lprofMergeValueProfData(struct ValueProfData *SrcValueProfData,
 
 VPDataReaderType *lprofGetVPDataReader();
 
+/* Compute size of VPData. Return -1 on error (0 is a valid return value). */
+int64_t __llvm_profile_getSizeOfValueProfData(VPDataReaderType *VPDataReader,
+                                      const __llvm_profile_data *DataBegin,
+                                      const __llvm_profile_data *DataEnd);
+
 /* Internal interface used by test to reset the max number of 
  * tracked values per value site to be \p MaxVals.
  */
diff --git a/compiler-rt/lib/profile/InstrProfilingWriter.c b/compiler-rt/lib/profile/InstrProfilingWriter.c
index 8816a71155511..4cfa2d1e96fbe 100644
--- a/compiler-rt/lib/profile/InstrProfilingWriter.c
+++ b/compiler-rt/lib/profile/InstrProfilingWriter.c
@@ -59,6 +59,7 @@ static void llvmInitBufferIO(ProfBufferIO *BufferIO, ProfDataWriter *FileWriter,
   BufferIO->BufferStart = Buffer;
   BufferIO->BufferSz = BufferSz;
   BufferIO->CurOffset = 0;
+  BufferIO->FlushedSize = 0;
 }
 
 COMPILER_RT_VISIBILITY ProfBufferIO *
@@ -108,6 +109,12 @@ lprofBufferIOWrite(ProfBufferIO *BufferIO, const uint8_t *Data, uint32_t Size) {
 }
 
 COMPILER_RT_VISIBILITY int lprofBufferIOFlush(ProfBufferIO *BufferIO) {
+  /* If this is a 'counter' buffer, just keep track of how much got flushed. */
+  if (BufferIO->FileWriter == NULL) {
+    BufferIO->FlushedSize += BufferIO->CurOffset;
+    BufferIO->CurOffset = 0;
+    return 0;
+  }
   if (BufferIO->CurOffset) {
     ProfDataIOVec IO[] = {
         {BufferIO->BufferStart, sizeof(uint8_t), BufferIO->CurOffset, 0}};
@@ -214,6 +221,37 @@ static int writeOneValueProfData(ProfBufferIO *BufferIO,
   return 0;
 }
 
+/* Compute size of VPData, without actually writing it. This function follows
+ * the logic of writeValueProfData below it. It is used for predicting how big
+ * a buffer is needed to store the ValueProfileData.
+ * Note that this function returns -1 as an error indicator and 0 is
+ * a potentially valid value.
+ */
+int64_t
+__llvm_profile_getSizeOfValueProfData(VPDataReaderType *VPDataReader,
+                                      const __llvm_profile_data *DataBegin,
+                                      const __llvm_profile_data *DataEnd) {
+  const __llvm_profile_data *DI = 0;
+  ProfBufferIO *BufferIO = lprofCreateBufferIO(NULL);
+  // This BufferIO is a special struct just for counting how much is being
+  // 'written'. The backing Writer is NULL. It collects number of bytes
+  // in CurOffset, and finally in FlushedSize.
+
+  for (DI = DataBegin; DI < DataEnd; DI++) {
+    if (writeOneValueProfData(BufferIO, VPDataReader, DI)) {
+      return -1;
+    }
+  }
+  if (lprofBufferIOFlush(BufferIO) != 0)
+    return -1;
+
+  int64_t rv = BufferIO->FlushedSize;
+  lprofDeleteBufferIO(BufferIO);
+  return rv;
+}
+
+
+
 static int writeValueProfData(ProfDataWriter *Writer,
                               VPDataReaderType *VPDataReader,
                               const __llvm_profile_data *DataBegin,
diff --git a/compiler-rt/test/profile/instrprof-without-libc.c b/compiler-rt/test/profile/instrprof-without-libc.c
index 3142138cdffc0..dcec4cd756f77 100644
--- a/compiler-rt/test/profile/instrprof-without-libc.c
+++ b/compiler-rt/test/profile/instrprof-without-libc.c
@@ -66,13 +66,7 @@ int main(int argc, const char *argv[]) {
 // CHECK-SYMBOLS-NOT: {{ }}_fdopen
 // CHECK-SYMBOLS-NOT: {{ }}_fopen
 // CHECK-SYMBOLS-NOT: {{ }}_fwrite
-// CHECK-SYMBOLS-NOT: {{ }}_getenv
-// CHECK-SYMBOLS-NOT: {{ }}getenv
 // CHECK-SYMBOLS-NOT: {{ }}_malloc
 // CHECK-SYMBOLS-NOT: {{ }}malloc
-// CHECK-SYMBOLS-NOT: {{ }}_calloc
-// CHECK-SYMBOLS-NOT: {{ }}calloc
-// CHECK-SYMBOLS-NOT: {{ }}_free
-// CHECK-SYMBOLS-NOT: {{ }}free
 // CHECK-SYMBOLS-NOT: {{ }}_open
 // CHECK-SYMBOLS-NOT: {{ }}_getpagesize
diff --git a/compiler-rt/test/profile/instrprof-write-buffer.c b/compiler-rt/test/profile/instrprof-write-buffer.c
new file mode 100755
index 0000000000000..79fba78394ccd
--- /dev/null
+++ b/compiler-rt/test/profile/instrprof-write-buffer.c
@@ -0,0 +1,42 @@
+// UNSUPPORTED: target={{.*windows.*}}
+// This test is derived from "compiler-rt/test/profile/instrprof-write-buffer-internal.c",
+// and that test was disabled on Windows due to sanitizer-windows bot failures.
+// Doing the same here. See 2fcc3f4b18.
+//
+// RUN: rm -f %t.buf.profraw
+// RUN: %clang_pgogen -o %t %s
+// RUN: %run %t %t.buf.profraw
+// RUN: llvm-profdata show %t.buf.profraw
+
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+void threadFunc(void* callback) // Needed for failure.
+{
+  typedef void (FuncPtr)();
+  (*(FuncPtr*)callback)();
+}
+
+uint64_t __llvm_profile_get_size_for_buffer();
+int __llvm_profile_write_buffer(char*);
+
+int main(int argc, const char *argv[])
+{
+  // Write to a buffer, and write that to a file.
+  uint64_t bufsize = __llvm_profile_get_size_for_buffer();
+  char *buf = malloc(bufsize);
+  int ret = __llvm_profile_write_buffer(buf);
+
+  if (ret != 0) {
+    fprintf(stderr, "failed to write buffer");
+    return ret;
+  }
+
+  FILE *f = fopen(argv[1], "w");
+  fwrite(buf, bufsize, 1, f);
+  fclose(f);
+  free(buf);
+
+  return 0;
+}

>From 522509ec372fc3442fb4055e915d9d6256e46d69 Mon Sep 17 00:00:00 2001
From: Sunil Srivastava <sunil.srivastava at sony.com>
Date: Mon, 1 Jul 2024 20:21:23 -0700
Subject: [PATCH 2/2] Added CHECK lines for llvm-profdata output, and
 clang-format fixes.

---
 compiler-rt/lib/profile/InstrProfilingBuffer.c    | 4 ++--
 compiler-rt/test/profile/instrprof-write-buffer.c | 7 ++++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/compiler-rt/lib/profile/InstrProfilingBuffer.c b/compiler-rt/lib/profile/InstrProfilingBuffer.c
index b4c3399477068..8fb9e9c95e73c 100644
--- a/compiler-rt/lib/profile/InstrProfilingBuffer.c
+++ b/compiler-rt/lib/profile/InstrProfilingBuffer.c
@@ -249,10 +249,10 @@ COMPILER_RT_VISIBILITY int __llvm_profile_write_buffer(char *Buffer) {
   const __llvm_profile_data *DataBegin = __llvm_profile_begin_data();
   const __llvm_profile_data *DataEnd = __llvm_profile_end_data();
   VPDataReaderType *Reader = lprofGetVPDataReader();
-  if (__llvm_profile_getSizeOfValueProfData(Reader,DataBegin, DataEnd) < 0) {
+  if (__llvm_profile_getSizeOfValueProfData(Reader, DataBegin, DataEnd) < 0) {
     /* Attempt to use lprofGetVPDataReader will result in an error, so
      * do not use it. Generate profile data without VPDataReader.
-    */
+     */
     Reader = 0;
   }
   return lprofWriteData(&BufferWriter, Reader, 0);
diff --git a/compiler-rt/test/profile/instrprof-write-buffer.c b/compiler-rt/test/profile/instrprof-write-buffer.c
index 79fba78394ccd..9e80575ab80aa 100755
--- a/compiler-rt/test/profile/instrprof-write-buffer.c
+++ b/compiler-rt/test/profile/instrprof-write-buffer.c
@@ -6,7 +6,12 @@
 // RUN: rm -f %t.buf.profraw
 // RUN: %clang_pgogen -o %t %s
 // RUN: %run %t %t.buf.profraw
-// RUN: llvm-profdata show %t.buf.profraw
+// RUN: llvm-profdata show %t.buf.profraw | FileCheck %s
+
+// CHECK: Instrumentation level: IR  entry_first = 0
+// CHECK: Total functions: 2
+// CHECK: Maximum function count: 0
+// CHECK: Maximum internal block count: 0
 
 #include <stdint.h>
 #include <stdio.h>



More information about the llvm-commits mailing list