Report the dlopen failure reason from LoadLibrary

include/CppInterOp/CXCppInterOp.h

@@ -63,6 +63,10 @@ CPPINTEROP_API intptr_t cppinterop_Evaluate(const char* code, bool* HadError);
 CPPINTEROP_API CppInterOpArray
 cppinterop_GetClassTemplatedMethods(const char* name, CppConstDeclRef parent);
 
+/// C-ABI bridge for \c Cpp::LoadLibrary. The optional \c std::string*
+/// reason argument is C++-only, so the generated wrapper is suppressed.
+CPPINTEROP_API bool cppinterop_LoadLibrary(const char* lib_stem, bool lookup);
+
 #ifdef __cplusplus
 } // extern "C"
 #pragma clang diagnostic pop

lib/CppInterOp/CXCppInterOp.cpp

@@ -61,6 +61,11 @@ CPPINTEROP_API intptr_t cppinterop_Evaluate(const char* code, bool* HadError) {
   return Cpp::Evaluate(code, HadError);
 }
 
+// The std::string* reason argument of Cpp::LoadLibrary is C++-only.
+CPPINTEROP_API bool cppinterop_LoadLibrary(const char* lib_stem, bool lookup) {
+  return Cpp::LoadLibrary(lib_stem, lookup);
+}
+
 // GetClassTemplatedMethods returns bool AND fills a vector out-param.
 // The C wrapper drops the bool (caller checks arr.size > 0 instead).
 CPPINTEROP_API Cpp::CppInterOpArray

lib/CppInterOp/CppInterOp.cpp

@@ -5746,10 +5746,26 @@ std::string LookupLibrary(const char* lib_name) {
       getInterp().getDynamicLibraryManager()->lookupLibrary(lib_name));
 }
 
-bool LoadLibrary(const char* lib_stem, bool lookup) {
-  INTEROP_TRACE(lib_stem, lookup);
+bool LoadLibrary(const char* lib_stem, bool lookup, std::string* error) {
+  INTEROP_TRACE(lib_stem, lookup, error);
+  if (error)
+    error->clear();
+#ifdef CPPINTEROP_USE_CLING
+  // cling::Interpreter::loadLibrary has no reason channel; report what the
+  // lookup alone can tell.
   compat::Interpreter::CompilationResult res =
       getInterp().loadLibrary(lib_stem, lookup);
+  if (res != compat::Interpreter::kSuccess && error) {
+    bool NotFound =
+        lookup &&
+        getInterp().getDynamicLibraryManager()->lookupLibrary(lib_stem).empty();
+    *error = std::string(lib_stem) +
+             (NotFound ? ": library not found" : ": failed to load");
+  }
+#else
+  compat::Interpreter::CompilationResult res =
+      getInterp().loadLibrary(lib_stem, lookup, error);
+#endif
 
   return INTEROP_RETURN(res == compat::Interpreter::kSuccess);
 }

lib/CppInterOp/CppInterOp.td

@@ -245,12 +245,20 @@ GetResourceDir() function.}];
 def LoadLibrary : CppInterOpAPI {
   let Doc = [{Finds \c lib_stem considering the list of search paths and loads it by
 calling dlopen.
+\param[out] error - if non-null, receives the reason a load failed (the
+           dlerror() text, or a not-found note when lookup fails). When it
+           is set the reason is not also written to stderr. The Cling
+           backend only reports a generic note.
 \returns true on success.}];
 
+  // std::string* cannot cross the C ABI; CXCppInterOp.cpp keeps the
+  // two-argument cppinterop_LoadLibrary wrapper by hand.
+  let NoCWrapper = true;
   let ReturnType = "bool";
   let Args = [
     Arg<"const char*", "lib_stem">,
-    Arg<"bool", "lookup", "true">
+    Arg<"bool", "lookup", "true">,
+    Arg<"std::string*", "error", "nullptr">
   ];
 }
 

lib/CppInterOp/CppInterOpInterpreter.h

@@ -586,16 +586,22 @@ class Interpreter {
                                          incpaths, withSystem, withFlags);
   }
 
-  CompilationResult loadLibrary(const std::string& filename, bool lookup) {
+  CompilationResult loadLibrary(const std::string& filename, bool lookup,
+                                std::string* error = nullptr) {
     llvm::Triple triple(getCompilerInstance()->getTargetOpts().Triple);
     if (triple.isWasm()) {
+      // LCOV_EXCL_START -- no coverage lane runs the wasm path.
       // On WASM, dlopen-style canonical lookup has no effect.
       if (auto Err = inner->LoadDynamicLibrary(filename.c_str())) {
-        llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
-                                    "loadLibrary: ");
+        std::string Msg = llvm::toString(std::move(Err));
+        if (error)
+          *error = std::move(Msg);
+        else
+          llvm::errs() << "loadLibrary: " << Msg << '\n';
         return kFailure;
       }
       return kSuccess;
+      // LCOV_EXCL_STOP
     }
 
     DynamicLibraryManager* DLM = getDynamicLibraryManager();
@@ -604,9 +610,14 @@ class Interpreter {
       canonicalLib = DLM->lookupLibrary(filename);
 
     const std::string& library = lookup ? canonicalLib : filename;
-    if (!library.empty()) {
-      switch (
-          DLM->loadLibrary(library, /*permanent*/ false, /*resolved*/ true)) {
+    if (library.empty()) {
+      if (error)
+        *error = filename + ": library not found";
+      return kMoreInputExpected;
+    }
+    {
+      switch (DLM->loadLibrary(library, /*permanent*/ false, /*resolved*/ true,
+                               error)) {
       case DynamicLibraryManager::kLoadLibSuccess: // Intentional fall through
       case DynamicLibraryManager::kLoadLibAlreadyLoaded:
         return kSuccess;
@@ -618,7 +629,6 @@ class Interpreter {
         return kFailure;
       }
     }
-    return kMoreInputExpected;
   }
 
   std::string toString(const char* type, void* obj) {

lib/CppInterOp/DynamicLibraryManager.cpp

@@ -24,6 +24,7 @@
 #endif
 
 #include <fstream>
+#include <iostream>
 #include <sys/stat.h>
 #include <system_error>
 
@@ -343,7 +344,7 @@ std::string DynamicLibraryManager::lookupLibrary(
 
 DynamicLibraryManager::LoadLibResult
 DynamicLibraryManager::loadLibrary(StringRef libStem, bool permanent,
-                                   bool resolved) {
+                                   bool resolved, std::string* errMsg) {
 #define DEBUG_TYPE "Dyld::loadLibrary:"
   LLVM_DEBUG(dbgs() << "Dyld::loadLibrary: " << libStem.str() << ", "
                     << (permanent ? "permanent" : "not-permanent") << ", "
@@ -363,14 +364,19 @@ DynamicLibraryManager::loadLibrary(StringRef libStem, bool permanent,
 
   // TODO: !permanent case
 
-  std::string errMsg;
-  DyLibHandle dyLibHandle = platform::DLOpen(canonicalLoadedLib, &errMsg);
+  std::string loadErr;
+  DyLibHandle dyLibHandle = platform::DLOpen(canonicalLoadedLib, &loadErr);
   if (!dyLibHandle) {
     // We emit callback to LibraryLoadingFailed when we get error with error
     // message.
     // TODO: Implement callbacks
 
-    LLVM_DEBUG(dbgs() << "DynamicLibraryManager::loadLibrary(): " << errMsg);
+    if (errMsg)
+      *errMsg = loadErr;
+    else if (!loadErr.empty())
+      std::cerr << loadErr << '\n';
+
+    LLVM_DEBUG(dbgs() << "DynamicLibraryManager::loadLibrary(): " << loadErr);
 
     return kLoadLibLoadError;
   }

lib/CppInterOp/DynamicLibraryManager.h

@@ -163,13 +163,16 @@ class DynamicLibraryManager {
   ///\param [in] permanent - If false, the file can be unloaded later.
   ///\param [in] resolved - Whether libStem is an absolute path or resolved
   ///               from a previous call to DynamicLibraryManager::lookupLibrary
+  ///\param [out] errMsg - If non-null, receives the loader's error text on
+  ///               kLoadLibLoadError instead of it going to stderr.
   ///
   ///\returns kLoadLibSuccess on success, kLoadLibAlreadyLoaded if the library
   /// was already loaded, kLoadLibError if the library cannot be found or any
   /// other error was encountered.
   ///
   LoadLibResult loadLibrary(llvm::StringRef, bool permanent,
-                            bool resolved = false);
+                            bool resolved = false,
+                            std::string* errMsg = nullptr);
 
   void unloadLibrary(llvm::StringRef libStem);
 

unittests/CppInterOp/CAPITest.cpp

@@ -18,6 +18,12 @@ TYPED_TEST(CppInterOpTest, CAPI_DeclareAndProcess) {
   EXPECT_EQ(0, cppinterop_Process("capi_var++;"));
 }
 
+TYPED_TEST(CppInterOpTest, CAPI_LoadLibrary) {
+  TestFixture::CreateInterpreter();
+  // The hand-written two-argument wrapper still exists and forwards.
+  EXPECT_FALSE(cppinterop_LoadLibrary("no-such-cppinterop-lib", true));
+}
+
 TYPED_TEST(CppInterOpTest, CAPI_ScopeQueries) {
   TestFixture::CreateInterpreter();
   Cpp::Declare("namespace CAPINs { class Foo {}; }");

unittests/CppInterOp/DynamicLibraryManagerTest.cpp

@@ -3,8 +3,11 @@
 
 #include "clang/Basic/Version.h"
 
+#include "llvm/ADT/SmallString.h"
+
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
 
 #include "gtest/gtest.h"
 
@@ -69,6 +72,63 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, DynamicLibraryManager_Sanity) {
   // EXPECT_FALSE(Cpp::GetFunctionAddress("ret_zero"));
 }
 
+// A failed dlopen must report the dlerror() text on stderr.
+TYPED_TEST(CPPINTEROP_TEST_MODE, DynamicLibraryManager_LoadFailureReason) {
+#if defined(EMSCRIPTEN) || defined(_WIN32)
+  GTEST_SKIP() << "Test checks dlerror-style messages";
+#endif
+  if (TypeParam::isOutOfProcess)
+    GTEST_SKIP() << "Test fails for OOP JIT builds";
+
+  EXPECT_TRUE(TestFixture::CreateInterpreter());
+
+  // A file that exists but is not a shared library makes dlopen fail.
+  int FD = -1;
+  llvm::SmallString<256> BadLib;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("bad-lib", "so", FD, BadLib));
+  {
+    llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true);
+    OS << "not a shared library";
+  }
+
+  testing::internal::CaptureStderr();
+  EXPECT_FALSE(Cpp::LoadLibrary(BadLib.c_str(), /*lookup=*/false));
+  std::string Err = testing::internal::GetCapturedStderr();
+  // dlerror() names the failing file.
+  EXPECT_NE(Err.find(llvm::sys::path::filename(BadLib).str()),
+            std::string::npos)
+      << "stderr was: '" << Err << "'";
+
+  // With an out-parameter the reason goes there and not to stderr.
+  std::string Reason = "stale";
+  testing::internal::CaptureStderr();
+  EXPECT_FALSE(Cpp::LoadLibrary(BadLib.c_str(), /*lookup=*/false, &Reason));
+#ifdef CPPINTEROP_USE_CLING
+  testing::internal::GetCapturedStderr(); // cling still prints its own text
+#else
+  EXPECT_EQ(testing::internal::GetCapturedStderr(), "");
+#endif
+  EXPECT_NE(Reason.find(llvm::sys::path::filename(BadLib).str()),
+            std::string::npos)
+      << "reason was: '" << Reason << "'";
+
+  // A failed lookup reports that too.
+  EXPECT_FALSE(
+      Cpp::LoadLibrary("no-such-cppinterop-lib", /*lookup=*/true, &Reason));
+  EXPECT_NE(Reason.find("no-such-cppinterop-lib"), std::string::npos)
+      << "reason was: '" << Reason << "'";
+
+  // Success clears a stale reason.
+  std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr);
+  Cpp::AddSearchPath(llvm::sys::path::parent_path(BinaryPath).str().c_str());
+  Reason = "stale";
+  EXPECT_TRUE(Cpp::LoadLibrary("TestSharedLib", /*lookup=*/true, &Reason))
+      << "reason was: '" << Reason << "'";
+  EXPECT_EQ(Reason, "");
+
+  EXPECT_FALSE(llvm::sys::fs::remove(BadLib));
+}
+
 TYPED_TEST(CPPINTEROP_TEST_MODE, DynamicLibraryManager_BasicSymbolLookup) {
 #ifndef EMSCRIPTEN
   GTEST_SKIP() << "This test is only intended for Emscripten builds.";