[profiling] Improve error message for raw profile header mismatches

[paquette]

Summary

When a user uses a mismatched clang + llvm-profdata, they didn't get a very informative error message. It would just say "unsupported version".

As a result, users are often confused as to what they are supposed to do and tend to assume that it's a bug in the profiling runtime.

This patch improves the error message by:

• Adding a new class of error (raw_profile_version_mismatch) to make it clear that, specifically, the *raw profile* version is unsupported because of a tool mismatch.

• Adding an error message that tells the user which raw profile version was encountered, which version was expected, and instructs them to align their tool versions.

To support this, this patch also updates InstrProfError::take to also propagate the optional error message.

[jroelofs]

LGTM

llvm/lib/ProfileData/InstrProfReader.cpp

540

Given how pervasive the problem is, maybe the error message should also link to the docs explaining what the compatibility guarantees are/are-not.

[paquette]

llvm/lib/ProfileData/InstrProfReader.cpp

540

That's a good idea, but the docs are in Clang right now. I'll see if I can move them to LLVM or duplicate them there so that other compiler frontend users aren't confused.

llvm/tools/llvm-profdata/llvm-profdata.cpp

311–314

will remove this when I push

[Dinistro]

llvm/unittests/ProfileData/InstrProfTest.cpp

1359

It seems that this and the following similar line do not compile. I assume that the necessary flags to activate this test were not provided in the CI executed for this revision.

Crashing build: https://lab.llvm.org/buildbot/#/builders/104/builds/11668/steps/6/logs/stdio

[Dinistro]

llvm/unittests/ProfileData/InstrProfTest.cpp

1359

I creatred and landed a revision that fixes this https://reviews.llvm.org/D149434

[paquette]

Sorry for the inconvenience and thanks for the fix!

clang/lib/CodeGen/CodeGenPGO.cpp

 
 void CodeGenPGO::loadRegionCounts(llvm::IndexedInstrProfReader *PGOReader,
                                   bool IsInMainFile) {
   CGM.getPGOStats().addVisited(IsInMainFile);
   RegionCounts.clear();
   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)
       CGM.getPGOStats().addMismatched(IsInMainFile);
     else if (IPE == llvm::instrprof_error::malformed)
       // TODO: Consider a more specific warning for this case.
       CGM.getPGOStats().addMismatched(IsInMainFile);
     return;

llvm/include/llvm/ProfileData/InstrProf.h

enum class instrprof_error {
   invalid_prof,
   hash_mismatch,
   count_mismatch,
   counter_overflow,
   value_site_count_mismatch,
   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
 /// INSTR_PROF_DATA
 struct TemporalProfTraceTy {
   uint64_t Weight = 1;
   std::vector<uint64_t> FunctionNameRefs;
 };
public:
 
   std::error_code convertToErrorCode() const override {
     return make_error_code(Err);
   }
 
   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;
 
 private:
   instrprof_error Err;
   std::string Msg;
 };

llvm/lib/ProfileData/Coverage/CoverageMapping.cpp

Error CoverageMapping::loadFunctionRecord(
   else
     OrigFuncName = getFuncNameWithoutPrefix(OrigFuncName, Record.Filenames[0]);
 
   CounterMappingContext Ctx(Record.Expressions);
 
   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);
       return Error::success();
     } else if (IPE != instrprof_error::unknown_function)
       return make_error<InstrProfError>(IPE);
     Counts.assign(getMaxCounterID(Ctx, Record) + 1, 0);
   }

llvm/lib/ProfileData/InstrProf.cpp

case instrprof_error::uncompress_failed:
     break;
   case instrprof_error::empty_raw_profile:
     OS << "empty raw profile file";
     break;
   case instrprof_error::zlib_unavailable:
     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.
   if (!ErrMsg.empty())
     OS << ": " << ErrMsg;
 
   return OS.str();
 }

llvm/lib/ProfileData/InstrProfReader.cpp

Error RawInstrProfReader<IntPtrT>::createSymtab(InstrProfSymtab &Symtab) {
   return success();
 }
 
 template <class IntPtrT>
 Error RawInstrProfReader<IntPtrT>::readHeader(
     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)
     return error(instrprof_error::unexpected_debug_info_for_correlation);
 
   BinaryIdsSize = swap(Header.BinaryIdsSize);
   if (BinaryIdsSize % sizeof(uint64_t))
     return error(instrprof_error::bad_header);

 }
 
 InstrProfSymtab &IndexedInstrProfReader::getSymtab() {
   if (Symtab)
     return *Symtab;
 
   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);
   return *Symtab;
 }
 
 Expected<InstrProfRecord> IndexedInstrProfReader::getInstrProfRecord(
     StringRef FuncName, uint64_t FuncHash, uint64_t *MismatchedFuncSum) {

llvm/test/tools/llvm-profdata/mismatched-raw-profile-header.test

+// 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.

llvm/tools/llvm-profdata/llvm-profdata.cpp

static void overlapInput(const std::string &BaseFilename,
                          const std::string &TestFilename, WriterContext *WC,
                          OverlapStats &Overlap,
                          const OverlapFuncFilters &FuncFilter,
                          raw_fd_ostream &OS, bool IsCS) {
   auto FS = vfs::getRealFileSystem();
   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;
   }
 
   auto Reader = std::move(ReaderOrErr.get());
   for (auto &I : *Reader) {
     OverlapStats FuncOverlap(OverlapStats::FunctionLevel);
     FuncOverlap.setFuncInfo(I.Name, I.Hash);
 
if (Error E = WC->Writer.mergeProfileKind(Reader->getProfileKind())) {
           make_error<StringError>(
               "Cannot merge MemProf profile with Clang generated profile.",
               std::error_code()),
           Filename);
       return;
     }
 
     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.
     const auto &IdToFrame = Reader->getFrameMapping();
     for (const auto &I : IdToFrame) {
       bool Succeeded = WC->Writer.addMemProfFrame(
           /*Id=*/I.first, /*Frame=*/I.getSecond(), MemProfError);
       // If we weren't able to add the frame mappings then it doesn't make sense
       // to try to add the records from this profile.
       if (!Succeeded)
         return;
     }
     const auto &FunctionProfileData = Reader->getProfileData();
     // Add the memprof records into the writer context.
     for (const auto &I : FunctionProfileData) {
       WC->Writer.addMemProfRecord(/*Id=*/I.first, /*Record=*/I.second);
     }
     return;
   }
 
   auto FS = vfs::getRealFileSystem();
   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;
   }
 
   auto Reader = std::move(ReaderOrErr.get());
   if (Error E = WC->Writer.mergeProfileKind(Reader->getProfileKind())) {
     consumeError(std::move(E));
     WC->Errors.emplace_back(
         make_error<StringError>(
for (auto &I : *Reader) {
     bool Reported = false;
     WC->Writer.addRecord(std::move(I), Input.Weight, [&](Error E) {
       if (Reported) {
         consumeError(std::move(E));
         return;
       }
       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);
     });
   }
 
   if (Reader->hasTemporalProfile()) {
     auto &Traces = Reader->getTemporalProfTraces(Input.Weight);
     if (!Traces.empty())
       WC->Writer.addTemporalProfileTraces(
           Traces, Reader->getTemporalProfTraceStreamSize());
static void mergeWriterContexts(WriterContext *Dst, WriterContext *Src) {
   for (auto &ErrorPair : Src->Errors)
     Dst->Errors.push_back(std::move(ErrorPair));
   Src->Errors.clear();
 
   if (Error E = Dst->Writer.mergeProfileKind(Src->Writer.getProfileKind()))
     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)));
   });
 }
 
 static void writeInstrProfile(StringRef OutputFilename,
                               ProfileFormat OutputFormat,
                               InstrProfWriter &Writer) {
   std::error_code EC;
   raw_fd_ostream Output(OutputFilename.data(), EC,

llvm/unittests/ProfileData/InstrProfTest.cpp

 
 TEST_P(MaybeSparseInstrProfTest, get_icall_data_merge1_saturation) {
   static const char bar[] = "bar";
 
   const uint64_t MaxValCount = std::numeric_limits<uint64_t>::max();
   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);
 
   // Verify counter overflow.
   Result = instrprof_error::success;
   Writer.addRecord({"foo", 0x1234, {MaxEdgeCount}}, Err);
   ASSERT_EQ(Result, instrprof_error::counter_overflow);

 #if defined(_LP64) && defined(EXPENSIVE_CHECKS)
 TEST(ProfileReaderTest, ReadsLargeFiles) {
   const size_t LargeSize = 1ULL << 32; // 4GB
 
   auto RawProfile = WritableMemoryBuffer::getNewUninitMemBuffer(LargeSize);
   if (!RawProfile)
     GTEST_SKIP();
   auto RawProfileReaderOrErr = InstrProfReader::create(std::move(RawProfile));
   ASSERT_TRUE(InstrProfError::take(RawProfileReaderOrErr.takeError()) ==
               instrprof_error::unrecognized_format);
 
   auto IndexedProfile = WritableMemoryBuffer::getNewUninitMemBuffer(LargeSize);
   if (!IndexedProfile)
     GTEST_SKIP();
   auto IndexedReaderOrErr =
       IndexedInstrProfReader::create(std::move(IndexedProfile), nullptr);
   ASSERT_TRUE(InstrProfError::take(IndexedReaderOrErr.takeError()) ==
               instrprof_error::bad_magic);
 }
 #endif
 
 } // end anonymous namespace