[llvm] [LTO] Avoid assert fail on failed pass plugin load (PR #96691)

[Joel E. Denny via llvm-commits]

https://github.com/jdenny-ornl updated https://github.com/llvm/llvm-project/pull/96691

>From 26d93acf02d2ab8933ebdf402015a068ba3f33a1 Mon Sep 17 00:00:00 2001
From: "Joel E. Denny" <jdenny.ornl at gmail.com>
Date: Tue, 25 Jun 2024 15:42:16 -0400
Subject: [PATCH 1/2] [LTO] Avoid assert fail on failed pass plugin load

Without this patch, passing -load-pass-plugin=nonexistent.so to
llvm-lto2 produces a backtrace because LTOBackend.cpp does not handle
the error correctly:

```
Failed to load passes from 'nonexistant.so'. Request ignored.
Expected<T> must be checked before access or destruction.
Unchecked Expected<T> contained error:
Could not load library 'nonexistant.so': nonexistant.so: cannot open shared object file: No such file or directoryPLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
```

Any tool using `lto::Config::PassPlugins` should suffer similarly.

Based on the message "Request ignored" and the continue statement in
the implementation, this patch assumes the intention was not to cause
the program to fail.
---
 llvm/lib/LTO/LTOBackend.cpp            |  5 +++--
 llvm/test/Feature/load_plugin_error.ll | 19 ++++++++++++++++---
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 84a69d9ff1a1f..7931108c0ec99 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -192,8 +192,9 @@ static void RegisterPassPlugins(ArrayRef<std::string> PassPlugins,
   for (auto &PluginFN : PassPlugins) {
     auto PassPlugin = PassPlugin::Load(PluginFN);
     if (!PassPlugin) {
-      errs() << "Failed to load passes from '" << PluginFN
-             << "'. Request ignored.\n";
+      errs() << "Failed to load passes from plugin '" << PluginFN
+             << "' (request ignored): " << toString(PassPlugin.takeError())
+             <<  "\n";
       continue;
     }
 
diff --git a/llvm/test/Feature/load_plugin_error.ll b/llvm/test/Feature/load_plugin_error.ll
index 24a7cce5d8d39..2b860a901ee41 100644
--- a/llvm/test/Feature/load_plugin_error.ll
+++ b/llvm/test/Feature/load_plugin_error.ll
@@ -1,5 +1,18 @@
-; REQUIRES: plugins, examples
+; REQUIRES: plugins
 ; UNSUPPORTED: target={{.*windows.*}}
 
-; RUN: not opt < %s -load-pass-plugin=%t/nonexistant.so -disable-output 2>&1 | FileCheck %s
-; CHECK: Could not load library {{.*}}nonexistant.so
+; RUN: not opt < %s -load-pass-plugin=%t/nonexistent.so -disable-output 2>&1 | FileCheck %s
+
+; RUN: opt %s -o %t.o
+; RUN: llvm-lto2 run -load-pass-plugin=%t/nonexistent.so %t.o -o %t \
+; RUN:     -r %t.o,test 2>&1 | \
+; RUN:   FileCheck %s
+
+; CHECK: Could not load library {{.*}}nonexistent.so
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @test() {
+  ret void
+}

>From 35090612646933e667bf84c77e6d7f489395b495 Mon Sep 17 00:00:00 2001
From: "Joel E. Denny" <jdenny.ornl at gmail.com>
Date: Tue, 25 Jun 2024 18:07:28 -0400
Subject: [PATCH 2/2] Terminate on failed plugin

Based on the message "Request ignored" and the continue statement, the
intention was apparently to continue on failure to load a plugin.
However, no one appears to rely on that behavior now given that it
crashes instead, and terminating is consistent with opt.
---
 llvm/lib/LTO/LTOBackend.cpp            | 9 ++-------
 llvm/test/Feature/load_plugin_error.ll | 2 +-
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 7931108c0ec99..d5d642f0d25e6 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -191,13 +191,8 @@ static void RegisterPassPlugins(ArrayRef<std::string> PassPlugins,
   // Load requested pass plugins and let them register pass builder callbacks
   for (auto &PluginFN : PassPlugins) {
     auto PassPlugin = PassPlugin::Load(PluginFN);
-    if (!PassPlugin) {
-      errs() << "Failed to load passes from plugin '" << PluginFN
-             << "' (request ignored): " << toString(PassPlugin.takeError())
-             <<  "\n";
-      continue;
-    }
-
+    if (!PassPlugin)
+      report_fatal_error(PassPlugin.takeError(), /*gen_crash_diag=*/false);
     PassPlugin->registerPassBuilderCallbacks(PB);
   }
 }
diff --git a/llvm/test/Feature/load_plugin_error.ll b/llvm/test/Feature/load_plugin_error.ll
index 2b860a901ee41..dc6e037d11387 100644
--- a/llvm/test/Feature/load_plugin_error.ll
+++ b/llvm/test/Feature/load_plugin_error.ll
@@ -4,7 +4,7 @@
 ; RUN: not opt < %s -load-pass-plugin=%t/nonexistent.so -disable-output 2>&1 | FileCheck %s
 
 ; RUN: opt %s -o %t.o
-; RUN: llvm-lto2 run -load-pass-plugin=%t/nonexistent.so %t.o -o %t \
+; RUN: not llvm-lto2 run -load-pass-plugin=%t/nonexistent.so %t.o -o %t \
 ; RUN:     -r %t.o,test 2>&1 | \
 ; RUN:   FileCheck %s