[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 14:29:40 PDT 2024
https://github.com/sks75220 created https://github.com/llvm/llvm-project/pull/97350
__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.
The new test, instrprof-write-buffer.c is an example of the failure.
The changes to instrprof-without-libc.c are a bit questionable. I don't understand the purpose of this test, but the new code pulls in three function that this test does not like.
I don't see an easy way to avoid this change (instrprof-without-libc.c), though it may be possible to achieve that by splitting some files to ensure that the new changes are not pulled in this test.
I am open to suggestions.
>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] 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;
+}
More information about the llvm-commits
mailing list