[llvm-profdata] Add mode to recover from profile read failures

Add a mode in which profile read errors are not immediately treated as
fatal. In this mode, merging makes forward progress and reports failure
only if no inputs can be read.

Differential Revision: https://reviews.llvm.org/D66985

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@370827 91177308-0d34-0410-b5e6-96231b3b80d8

docs/CommandGuide/llvm-profdata.rst

@@ -124,6 +124,14 @@ OPTIONS
  Use N threads to perform profile merging. When N=0, llvm-profdata auto-detects
  an appropriate number of threads to use. This is the default.
 
+.. option:: -failure-mode=[any|all]
+
+ Set the failure mode. There are two options: 'any' causes the merge command to
+ fail if any profiles are invalid, and 'all' causes the merge command to fail
+ only if all profiles are invalid. If 'all' is set, information from any
+ invalid profiles is excluded from the final merged product. The default
+ failure mode is 'any'.
+
 EXAMPLES
 ^^^^^^^^
 Basic Usage

test/tools/llvm-profdata/invalid-profdata.test

@@ -21,7 +21,8 @@ RUN: echo "1"                                            >> %t.input
 RUN: echo ":10"                                          >> %t.input
 
 RUN: not llvm-profdata merge %t.input -text -output=/dev/null 2>&1  | FileCheck %s --check-prefix=BROKEN
-BROKEN: Malformed instrumentation profile data
+BROKEN: warning: {{.*}}invalid-profdata.test.tmp.input: Malformed instrumentation profile data
+BROKEN-NEXT: error: No profiles could be merged.
 
 RUN: echo ":ir"                     > %t.input
 RUN: echo "_ZN6Thread5StartEv"      >> %t.input

test/tools/llvm-profdata/text-format-errors.test

@@ -1,14 +1,22 @@
 Tests for instrumentation profile bad encoding.
 
 1- Detect invalid count
-RUN: not llvm-profdata show %p/Inputs/invalid-count-later.proftext 2>&1 | FileCheck %s --check-prefix=INVALID-COUNT-LATER
-RUN: not llvm-profdata merge %p/Inputs/invalid-count-later.proftext %p/Inputs/invalid-count-later.proftext -o %t.out 2>&1 | FileCheck %s --check-prefix=INVALID-COUNT-LATER
-INVALID-COUNT-LATER: error: {{.*}}invalid-count-later.proftext: Malformed instrumentation profile data
+RUN: not llvm-profdata show %p/Inputs/invalid-count-later.proftext 2>&1 | FileCheck %s --check-prefix=INVALID-COUNT-LATER-SHOW
+INVALID-COUNT-LATER-SHOW: error: {{.*}}invalid-count-later.proftext: Malformed instrumentation profile data
+
+RUN: not llvm-profdata merge %p/Inputs/invalid-count-later.proftext %p/Inputs/invalid-count-later.proftext -o %t.out 2>&1 | FileCheck %s --check-prefix=INVALID-COUNT-LATER-MERGE
+RUN: not llvm-profdata merge -failure-mode=all %p/Inputs/invalid-count-later.proftext %p/Inputs/invalid-count-later.proftext -o %t.out 2>&1 | FileCheck %s --check-prefix=INVALID-COUNT-LATER-MERGE
+INVALID-COUNT-LATER-MERGE: warning: {{.*}}invalid-count-later.proftext: Malformed instrumentation profile data
+INVALID-COUNT-LATER-MERGE-NEXT: warning: {{.*}}invalid-count-later.proftext: Malformed instrumentation profile data
+INVALID-COUNT-LATER-MERGE-NEXT: error: No profiles could be merged.
 
 2- Detect bad hash
-RUN: not llvm-profdata show %p/Inputs/bad-hash.proftext 2>&1 | FileCheck %s --check-prefix=BAD-HASH
-RUN: not llvm-profdata merge %p/Inputs/bad-hash.proftext %p/Inputs/bad-hash.proftext -o %t.out 2>&1 | FileCheck %s --check-prefix=BAD-HASH
-BAD-HASH: error: {{.*}}bad-hash.proftext: Malformed instrumentation profile data
+RUN: not llvm-profdata show %p/Inputs/bad-hash.proftext 2>&1 | FileCheck %s --check-prefix=BAD-HASH-SHOW
+BAD-HASH-SHOW: error: {{.*}}bad-hash.proftext: Malformed instrumentation profile data
+
+RUN: not llvm-profdata merge %p/Inputs/bad-hash.proftext %p/Inputs/bad-hash.proftext -o %t.out 2>&1 | FileCheck %s --check-prefix=BAD-HASH-MERGE
+BAD-HASH-MERGE: warning: {{.*}}bad-hash.proftext: Malformed instrumentation profile data
+BAD-HASH-NEXT: error: No profiles could be merged.
 
 3- Detect no counts
 RUN: not llvm-profdata show %p/Inputs/no-counts.proftext 2>&1 | FileCheck %s --check-prefix=NO-COUNTS

tools/llvm-profdata/llvm-profdata.cpp

@@ -85,6 +85,15 @@ static void exitWithErrorCode(std::error_code EC, StringRef Whence = "") {
 
 namespace {
 enum ProfileKinds { instr, sample };
+enum FailureMode { failIfAnyAreInvalid, failIfAllAreInvalid };
+}
+
+static void warnOrExitGivenError(FailureMode FailMode, std::error_code EC,
+                                 StringRef Whence = "") {
+  if (FailMode == failIfAnyAreInvalid)
+    exitWithErrorCode(EC, Whence);
+  else
+    warn(EC.message(), Whence);
 }
 
 static void handleMergeWriterError(Error E, StringRef WhenceFile = "",
@@ -174,33 +183,16 @@ typedef SmallVector<WeightedFile, 5> WeightedFileVector;
 struct WriterContext {
   std::mutex Lock;
   InstrProfWriter Writer;
-  Error Err;
-  std::string ErrWhence;
+  std::vector<std::pair<Error, std::string>> Errors;
   std::mutex &ErrLock;
   SmallSet<instrprof_error, 4> &WriterErrorCodes;
 
   WriterContext(bool IsSparse, std::mutex &ErrLock,
                 SmallSet<instrprof_error, 4> &WriterErrorCodes)
-      : Lock(), Writer(IsSparse), Err(Error::success()), ErrWhence(""),
-        ErrLock(ErrLock), WriterErrorCodes(WriterErrorCodes) {}
+      : Lock(), Writer(IsSparse), Errors(), ErrLock(ErrLock),
+        WriterErrorCodes(WriterErrorCodes) {}
 };
 
-/// Determine whether an error is fatal for profile merging.
-static bool isFatalError(instrprof_error IPE) {
-  switch (IPE) {
-  default:
-    return true;
-  case instrprof_error::success:
-  case instrprof_error::eof:
-  case instrprof_error::unknown_function:
-  case instrprof_error::hash_mismatch:
-  case instrprof_error::count_mismatch:
-  case instrprof_error::counter_overflow:
-  case instrprof_error::value_site_count_mismatch:
-    return false;
-  }
-}
-
 /// Computer the overlap b/w profile BaseFilename and TestFileName,
 /// and store the program level result to Overlap.
 static void overlapInput(const std::string &BaseFilename,
@@ -213,7 +205,7 @@ static void overlapInput(const std::string &BaseFilename,
     // Skip the empty profiles by returning sliently.
     instrprof_error IPE = InstrProfError::take(std::move(E));
     if (IPE != instrprof_error::empty_raw_profile)
-      WC->Err = make_error<InstrProfError>(IPE);
+      WC->Errors.emplace_back(make_error<InstrProfError>(IPE), TestFilename);
     return;
   }
 
@@ -232,21 +224,17 @@ static void loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
                       WriterContext *WC) {
   std::unique_lock<std::mutex> CtxGuard{WC->Lock};
 
-  // If there's a pending hard error, don't do more work.
-  if (WC->Err)
-    return;
-
   // Copy the filename, because llvm::ThreadPool copied the input "const
   // WeightedFile &" by value, making a reference to the filename within it
   // invalid outside of this packaged task.
-  WC->ErrWhence = Input.Filename;
+  std::string Filename = Input.Filename;
 
   auto ReaderOrErr = InstrProfReader::create(Input.Filename);
   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->Err = make_error<InstrProfError>(IPE);
+      WC->Errors.emplace_back(make_error<InstrProfError>(IPE), Filename);
     return;
   }
 
@@ -254,9 +242,11 @@ static void loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
   bool IsIRProfile = Reader->isIRLevelProfile();
   bool HasCSIRProfile = Reader->hasCSIRLevelProfile();
   if (WC->Writer.setIsIRLevelProfile(IsIRProfile, HasCSIRProfile)) {
-    WC->Err = make_error<StringError>(
-        "Merge IR generated profile with Clang generated profile.",
-        std::error_code());
+    WC->Errors.emplace_back(
+        make_error<StringError>(
+            "Merge IR generated profile with Clang generated profile.",
+            std::error_code()),
+        Filename);
     return;
   }
 
@@ -279,30 +269,23 @@ static void loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
                              FuncName, firstTime);
     });
   }
-  if (Reader->hasError()) {
-    if (Error E = Reader->getError()) {
-      instrprof_error IPE = InstrProfError::take(std::move(E));
-      if (isFatalError(IPE))
-        WC->Err = make_error<InstrProfError>(IPE);
-    }
-  }
+  if (Reader->hasError())
+    if (Error E = Reader->getError())
+      WC->Errors.emplace_back(std::move(E), Filename);
 }
 
 /// Merge the \p Src writer context into \p Dst.
 static void mergeWriterContexts(WriterContext *Dst, WriterContext *Src) {
-  // If we've already seen a hard error, continuing with the merge would
-  // clobber it.
-  if (Dst->Err || Src->Err)
-    return;
+  for (auto &ErrorPair : Src->Errors)
+    Dst->Errors.push_back(std::move(ErrorPair));
+  Src->Errors.clear();
 
-  bool Reported = false;
   Dst->Writer.mergeRecordsFromWriter(std::move(Src->Writer), [&](Error E) {
-    if (Reported) {
-      consumeError(std::move(E));
-      return;
-    }
-    Reported = true;
-    Dst->Err = std::move(E);
+    instrprof_error IPE = InstrProfError::take(std::move(E));
+    std::unique_lock<std::mutex> ErrGuard{Dst->ErrLock};
+    bool firstTime = Dst->WriterErrorCodes.insert(IPE).second;
+    if (firstTime)
+      warn(toString(make_error<InstrProfError>(IPE)));
   });
 }
 
@@ -310,7 +293,7 @@ static void mergeInstrProfile(const WeightedFileVector &Inputs,
                               SymbolRemapper *Remapper,
                               StringRef OutputFilename,
                               ProfileFormat OutputFormat, bool OutputSparse,
-                              unsigned NumThreads) {
+                              unsigned NumThreads, FailureMode FailMode) {
   if (OutputFilename.compare("-") == 0)
     exitWithError("Cannot write indexed profdata format to stdout.");
 
@@ -365,20 +348,18 @@ static void mergeInstrProfile(const WeightedFileVector &Inputs,
     } while (Mid > 0);
   }
 
-  // Handle deferred hard errors encountered during merging.
+  // Handle deferred errors encountered during merging. If the number of errors
+  // is equal to the number of inputs the merge failed.
+  unsigned NumErrors = 0;
   for (std::unique_ptr<WriterContext> &WC : Contexts) {
-    if (!WC->Err)
-      continue;
-    if (!WC->Err.isA<InstrProfError>())
-      exitWithError(std::move(WC->Err), WC->ErrWhence);
-
-    instrprof_error IPE = InstrProfError::take(std::move(WC->Err));
-    if (isFatalError(IPE))
-      exitWithError(make_error<InstrProfError>(IPE), WC->ErrWhence);
-    else
-      warn(toString(make_error<InstrProfError>(IPE)),
-           WC->ErrWhence);
+    for (auto &ErrorPair : WC->Errors) {
+      ++NumErrors;
+      warn(toString(std::move(ErrorPair.first)), ErrorPair.second);
+    }
   }
+  if (NumErrors == Inputs.size() ||
+      (NumErrors > 0 && FailMode == failIfAnyAreInvalid))
+    exitWithError("No profiles could be merged.");
 
   std::error_code EC;
   raw_fd_ostream Output(OutputFilename.data(), EC, sys::fs::OF_None);
@@ -458,10 +439,12 @@ static void populateProfileSymbolList(MemoryBuffer *Buffer,
     PSL.add(symbol);
 }
 
-static void
-mergeSampleProfile(const WeightedFileVector &Inputs, SymbolRemapper *Remapper,
-                   StringRef OutputFilename, ProfileFormat OutputFormat,
-                   StringRef ProfileSymbolListFile, bool CompressProfSymList) {
+static void mergeSampleProfile(const WeightedFileVector &Inputs,
+                               SymbolRemapper *Remapper,
+                               StringRef OutputFilename,
+                               ProfileFormat OutputFormat,
+                               StringRef ProfileSymbolListFile,
+                               bool CompressProfSymList, FailureMode FailMode) {
   using namespace sampleprof;
   StringMap<FunctionSamples> ProfileMap;
   SmallVector<std::unique_ptr<sampleprof::SampleProfileReader>, 5> Readers;
@@ -469,8 +452,10 @@ mergeSampleProfile(const WeightedFileVector &Inputs, SymbolRemapper *Remapper,
   sampleprof::ProfileSymbolList WriterList;
   for (const auto &Input : Inputs) {
     auto ReaderOrErr = SampleProfileReader::create(Input.Filename, Context);
-    if (std::error_code EC = ReaderOrErr.getError())
-      exitWithErrorCode(EC, Input.Filename);
+    if (std::error_code EC = ReaderOrErr.getError()) {
+      warnOrExitGivenError(FailMode, EC, Input.Filename);
+      continue;
+    }
 
     // We need to keep the readers around until after all the files are
     // read so that we do not lose the function names stored in each
@@ -478,8 +463,11 @@ mergeSampleProfile(const WeightedFileVector &Inputs, SymbolRemapper *Remapper,
     // merged profile map.
     Readers.push_back(std::move(ReaderOrErr.get()));
     const auto Reader = Readers.back().get();
-    if (std::error_code EC = Reader->read())
-      exitWithErrorCode(EC, Input.Filename);
+    if (std::error_code EC = Reader->read()) {
+      warnOrExitGivenError(FailMode, EC, Input.Filename);
+      Readers.pop_back();
+      continue;
+    }
 
     StringMap<FunctionSamples> &Profiles = Reader->getProfiles();
     for (StringMap<FunctionSamples>::iterator I = Profiles.begin(),
@@ -625,6 +613,12 @@ static int merge_main(int argc, const char *argv[]) {
           clEnumValN(PF_Text, "text", "Text encoding"),
           clEnumValN(PF_GCC, "gcc",
                      "GCC encoding (only meaningful for -sample)")));
+  cl::opt<FailureMode> FailureMode(
+      "failure-mode", cl::init(failIfAnyAreInvalid), cl::desc("Failure mode:"),
+      cl::values(clEnumValN(failIfAnyAreInvalid, "any",
+                            "Fail if any profile is invalid."),
+                 clEnumValN(failIfAllAreInvalid, "all",
+                            "Fail only if all profiles are invalid.")));
   cl::opt<bool> OutputSparse("sparse", cl::init(false),
       cl::desc("Generate a sparse profile (only meaningful for -instr)"));
   cl::opt<unsigned> NumThreads(
@@ -669,11 +663,11 @@ static int merge_main(int argc, const char *argv[]) {
 
   if (ProfileKind == instr)
     mergeInstrProfile(WeightedInputs, Remapper.get(), OutputFilename,
-                      OutputFormat, OutputSparse, NumThreads);
+                      OutputFormat, OutputSparse, NumThreads, FailureMode);
   else
     mergeSampleProfile(WeightedInputs, Remapper.get(), OutputFilename,
                        OutputFormat, ProfileSymbolListFile,
-                       CompressProfSymList);
+                       CompressProfSymList, FailureMode);
 
   return 0;
 }