[llvm-exegesis] Use explicit error classes for different snippet crashes

[boomanaiden154]

This patch switches to using explicit snippet crashes that contain more information about the specific type of error (like the address for a segmentation fault) that occurred. All these new error classes inherit from SnippetExecutionFailure to allow for easily grabbing all of them in addition to filtering for specific types using the standard LLVM error primitives.

[llvmbot]

@llvm/pr-subscribers-tools-llvm-exegesis

Author: Aiden Grossman (boomanaiden154)

Changes

This patch special cases the segfault case for the subprocess executor, giving the exact address of a segfault when one occurs in the subprocess. This makes it a lot easier to debug where things are going wrong in the snippet.

---

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

4 Files Affected:

• (modified) llvm/test/tools/llvm-exegesis/X86/latency/subprocess-segfault.s (+2-2)
• (modified) llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp (+2-2)
• (modified) llvm/tools/llvm-exegesis/lib/Error.cpp (+16-1)
• (modified) llvm/tools/llvm-exegesis/lib/Error.h (+8-1)

diff --git a/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-segfault.s b/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-segfault.s
index 92b08e1c18c62..db04f8522e4e0 100644
--- a/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-segfault.s
+++ b/llvm/test/tools/llvm-exegesis/X86/latency/subprocess-segfault.s
@@ -2,7 +2,7 @@
 
 # RUN: llvm-exegesis -mtriple=x86_64-unknown-unknown -mode=latency -snippets-file=%s -execution-mode=subprocess | FileCheck %s
 
-# CHECK: error:           'The benchmarking subprocess sent unexpected signal: Segmentation fault'
+# CHECK: error:           The snippet crashed with signal Segmentation fault at address 10000
 
-# LLVM-EXEGESIS-DEFREG RBX 0
+# LLVM-EXEGESIS-DEFREG RBX 10000
 movq (%rbx), %rax
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
index 85375dec2a44c..f5d73a8bd6a31 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkRunner.cpp
@@ -374,8 +374,8 @@ class SubProcessFunctionExecutorImpl
     }
 
     return make_error<SnippetCrash>(
-        "The benchmarking subprocess sent unexpected signal: " +
-        Twine(strsignal(ChildSignalInfo.si_signo)));
+        ChildSignalInfo.si_signo,
+        reinterpret_cast<intptr_t>(ChildSignalInfo.si_addr));
   }
 
   [[noreturn]] void prepareAndRunBenchmark(int Pipe,
diff --git a/llvm/tools/llvm-exegesis/lib/Error.cpp b/llvm/tools/llvm-exegesis/lib/Error.cpp
index 51ce41bf00bf5..213e54e3bcf53 100644
--- a/llvm/tools/llvm-exegesis/lib/Error.cpp
+++ b/llvm/tools/llvm-exegesis/lib/Error.cpp
@@ -8,6 +8,10 @@
 
 #include "Error.h"
 
+#ifdef LLVM_ON_UNIX
+#include <string.h>
+#endif // LLVM_ON_UNIX
+
 namespace llvm {
 namespace exegesis {
 
@@ -21,7 +25,18 @@ std::error_code ClusteringError::convertToErrorCode() const {
 
 char SnippetCrash::ID;
 
-void SnippetCrash::log(raw_ostream &OS) const { OS << Msg; }
+void SnippetCrash::log(raw_ostream &OS) const {
+  if (SISignalNumber == -1) {
+    OS << Msg;
+    return;
+  }
+#ifdef LLVM_ON_UNIX
+  OS << "The snippet crashed with signal " << strsignal(SISignalNumber)
+     << " at address " << Twine::utohexstr(SIAddress);
+#else
+  OS << "The snippet crashed with a signal";
+#endif // LLVM_ON_UNIX
+}
 
 std::error_code SnippetCrash::convertToErrorCode() const {
   return inconvertibleErrorCode();
diff --git a/llvm/tools/llvm-exegesis/lib/Error.h b/llvm/tools/llvm-exegesis/lib/Error.h
index e5fa093e6e125..8d3f394ed8d6e 100644
--- a/llvm/tools/llvm-exegesis/lib/Error.h
+++ b/llvm/tools/llvm-exegesis/lib/Error.h
@@ -12,6 +12,8 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Error.h"
 
+#include <cstdint>
+
 namespace llvm {
 namespace exegesis {
 
@@ -41,7 +43,10 @@ class ClusteringError : public ErrorInfo<ClusteringError> {
 class SnippetCrash : public ErrorInfo<SnippetCrash> {
 public:
   static char ID;
-  SnippetCrash(const Twine &S) : Msg(S.str()) {}
+  SnippetCrash(const Twine &S)
+      : Msg(S.str()), SIAddress(-1), SISignalNumber(-1) {}
+  SnippetCrash(int SISignalNumber_, intptr_t SIAddress_)
+      : Msg(""), SIAddress(SIAddress_), SISignalNumber(SISignalNumber_) {}
 
   void log(raw_ostream &OS) const override;
 
@@ -49,6 +54,8 @@ class SnippetCrash : public ErrorInfo<SnippetCrash> {
 
 private:
   std::string Msg;
+  intptr_t SIAddress;
+  int SISignalNumber;
 };
 
 } // namespace exegesis

[boomanaiden154]

Bump on this when reviewers have a chance. Thanks!

[legrosbuffle]

Sorry forgot to hit send.

[boomanaiden154]

@legrosbuffle This should roughly file the design that you outlined when we met last week (might've missed a minor detail or two, my notes ended up being relatively sparse). Two slight deviations:

• Added a new error class, SnippetSignal as the only other error type we need involves dealing with signals, so having a first-class method for dealing with that seems to make sense.

I also switched one failure mode in the subprocess function executor from creating a SnippetCrash/new equivalent to Failure as the failure modes there do happen in the generated snippet, but they happen during the snippet rather than the user-defined code and thus should be classified as failures within llvm-exegesis rather than with the snippet itself. The unknown exit code is borderline and could be considered a snippet failure, but I think leaving it as a failure is fine for now.

Would appreciate a review when you get a chance. Thank you very much for suggesting this design. This is definitely a lot cleaner.

[legrosbuffle]

Design looks good to me; just a couple remarks on the code.