[llvm] [llvm-profdata] Emit error when counter value is greater than 2^56. (PR #69513)
Zequan Wu via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 20 11:33:20 PDT 2023
https://github.com/ZequanWu updated https://github.com/llvm/llvm-project/pull/69513
>From 9a1af6e1d47ab622979796f2319edec8a9c77928 Mon Sep 17 00:00:00 2001
From: Zequan Wu <zequanwu at google.com>
Date: Wed, 18 Oct 2023 16:28:30 -0400
Subject: [PATCH 1/3] [llvm-profdata] Emit error when counter value is greater
than 2^56.
---
llvm/lib/ProfileData/InstrProfReader.cpp | 13 +++++++++++--
.../malformed-num-counters-zero.test | 17 +++++++++++++++--
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index a920a31d0a4b229..d62a816cdeed4e6 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -38,6 +38,9 @@
using namespace llvm;
+// Maxium counter value 2^56.
+static uint64_t MaxCounterValue = 0xffffffffffffff;
+
// Extracts the variant information from the top 32 bits in the version and
// returns an enum specifying the variants present.
static InstrProfKind getProfileKindFromVersion(uint64_t Version) {
@@ -676,8 +679,14 @@ Error RawInstrProfReader<IntPtrT>::readRawCounts(
// A value of zero signifies the block is covered.
Record.Counts.push_back(*Ptr == 0 ? 1 : 0);
} else {
- const auto *CounterValue = reinterpret_cast<const uint64_t *>(Ptr);
- Record.Counts.push_back(swap(*CounterValue));
+ uint64_t CounterValue = swap(*reinterpret_cast<const uint64_t *>(Ptr));
+ if (CounterValue > MaxCounterValue)
+ return error(instrprof_error::malformed,
+ ("counter value " + Twine(CounterValue) +
+ " is greater than " + Twine(MaxCounterValue))
+ .str());
+
+ Record.Counts.push_back(CounterValue);
}
}
diff --git a/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test b/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test
index b718cf0fd8e9723..011c1cbf73af3bb 100644
--- a/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test
+++ b/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test
@@ -35,11 +35,24 @@ RUN: printf '\1\0\0\0\0\0\0\0' >> %t.profraw
RUN: printf '\0\0\4\0\1\0\0\0' >> %t.profraw
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
+
+// Make a copy for another test.
+RUN: cp %t.profraw %t1.profraw
+
// Make NumCounters = 0 so that we get "number of counters is zero" error message
RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
RUN: printf '\023\0\0\0\0\0\0\0' >> %t.profraw
RUN: printf '\3\0foo\0\0\0' >> %t.profraw
-RUN: not llvm-profdata show %t.profraw 2>&1 | FileCheck %s
-CHECK: malformed instrumentation profile data: number of counters is zero
+RUN: not llvm-profdata show %t.profraw 2>&1 | FileCheck %s --check-prefix=ZERO
+ZERO: malformed instrumentation profile data: number of counters is zero
+
+// Test a counter value greater than 2^56.
+RUN: printf '\1\0\0\0\0\0\0\0' >> %t1.profraw
+
+RUN: printf '\0\0\0\0\0\0\0\1' >> %t1.profraw
+RUN: printf '\3\0foo\0\0\0' >> %t1.profraw
+
+RUN: not llvm-profdata show %t1.profraw 2>&1 | FileCheck %s --check-prefix=MAX
+MAX: malformed instrumentation profile data: counter value 72057594037927936 is greater than 72057594037927935
>From 5d4c2ce9b8f5c49041bcbdf12f7890f75e4754c7 Mon Sep 17 00:00:00 2001
From: Zequan Wu <zequanwu at google.com>
Date: Thu, 19 Oct 2023 15:41:38 -0400
Subject: [PATCH 2/3] address comments
---
llvm/include/llvm/ProfileData/InstrProfReader.h | 3 +++
llvm/lib/ProfileData/InstrProfReader.cpp | 10 +++-------
.../llvm-profdata/malformed-num-counters-zero.test | 6 +++---
3 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProfReader.h b/llvm/include/llvm/ProfileData/InstrProfReader.h
index 5f54cbeb1b01eda..e62ee42d09f4145 100644
--- a/llvm/include/llvm/ProfileData/InstrProfReader.h
+++ b/llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -341,6 +341,9 @@ class RawInstrProfReader : public InstrProfReader {
/// Start address of binary id length and data pairs.
const uint8_t *BinaryIdsStart;
+ // Maxium counter value 2^56.
+ static const uint64_t MaxCounterValue = (1ULL << 56);
+
public:
RawInstrProfReader(std::unique_ptr<MemoryBuffer> DataBuffer,
const InstrProfCorrelator *Correlator)
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index d62a816cdeed4e6..3a1d5ef3a9d827e 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -27,6 +27,7 @@
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/SwapByteOrder.h"
#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/WithColor.h"
#include <algorithm>
#include <cstddef>
#include <cstdint>
@@ -38,9 +39,6 @@
using namespace llvm;
-// Maxium counter value 2^56.
-static uint64_t MaxCounterValue = 0xffffffffffffff;
-
// Extracts the variant information from the top 32 bits in the version and
// returns an enum specifying the variants present.
static InstrProfKind getProfileKindFromVersion(uint64_t Version) {
@@ -681,10 +679,8 @@ Error RawInstrProfReader<IntPtrT>::readRawCounts(
} else {
uint64_t CounterValue = swap(*reinterpret_cast<const uint64_t *>(Ptr));
if (CounterValue > MaxCounterValue)
- return error(instrprof_error::malformed,
- ("counter value " + Twine(CounterValue) +
- " is greater than " + Twine(MaxCounterValue))
- .str());
+ WithColor::warning() << "counter value " + Twine(CounterValue) +
+ " suggests corrupted profile data";
Record.Counts.push_back(CounterValue);
}
diff --git a/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test b/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test
index 011c1cbf73af3bb..a5f05095d6d52de 100644
--- a/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test
+++ b/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test
@@ -51,8 +51,8 @@ ZERO: malformed instrumentation profile data: number of counters is zero
// Test a counter value greater than 2^56.
RUN: printf '\1\0\0\0\0\0\0\0' >> %t1.profraw
-RUN: printf '\0\0\0\0\0\0\0\1' >> %t1.profraw
+RUN: printf '\1\0\0\0\0\0\0\1' >> %t1.profraw
RUN: printf '\3\0foo\0\0\0' >> %t1.profraw
-RUN: not llvm-profdata show %t1.profraw 2>&1 | FileCheck %s --check-prefix=MAX
-MAX: malformed instrumentation profile data: counter value 72057594037927936 is greater than 72057594037927935
+RUN: llvm-profdata show %t1.profraw 2>&1 | FileCheck %s --check-prefix=MAX
+MAX: warning: counter value 72057594037927937 suggests corrupted profile data
>From fb2339c8764bcf515fbd749d50b4992381a65016 Mon Sep 17 00:00:00 2001
From: Zequan Wu <zequanwu at google.com>
Date: Fri, 20 Oct 2023 14:33:03 -0400
Subject: [PATCH 3/3] Use callback function to print warning for the first
time.
---
.../llvm/ProfileData/InstrProfReader.h | 16 +++++++----
llvm/lib/ProfileData/InstrProfReader.cpp | 28 +++++++++++--------
.../malformed-num-counters-zero.test | 4 +--
llvm/tools/llvm-profdata/llvm-profdata.cpp | 14 +++++++++-
4 files changed, 43 insertions(+), 19 deletions(-)
diff --git a/llvm/include/llvm/ProfileData/InstrProfReader.h b/llvm/include/llvm/ProfileData/InstrProfReader.h
index e62ee42d09f4145..fbda29d5217d744 100644
--- a/llvm/include/llvm/ProfileData/InstrProfReader.h
+++ b/llvm/include/llvm/ProfileData/InstrProfReader.h
@@ -202,11 +202,13 @@ class InstrProfReader {
/// instrprof file.
static Expected<std::unique_ptr<InstrProfReader>>
create(const Twine &Path, vfs::FileSystem &FS,
- const InstrProfCorrelator *Correlator = nullptr);
+ const InstrProfCorrelator *Correlator = nullptr,
+ std::optional<std::function<void(Error)>> MaybeWarnFn = {});
static Expected<std::unique_ptr<InstrProfReader>>
create(std::unique_ptr<MemoryBuffer> Buffer,
- const InstrProfCorrelator *Correlator = nullptr);
+ const InstrProfCorrelator *Correlator = nullptr,
+ std::optional<std::function<void(Error)>> MaybeWarnFn = {});
/// \param Weight for raw profiles use this as the temporal profile trace
/// weight
@@ -341,15 +343,19 @@ class RawInstrProfReader : public InstrProfReader {
/// Start address of binary id length and data pairs.
const uint8_t *BinaryIdsStart;
- // Maxium counter value 2^56.
+ std::optional<std::function<void(Error)>> MaybeWarnFn;
+
+ /// Maxium counter value 2^56.
static const uint64_t MaxCounterValue = (1ULL << 56);
public:
RawInstrProfReader(std::unique_ptr<MemoryBuffer> DataBuffer,
- const InstrProfCorrelator *Correlator)
+ const InstrProfCorrelator *Correlator,
+ std::optional<std::function<void(Error)>> MaybeWarnFn)
: DataBuffer(std::move(DataBuffer)),
Correlator(dyn_cast_or_null<const InstrProfCorrelatorImpl<IntPtrT>>(
- Correlator)) {}
+ Correlator)),
+ MaybeWarnFn(MaybeWarnFn) {}
RawInstrProfReader(const RawInstrProfReader &) = delete;
RawInstrProfReader &operator=(const RawInstrProfReader &) = delete;
diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp
index 3a1d5ef3a9d827e..286d7a877f673cb 100644
--- a/llvm/lib/ProfileData/InstrProfReader.cpp
+++ b/llvm/lib/ProfileData/InstrProfReader.cpp
@@ -27,7 +27,6 @@
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/SwapByteOrder.h"
#include "llvm/Support/VirtualFileSystem.h"
-#include "llvm/Support/WithColor.h"
#include <algorithm>
#include <cstddef>
#include <cstdint>
@@ -166,19 +165,22 @@ static Error printBinaryIdsInternal(raw_ostream &OS,
return Error::success();
}
-Expected<std::unique_ptr<InstrProfReader>>
-InstrProfReader::create(const Twine &Path, vfs::FileSystem &FS,
- const InstrProfCorrelator *Correlator) {
+Expected<std::unique_ptr<InstrProfReader>> InstrProfReader::create(
+ const Twine &Path, vfs::FileSystem &FS,
+ const InstrProfCorrelator *Correlator,
+ std::optional<std::function<void(Error)>> MaybeWarnFn) {
// Set up the buffer to read.
auto BufferOrError = setupMemoryBuffer(Path, FS);
if (Error E = BufferOrError.takeError())
return std::move(E);
- return InstrProfReader::create(std::move(BufferOrError.get()), Correlator);
+ return InstrProfReader::create(std::move(BufferOrError.get()), Correlator,
+ MaybeWarnFn);
}
Expected<std::unique_ptr<InstrProfReader>>
InstrProfReader::create(std::unique_ptr<MemoryBuffer> Buffer,
- const InstrProfCorrelator *Correlator) {
+ const InstrProfCorrelator *Correlator,
+ std::optional<std::function<void(Error)>> MaybeWarnFn) {
if (Buffer->getBufferSize() == 0)
return make_error<InstrProfError>(instrprof_error::empty_raw_profile);
@@ -187,9 +189,11 @@ InstrProfReader::create(std::unique_ptr<MemoryBuffer> Buffer,
if (IndexedInstrProfReader::hasFormat(*Buffer))
Result.reset(new IndexedInstrProfReader(std::move(Buffer)));
else if (RawInstrProfReader64::hasFormat(*Buffer))
- Result.reset(new RawInstrProfReader64(std::move(Buffer), Correlator));
+ Result.reset(
+ new RawInstrProfReader64(std::move(Buffer), Correlator, MaybeWarnFn));
else if (RawInstrProfReader32::hasFormat(*Buffer))
- Result.reset(new RawInstrProfReader32(std::move(Buffer), Correlator));
+ Result.reset(
+ new RawInstrProfReader32(std::move(Buffer), Correlator, MaybeWarnFn));
else if (TextInstrProfReader::hasFormat(*Buffer))
Result.reset(new TextInstrProfReader(std::move(Buffer)));
else
@@ -678,9 +682,11 @@ Error RawInstrProfReader<IntPtrT>::readRawCounts(
Record.Counts.push_back(*Ptr == 0 ? 1 : 0);
} else {
uint64_t CounterValue = swap(*reinterpret_cast<const uint64_t *>(Ptr));
- if (CounterValue > MaxCounterValue)
- WithColor::warning() << "counter value " + Twine(CounterValue) +
- " suggests corrupted profile data";
+ if (CounterValue > MaxCounterValue && MaybeWarnFn)
+ (*MaybeWarnFn)(make_error<InstrProfError>(
+ instrprof_error::malformed,
+ "excessively large counter value " + Twine(CounterValue) +
+ " suggests corrupted profile data"));
Record.Counts.push_back(CounterValue);
}
diff --git a/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test b/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test
index a5f05095d6d52de..293e7c862b516c4 100644
--- a/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test
+++ b/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test
@@ -54,5 +54,5 @@ RUN: printf '\1\0\0\0\0\0\0\0' >> %t1.profraw
RUN: printf '\1\0\0\0\0\0\0\1' >> %t1.profraw
RUN: printf '\3\0foo\0\0\0' >> %t1.profraw
-RUN: llvm-profdata show %t1.profraw 2>&1 | FileCheck %s --check-prefix=MAX
-MAX: warning: counter value 72057594037927937 suggests corrupted profile data
+RUN: llvm-profdata merge %t1.profraw -o %t.profdata 2>&1 | FileCheck %s --check-prefix=MAX
+MAX: warning: excessively large counter value 72057594037927937 suggests corrupted profile data
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 7d665a8005b0d62..9a658cdf8356ad8 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -314,7 +314,19 @@ static void loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
}
auto FS = vfs::getRealFileSystem();
- auto ReaderOrErr = InstrProfReader::create(Input.Filename, *FS, Correlator);
+ bool Reported = false;
+ auto WarnFn = [&](Error E) {
+ if (Reported) {
+ consumeError(std::move(E));
+ return;
+ }
+ Reported = true;
+ // Only show hint the first time an error occurs.
+ auto [ErrCode, Msg] = InstrProfError::take(std::move(E));
+ warn(Msg);
+ };
+ auto ReaderOrErr =
+ InstrProfReader::create(Input.Filename, *FS, Correlator, WarnFn);
if (Error E = ReaderOrErr.takeError()) {
// Skip the empty profiles by returning silently.
auto [ErrCode, Msg] = InstrProfError::take(std::move(E));
More information about the llvm-commits
mailing list