[llvm] [profdata] Consume reader error if returned early (PR #163671)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-pgo

Author: Ellis Hoag (ellishg)

<details>
<summary>Changes</summary>

https://github.com/llvm/llvm-project/pull/69513 added logic to emit an error when something goes wrong in `readRawCounts()` by passing a `Warn()` lambda to capture errors. If we capture an error in this way and we also early return due to some other error (say from `readBinaryIds()`) then we will hit an assert due to an unconsumed error. This is because we don't consume the error at the end of this function.

https://github.com/llvm/llvm-project/blob/1c7ae8927439e5dbc3c1e9a9df4769ace491f9f6/llvm/tools/llvm-profdata/llvm-profdata.cpp#L864-L867

Fix this assert by calling `consumeError()` via `llvm::make_scope_exit()` when the function returns for any reason.

We might be able to simplify this by directly adding the error to `WC->Errors` in `Warn()`, but that might change the order of the errors reported and I am less confident in that change.

---
Full diff: https://github.com/llvm/llvm-project/pull/163671.diff


1 Files Affected:

- (modified) llvm/tools/llvm-profdata/llvm-profdata.cpp (+7) 


``````````diff
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 15ddb05f953ee..585a39de9985f 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
@@ -778,6 +779,12 @@ loadInput(const WeightedFile &Input, SymbolRemapper *Remapper,
   // we have more non-fatal errors from InstrProfReader in the future. How
   // should this interact with different -failure-mode?
   std::optional<std::pair<Error, std::string>> ReaderWarning;
+  auto ReaderWarningScope = llvm::make_scope_exit([&] {
+    // If we hit a different error we may still have an error in ReaderWarning.
+    // Consume it now to avoid an assert
+    if (ReaderWarning)
+      consumeError(std::move(ReaderWarning->first));
+  });
   auto Warn = [&](Error E) {
     if (ReaderWarning) {
       consumeError(std::move(E));

``````````

</details>


https://github.com/llvm/llvm-project/pull/163671