From a4242e94c27f4edb9d6602588833f63f7821a62e Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Wed, 10 Jul 2024 17:27:57 +0000 Subject: [PATCH 01/25] Library autoloader --- include/clang/Interpreter/CppInterOp.h | 8 + lib/Interpreter/CppInterOp.cpp | 172 ++++++++++++++++++ lib/Interpreter/DynamicLibraryManager.cpp | 7 +- .../DynamicLibraryManagerSymbol.cpp | 4 +- unittests/CppInterOp/CMakeLists.txt | 4 + .../CppInterOp/DynamicLibraryManagerTest.cpp | 95 ++++++++-- .../CppInterOp/TestSharedLib1/CMakeLists.txt | 12 ++ .../TestSharedLib1/TestSharedLib1.cpp | 5 + .../TestSharedLib1/TestSharedLib1.h | 13 ++ 9 files changed, 303 insertions(+), 17 deletions(-) create mode 100644 unittests/CppInterOp/TestSharedLib1/CMakeLists.txt create mode 100644 unittests/CppInterOp/TestSharedLib1/TestSharedLib1.cpp create mode 100644 unittests/CppInterOp/TestSharedLib1/TestSharedLib1.h diff --git a/include/clang/Interpreter/CppInterOp.h b/include/clang/Interpreter/CppInterOp.h index 3d4f6a855..79e098bc7 100644 --- a/include/clang/Interpreter/CppInterOp.h +++ b/include/clang/Interpreter/CppInterOp.h @@ -757,6 +757,14 @@ namespace Cpp { unsigned complete_line = 1U, unsigned complete_column = 1U); + /// Set libraries autoload. + ///\param[in] autoload - true = libraries autoload is on. + CPPINTEROP_API void SetLibrariesAutoload(bool autoload = true); + + /// Get libraries autoload. + ///\returns LibraryAutoLoad state (true = libraries autoload is on). + CPPINTEROP_API bool GetLibrariesAutoload(); + } // end namespace Cpp #endif // CPPINTEROP_CPPINTEROP_H diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index 15a177281..339751dc1 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -32,6 +32,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Demangle/Demangle.h" +#include "llvm/ExecutionEngine/Orc/Core.h" #include "llvm/Support/Casting.h" #include "llvm/Support/Debug.h" #include "llvm/Support/raw_os_ostream.h" @@ -3546,4 +3547,175 @@ namespace Cpp { complete_column); } +#define DEBUG_TYPE "autoload" + + static inline std::string DemangleNameForDlsym(const std::string& name) { + std::string nameForDlsym = name; + +#if defined(R__MACOSX) || defined(R__WIN32) + // The JIT gives us a mangled name which has an additional leading underscore + // on macOS and Windows, for instance __ZN8TRandom34RndmEv. However, dlsym + // requires us to remove it. + // FIXME: get this information from the DataLayout via getGlobalPrefix()! + if (nameForDlsym[0] == '_') + nameForDlsym.erase(0, 1); +#endif //R__MACOSX + + return nameForDlsym; + } + + class AutoLoadLibrarySearchGenerator; + static AutoLoadLibrarySearchGenerator *ALLSG = nullptr; + + class AutoLoadLibrarySearchGenerator : public llvm::orc::DefinitionGenerator { + public: + bool Enabled = false; + + // Lazy materialization unit class helper + class AutoloadLibraryMU : public llvm::orc::MaterializationUnit { + std::string lib; + llvm::orc::SymbolNameVector syms; + public: + AutoloadLibraryMU(const std::string &Library, const llvm::orc::SymbolNameVector &Symbols) + : MaterializationUnit({getSymbolFlagsMap(Symbols), nullptr}), lib(Library), syms(Symbols) {} + + StringRef getName() const override { + return ""; + } + + void materialize(std::unique_ptr R) override { + if (!ALLSG || !ALLSG->Enabled) { + R->failMaterialization(); + return; + } + + LLVM_DEBUG(dbgs() << "Materialize " << lib << " syms=" << syms); + + auto& I = getInterp(); + auto DLM = I.getDynamicLibraryManager(); + + llvm::orc::SymbolMap loadedSymbols; + llvm::orc::SymbolNameSet failedSymbols; + bool loadedLibrary = false; + + for (auto symbol : syms) { + std::string symbolStr = (*symbol).str(); + std::string nameForDlsym = DemangleNameForDlsym(symbolStr); + + // Check if the symbol is available without loading the library. + void *addr = llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(nameForDlsym); + + if (!addr && !loadedLibrary) { + // Try to load the library which should provide the symbol definition. + if (DLM->loadLibrary(lib, false) != DynamicLibraryManager::LoadLibResult::kLoadLibSuccess) { + LLVM_DEBUG(dbgs() << "MU: Failed to load library " << lib); + string err = "MU: Failed to load library! " + lib; + perror(err.c_str()); + } else { + LLVM_DEBUG(dbgs() << "MU: Autoload library " << lib); + } + + // Only try loading the library once. + loadedLibrary = true; + + addr = llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(nameForDlsym); + } + + if (addr) { + loadedSymbols[symbol] = + llvm::orc::ExecutorSymbolDef(llvm::orc::ExecutorAddr::fromPtr(addr), JITSymbolFlags::Exported); + } else { + // Collect all failing symbols, delegate their responsibility and then + // fail their materialization. R->defineNonExistent() sounds like it + // should do that, but it's not implemented?! + failedSymbols.insert(symbol); + } + } + + if (!failedSymbols.empty()) { + auto failingMR = R->delegate(failedSymbols); + if (failingMR) { + (*failingMR)->failMaterialization(); + } + } + + if (!loadedSymbols.empty()) { + llvm::cantFail(R->notifyResolved(loadedSymbols)); + llvm::cantFail(R->notifyEmitted()); + } + } + + void discard(const llvm::orc::JITDylib &JD, const llvm::orc::SymbolStringPtr &Name) override {} + + private: + static llvm::orc::SymbolFlagsMap getSymbolFlagsMap(const llvm::orc::SymbolNameVector &Symbols) { + llvm::orc::SymbolFlagsMap map; + for (auto symbolName : Symbols) + map[symbolName] = llvm::JITSymbolFlags::Exported; + return map; + } + }; + + llvm::Error tryToGenerate(llvm::orc::LookupState &LS, llvm::orc::LookupKind K, llvm::orc::JITDylib &JD, + llvm::orc::JITDylibLookupFlags JDLookupFlags, const llvm::orc::SymbolLookupSet &Symbols) override { + if (Enabled) { + LLVM_DEBUG(dbgs() << "tryToGenerate"); + + auto& I = getInterp(); + auto DLM = I.getDynamicLibraryManager(); + + std::unordered_map found; + llvm::orc::SymbolMap NewSymbols; + for (auto &KV : Symbols) { + auto &Name = KV.first; + if ((*Name).empty()) + continue; + + auto lib = DLM->searchLibrariesForSymbol(*Name, /*searchSystem=*/true); // false? + if (lib.empty()) + continue; + + found[lib].push_back(Name); + + // Workaround: This getAddressOfGlobal call make first symbol search + // to work, immediatelly after library auto load. This approach do not + // use MU + //DLM->loadLibrary(lib, true); + //I.getAddressOfGlobal(*Name); + } + + for (auto &&KV : found) { + auto MU = std::make_unique(KV.first, std::move(KV.second)); + if (auto Err = JD.define(MU)) + return Err; + } + } + + return llvm::Error::success(); + } + }; + + void SetLibrariesAutoload(bool autoload /* = true */) { + auto& I = getInterp(); + llvm::orc::LLJIT& EE = *compat::getExecutionEngine(I); +#if CLANG_VERSION_MAJOR < 17 + llvm::orc::JITDylib& DyLib = EE.getMainJITDylib(); +#else + llvm::orc::JITDylib& DyLib = *EE.getProcessSymbolsJITDylib().get(); +#endif // CLANG_VERSION_MAJOR + + if (!ALLSG) + ALLSG = &DyLib.addGenerator(std::make_unique()); + ALLSG->Enabled = autoload; + + LLVM_DEBUG(dbgs() << "Autoload=" << (ALLSG && ALLSG->Enabled ? "ON" : "OFF")); + } + + bool GetLibrariesAutoload() { + LLVM_DEBUG(dbgs() << "Autoload is " << (ALLSG && ALLSG->Enabled ? "ON" : "OFF")); + return ALLSG && ALLSG->Enabled; + } + +#undef DEBUG_TYPE + } // end namespace Cpp diff --git a/lib/Interpreter/DynamicLibraryManager.cpp b/lib/Interpreter/DynamicLibraryManager.cpp index 630ad23a6..4265024e1 100644 --- a/lib/Interpreter/DynamicLibraryManager.cpp +++ b/lib/Interpreter/DynamicLibraryManager.cpp @@ -243,7 +243,7 @@ namespace Cpp { // get canonical path name and check if already loaded const std::string Path = platform::NormalizePath(foundDyLib); if (Path.empty()) { - LLVM_DEBUG(dbgs() << "cling::DynamicLibraryManager::lookupLibMaybeAddExt(): " + LLVM_DEBUG(dbgs() << "DynamicLibraryManager::lookupLibMaybeAddExt(): " << "error getting real (canonical) path of library " << foundDyLib << '\n'); return foundDyLib; } @@ -392,8 +392,7 @@ namespace Cpp { return; DyLibHandle dyLibHandle = nullptr; - for (DyLibs::const_iterator I = m_DyLibs.begin(), E = m_DyLibs.end(); - I != E; ++I) { + for (DyLibs::const_iterator I = m_DyLibs.begin(), E = m_DyLibs.end(); I != E; ++I) { if (I->second == canonicalLoadedLib) { dyLibHandle = I->first; break; @@ -405,7 +404,7 @@ namespace Cpp { std::string errMsg; platform::DLClose(dyLibHandle, &errMsg); if (!errMsg.empty()) { - LLVM_DEBUG(dbgs() << "cling::DynamicLibraryManager::unloadLibrary(): " + LLVM_DEBUG(dbgs() << "DynamicLibraryManager::unloadLibrary(): " << errMsg << '\n'); } diff --git a/lib/Interpreter/DynamicLibraryManagerSymbol.cpp b/lib/Interpreter/DynamicLibraryManagerSymbol.cpp index ac1857fa5..32dfc4c95 100644 --- a/lib/Interpreter/DynamicLibraryManagerSymbol.cpp +++ b/lib/Interpreter/DynamicLibraryManagerSymbol.cpp @@ -1153,8 +1153,8 @@ namespace Cpp { std::string Dyld::searchLibrariesForSymbol(StringRef mangledName, bool searchSystem/* = true*/) { #define DEBUG_TYPE "Dyld:searchLibrariesForSymbol:" - assert(!llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(mangledName.str()) && - "Library already loaded, please use dlsym!"); +// assert(!llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(mangledName.str()) && +// "Library already loaded, please use dlsym!"); assert(!mangledName.empty()); using namespace llvm::sys::path; diff --git a/unittests/CppInterOp/CMakeLists.txt b/unittests/CppInterOp/CMakeLists.txt index 25f59b394..e80de9059 100644 --- a/unittests/CppInterOp/CMakeLists.txt +++ b/unittests/CppInterOp/CMakeLists.txt @@ -50,3 +50,7 @@ set_output_directory(DynamicLibraryManagerTests add_dependencies(DynamicLibraryManagerTests TestSharedLib) #export_executable_symbols_for_plugins(TestSharedLib) add_subdirectory(TestSharedLib) +# TODO: Remove when libraryunload work correctly +add_dependencies(DynamicLibraryManagerTests TestSharedLib1) +#export_executable_symbols_for_plugins(TestSharedLib1) +add_subdirectory(TestSharedLib1) diff --git a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp index 01624ef78..fb13bcd94 100644 --- a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp +++ b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp @@ -5,6 +5,9 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" + +// Helper functions + // This function isn't referenced outside its translation unit, but it // can't use the "static" keyword because its address is used for // GetMainExecutable (since some platforms don't support taking the @@ -17,9 +20,20 @@ std::string GetExecutablePath(const char* Argv0) { return llvm::sys::fs::getMainExecutable(Argv0, MainAddr); } +static inline std::string MangleNameForDlsym(const std::string& name) { + std::string nameForDlsym = name; +#if defined(R__MACOSX) || defined(R__WIN32) + if (nameForDlsym[0] != '_') + nameForDlsym.insert (0, 1, '_'); +#endif //R__MACOSX + return nameForDlsym; +} + +// Tests + TEST(DynamicLibraryManagerTest, Sanity) { EXPECT_TRUE(Cpp::CreateInterpreter()); - EXPECT_FALSE(Cpp::GetFunctionAddress("ret_zero")); + EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_zero").c_str())); std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr); llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath); @@ -28,13 +42,9 @@ TEST(DynamicLibraryManagerTest, Sanity) { // FIXME: dlsym on mach-o takes the C-level name, however, the macho-o format // adds an additional underscore (_) prefix to the lowered names. Figure out // how to harmonize that API. -#ifdef __APPLE__ - std::string PathToTestSharedLib = - Cpp::SearchLibrariesForSymbol("_ret_zero", /*system_search=*/false); -#else + std::string PathToTestSharedLib = - Cpp::SearchLibrariesForSymbol("ret_zero", /*system_search=*/false); -#endif // __APPLE__ + Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_zero").c_str(), /*system_search=*/false); EXPECT_STRNE("", PathToTestSharedLib.c_str()) << "Cannot find: '" << PathToTestSharedLib << "' in '" << Dir.str() @@ -44,13 +54,76 @@ TEST(DynamicLibraryManagerTest, Sanity) { // Force ExecutionEngine to be created. Cpp::Process(""); // FIXME: Conda returns false to run this code on osx. -#ifndef __APPLE__ - EXPECT_TRUE(Cpp::GetFunctionAddress("ret_zero")); -#endif //__APPLE__ + EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_zero").c_str())); Cpp::UnloadLibrary("TestSharedLib"); // We have no reliable way to check if it was unloaded because posix does not // require the library to be actually unloaded but just the handle to be // invalidated... - // EXPECT_FALSE(Cpp::GetFunctionAddress("ret_zero")); + // EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_zero").c_str())); +} + +TEST(DynamicLibraryManagerTest, LibrariesAutoload) { + EXPECT_TRUE(Cpp::CreateInterpreter()); + + // Autoload by default is OFF. Symbol search must fail. + EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); + + // Libraries Search path by default is set to main executable directory. + std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr); + llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath); + Cpp::AddSearchPath(Dir.str().c_str()); + + // Find library with "rec_one" symbol defined and exported + // + // FIXME: dlsym on mach-o takes the C-level name, however, the macho-o format + // adds an additional underscore (_) prefix to the lowered names. Figure out + // how to harmonize that API. For now we use out minimal implementation of + // helper function. + std::string PathToTestSharedLib1 = + Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_one").c_str(), /*system_search=*/false); + + // If result is "" then we cannot find this library. + EXPECT_STRNE("", PathToTestSharedLib1.c_str()) + << "Cannot find: '" << PathToTestSharedLib1 << "' in '" << Dir.str() << "'"; + + // Force ExecutionEngine to be created. + Cpp::Process(""); + + // Check default Autoload is OFF + EXPECT_FALSE(Cpp::GetLibrariesAutoload()); + // Find symbol must fail (when auotload=off) + EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); + + // Autoload turn ON + Cpp::SetLibrariesAutoload(true); + // Check autorum status (must be turned ON) + EXPECT_TRUE(Cpp::GetLibrariesAutoload()); + + // FIXME: Conda returns false to run this code on osx. + EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); + // Check for some symbols (exists and not exists) + EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_not_exist").c_str())); + EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_not_exist1").c_str())); + EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_two").c_str())); + + //Cpp::UnloadLibrary("TestSharedLib1"); + //// We have no reliable way to check if it was unloaded because posix does not + //// require the library to be actually unloaded but just the handle to be + //// invalidated... + //EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); + + // Autoload turn OFF + Cpp::SetLibrariesAutoload(false); + // Check autorum status (must be turned OFF) + EXPECT_FALSE(Cpp::GetLibrariesAutoload()); + //EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); // if unload works + //EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); + + // Autoload turn ON again + Cpp::SetLibrariesAutoload(true); + // Check autorum status (must be turned ON) + EXPECT_TRUE(Cpp::GetLibrariesAutoload()); + //EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); // if unload works + //EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); } diff --git a/unittests/CppInterOp/TestSharedLib1/CMakeLists.txt b/unittests/CppInterOp/TestSharedLib1/CMakeLists.txt new file mode 100644 index 000000000..641d6f2e7 --- /dev/null +++ b/unittests/CppInterOp/TestSharedLib1/CMakeLists.txt @@ -0,0 +1,12 @@ +# TODO: Remove when library unload work correctly +add_llvm_library(TestSharedLib1 + SHARED + DISABLE_LLVM_LINK_LLVM_DYLIB + BUILDTREE_ONLY + TestSharedLib1.cpp) +# Put TestSharedLib1 next to the unit test executable. +set_output_directory(TestSharedLib1 + BINARY_DIR ${CMAKE_BINARY_DIR}/unittests/bin/$/ + LIBRARY_DIR ${CMAKE_BINARY_DIR}/unittests/bin/$/ + ) +set_target_properties(TestSharedLib1 PROPERTIES FOLDER "Tests") diff --git a/unittests/CppInterOp/TestSharedLib1/TestSharedLib1.cpp b/unittests/CppInterOp/TestSharedLib1/TestSharedLib1.cpp new file mode 100644 index 000000000..ef11b565a --- /dev/null +++ b/unittests/CppInterOp/TestSharedLib1/TestSharedLib1.cpp @@ -0,0 +1,5 @@ +#include "TestSharedLib1.h" + +int ret_one() { return 1; } + +int ret_two() { return 2; } diff --git a/unittests/CppInterOp/TestSharedLib1/TestSharedLib1.h b/unittests/CppInterOp/TestSharedLib1/TestSharedLib1.h new file mode 100644 index 000000000..8fa3d4f5b --- /dev/null +++ b/unittests/CppInterOp/TestSharedLib1/TestSharedLib1.h @@ -0,0 +1,13 @@ +#ifndef UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB1_H +#define UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB1_H + +// Avoid having to mangle/demangle the symbol name in tests +#ifdef _WIN32 +extern "C" __declspec(dllexport) int ret_one(); +extern "C" __declspec(dllexport) int ret_two(); +#else +extern "C" int ret_one(); +extern "C" int ret_two(); +#endif + +#endif // UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB1_H From aae272bd1592db2bcde1e5771c9ad22e8352c7e9 Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Thu, 11 Jul 2024 13:42:43 +0000 Subject: [PATCH 02/25] Fix some clang-tidy suggested issues --- lib/Interpreter/CppInterOp.cpp | 50 +++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index 339751dc1..b3d213a91 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -41,6 +41,7 @@ #include #include #include +#include // Stream redirect. #ifdef _WIN32 @@ -3565,26 +3566,36 @@ namespace Cpp { } class AutoLoadLibrarySearchGenerator; - static AutoLoadLibrarySearchGenerator *ALLSG = nullptr; + + // Last assigned Autoload SearchGenerator + // TODO: Test for thread safe. + static AutoLoadLibrarySearchGenerator *AutoSG = nullptr; class AutoLoadLibrarySearchGenerator : public llvm::orc::DefinitionGenerator { - public: bool Enabled = false; + public: + bool isEnabled() { + return Enabled; + } + + void setEnabled(bool enabled) { + Enabled = enabled; + } // Lazy materialization unit class helper class AutoloadLibraryMU : public llvm::orc::MaterializationUnit { - std::string lib; + const std::string lib; llvm::orc::SymbolNameVector syms; public: - AutoloadLibraryMU(const std::string &Library, const llvm::orc::SymbolNameVector &Symbols) + AutoloadLibraryMU(const std::string Library, const llvm::orc::SymbolNameVector &Symbols) : MaterializationUnit({getSymbolFlagsMap(Symbols), nullptr}), lib(Library), syms(Symbols) {} - StringRef getName() const override { + [[nodiscard]] StringRef getName() const override { return ""; } void materialize(std::unique_ptr R) override { - if (!ALLSG || !ALLSG->Enabled) { + if (!AutoSG || !AutoSG->isEnabled()) { R->failMaterialization(); return; } @@ -3592,13 +3603,13 @@ namespace Cpp { LLVM_DEBUG(dbgs() << "Materialize " << lib << " syms=" << syms); auto& I = getInterp(); - auto DLM = I.getDynamicLibraryManager(); + auto *DLM = I.getDynamicLibraryManager(); llvm::orc::SymbolMap loadedSymbols; llvm::orc::SymbolNameSet failedSymbols; bool loadedLibrary = false; - for (auto symbol : syms) { + for (const auto &symbol : syms) { std::string symbolStr = (*symbol).str(); std::string nameForDlsym = DemangleNameForDlsym(symbolStr); @@ -3650,7 +3661,7 @@ namespace Cpp { private: static llvm::orc::SymbolFlagsMap getSymbolFlagsMap(const llvm::orc::SymbolNameVector &Symbols) { llvm::orc::SymbolFlagsMap map; - for (auto symbolName : Symbols) + for (const auto &symbolName : Symbols) map[symbolName] = llvm::JITSymbolFlags::Exported; return map; } @@ -3658,16 +3669,16 @@ namespace Cpp { llvm::Error tryToGenerate(llvm::orc::LookupState &LS, llvm::orc::LookupKind K, llvm::orc::JITDylib &JD, llvm::orc::JITDylibLookupFlags JDLookupFlags, const llvm::orc::SymbolLookupSet &Symbols) override { - if (Enabled) { + if (isEnabled()) { LLVM_DEBUG(dbgs() << "tryToGenerate"); auto& I = getInterp(); - auto DLM = I.getDynamicLibraryManager(); + auto *DLM = I.getDynamicLibraryManager(); std::unordered_map found; llvm::orc::SymbolMap NewSymbols; - for (auto &KV : Symbols) { - auto &Name = KV.first; + for (const auto &KV : Symbols) { + const auto &Name = KV.first; if ((*Name).empty()) continue; @@ -3693,6 +3704,7 @@ namespace Cpp { return llvm::Error::success(); } + }; void SetLibrariesAutoload(bool autoload /* = true */) { @@ -3704,16 +3716,16 @@ namespace Cpp { llvm::orc::JITDylib& DyLib = *EE.getProcessSymbolsJITDylib().get(); #endif // CLANG_VERSION_MAJOR - if (!ALLSG) - ALLSG = &DyLib.addGenerator(std::make_unique()); - ALLSG->Enabled = autoload; + if (!AutoSG) + AutoSG = &DyLib.addGenerator(std::make_unique()); + AutoSG->setEnabled(autoload); - LLVM_DEBUG(dbgs() << "Autoload=" << (ALLSG && ALLSG->Enabled ? "ON" : "OFF")); + LLVM_DEBUG(dbgs() << "Autoload=" << (AutoSG->isEnabled() ? "ON" : "OFF")); } bool GetLibrariesAutoload() { - LLVM_DEBUG(dbgs() << "Autoload is " << (ALLSG && ALLSG->Enabled ? "ON" : "OFF")); - return ALLSG && ALLSG->Enabled; + LLVM_DEBUG(dbgs() << "Autoload is " << (AutoSG && AutoSG->isEnabled() ? "ON" : "OFF")); + return AutoSG && AutoSG->isEnabled(); } #undef DEBUG_TYPE From 1cbc857dab9083008ae88635875d63393e6924b3 Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Thu, 11 Jul 2024 15:32:43 +0000 Subject: [PATCH 03/25] Fix some clang-tidy suggested issues 2 --- lib/Interpreter/CppInterOp.cpp | 4 ++-- lib/Interpreter/DynamicLibraryManager.cpp | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index b3d213a91..80783b47b 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -3584,10 +3584,10 @@ namespace Cpp { // Lazy materialization unit class helper class AutoloadLibraryMU : public llvm::orc::MaterializationUnit { - const std::string lib; + std::string lib; llvm::orc::SymbolNameVector syms; public: - AutoloadLibraryMU(const std::string Library, const llvm::orc::SymbolNameVector &Symbols) + AutoloadLibraryMU(const std::string& Library, const llvm::orc::SymbolNameVector &Symbols) : MaterializationUnit({getSymbolFlagsMap(Symbols), nullptr}), lib(Library), syms(Symbols) {} [[nodiscard]] StringRef getName() const override { diff --git a/lib/Interpreter/DynamicLibraryManager.cpp b/lib/Interpreter/DynamicLibraryManager.cpp index 4265024e1..38ac626d7 100644 --- a/lib/Interpreter/DynamicLibraryManager.cpp +++ b/lib/Interpreter/DynamicLibraryManager.cpp @@ -392,9 +392,9 @@ namespace Cpp { return; DyLibHandle dyLibHandle = nullptr; - for (DyLibs::const_iterator I = m_DyLibs.begin(), E = m_DyLibs.end(); I != E; ++I) { - if (I->second == canonicalLoadedLib) { - dyLibHandle = I->first; + for (const auto& dylib : m_DyLibs) { + if (dylib.second == canonicalLoadedLib) { + dyLibHandle = dylib.first; break; } } From ae4b0abe00ee62e5ed9c45a4be784f2d733270a9 Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Sun, 28 Jul 2024 04:33:13 +0000 Subject: [PATCH 04/25] Fix --- include/clang/Interpreter/CppInterOp.h | 2 +- lib/Interpreter/CppInterOp.cpp | 83 ++++++------ unittests/CppInterOp/CMakeLists.txt | 16 ++- .../CppInterOp/DynamicLibraryManagerTest.cpp | 120 +++++++++++++++--- .../CppInterOp/TestSharedLib2/CMakeLists.txt | 11 ++ .../TestSharedLib2/TestSharedLib2.cpp | 5 + .../TestSharedLib2/TestSharedLib2.h | 13 ++ .../CppInterOp/TestSharedLib3/CMakeLists.txt | 11 ++ .../TestSharedLib3/TestSharedLib3.cpp | 3 + .../TestSharedLib3/TestSharedLib3.h | 11 ++ 10 files changed, 213 insertions(+), 62 deletions(-) create mode 100644 unittests/CppInterOp/TestSharedLib2/CMakeLists.txt create mode 100644 unittests/CppInterOp/TestSharedLib2/TestSharedLib2.cpp create mode 100644 unittests/CppInterOp/TestSharedLib2/TestSharedLib2.h create mode 100644 unittests/CppInterOp/TestSharedLib3/CMakeLists.txt create mode 100644 unittests/CppInterOp/TestSharedLib3/TestSharedLib3.cpp create mode 100644 unittests/CppInterOp/TestSharedLib3/TestSharedLib3.h diff --git a/include/clang/Interpreter/CppInterOp.h b/include/clang/Interpreter/CppInterOp.h index 79e098bc7..abd5438c4 100644 --- a/include/clang/Interpreter/CppInterOp.h +++ b/include/clang/Interpreter/CppInterOp.h @@ -761,7 +761,7 @@ namespace Cpp { ///\param[in] autoload - true = libraries autoload is on. CPPINTEROP_API void SetLibrariesAutoload(bool autoload = true); - /// Get libraries autoload. + /// Get libraries autoload status. ///\returns LibraryAutoLoad state (true = libraries autoload is on). CPPINTEROP_API bool GetLibrariesAutoload(); diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index 80783b47b..7dd9d818d 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -1137,9 +1137,9 @@ namespace Cpp { auto FDAorErr = compat::getSymbolAddress(I, mangled_name); if (llvm::Error Err = FDAorErr.takeError()) llvm::consumeError(std::move(Err)); // nullptr if missing - else + else { return llvm::jitTargetAddressToPointer(*FDAorErr); - + } return nullptr; } @@ -2730,7 +2730,7 @@ namespace Cpp { std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr, MainAddr); // build/tools/clang/unittests/Interpreter/Executable -> build/ - StringRef Dir = sys::path::parent_path(BinaryPath); + Dir = sys::path::parent_path(BinaryPath); Dir = sys::path::parent_path(Dir); Dir = sys::path::parent_path(Dir); @@ -3565,12 +3565,6 @@ namespace Cpp { return nameForDlsym; } - class AutoLoadLibrarySearchGenerator; - - // Last assigned Autoload SearchGenerator - // TODO: Test for thread safe. - static AutoLoadLibrarySearchGenerator *AutoSG = nullptr; - class AutoLoadLibrarySearchGenerator : public llvm::orc::DefinitionGenerator { bool Enabled = false; public: @@ -3587,15 +3581,15 @@ namespace Cpp { std::string lib; llvm::orc::SymbolNameVector syms; public: - AutoloadLibraryMU(const std::string& Library, const llvm::orc::SymbolNameVector &Symbols) - : MaterializationUnit({getSymbolFlagsMap(Symbols), nullptr}), lib(Library), syms(Symbols) {} + AutoloadLibraryMU(std::string Library, const llvm::orc::SymbolNameVector &Symbols) + : MaterializationUnit({getSymbolFlagsMap(Symbols), nullptr}), lib(std::move(Library)), syms(Symbols) {} - [[nodiscard]] StringRef getName() const override { + StringRef getName() const override { return ""; } void materialize(std::unique_ptr R) override { - if (!AutoSG || !AutoSG->isEnabled()) { + if (!sAutoSG || !sAutoSG->isEnabled()) { R->failMaterialization(); return; } @@ -3669,37 +3663,37 @@ namespace Cpp { llvm::Error tryToGenerate(llvm::orc::LookupState &LS, llvm::orc::LookupKind K, llvm::orc::JITDylib &JD, llvm::orc::JITDylibLookupFlags JDLookupFlags, const llvm::orc::SymbolLookupSet &Symbols) override { - if (isEnabled()) { - LLVM_DEBUG(dbgs() << "tryToGenerate"); + if (!isEnabled()) + return llvm::Error::success(); + LLVM_DEBUG(dbgs() << "tryToGenerate"); - auto& I = getInterp(); - auto *DLM = I.getDynamicLibraryManager(); + auto& I = getInterp(); + auto *DLM = I.getDynamicLibraryManager(); - std::unordered_map found; - llvm::orc::SymbolMap NewSymbols; - for (const auto &KV : Symbols) { - const auto &Name = KV.first; - if ((*Name).empty()) - continue; + std::unordered_map found; + llvm::orc::SymbolMap NewSymbols; + for (const auto &KV : Symbols) { + const auto &Name = KV.first; + if ((*Name).empty()) + continue; - auto lib = DLM->searchLibrariesForSymbol(*Name, /*searchSystem=*/true); // false? - if (lib.empty()) - continue; + auto lib = DLM->searchLibrariesForSymbol(*Name, /*searchSystem=*/true); // false? + if (lib.empty()) + continue; - found[lib].push_back(Name); + found[lib].push_back(Name); - // Workaround: This getAddressOfGlobal call make first symbol search - // to work, immediatelly after library auto load. This approach do not - // use MU - //DLM->loadLibrary(lib, true); - //I.getAddressOfGlobal(*Name); - } + // Workaround: This getAddressOfGlobal call make first symbol search + // to work, immediatelly after library auto load. This approach do not + // use MU + //DLM->loadLibrary(lib, true); + //I.getAddressOfGlobal(*Name); + } - for (auto &&KV : found) { - auto MU = std::make_unique(KV.first, std::move(KV.second)); - if (auto Err = JD.define(MU)) - return Err; - } + for (auto &&KV : found) { + auto MU = std::make_unique(KV.first, std::move(KV.second)); + if (auto Err = JD.define(MU)) + return Err; } return llvm::Error::success(); @@ -3716,16 +3710,17 @@ namespace Cpp { llvm::orc::JITDylib& DyLib = *EE.getProcessSymbolsJITDylib().get(); #endif // CLANG_VERSION_MAJOR - if (!AutoSG) - AutoSG = &DyLib.addGenerator(std::make_unique()); - AutoSG->setEnabled(autoload); + if (!sAutoSG) { + sAutoSG = &DyLib.addGenerator(std::make_unique()); + } + sAutoSG->setEnabled(autoload); - LLVM_DEBUG(dbgs() << "Autoload=" << (AutoSG->isEnabled() ? "ON" : "OFF")); + LLVM_DEBUG(dbgs() << "Autoload=" << (sAutoSG->isEnabled() ? "ON" : "OFF")); } bool GetLibrariesAutoload() { - LLVM_DEBUG(dbgs() << "Autoload is " << (AutoSG && AutoSG->isEnabled() ? "ON" : "OFF")); - return AutoSG && AutoSG->isEnabled(); + LLVM_DEBUG(dbgs() << "Autoload is " << (sAutoSG && sAutoSG->isEnabled() ? "ON" : "OFF")); + return sAutoSG && sAutoSG->isEnabled(); } #undef DEBUG_TYPE diff --git a/unittests/CppInterOp/CMakeLists.txt b/unittests/CppInterOp/CMakeLists.txt index e80de9059..1c7c93f13 100644 --- a/unittests/CppInterOp/CMakeLists.txt +++ b/unittests/CppInterOp/CMakeLists.txt @@ -43,6 +43,15 @@ target_link_libraries(DynamicLibraryManagerTests clangCppInterOp ) +set_source_files_properties(DynamicLibraryManagerTest.cpp PROPERTIES COMPILE_DEFINITIONS + "LLVM_BINARY_DIR=\"${LLVM_BINARY_DIR}\"" + ) +set_source_files_properties(DynamicLibraryManagerTest.cpp PROPERTIES COMPILE_DEFINITIONS + "CPPINTEROP_VERSION=\"${CPPINTEROP_VERSION}\"" + ) + +set_output_directory(DynamicLibraryManagerTests BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/TestSharedLib/unittests/bin/$/) + set_output_directory(DynamicLibraryManagerTests BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/TestSharedLib/unittests/bin/$/ LIBRARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/TestSharedLib/unittests/bin/$/ @@ -50,7 +59,12 @@ set_output_directory(DynamicLibraryManagerTests add_dependencies(DynamicLibraryManagerTests TestSharedLib) #export_executable_symbols_for_plugins(TestSharedLib) add_subdirectory(TestSharedLib) -# TODO: Remove when libraryunload work correctly add_dependencies(DynamicLibraryManagerTests TestSharedLib1) #export_executable_symbols_for_plugins(TestSharedLib1) add_subdirectory(TestSharedLib1) +add_dependencies(DynamicLibraryManagerTests TestSharedLib2) +#export_executable_symbols_for_plugins(TestSharedLib2) +add_subdirectory(TestSharedLib2) +add_dependencies(DynamicLibraryManagerTests TestSharedLib3) +#export_executable_symbols_for_plugins(TestSharedLib3) +add_subdirectory(TestSharedLib3) diff --git a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp index fb13bcd94..bb511f0ea 100644 --- a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp +++ b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp @@ -2,9 +2,12 @@ #include "clang/Interpreter/CppInterOp.h" + +#include "llvm/ExecutionEngine/Orc/Core.h" +#include "llvm/ExecutionEngine/Orc/DebugUtils.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" - +#include "llvm/Support/raw_ostream.h" // Helper functions @@ -13,13 +16,14 @@ // GetMainExecutable (since some platforms don't support taking the // address of main, and some platforms can't implement GetMainExecutable // without being given the address of a function in the main executable). -std::string GetExecutablePath(const char* Argv0) { +std::string GetExecutablePath(const char* Argv0, void* mainAddr = nullptr) { // This just needs to be some symbol in the binary; C++ doesn't // allow taking the address of ::main however. - void* MainAddr = (void*)intptr_t(GetExecutablePath); + void* MainAddr = mainAddr ? mainAddr : (void*)intptr_t(GetExecutablePath); return llvm::sys::fs::getMainExecutable(Argv0, MainAddr); } +// Mangle symbol name static inline std::string MangleNameForDlsym(const std::string& name) { std::string nameForDlsym = name; #if defined(R__MACOSX) || defined(R__WIN32) @@ -29,30 +33,33 @@ static inline std::string MangleNameForDlsym(const std::string& name) { return nameForDlsym; } +#include "../../lib/Interpreter/CppInterOp.cpp" + + // Tests TEST(DynamicLibraryManagerTest, Sanity) { EXPECT_TRUE(Cpp::CreateInterpreter()); EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_zero").c_str())); - std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr); + std::string BinaryPath = GetExecutablePath(nullptr, nullptr); llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath); Cpp::AddSearchPath(Dir.str().c_str()); + // Find and load library with "ret_zero" symbol defined and exported + // // FIXME: dlsym on mach-o takes the C-level name, however, the macho-o format // adds an additional underscore (_) prefix to the lowered names. Figure out // how to harmonize that API. - std::string PathToTestSharedLib = Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_zero").c_str(), /*system_search=*/false); - EXPECT_STRNE("", PathToTestSharedLib.c_str()) - << "Cannot find: '" << PathToTestSharedLib << "' in '" << Dir.str() - << "'"; - + << "Cannot find: '" << PathToTestSharedLib << "' in '" << Dir.str() << "'"; EXPECT_TRUE(Cpp::LoadLibrary(PathToTestSharedLib.c_str())); + // Force ExecutionEngine to be created. Cpp::Process(""); + // FIXME: Conda returns false to run this code on osx. EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_zero").c_str())); @@ -70,11 +77,11 @@ TEST(DynamicLibraryManagerTest, LibrariesAutoload) { EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); // Libraries Search path by default is set to main executable directory. - std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr); + std::string BinaryPath = GetExecutablePath(nullptr, nullptr); llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath); Cpp::AddSearchPath(Dir.str().c_str()); - // Find library with "rec_one" symbol defined and exported + // Find library with "ret_one" symbol defined and exported // // FIXME: dlsym on mach-o takes the C-level name, however, the macho-o format // adds an additional underscore (_) prefix to the lowered names. Figure out @@ -82,7 +89,6 @@ TEST(DynamicLibraryManagerTest, LibrariesAutoload) { // helper function. std::string PathToTestSharedLib1 = Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_one").c_str(), /*system_search=*/false); - // If result is "" then we cannot find this library. EXPECT_STRNE("", PathToTestSharedLib1.c_str()) << "Cannot find: '" << PathToTestSharedLib1 << "' in '" << Dir.str() << "'"; @@ -117,13 +123,95 @@ TEST(DynamicLibraryManagerTest, LibrariesAutoload) { Cpp::SetLibrariesAutoload(false); // Check autorum status (must be turned OFF) EXPECT_FALSE(Cpp::GetLibrariesAutoload()); - //EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); // if unload works - //EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); + EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); // false if unload works // Autoload turn ON again Cpp::SetLibrariesAutoload(true); // Check autorum status (must be turned ON) EXPECT_TRUE(Cpp::GetLibrariesAutoload()); - //EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); // if unload works - //EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); + EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); + + // Find library with "ret_1" symbol defined and exported + std::string PathToTestSharedLib2 = + Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_1").c_str(), /*system_search=*/false); + // If result is "" then we cannot find this library. + EXPECT_STRNE("", PathToTestSharedLib2.c_str()) + << "Cannot find: '" << PathToTestSharedLib2 << "' in '" << Dir.str() << "'"; + // Find symbol must success (when auotload=on) + EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_1").c_str())); + // Find symbol must success (when auotload=on) + EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_2").c_str())); +} + +TEST(DynamicLibraryManagerTest, LibrariesAutoloadExtraCoverage) { + EXPECT_TRUE(Cpp::CreateInterpreter()); + + // Libraries Search path by default is set to main executable directory. + std::string BinaryPath = GetExecutablePath(nullptr, nullptr); + llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath); + Cpp::AddSearchPath(Dir.str().c_str()); + + // Force ExecutionEngine to be created. + Cpp::Process(""); + + // Autoload turn ON + Cpp::SetLibrariesAutoload(true); + // Check autorum status (must be turned ON) + EXPECT_TRUE(Cpp::GetLibrariesAutoload()); + + // Cover: MU getName() + std::string res; + llvm::raw_string_ostream rss(res); + const llvm::orc::SymbolNameVector syms; + Cpp::AutoLoadLibrarySearchGenerator::AutoloadLibraryMU MU("test", syms); + rss << MU; + EXPECT_STRNE("", rss.str().c_str()) << "MU problem!"; + + // Cover: LoadLibrary error + // if (DLM->loadLibrary(lib, false) != DynamicLibraryManager::LoadLibResult::kLoadLibSuccess) { + // LLVM_DEBUG(dbgs() << "MU: Failed to load library " << lib); + // string err = "MU: Failed to load library! " + lib; + // perror(err.c_str()); + // } else { + // Find library with "ret_value" symbol defined and exported + std::string PathToTestSharedLib3 = + Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_val").c_str(), /*system_search=*/false); + // If result is "" then we cannot find this library. + EXPECT_STRNE("", PathToTestSharedLib3.c_str()) + << "Cannot find: '" << PathToTestSharedLib3 << "' in '" << Dir.str() << "'"; + // Remove library for simulate load error + llvm::sys::fs::remove(PathToTestSharedLib3, true); + EXPECT_TRUE(Cpp::GetLibrariesAutoload()); + // FIXME: Conda returns false to run this code on osx. + EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_val").c_str())); + + // Cover + // } else { + // // Collect all failing symbols, delegate their responsibility and then + // // fail their materialization. R->defineNonExistent() sounds like it + // // should do that, but it's not implemented?! + // failedSymbols.insert(symbol); + // TODO: implement test this case + + // Cover + // if (!failedSymbols.empty()) { + // auto failingMR = R->delegate(failedSymbols); + // if (failingMR) { + // (*failingMR)->failMaterialization(); + // TODO: implement test this case + + // Cover + // void discard(const llvm::orc::JITDylib &JD, const llvm::orc::SymbolStringPtr &Name) override {} + // TODO: implement test this case + + // Cover + // if (Path.empty()) { + // LLVM_DEBUG(dbgs() << "DynamicLibraryManager::lookupLibMaybeAddExt(): " + // TODO: implement test this case + + // Cover + // platform::DLClose(dyLibHandle, &errMsg); + // if (!errMsg.empty()) { + // LLVM_DEBUG(dbgs() << "DynamicLibraryManager::unloadLibrary(): " + // TODO: implement test this case } diff --git a/unittests/CppInterOp/TestSharedLib2/CMakeLists.txt b/unittests/CppInterOp/TestSharedLib2/CMakeLists.txt new file mode 100644 index 000000000..275d0af11 --- /dev/null +++ b/unittests/CppInterOp/TestSharedLib2/CMakeLists.txt @@ -0,0 +1,11 @@ +add_llvm_library(TestSharedLib2 + SHARED + DISABLE_LLVM_LINK_LLVM_DYLIB + BUILDTREE_ONLY + TestSharedLib2.cpp) +# Put TestSharedLib2 next to the unit test executable. +set_output_directory(TestSharedLib2 + BINARY_DIR ${CMAKE_BINARY_DIR}/unittests/bin/$/ + LIBRARY_DIR ${CMAKE_BINARY_DIR}/unittests/bin/$/ + ) +set_target_properties(TestSharedLib2 PROPERTIES FOLDER "Tests") diff --git a/unittests/CppInterOp/TestSharedLib2/TestSharedLib2.cpp b/unittests/CppInterOp/TestSharedLib2/TestSharedLib2.cpp new file mode 100644 index 000000000..dd7c1f0da --- /dev/null +++ b/unittests/CppInterOp/TestSharedLib2/TestSharedLib2.cpp @@ -0,0 +1,5 @@ +#include "TestSharedLib2.h" + +int ret_1() { return 1; } + +int ret_2() { return 2; } diff --git a/unittests/CppInterOp/TestSharedLib2/TestSharedLib2.h b/unittests/CppInterOp/TestSharedLib2/TestSharedLib2.h new file mode 100644 index 000000000..d0095f89d --- /dev/null +++ b/unittests/CppInterOp/TestSharedLib2/TestSharedLib2.h @@ -0,0 +1,13 @@ +#ifndef UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB2_H +#define UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB2_H + +// Avoid having to mangle/demangle the symbol name in tests +#ifdef _WIN32 +extern "C" __declspec(dllexport) int ret_1(); +extern "C" __declspec(dllexport) int ret_2(); +#else +extern "C" int ret_1(); +extern "C" int ret_2(); +#endif + +#endif // UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB2_H diff --git a/unittests/CppInterOp/TestSharedLib3/CMakeLists.txt b/unittests/CppInterOp/TestSharedLib3/CMakeLists.txt new file mode 100644 index 000000000..b170f0bb0 --- /dev/null +++ b/unittests/CppInterOp/TestSharedLib3/CMakeLists.txt @@ -0,0 +1,11 @@ +add_llvm_library(TestSharedLib3 + SHARED + DISABLE_LLVM_LINK_LLVM_DYLIB + BUILDTREE_ONLY + TestSharedLib3.cpp) +# Put TestSharedLib3 next to the unit test executable. +set_output_directory(TestSharedLib3 + BINARY_DIR ${CMAKE_BINARY_DIR}/unittests/bin/$/ + LIBRARY_DIR ${CMAKE_BINARY_DIR}/unittests/bin/$/ + ) +set_target_properties(TestSharedLib2 PROPERTIES FOLDER "Tests") diff --git a/unittests/CppInterOp/TestSharedLib3/TestSharedLib3.cpp b/unittests/CppInterOp/TestSharedLib3/TestSharedLib3.cpp new file mode 100644 index 000000000..cd5c189a4 --- /dev/null +++ b/unittests/CppInterOp/TestSharedLib3/TestSharedLib3.cpp @@ -0,0 +1,3 @@ +#include "TestSharedLib3.h" + +int ret_val() { return 42; } diff --git a/unittests/CppInterOp/TestSharedLib3/TestSharedLib3.h b/unittests/CppInterOp/TestSharedLib3/TestSharedLib3.h new file mode 100644 index 000000000..0752d50a2 --- /dev/null +++ b/unittests/CppInterOp/TestSharedLib3/TestSharedLib3.h @@ -0,0 +1,11 @@ +#ifndef UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB3_H +#define UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB3_H + +// Avoid having to mangle/demangle the symbol name in tests +#ifdef _WIN32 +extern "C" __declspec(dllexport) int ret_val(); +#else +extern "C" int ret_val(); +#endif + +#endif // UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB3_H From f55485ac72a29db38e0a334deea7f0a410967225 Mon Sep 17 00:00:00 2001 From: Alexander Penev <7923188+alexander-penev@users.noreply.github.com> Date: Tue, 30 Jul 2024 16:06:21 +0300 Subject: [PATCH 05/25] Update CppInterOp.cpp --- lib/Interpreter/CppInterOp.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index 7dd9d818d..a94f6020f 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -1137,9 +1137,8 @@ namespace Cpp { auto FDAorErr = compat::getSymbolAddress(I, mangled_name); if (llvm::Error Err = FDAorErr.takeError()) llvm::consumeError(std::move(Err)); // nullptr if missing - else { + else return llvm::jitTargetAddressToPointer(*FDAorErr); - } return nullptr; } From 4af71859b0832107da98a993fcd3b1a47a2ecc3b Mon Sep 17 00:00:00 2001 From: Alexander Penev <7923188+alexander-penev@users.noreply.github.com> Date: Wed, 31 Jul 2024 08:00:21 +0300 Subject: [PATCH 06/25] Fix demangle --- lib/Interpreter/CppInterOp.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index a94f6020f..98d5478c5 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -3552,15 +3552,23 @@ namespace Cpp { static inline std::string DemangleNameForDlsym(const std::string& name) { std::string nameForDlsym = name; -#if defined(R__MACOSX) || defined(R__WIN32) + static bool is_demangle_active = false; + static bool demangle = false; + if (!is_demangle_active) { + auto& I = getInterp(); + llvm::orc::LLJIT& EE = *compat::getExecutionEngine(I); + auto t = EE.getTargetMachine().getTargetTriple(); + demangle = t.isOSDarwin() || t.isWindows(); + is_demangle_active = true; + } + // The JIT gives us a mangled name which has an additional leading underscore // on macOS and Windows, for instance __ZN8TRandom34RndmEv. However, dlsym // requires us to remove it. // FIXME: get this information from the DataLayout via getGlobalPrefix()! - if (nameForDlsym[0] == '_') + if (demangle && nameForDlsym[0] == '_') nameForDlsym.erase(0, 1); -#endif //R__MACOSX - + } return nameForDlsym; } From 38066ab05d386f37f5f01f0dab149cb34e48d276 Mon Sep 17 00:00:00 2001 From: Alexander Penev <7923188+alexander-penev@users.noreply.github.com> Date: Wed, 31 Jul 2024 08:03:12 +0300 Subject: [PATCH 07/25] Apply suggestions from code review Co-authored-by: Vassil Vassilev --- include/clang/Interpreter/CppInterOp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/clang/Interpreter/CppInterOp.h b/include/clang/Interpreter/CppInterOp.h index abd5438c4..20da268ee 100644 --- a/include/clang/Interpreter/CppInterOp.h +++ b/include/clang/Interpreter/CppInterOp.h @@ -758,7 +758,7 @@ namespace Cpp { unsigned complete_column = 1U); /// Set libraries autoload. - ///\param[in] autoload - true = libraries autoload is on. + ///\param[in] autoload - true to enable library autoload. CPPINTEROP_API void SetLibrariesAutoload(bool autoload = true); /// Get libraries autoload status. From 724043438c7b05e5cac09279c4e7293edebdd5cd Mon Sep 17 00:00:00 2001 From: Alexander Penev <7923188+alexander-penev@users.noreply.github.com> Date: Wed, 31 Jul 2024 08:06:02 +0300 Subject: [PATCH 08/25] Update DynamicLibraryManagerSymbol.cpp --- lib/Interpreter/DynamicLibraryManagerSymbol.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Interpreter/DynamicLibraryManagerSymbol.cpp b/lib/Interpreter/DynamicLibraryManagerSymbol.cpp index 32dfc4c95..3629c324f 100644 --- a/lib/Interpreter/DynamicLibraryManagerSymbol.cpp +++ b/lib/Interpreter/DynamicLibraryManagerSymbol.cpp @@ -1260,7 +1260,7 @@ namespace Cpp { void DynamicLibraryManager::initializeDyld( std::function shouldPermanentlyIgnore) { - //assert(!m_Dyld && "Already initialized!"); + assert(!m_Dyld && "Already initialized!"); if (m_Dyld) delete m_Dyld; From f815bace12214538caefbde0832f6d0ce971c6e9 Mon Sep 17 00:00:00 2001 From: Alexander Penev <7923188+alexander-penev@users.noreply.github.com> Date: Wed, 31 Jul 2024 08:24:41 +0300 Subject: [PATCH 09/25] Update DynamicLibraryManagerSymbol.cpp --- lib/Interpreter/DynamicLibraryManagerSymbol.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Interpreter/DynamicLibraryManagerSymbol.cpp b/lib/Interpreter/DynamicLibraryManagerSymbol.cpp index 3629c324f..a7c7ded72 100644 --- a/lib/Interpreter/DynamicLibraryManagerSymbol.cpp +++ b/lib/Interpreter/DynamicLibraryManagerSymbol.cpp @@ -1153,8 +1153,8 @@ namespace Cpp { std::string Dyld::searchLibrariesForSymbol(StringRef mangledName, bool searchSystem/* = true*/) { #define DEBUG_TYPE "Dyld:searchLibrariesForSymbol:" -// assert(!llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(mangledName.str()) && -// "Library already loaded, please use dlsym!"); + assert(!llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(mangledName.str()) && + "Library already loaded, please use dlsym!"); assert(!mangledName.empty()); using namespace llvm::sys::path; From a8f441d629770174cfcec5a0cc7a1952e2c14304 Mon Sep 17 00:00:00 2001 From: Alexander Penev <7923188+alexander-penev@users.noreply.github.com> Date: Wed, 31 Jul 2024 08:51:14 +0300 Subject: [PATCH 10/25] Update DynamicLibraryManagerTest.cpp --- unittests/CppInterOp/DynamicLibraryManagerTest.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp index bb511f0ea..bf9ec5a3a 100644 --- a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp +++ b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp @@ -52,7 +52,7 @@ TEST(DynamicLibraryManagerTest, Sanity) { // adds an additional underscore (_) prefix to the lowered names. Figure out // how to harmonize that API. std::string PathToTestSharedLib = - Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_zero").c_str(), /*system_search=*/false); + Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_zero").c_str(), /*search_system=*/false); EXPECT_STRNE("", PathToTestSharedLib.c_str()) << "Cannot find: '" << PathToTestSharedLib << "' in '" << Dir.str() << "'"; EXPECT_TRUE(Cpp::LoadLibrary(PathToTestSharedLib.c_str())); @@ -88,7 +88,7 @@ TEST(DynamicLibraryManagerTest, LibrariesAutoload) { // how to harmonize that API. For now we use out minimal implementation of // helper function. std::string PathToTestSharedLib1 = - Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_one").c_str(), /*system_search=*/false); + Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_one").c_str(), /*search_system=*/false); // If result is "" then we cannot find this library. EXPECT_STRNE("", PathToTestSharedLib1.c_str()) << "Cannot find: '" << PathToTestSharedLib1 << "' in '" << Dir.str() << "'"; @@ -133,7 +133,7 @@ TEST(DynamicLibraryManagerTest, LibrariesAutoload) { // Find library with "ret_1" symbol defined and exported std::string PathToTestSharedLib2 = - Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_1").c_str(), /*system_search=*/false); + Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_1").c_str(), /*search_system=*/false); // If result is "" then we cannot find this library. EXPECT_STRNE("", PathToTestSharedLib2.c_str()) << "Cannot find: '" << PathToTestSharedLib2 << "' in '" << Dir.str() << "'"; @@ -175,7 +175,7 @@ TEST(DynamicLibraryManagerTest, LibrariesAutoloadExtraCoverage) { // } else { // Find library with "ret_value" symbol defined and exported std::string PathToTestSharedLib3 = - Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_val").c_str(), /*system_search=*/false); + Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_val").c_str(), /*search_system=*/false); // If result is "" then we cannot find this library. EXPECT_STRNE("", PathToTestSharedLib3.c_str()) << "Cannot find: '" << PathToTestSharedLib3 << "' in '" << Dir.str() << "'"; From b5eb022d8527c9afd8d481f75e2908931d8c86cb Mon Sep 17 00:00:00 2001 From: Alexander Penev <7923188+alexander-penev@users.noreply.github.com> Date: Wed, 31 Jul 2024 09:03:51 +0300 Subject: [PATCH 11/25] Update CMakeLists.txt --- unittests/CppInterOp/TestSharedLib1/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/unittests/CppInterOp/TestSharedLib1/CMakeLists.txt b/unittests/CppInterOp/TestSharedLib1/CMakeLists.txt index 641d6f2e7..a5b2a3d80 100644 --- a/unittests/CppInterOp/TestSharedLib1/CMakeLists.txt +++ b/unittests/CppInterOp/TestSharedLib1/CMakeLists.txt @@ -1,4 +1,3 @@ -# TODO: Remove when library unload work correctly add_llvm_library(TestSharedLib1 SHARED DISABLE_LLVM_LINK_LLVM_DYLIB From 80c76c133d78422763b4b55ba46edea8927d50f0 Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Wed, 31 Jul 2024 06:49:00 +0000 Subject: [PATCH 12/25] Fix demangle build --- lib/Interpreter/CppInterOp.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index 98d5478c5..960cf221d 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -3557,8 +3557,8 @@ namespace Cpp { if (!is_demangle_active) { auto& I = getInterp(); llvm::orc::LLJIT& EE = *compat::getExecutionEngine(I); - auto t = EE.getTargetMachine().getTargetTriple(); - demangle = t.isOSDarwin() || t.isWindows(); + auto t = EE.getTargetTriple(); + demangle = t.isOSDarwin() || t.isOSWindows(); is_demangle_active = true; } @@ -3568,7 +3568,6 @@ namespace Cpp { // FIXME: get this information from the DataLayout via getGlobalPrefix()! if (demangle && nameForDlsym[0] == '_') nameForDlsym.erase(0, 1); - } return nameForDlsym; } From 1f5111e6ac587b08e7941b4d3ed548ebd328488c Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Fri, 6 Sep 2024 06:13:08 +0000 Subject: [PATCH 13/25] Cleanup --- unittests/CppInterOp/CMakeLists.txt | 5 +- .../CppInterOp/DynamicLibraryManagerTest.cpp | 53 +++---------------- .../CppInterOp/TestSharedLib3/CMakeLists.txt | 11 ---- .../TestSharedLib3/TestSharedLib3.cpp | 3 -- .../TestSharedLib3/TestSharedLib3.h | 11 ---- 5 files changed, 8 insertions(+), 75 deletions(-) delete mode 100644 unittests/CppInterOp/TestSharedLib3/CMakeLists.txt delete mode 100644 unittests/CppInterOp/TestSharedLib3/TestSharedLib3.cpp delete mode 100644 unittests/CppInterOp/TestSharedLib3/TestSharedLib3.h diff --git a/unittests/CppInterOp/CMakeLists.txt b/unittests/CppInterOp/CMakeLists.txt index 1c7c93f13..e8b39764e 100644 --- a/unittests/CppInterOp/CMakeLists.txt +++ b/unittests/CppInterOp/CMakeLists.txt @@ -59,12 +59,11 @@ set_output_directory(DynamicLibraryManagerTests add_dependencies(DynamicLibraryManagerTests TestSharedLib) #export_executable_symbols_for_plugins(TestSharedLib) add_subdirectory(TestSharedLib) + add_dependencies(DynamicLibraryManagerTests TestSharedLib1) #export_executable_symbols_for_plugins(TestSharedLib1) add_subdirectory(TestSharedLib1) + add_dependencies(DynamicLibraryManagerTests TestSharedLib2) #export_executable_symbols_for_plugins(TestSharedLib2) add_subdirectory(TestSharedLib2) -add_dependencies(DynamicLibraryManagerTests TestSharedLib3) -#export_executable_symbols_for_plugins(TestSharedLib3) -add_subdirectory(TestSharedLib3) diff --git a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp index bf9ec5a3a..5bddb915f 100644 --- a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp +++ b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp @@ -167,51 +167,10 @@ TEST(DynamicLibraryManagerTest, LibrariesAutoloadExtraCoverage) { rss << MU; EXPECT_STRNE("", rss.str().c_str()) << "MU problem!"; - // Cover: LoadLibrary error - // if (DLM->loadLibrary(lib, false) != DynamicLibraryManager::LoadLibResult::kLoadLibSuccess) { - // LLVM_DEBUG(dbgs() << "MU: Failed to load library " << lib); - // string err = "MU: Failed to load library! " + lib; - // perror(err.c_str()); - // } else { - // Find library with "ret_value" symbol defined and exported - std::string PathToTestSharedLib3 = - Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_val").c_str(), /*search_system=*/false); - // If result is "" then we cannot find this library. - EXPECT_STRNE("", PathToTestSharedLib3.c_str()) - << "Cannot find: '" << PathToTestSharedLib3 << "' in '" << Dir.str() << "'"; - // Remove library for simulate load error - llvm::sys::fs::remove(PathToTestSharedLib3, true); - EXPECT_TRUE(Cpp::GetLibrariesAutoload()); - // FIXME: Conda returns false to run this code on osx. - EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_val").c_str())); - - // Cover - // } else { - // // Collect all failing symbols, delegate their responsibility and then - // // fail their materialization. R->defineNonExistent() sounds like it - // // should do that, but it's not implemented?! - // failedSymbols.insert(symbol); - // TODO: implement test this case - - // Cover - // if (!failedSymbols.empty()) { - // auto failingMR = R->delegate(failedSymbols); - // if (failingMR) { - // (*failingMR)->failMaterialization(); - // TODO: implement test this case - - // Cover - // void discard(const llvm::orc::JITDylib &JD, const llvm::orc::SymbolStringPtr &Name) override {} - // TODO: implement test this case - - // Cover - // if (Path.empty()) { - // LLVM_DEBUG(dbgs() << "DynamicLibraryManager::lookupLibMaybeAddExt(): " - // TODO: implement test this case - - // Cover - // platform::DLClose(dyLibHandle, &errMsg); - // if (!errMsg.empty()) { - // LLVM_DEBUG(dbgs() << "DynamicLibraryManager::unloadLibrary(): " - // TODO: implement test this case + //TODO: Test and cover also if it is possible: + // 1. Error when LoadLibrary + // 2. if (!failedSymbols.empty()) { ... + // 3. void discard(const llvm::orc::JITDylib &JD, const llvm::orc::SymbolStringPtr &Name) override {} + // 4. if (Path.empty()) { ... + // 5. platform::DLClose(dyLibHandle, &errMsg); } diff --git a/unittests/CppInterOp/TestSharedLib3/CMakeLists.txt b/unittests/CppInterOp/TestSharedLib3/CMakeLists.txt deleted file mode 100644 index b170f0bb0..000000000 --- a/unittests/CppInterOp/TestSharedLib3/CMakeLists.txt +++ /dev/null @@ -1,11 +0,0 @@ -add_llvm_library(TestSharedLib3 - SHARED - DISABLE_LLVM_LINK_LLVM_DYLIB - BUILDTREE_ONLY - TestSharedLib3.cpp) -# Put TestSharedLib3 next to the unit test executable. -set_output_directory(TestSharedLib3 - BINARY_DIR ${CMAKE_BINARY_DIR}/unittests/bin/$/ - LIBRARY_DIR ${CMAKE_BINARY_DIR}/unittests/bin/$/ - ) -set_target_properties(TestSharedLib2 PROPERTIES FOLDER "Tests") diff --git a/unittests/CppInterOp/TestSharedLib3/TestSharedLib3.cpp b/unittests/CppInterOp/TestSharedLib3/TestSharedLib3.cpp deleted file mode 100644 index cd5c189a4..000000000 --- a/unittests/CppInterOp/TestSharedLib3/TestSharedLib3.cpp +++ /dev/null @@ -1,3 +0,0 @@ -#include "TestSharedLib3.h" - -int ret_val() { return 42; } diff --git a/unittests/CppInterOp/TestSharedLib3/TestSharedLib3.h b/unittests/CppInterOp/TestSharedLib3/TestSharedLib3.h deleted file mode 100644 index 0752d50a2..000000000 --- a/unittests/CppInterOp/TestSharedLib3/TestSharedLib3.h +++ /dev/null @@ -1,11 +0,0 @@ -#ifndef UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB3_H -#define UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB3_H - -// Avoid having to mangle/demangle the symbol name in tests -#ifdef _WIN32 -extern "C" __declspec(dllexport) int ret_val(); -#else -extern "C" int ret_val(); -#endif - -#endif // UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB3_H From 2ab35afc46c4422a48cadf94c38110299db29f81 Mon Sep 17 00:00:00 2001 From: Alexander Penev <7923188+alexander-penev@users.noreply.github.com> Date: Wed, 18 Dec 2024 20:56:27 +0200 Subject: [PATCH 14/25] Restore sAutoSG in CppInterOp.cpp --- lib/Interpreter/CppInterOp.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index 960cf221d..4b4787dab 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -66,6 +66,10 @@ namespace Cpp { using namespace llvm; using namespace std; + // Last assigned Autoload SearchGenerator + // TODO: Test fot thread safe. + class AutoLoadLibrarySearchGenerator; + static AutoLoadLibrarySearchGenerator *sAutoSG = null; // Flag to indicate ownership when an external interpreter instance is used. static bool OwningSInterpreter = true; static compat::Interpreter* sInterpreter = nullptr; From b307433e2e7cdf904dda03417b2c2ac966703878 Mon Sep 17 00:00:00 2001 From: Alexander Penev <7923188+alexander-penev@users.noreply.github.com> Date: Thu, 19 Dec 2024 01:04:31 +0200 Subject: [PATCH 15/25] Fix null -> nullptr in CppInterOp.cpp --- lib/Interpreter/CppInterOp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index 4b4787dab..c9c6f3763 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -69,7 +69,7 @@ namespace Cpp { // Last assigned Autoload SearchGenerator // TODO: Test fot thread safe. class AutoLoadLibrarySearchGenerator; - static AutoLoadLibrarySearchGenerator *sAutoSG = null; + static AutoLoadLibrarySearchGenerator *sAutoSG = nullptr; // Flag to indicate ownership when an external interpreter instance is used. static bool OwningSInterpreter = true; static compat::Interpreter* sInterpreter = nullptr; From a36996276739610bd7de2f421383b32e8ab8eb10 Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Sat, 11 Jan 2025 17:44:47 +0000 Subject: [PATCH 16/25] Fix ExecutorSymbolDef compat in llvm<17 --- lib/Interpreter/CppInterOp.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index c9c6f3763..b52444a1c 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -3638,7 +3638,11 @@ namespace Cpp { if (addr) { loadedSymbols[symbol] = +#if CLANG_VERSION_MAJOR < 17 + JITEvaluatedSymbol(addr, JITSymbolFlags::Exported); +#else llvm::orc::ExecutorSymbolDef(llvm::orc::ExecutorAddr::fromPtr(addr), JITSymbolFlags::Exported); +#endif // CLANG_VERSION_MAJOR < 17 } else { // Collect all failing symbols, delegate their responsibility and then // fail their materialization. R->defineNonExistent() sounds like it From ea75cb40ff7a9b5ca4f26f970b8dfeff9cff9da9 Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Sat, 11 Jan 2025 17:55:49 +0000 Subject: [PATCH 17/25] Fix compat in llvm<17 --- lib/Interpreter/CppInterOp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index b52444a1c..15c772c0b 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -3639,7 +3639,7 @@ namespace Cpp { if (addr) { loadedSymbols[symbol] = #if CLANG_VERSION_MAJOR < 17 - JITEvaluatedSymbol(addr, JITSymbolFlags::Exported); + llvm::orc::JITEvaluatedSymbol(addr, JITSymbolFlags::Exported); #else llvm::orc::ExecutorSymbolDef(llvm::orc::ExecutorAddr::fromPtr(addr), JITSymbolFlags::Exported); #endif // CLANG_VERSION_MAJOR < 17 From 90e9c856da0b482db7eede59dfb5b9da58d3f34b Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Sat, 11 Jan 2025 18:12:34 +0000 Subject: [PATCH 18/25] Fix compat in llvm<17 (2) --- lib/Interpreter/CppInterOp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index 15c772c0b..5d34c2ee3 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -3639,7 +3639,7 @@ namespace Cpp { if (addr) { loadedSymbols[symbol] = #if CLANG_VERSION_MAJOR < 17 - llvm::orc::JITEvaluatedSymbol(addr, JITSymbolFlags::Exported); + llvm::JITEvaluatedSymbol(addr, JITSymbolFlags::Exported); #else llvm::orc::ExecutorSymbolDef(llvm::orc::ExecutorAddr::fromPtr(addr), JITSymbolFlags::Exported); #endif // CLANG_VERSION_MAJOR < 17 From 3e7e87ea95eb8ae5971108fe1ecebae1461dbbb8 Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Sat, 11 Jan 2025 19:15:20 +0000 Subject: [PATCH 19/25] Fix compat in llvm<17 (3) --- lib/Interpreter/CppInterOp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index 5d34c2ee3..b9e7d1025 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -3639,7 +3639,7 @@ namespace Cpp { if (addr) { loadedSymbols[symbol] = #if CLANG_VERSION_MAJOR < 17 - llvm::JITEvaluatedSymbol(addr, JITSymbolFlags::Exported); + llvm::JITEvaluatedSymbol(llvm::JITEvaluatedSymbol::fromPointer(addr), JITSymbolFlags::Exported); #else llvm::orc::ExecutorSymbolDef(llvm::orc::ExecutorAddr::fromPtr(addr), JITSymbolFlags::Exported); #endif // CLANG_VERSION_MAJOR < 17 From bbcb9f0cfc2b2059a8863bd6790f03844bf28b48 Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Sat, 11 Jan 2025 19:31:21 +0000 Subject: [PATCH 20/25] Fix compat in llvm<17 (4) --- lib/Interpreter/CppInterOp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index b9e7d1025..19acaa705 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -3639,7 +3639,7 @@ namespace Cpp { if (addr) { loadedSymbols[symbol] = #if CLANG_VERSION_MAJOR < 17 - llvm::JITEvaluatedSymbol(llvm::JITEvaluatedSymbol::fromPointer(addr), JITSymbolFlags::Exported); + llvm::JITEvaluatedSymbol::fromPointer(addr, JITSymbolFlags::Exported); #else llvm::orc::ExecutorSymbolDef(llvm::orc::ExecutorAddr::fromPtr(addr), JITSymbolFlags::Exported); #endif // CLANG_VERSION_MAJOR < 17 From c280c702f607f176b1788879dba6c0a7a60f4a7c Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Sun, 12 Jan 2025 07:42:10 +0000 Subject: [PATCH 21/25] Fix compat in llvm<17 (5) --- lib/Interpreter/CppInterOp.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index 19acaa705..f5021bf51 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -3660,7 +3660,9 @@ namespace Cpp { if (!loadedSymbols.empty()) { llvm::cantFail(R->notifyResolved(loadedSymbols)); - llvm::cantFail(R->notifyEmitted()); + + llvm::orc::SymbolDependenceGroup DepGroup; + llvm::cantFail(R->notifyEmitted({DepGroup})); } } From c4c646f1c88057005ddd0b3982c8d5c8ff55a155 Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Sun, 12 Jan 2025 08:14:13 +0000 Subject: [PATCH 22/25] Fix compat in llvm<17 (6) --- lib/Interpreter/CppInterOp.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index f5021bf51..978b99b62 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -34,6 +34,7 @@ #include "llvm/Demangle/Demangle.h" #include "llvm/ExecutionEngine/Orc/Core.h" #include "llvm/Support/Casting.h" +#include "llvm/Support/Error.h" #include "llvm/Support/Debug.h" #include "llvm/Support/raw_os_ostream.h" From 6ef47d7f35859bbacd82c0da1e18534df12b546e Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Fri, 31 Jan 2025 05:37:21 +0000 Subject: [PATCH 23/25] Fix --- lib/Interpreter/CppInterOp.cpp | 23 ++++++++++++++--------- unittests/CppInterOp/CMakeLists.txt | 4 ++-- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index 978b99b62..b704a960f 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -3590,22 +3590,23 @@ namespace Cpp { // Lazy materialization unit class helper class AutoloadLibraryMU : public llvm::orc::MaterializationUnit { std::string lib; - llvm::orc::SymbolNameVector syms; + std::string fLibrary; + llvm::orc::SymbolNameVector fSymbols; public: - AutoloadLibraryMU(std::string Library, const llvm::orc::SymbolNameVector &Symbols) - : MaterializationUnit({getSymbolFlagsMap(Symbols), nullptr}), lib(std::move(Library)), syms(Symbols) {} + AutoloadLibraryMU(const std::string &Library, const llvm::orc::SymbolNameVector &Symbols) + : MaterializationUnit({getSymbolFlagsMap(Symbols), nullptr}), fLibrary(Library), fSymbols(Symbols) {} StringRef getName() const override { return ""; } void materialize(std::unique_ptr R) override { - if (!sAutoSG || !sAutoSG->isEnabled()) { - R->failMaterialization(); - return; - } + //if (!sAutoSG || !sAutoSG->isEnabled()) { + // R->failMaterialization(); + // return; + //} - LLVM_DEBUG(dbgs() << "Materialize " << lib << " syms=" << syms); + LLVM_DEBUG(dbgs() << "Materialize " << lib << " syms=" << fSymbols); auto& I = getInterp(); auto *DLM = I.getDynamicLibraryManager(); @@ -3614,7 +3615,7 @@ namespace Cpp { llvm::orc::SymbolNameSet failedSymbols; bool loadedLibrary = false; - for (const auto &symbol : syms) { + for (const auto &symbol : fSymbols) { std::string symbolStr = (*symbol).str(); std::string nameForDlsym = DemangleNameForDlsym(symbolStr); @@ -3662,8 +3663,12 @@ namespace Cpp { if (!loadedSymbols.empty()) { llvm::cantFail(R->notifyResolved(loadedSymbols)); +#if CLANG_VERSION_MAJOR < 18 llvm::orc::SymbolDependenceGroup DepGroup; llvm::cantFail(R->notifyEmitted({DepGroup})); +#else + llvm::cantFail(R->notifyEmitted()); +#endif } } diff --git a/unittests/CppInterOp/CMakeLists.txt b/unittests/CppInterOp/CMakeLists.txt index e8b39764e..fb1398371 100644 --- a/unittests/CppInterOp/CMakeLists.txt +++ b/unittests/CppInterOp/CMakeLists.txt @@ -20,8 +20,8 @@ target_link_libraries(CppInterOpTests ) set_output_directory(CppInterOpTests - BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/CppInterOpTests/unittests/bin/$/ - LIBRARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/CppInterOpTests/unittests/bin/$/ + BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/bin/$/ + LIBRARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/bin/$/ ) if(NOT WIN32) From 2cb8352f734e2fc0b3ea896bb6b433b1dc93adda Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Fri, 31 Jan 2025 14:54:04 +0000 Subject: [PATCH 24/25] Fix1 --- lib/Interpreter/CppInterOp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index b704a960f..2fdaa32ce 100755 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -3663,7 +3663,7 @@ namespace Cpp { if (!loadedSymbols.empty()) { llvm::cantFail(R->notifyResolved(loadedSymbols)); -#if CLANG_VERSION_MAJOR < 18 +#if CLANG_VERSION_MAJOR > 18 llvm::orc::SymbolDependenceGroup DepGroup; llvm::cantFail(R->notifyEmitted({DepGroup})); #else From 03a8e0be3b978e7e517a7b35c88330f0f4ceaff1 Mon Sep 17 00:00:00 2001 From: Alexander Penev Date: Tue, 4 Feb 2025 17:06:15 +0000 Subject: [PATCH 25/25] Fix3 --- unittests/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/unittests/CMakeLists.txt b/unittests/CMakeLists.txt index 972b1e475..04a6c59ed 100644 --- a/unittests/CMakeLists.txt +++ b/unittests/CMakeLists.txt @@ -38,6 +38,7 @@ endif() DEPENDS) # FIXME: Just call gtest_add_tests this function is available. #gtest_add_tests(${name} "${Arguments}" AUTO) + llvm_update_compile_flags(${name}) endfunction() add_subdirectory(CppInterOp)