This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG17cfd2e025cb: [profiling] Improve error message for raw
profile header mismatches (authored by paquette).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D149361?vs=517644&id=517723#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149361/new/
https://reviews.llvm.org/D149361
Files:
clang/lib/CodeGen/CodeGenPGO.cpp
llvm/include/llvm/ProfileData/InstrProf.h
llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/ProfileData/InstrProfReader.cpp
llvm/test/tools/llvm-profdata/mismatched-raw-profile-header.test
llvm/tools/llvm-profdata/llvm-profdata.cpp
llvm/unittests/ProfileData/InstrProfTest.cpp
Index: llvm/unittests/ProfileData/InstrProfTest.cpp
===================================================================
--- llvm/unittests/ProfileData/InstrProfTest.cpp
+++ llvm/unittests/ProfileData/InstrProfTest.cpp
@@ -869,7 +869,9 @@
const uint64_t MaxEdgeCount = getInstrMaxCountValue();
instrprof_error Result;
- auto Err = [&](Error E) { Result = InstrProfError::take(std::move(E)); };
+ auto Err = [&](Error E) {
+ Result = std::get<0>(InstrProfError::take(std::move(E)));
+ };
Result = instrprof_error::success;
Writer.addRecord({"foo", 0x1234, {1}}, Err);
ASSERT_EQ(Result, instrprof_error::success);
Index: llvm/tools/llvm-profdata/llvm-profdata.cpp
===================================================================
--- llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -233,9 +233,10 @@
auto ReaderOrErr = InstrProfReader::create(TestFilename, *FS);
if (Error E = ReaderOrErr.takeError()) {
// Skip the empty profiles by returning sliently.
- instrprof_error IPE = InstrProfError::take(std::move(E));
- if (IPE != instrprof_error::empty_raw_profile)
- WC->Errors.emplace_back(make_error<InstrProfError>(IPE), TestFilename);
+ auto [ErrorCode, Msg] = InstrProfError::take(std::move(E));
+ if (ErrorCode != instrprof_error::empty_raw_profile)
+ WC->Errors.emplace_back(make_error<InstrProfError>(ErrorCode, Msg),
+ TestFilename);
return;
}
@@ -280,8 +281,9 @@
}
auto MemProfError = [&](Error E) {
- instrprof_error IPE = InstrProfError::take(std::move(E));
- WC->Errors.emplace_back(make_error<InstrProfError>(IPE), Filename);
+ auto [ErrorCode, Msg] = InstrProfError::take(std::move(E));
+ WC->Errors.emplace_back(make_error<InstrProfError>(ErrorCode, Msg),
+ Filename);
};
// Add the frame mappings into the writer context.
@@ -306,9 +308,10 @@
auto ReaderOrErr = InstrProfReader::create(Input.Filename, *FS, Correlator);
if (Error E = ReaderOrErr.takeError()) {
// Skip the empty profiles by returning silently.
- instrprof_error IPE = InstrProfError::take(std::move(E));
- if (IPE != instrprof_error::empty_raw_profile)
- WC->Errors.emplace_back(make_error<InstrProfError>(IPE), Filename);
+ auto [ErrCode, Msg] = InstrProfError::take(std::move(E));
+ if (ErrCode != instrprof_error::empty_raw_profile)
+ WC->Errors.emplace_back(make_error<InstrProfError>(ErrCode, Msg),
+ Filename);
return;
}
@@ -335,11 +338,11 @@
}
Reported = true;
// Only show hint the first time an error occurs.
- instrprof_error IPE = InstrProfError::take(std::move(E));
+ auto [ErrCode, Msg] = InstrProfError::take(std::move(E));
std::unique_lock<std::mutex> ErrGuard{WC->ErrLock};
- bool firstTime = WC->WriterErrorCodes.insert(IPE).second;
- handleMergeWriterError(make_error<InstrProfError>(IPE), Input.Filename,
- FuncName, firstTime);
+ bool firstTime = WC->WriterErrorCodes.insert(ErrCode).second;
+ handleMergeWriterError(make_error<InstrProfError>(ErrCode, Msg),
+ Input.Filename, FuncName, firstTime);
});
}
@@ -370,11 +373,11 @@
exitWithError(std::move(E));
Dst->Writer.mergeRecordsFromWriter(std::move(Src->Writer), [&](Error E) {
- instrprof_error IPE = InstrProfError::take(std::move(E));
+ auto [ErrorCode, Msg] = InstrProfError::take(std::move(E));
std::unique_lock<std::mutex> ErrGuard{Dst->ErrLock};
- bool firstTime = Dst->WriterErrorCodes.insert(IPE).second;
+ bool firstTime = Dst->WriterErrorCodes.insert(ErrorCode).second;
if (firstTime)
- warn(toString(make_error<InstrProfError>(IPE)));
+ warn(toString(make_error<InstrProfError>(ErrorCode, Msg)));
});
}
Index: llvm/test/tools/llvm-profdata/mismatched-raw-profile-header.test
===================================================================
--- /dev/null
+++ llvm/test/tools/llvm-profdata/mismatched-raw-profile-header.test
@@ -0,0 +1,19 @@
+// Magic
+RUN: printf '\377lprofr\201' > %t
+// Version
+RUN: printf '\0\01\0\0\0\0\0\10' >> %t
+// The rest of the header needs to be there to prevent a broken header error.
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
+RUN: printf '\0\0\0\0\0\0\0\2' >> %t
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
+RUN: printf '\0\0\0\0\0\0\0\3' >> %t
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
+RUN: printf '\0\0\0\0\0\0\0\20' >> %t
+RUN: printf '\0\0\0\1\0\4\0\0' >> %t
+RUN: printf '\0\0\0\2\0\4\0\0' >> %t
+RUN: printf '\0\0\0\0\0\0\0\0' >> %t
+
+RUN: not llvm-profdata show %t -o /dev/null 2>&1 | FileCheck %s
+
+CHECK: raw profile version mismatch: Profile uses raw profile format version = 281474976710664; expected version = {{[0-9]+}}
+CHECK-NEXT: PLEASE update this tool to version in the raw profile, or regenerate raw profile with expected version.
Index: llvm/lib/ProfileData/InstrProfReader.cpp
===================================================================
--- llvm/lib/ProfileData/InstrProfReader.cpp
+++ llvm/lib/ProfileData/InstrProfReader.cpp
@@ -532,7 +532,13 @@
const RawInstrProf::Header &Header) {
Version = swap(Header.Version);
if (GET_VERSION(Version) != RawInstrProf::Version)
- return error(instrprof_error::unsupported_version);
+ return error(instrprof_error::raw_profile_version_mismatch,
+ ("Profile uses raw profile format version = " +
+ Twine(GET_VERSION(Version)) +
+ "; expected version = " + Twine(RawInstrProf::Version) +
+ "\nPLEASE update this tool to version in the raw profile, or "
+ "regenerate raw profile with expected version.")
+ .str());
if (useDebugInfoCorrelate() && !Correlator)
return error(instrprof_error::missing_debug_info_for_correlation);
if (!useDebugInfoCorrelate() && Correlator)
@@ -1199,7 +1205,8 @@
std::unique_ptr<InstrProfSymtab> NewSymtab = std::make_unique<InstrProfSymtab>();
if (Error E = Index->populateSymtab(*NewSymtab)) {
- consumeError(error(InstrProfError::take(std::move(E))));
+ auto [ErrCode, Msg] = InstrProfError::take(std::move(E));
+ consumeError(error(ErrCode, Msg));
}
Symtab = std::move(NewSymtab);
Index: llvm/lib/ProfileData/InstrProf.cpp
===================================================================
--- llvm/lib/ProfileData/InstrProf.cpp
+++ llvm/lib/ProfileData/InstrProf.cpp
@@ -153,6 +153,9 @@
OS << "profile uses zlib compression but the profile reader was built "
"without zlib support";
break;
+ case instrprof_error::raw_profile_version_mismatch:
+ OS << "raw profile version mismatch";
+ break;
}
// If optional error message is not empty, append it to the message.
Index: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
===================================================================
--- llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -249,7 +249,7 @@
std::vector<uint64_t> Counts;
if (Error E = ProfileReader.getFunctionCounts(Record.FunctionName,
Record.FunctionHash, Counts)) {
- instrprof_error IPE = InstrProfError::take(std::move(E));
+ instrprof_error IPE = std::get<0>(InstrProfError::take(std::move(E)));
if (IPE == instrprof_error::hash_mismatch) {
FuncHashMismatches.emplace_back(std::string(Record.FunctionName),
Record.FunctionHash);
Index: llvm/include/llvm/ProfileData/InstrProf.h
===================================================================
--- llvm/include/llvm/ProfileData/InstrProf.h
+++ llvm/include/llvm/ProfileData/InstrProf.h
@@ -330,7 +330,8 @@
compress_failed,
uncompress_failed,
empty_raw_profile,
- zlib_unavailable
+ zlib_unavailable,
+ raw_profile_version_mismatch
};
/// An ordered list of functions identified by their NameRef found in
@@ -362,15 +363,18 @@
instrprof_error get() const { return Err; }
const std::string &getMessage() const { return Msg; }
- /// Consume an Error and return the raw enum value contained within it. The
- /// Error must either be a success value, or contain a single InstrProfError.
- static instrprof_error take(Error E) {
+ /// Consume an Error and return the raw enum value contained within it, and
+ /// the optional error message. The Error must either be a success value, or
+ /// contain a single InstrProfError.
+ static std::pair<instrprof_error, std::string> take(Error E) {
auto Err = instrprof_error::success;
- handleAllErrors(std::move(E), [&Err](const InstrProfError &IPE) {
+ std::string Msg = "";
+ handleAllErrors(std::move(E), [&Err, &Msg](const InstrProfError &IPE) {
assert(Err == instrprof_error::success && "Multiple errors encountered");
Err = IPE.get();
+ Msg = IPE.getMessage();
});
- return Err;
+ return {Err, Msg};
}
static char ID;
Index: clang/lib/CodeGen/CodeGenPGO.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenPGO.cpp
+++ clang/lib/CodeGen/CodeGenPGO.cpp
@@ -1036,7 +1036,7 @@
llvm::Expected<llvm::InstrProfRecord> RecordExpected =
PGOReader->getInstrProfRecord(FuncName, FunctionHash);
if (auto E = RecordExpected.takeError()) {
- auto IPE = llvm::InstrProfError::take(std::move(E));
+ auto IPE = std::get<0>(llvm::InstrProfError::take(std::move(E)));
if (IPE == llvm::instrprof_error::unknown_function)
CGM.getPGOStats().addMissing(IsInMainFile);
else if (IPE == llvm::instrprof_error::hash_mismatch)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
