From 2783d9cf2d03fe15501b45a3c9807bbeda44282c Mon Sep 17 00:00:00 2001 From: Vassil Vassilev Date: Mon, 30 Oct 2023 11:37:16 +0000 Subject: [PATCH] Expand testing coverage to cover the dynamic library manager --- include/clang/Interpreter/CppInterOp.h | 21 +++++++++-- lib/Interpreter/CppInterOp.cpp | 35 +++++++++++++------ lib/Interpreter/CppInterOpInterpreter.h | 6 +++- .../CppInterOp/DynamicLibraryManagerTest.cpp | 33 ++++++++++++++++- .../CppInterOp/TestSharedLib/TestSharedLib.h | 3 +- 5 files changed, 82 insertions(+), 16 deletions(-) diff --git a/include/clang/Interpreter/CppInterOp.h b/include/clang/Interpreter/CppInterOp.h index e870fb3ac..7e1d56d07 100644 --- a/include/clang/Interpreter/CppInterOp.h +++ b/include/clang/Interpreter/CppInterOp.h @@ -340,7 +340,10 @@ namespace Cpp { /// Checks if the provided parameter is a 'Static' method. bool IsStaticMethod(TCppConstFunction_t method); - /// Gets the address of the function to be able to call it. + ///\returns the address of the function given its potentially mangled name. + TCppFuncAddr_t GetFunctionAddress(const char* mangled_name); + + ///\returns the address of the function given its function declaration. TCppFuncAddr_t GetFunctionAddress(TCppFunction_t method); /// Checks if the provided parameter is a 'Virtual' method. @@ -461,9 +464,21 @@ namespace Cpp { ///\returns the path to the library. const std::string LookupLibrary(const char *lib_name); - /// Loads the library based on the path returned by the LookupLibrary() + /// Finds \c lib_stem considering the list of search paths and loads it by + /// calling dlopen. + /// \returns true on success. + bool LoadLibrary(const char* lib_stem, bool lookup = true); + + /// Finds \c lib_stem considering the list of search paths and unloads it by + /// calling dlclose. /// function. - bool LoadLibrary(const char *lib_path, bool lookup = true); + void UnloadLibrary(const char* lib_stem); + + /// Scans all libraries on the library search path for a given potentially + /// mangled symbol name. + ///\returns the path to the first library that contains the symbol definition. + std::string SearchLibrariesForSymbol(const char* mangled_name, + bool search_system /*true*/); /// Inserts or replaces a symbol in the JIT with the one provided. This is /// useful for providing our own implementations of facilities such as printf. diff --git a/lib/Interpreter/CppInterOp.cpp b/lib/Interpreter/CppInterOp.cpp index fad1c385f..72063cadd 100644 --- a/lib/Interpreter/CppInterOp.cpp +++ b/lib/Interpreter/CppInterOp.cpp @@ -980,6 +980,17 @@ namespace Cpp { return false; } + TCppFuncAddr_t GetFunctionAddress(const char* mangled_name) { + auto& I = getInterp(); + auto FDAorErr = compat::getSymbolAddress(I, mangled_name); + if (llvm::Error Err = FDAorErr.takeError()) + llvm::consumeError(std::move(Err)); // nullptr if missing + else + return llvm::jitTargetAddressToPointer(*FDAorErr); + + return nullptr; + } + TCppFuncAddr_t GetFunctionAddress(TCppFunction_t method) { auto *D = (Decl *) method; @@ -1002,14 +1013,8 @@ namespace Cpp { return mangled_name; }; - auto &I = getInterp(); - if (auto *FD = llvm::dyn_cast_or_null(D)) { - auto FDAorErr = compat::getSymbolAddress(I, StringRef(get_mangled_name(FD))); - if (llvm::Error Err = FDAorErr.takeError()) - llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "Failed to GetFunctionAdress:"); - else - return llvm::jitTargetAddressToPointer(*FDAorErr); - } + if (auto* FD = llvm::dyn_cast_or_null(D)) + return GetFunctionAddress(get_mangled_name(FD).c_str()); return 0; } @@ -2588,13 +2593,23 @@ namespace Cpp { return getInterp().getDynamicLibraryManager()->lookupLibrary(lib_name); } - bool LoadLibrary(const char *lib_name, bool lookup) { + bool LoadLibrary(const char* lib_stem, bool lookup) { compat::Interpreter::CompilationResult res = - getInterp().loadLibrary(lib_name, lookup); + getInterp().loadLibrary(lib_stem, lookup); return res == compat::Interpreter::kSuccess; } + void UnloadLibrary(const char* lib_stem) { + getInterp().getDynamicLibraryManager()->unloadLibrary(lib_stem); + } + + std::string SearchLibrariesForSymbol(const char* mangled_name, + bool search_system /*true*/) { + auto* DLM = getInterp().getDynamicLibraryManager(); + return DLM->searchLibrariesForSymbol(mangled_name, search_system); + } + bool InsertOrReplaceJitSymbol(const char* linker_mangled_name, uint64_t address) { // FIXME: This approach is problematic since we could replace a symbol diff --git a/lib/Interpreter/CppInterOpInterpreter.h b/lib/Interpreter/CppInterOpInterpreter.h index 4cb9a3024..ea5ba2688 100644 --- a/lib/Interpreter/CppInterOpInterpreter.h +++ b/lib/Interpreter/CppInterOpInterpreter.h @@ -330,7 +330,11 @@ class Interpreter { const DynamicLibraryManager* getDynamicLibraryManager() const { assert(compat::getExecutionEngine(*inner) && "We must have an executor"); - static const DynamicLibraryManager* DLM = new DynamicLibraryManager(); + static DynamicLibraryManager* DLM = nullptr; + if (!DLM) { + DLM = new DynamicLibraryManager(); + DLM->initializeDyld([](llvm::StringRef) { /*ignore*/ return false; }); + } return DLM; // TODO: Add DLM to InternalExecutor and use executor->getDML() // return inner->getExecutionEngine()->getDynamicLibraryManager(); diff --git a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp index c7dea4c2f..99fe42d41 100644 --- a/unittests/CppInterOp/DynamicLibraryManagerTest.cpp +++ b/unittests/CppInterOp/DynamicLibraryManagerTest.cpp @@ -2,7 +2,38 @@ #include "clang/Interpreter/CppInterOp.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/Path.h" + +// 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 +// 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) { + // 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); + return llvm::sys::fs::getMainExecutable(Argv0, MainAddr); +} + TEST(DynamicLibraryManagerTest, Sanity) { EXPECT_TRUE(Cpp::CreateInterpreter()); - EXPECT_TRUE(Cpp::LoadLibrary("TestSharedLib")); + EXPECT_FALSE(Cpp::GetFunctionAddress("ret_zero")); + + std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr); + llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath); + Cpp::AddSearchPath(Dir.str().c_str()); + + std::string PathToTestSharedLib = + Cpp::SearchLibrariesForSymbol("ret_zero", /*system_search=*/false); + EXPECT_STRNE("", PathToTestSharedLib.c_str()); + + EXPECT_TRUE(Cpp::LoadLibrary(PathToTestSharedLib.c_str())); + EXPECT_TRUE(Cpp::GetFunctionAddress("ret_zero")); + 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")); } diff --git a/unittests/CppInterOp/TestSharedLib/TestSharedLib.h b/unittests/CppInterOp/TestSharedLib/TestSharedLib.h index 9577f66d6..41fe7f758 100644 --- a/unittests/CppInterOp/TestSharedLib/TestSharedLib.h +++ b/unittests/CppInterOp/TestSharedLib/TestSharedLib.h @@ -1,6 +1,7 @@ #ifndef UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB_H #define UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB_H -int ret_zero(); +// Avoid having to mangle/demangle the symbol name in tests. +extern "C" int ret_zero(); #endif // UNITTESTS_CPPINTEROP_TESTSHAREDLIB_TESTSHAREDLIB_H