From 378051061ea63a23be9199c71c47e828bac2ea83 Mon Sep 17 00:00:00 2001 From: Emery Conrad Date: Fri, 21 Aug 2026 14:14:47 -0500 Subject: [PATCH 1/2] Report the dlopen failure reason on stderr DynamicLibraryManager::loadLibrary got the dlerror() text from platform::DLOpen but only emitted it under LLVM_DEBUG. cppjit's load_library builds its Python error from captured stderr, so the reason was empty. Emit the text to std::cerr; cppjit captures only std::cerr's rdbuf, not llvm::errs(). Paths.cpp already calls ::dlerror() to build this text. Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop) --- lib/CppInterOp/DynamicLibraryManager.cpp | 4 +++ .../CppInterOp/DynamicLibraryManagerTest.cpp | 33 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/lib/CppInterOp/DynamicLibraryManager.cpp b/lib/CppInterOp/DynamicLibraryManager.cpp index a1ddd57ab..36828cd9f 100644 --- a/lib/CppInterOp/DynamicLibraryManager.cpp +++ b/lib/CppInterOp/DynamicLibraryManager.cpp @@ -24,6 +24,7 @@ #endif #include +#include #include #include @@ -370,6 +371,9 @@ DynamicLibraryManager::loadLibrary(StringRef libStem, bool permanent, // message. // TODO: Implement callbacks + if (!errMsg.empty()) + std::cerr << errMsg << '\n'; + LLVM_DEBUG(dbgs() << "DynamicLibraryManager::loadLibrary(): " << errMsg); return kLoadLibLoadError; diff --git a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp index 136d5acc6..10cdbfedf 100644 --- a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp +++ b/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,36 @@ 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 << "'"; + + 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."; From 1764cbcd7c7b4ac5e98a0b96904b9ab0dfb30d25 Mon Sep 17 00:00:00 2001 From: Emery Conrad Date: Thu, 3 Sep 2026 12:13:38 -0500 Subject: [PATCH 2/2] Add an optional reason out-parameter to LoadLibrary cppjit builds its Python error from the text a failed LoadLibrary writes to stderr, which the caller has to capture. Callers can now pass a std::string* and get the dlerror() text, or a not-found note when the lookup fails, directly. When the pointer is set the reason is not also written to stderr. std::string* cannot cross the C ABI, so the generated C wrapper is suppressed and cppinterop_LoadLibrary(const char*, bool) is kept by hand in CXCppInterOp.cpp. Co-developed-with-the-help-of: Claude Code (Fable 5.1, human in the loop) --- include/CppInterOp/CXCppInterOp.h | 4 +++ lib/CppInterOp/CXCppInterOp.cpp | 5 ++++ lib/CppInterOp/CppInterOp.cpp | 20 ++++++++++++-- lib/CppInterOp/CppInterOp.td | 10 ++++++- lib/CppInterOp/CppInterOpInterpreter.h | 24 ++++++++++++----- lib/CppInterOp/DynamicLibraryManager.cpp | 14 +++++----- lib/CppInterOp/DynamicLibraryManager.h | 5 +++- unittests/CppInterOp/CAPITest.cpp | 6 +++++ .../CppInterOp/DynamicLibraryManagerTest.cpp | 27 +++++++++++++++++++ 9 files changed, 98 insertions(+), 17 deletions(-) diff --git a/include/CppInterOp/CXCppInterOp.h b/include/CppInterOp/CXCppInterOp.h index 95f025f53..baa23b701 100644 --- a/include/CppInterOp/CXCppInterOp.h +++ b/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 diff --git a/lib/CppInterOp/CXCppInterOp.cpp b/lib/CppInterOp/CXCppInterOp.cpp index 09c32cfa7..c2347d4be 100644 --- a/lib/CppInterOp/CXCppInterOp.cpp +++ b/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 diff --git a/lib/CppInterOp/CppInterOp.cpp b/lib/CppInterOp/CppInterOp.cpp index 7982a3f4c..1c58f9f84 100644 --- a/lib/CppInterOp/CppInterOp.cpp +++ b/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); } diff --git a/lib/CppInterOp/CppInterOp.td b/lib/CppInterOp/CppInterOp.td index 4f4c45e86..82f79750c 100644 --- a/lib/CppInterOp/CppInterOp.td +++ b/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"> ]; } diff --git a/lib/CppInterOp/CppInterOpInterpreter.h b/lib/CppInterOp/CppInterOpInterpreter.h index cd3a9ba73..d11de0935 100644 --- a/lib/CppInterOp/CppInterOpInterpreter.h +++ b/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) { diff --git a/lib/CppInterOp/DynamicLibraryManager.cpp b/lib/CppInterOp/DynamicLibraryManager.cpp index 36828cd9f..3b93e10eb 100644 --- a/lib/CppInterOp/DynamicLibraryManager.cpp +++ b/lib/CppInterOp/DynamicLibraryManager.cpp @@ -344,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") << ", " @@ -364,17 +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 - if (!errMsg.empty()) - std::cerr << errMsg << '\n'; + if (errMsg) + *errMsg = loadErr; + else if (!loadErr.empty()) + std::cerr << loadErr << '\n'; - LLVM_DEBUG(dbgs() << "DynamicLibraryManager::loadLibrary(): " << errMsg); + LLVM_DEBUG(dbgs() << "DynamicLibraryManager::loadLibrary(): " << loadErr); return kLoadLibLoadError; } diff --git a/lib/CppInterOp/DynamicLibraryManager.h b/lib/CppInterOp/DynamicLibraryManager.h index 167bd0c7c..54c5c8776 100644 --- a/lib/CppInterOp/DynamicLibraryManager.h +++ b/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); diff --git a/unittests/CppInterOp/CAPITest.cpp b/unittests/CppInterOp/CAPITest.cpp index 71f709780..aba59c709 100644 --- a/unittests/CppInterOp/CAPITest.cpp +++ b/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 {}; }"); diff --git a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp index 10cdbfedf..38a160b45 100644 --- a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp +++ b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp @@ -99,6 +99,33 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, DynamicLibraryManager_LoadFailureReason) { 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)); }