[profdata] Return Expected from showSampleProfile()

[ellishg]

#149433 broke the hwasan buildbot: https://lab.llvm.org/buildbot/#/builders/55/builds/14455 and it is unclear why it triggered the failures. As far as I can tell, the memory leak existed before because LLVMContext leaks memory when exit(1) is called, which happens in this test. To fix this, showSampleProfile() now returns Expected and the caller handles the error with ExitOnError. Then LLVMContext's stack is cleaned up when showSampleProfile() returns.

If the bots don't break with this change, I will follow up to return Expected from the remaining functions. I believe we can then remove exitWithError*() entirely and replace it with ExitOnError.

[llvmbot]

@llvm/pr-subscribers-pgo

Author: Ellis Hoag (ellishg)

Changes

#149433 broke the hwasan buildbot: https://lab.llvm.org/buildbot/#/builders/55/builds/14455 and it is unclear why it triggered the failures. As far as I can tell, the memory leak existed before because LLVMContext leaks memory when exit(1) is called, which happens in this test. To fix this, showSampleProfile() now returns Expected and the caller handles the error with ExitOnError. Then LLVMContext's stack is cleaned up when showSampleProfile() returns.

If the bots don't break with this change, I will follow up to return Expected from the remaining functions. I believe we can then remove exitWithError*() entirely and replace it with ExitOnError.

---

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

2 Files Affected:

• (modified) llvm/test/tools/llvm-profdata/sample-profile-basic.test (+1-1)
• (modified) llvm/tools/llvm-profdata/llvm-profdata.cpp (+13-8)

diff --git a/llvm/test/tools/llvm-profdata/sample-profile-basic.test b/llvm/test/tools/llvm-profdata/sample-profile-basic.test
index 0b0d37aac36cf..f8c657814f4fa 100644
--- a/llvm/test/tools/llvm-profdata/sample-profile-basic.test
+++ b/llvm/test/tools/llvm-profdata/sample-profile-basic.test
@@ -32,4 +32,4 @@ MERGE1: _Z3fooi:15422:1220
 
 5- Detect invalid text encoding (e.g. instrumentation profile text format).
 RUN: not llvm-profdata show --sample %p/Inputs/foo3bar3-1.proftext 2>&1 | FileCheck %s --check-prefix=BADTEXT
-BADTEXT: error: {{.+}}: Unrecognized sample profile encoding format
+BADTEXT: Unrecognized sample profile encoding format
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 207ae2ddd4cf2..8f868f3511143 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -3225,16 +3225,19 @@ static int showHotFunctionList(const sampleprof::SampleProfileMap &Profiles,
   return 0;
 }
 
-static int showSampleProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
+static Expected<int> showSampleProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
   if (SFormat == ShowFormat::Yaml)
-    exitWithError("YAML output is not supported for sample profiles");
+    return createStringError(
+        errc::invalid_argument,
+        "YAML output is not supported for sample profiles");
   using namespace sampleprof;
   LLVMContext Context;
   auto FS = vfs::getRealFileSystem();
   auto ReaderOrErr = SampleProfileReader::create(Filename, Context, *FS,
                                                  FSDiscriminatorPassOption);
   if (std::error_code EC = ReaderOrErr.getError())
-    exitWithErrorCode(EC, Filename);
+    return createStringError(EC, "%s: %s", Filename.c_str(),
+                             EC.message().c_str());
 
   auto Reader = std::move(ReaderOrErr.get());
   if (ShowSectionInfoOnly) {
@@ -3243,7 +3246,8 @@ static int showSampleProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
   }
 
   if (std::error_code EC = Reader->read())
-    exitWithErrorCode(EC, Filename);
+    return createStringError(EC, "%s: %s", Filename.c_str(),
+                             EC.message().c_str());
 
   if (ShowAllFunctions || FuncNameFilter.empty()) {
     if (SFormat == ShowFormat::Json)
@@ -3252,9 +3256,9 @@ static int showSampleProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
       Reader->dump(OS);
   } else {
     if (SFormat == ShowFormat::Json)
-      exitWithError(
-          "the JSON format is supported only when all functions are to "
-          "be printed");
+      return createStringError(errc::invalid_argument,
+                               "the JSON format is supported only when all "
+                               "functions are to be printed");
 
     // TODO: parse context string to support filtering by contexts.
     FunctionSamples *FS = Reader->getSamplesFor(StringRef(FuncNameFilter));
@@ -3366,6 +3370,7 @@ static int showDebugInfoCorrelation(const std::string &Filename,
 }
 
 static int show_main(StringRef ProgName) {
+  ExitOnError ExitOnErr;
   if (Filename.empty() && DebugInfoFilename.empty())
     exitWithError(
         "the positional argument '<profdata-file>' is required unless '--" +
@@ -3394,7 +3399,7 @@ static int show_main(StringRef ProgName) {
   if (ShowProfileKind == instr)
     return showInstrProfile(SFormat, OS);
   if (ShowProfileKind == sample)
-    return showSampleProfile(SFormat, OS);
+    return ExitOnErr(showSampleProfile(SFormat, OS));
   return showMemProfProfile(SFormat, OS);
 }
 

[mingmingl-llvm]

The change itself lgtm.

It's not very clear to me why #149433 caused failure in the first place and whether this change will fix it though..

fyi, https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild#try-local-changes is useful to reproduce a certain (e.g., sanitizer) build bot failures with a local llvm repo and verify fixes, but the running multi-stage script can take O(hours). Based on the host info of one hwsan worker (https://lab.llvm.org/buildbot/#/workers/17), I'm guessing https://github.com/llvm/llvm-zorg/blob/main/zorg/buildbot/builders/sanitizers/buildbot_bootstrap_hwasan.sh is the script.