Skip to content

Commit 0630b42

Browse files
Fix
1 parent 2a17cbf commit 0630b42

File tree

10 files changed

+220
-67
lines changed

10 files changed

+220
-67
lines changed

include/clang/Interpreter/CppInterOp.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,7 @@ namespace Cpp {
715715
///\param[in] autoload - true = libraries autoload is on.
716716
CPPINTEROP_API void SetLibrariesAutoload(bool autoload = true);
717717

718-
/// Get libraries autoload.
718+
/// Get libraries autoload status.
719719
///\returns LibraryAutoLoad state (true = libraries autoload is on).
720720
CPPINTEROP_API bool GetLibrariesAutoload();
721721

lib/Interpreter/CppInterOp.cpp

Lines changed: 49 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ namespace Cpp {
6363
using namespace std;
6464

6565
static std::unique_ptr<compat::Interpreter> sInterpreter;
66+
// Last assigned Autoload SearchGenerator
67+
// TODO: Test for thread safe.
68+
class AutoLoadLibrarySearchGenerator;
69+
static AutoLoadLibrarySearchGenerator *sAutoSG = nullptr;
6670
// Valgrind complains about __cxa_pure_virtual called when deleting
6771
// llvm::SectionMemoryManager::~SectionMemoryManager as part of the dtor chain
6872
// of the Interpreter.
@@ -1081,9 +1085,9 @@ namespace Cpp {
10811085
auto FDAorErr = compat::getSymbolAddress(I, mangled_name);
10821086
if (llvm::Error Err = FDAorErr.takeError())
10831087
llvm::consumeError(std::move(Err)); // nullptr if missing
1084-
else
1088+
else {
10851089
return llvm::jitTargetAddressToPointer<void*>(*FDAorErr);
1086-
1090+
}
10871091
return nullptr;
10881092
}
10891093

@@ -2590,7 +2594,7 @@ namespace Cpp {
25902594
std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr, MainAddr);
25912595

25922596
// build/tools/clang/unittests/Interpreter/Executable -> build/
2593-
StringRef Dir = sys::path::parent_path(BinaryPath);
2597+
Dir = sys::path::parent_path(BinaryPath);
25942598

25952599
Dir = sys::path::parent_path(Dir);
25962600
Dir = sys::path::parent_path(Dir);
@@ -2665,6 +2669,7 @@ namespace Cpp {
26652669
// calls to CreateInterpreter.
26662670
//assert(!sInterpreter && "Interpreter already set.");
26672671
sInterpreter.reset(I);
2672+
sAutoSG = nullptr;
26682673
return I;
26692674
}
26702675

@@ -3370,12 +3375,6 @@ namespace Cpp {
33703375
return nameForDlsym;
33713376
}
33723377

3373-
class AutoLoadLibrarySearchGenerator;
3374-
3375-
// Last assigned Autoload SearchGenerator
3376-
// TODO: Test for thread safe.
3377-
static AutoLoadLibrarySearchGenerator *AutoSG = nullptr;
3378-
33793378
class AutoLoadLibrarySearchGenerator : public llvm::orc::DefinitionGenerator {
33803379
bool Enabled = false;
33813380
public:
@@ -3392,15 +3391,15 @@ namespace Cpp {
33923391
std::string lib;
33933392
llvm::orc::SymbolNameVector syms;
33943393
public:
3395-
AutoloadLibraryMU(const std::string& Library, const llvm::orc::SymbolNameVector &Symbols)
3396-
: MaterializationUnit({getSymbolFlagsMap(Symbols), nullptr}), lib(Library), syms(Symbols) {}
3394+
AutoloadLibraryMU(std::string Library, const llvm::orc::SymbolNameVector &Symbols)
3395+
: MaterializationUnit({getSymbolFlagsMap(Symbols), nullptr}), lib(std::move(Library)), syms(Symbols) {}
33973396

3398-
[[nodiscard]] StringRef getName() const override {
3397+
StringRef getName() const override {
33993398
return "<Symbols from Autoloaded Library>";
34003399
}
34013400

34023401
void materialize(std::unique_ptr<llvm::orc::MaterializationResponsibility> R) override {
3403-
if (!AutoSG || !AutoSG->isEnabled()) {
3402+
if (!sAutoSG || !sAutoSG->isEnabled()) {
34043403
R->failMaterialization();
34053404
return;
34063405
}
@@ -3474,37 +3473,37 @@ namespace Cpp {
34743473

34753474
llvm::Error tryToGenerate(llvm::orc::LookupState &LS, llvm::orc::LookupKind K, llvm::orc::JITDylib &JD,
34763475
llvm::orc::JITDylibLookupFlags JDLookupFlags, const llvm::orc::SymbolLookupSet &Symbols) override {
3477-
if (isEnabled()) {
3478-
LLVM_DEBUG(dbgs() << "tryToGenerate");
3479-
3480-
auto& I = getInterp();
3481-
auto *DLM = I.getDynamicLibraryManager();
3482-
3483-
std::unordered_map<std::string, llvm::orc::SymbolNameVector> found;
3484-
llvm::orc::SymbolMap NewSymbols;
3485-
for (const auto &KV : Symbols) {
3486-
const auto &Name = KV.first;
3487-
if ((*Name).empty())
3488-
continue;
3489-
3490-
auto lib = DLM->searchLibrariesForSymbol(*Name, /*searchSystem=*/true); // false?
3491-
if (lib.empty())
3492-
continue;
3493-
3494-
found[lib].push_back(Name);
3495-
3496-
// Workaround: This getAddressOfGlobal call make first symbol search
3497-
// to work, immediatelly after library auto load. This approach do not
3498-
// use MU
3499-
//DLM->loadLibrary(lib, true);
3500-
//I.getAddressOfGlobal(*Name);
3501-
}
3476+
if (!isEnabled())
3477+
return llvm::Error::success();
3478+
LLVM_DEBUG(dbgs() << "tryToGenerate");
3479+
3480+
auto& I = getInterp();
3481+
auto *DLM = I.getDynamicLibraryManager();
3482+
3483+
std::unordered_map<std::string, llvm::orc::SymbolNameVector> found;
3484+
llvm::orc::SymbolMap NewSymbols;
3485+
for (const auto &KV : Symbols) {
3486+
const auto &Name = KV.first;
3487+
if ((*Name).empty())
3488+
continue;
3489+
3490+
auto lib = DLM->searchLibrariesForSymbol(*Name, /*searchSystem=*/true); // false?
3491+
if (lib.empty())
3492+
continue;
3493+
3494+
found[lib].push_back(Name);
3495+
3496+
// Workaround: This getAddressOfGlobal call make first symbol search
3497+
// to work, immediatelly after library auto load. This approach do not
3498+
// use MU
3499+
//DLM->loadLibrary(lib, true);
3500+
//I.getAddressOfGlobal(*Name);
3501+
}
35023502

3503-
for (auto &&KV : found) {
3504-
auto MU = std::make_unique<AutoloadLibraryMU>(KV.first, std::move(KV.second));
3505-
if (auto Err = JD.define(MU))
3506-
return Err;
3507-
}
3503+
for (auto &&KV : found) {
3504+
auto MU = std::make_unique<AutoloadLibraryMU>(KV.first, std::move(KV.second));
3505+
if (auto Err = JD.define(MU))
3506+
return Err;
35083507
}
35093508

35103509
return llvm::Error::success();
@@ -3521,16 +3520,17 @@ namespace Cpp {
35213520
llvm::orc::JITDylib& DyLib = *EE.getProcessSymbolsJITDylib().get();
35223521
#endif // CLANG_VERSION_MAJOR
35233522

3524-
if (!AutoSG)
3525-
AutoSG = &DyLib.addGenerator(std::make_unique<AutoLoadLibrarySearchGenerator>());
3526-
AutoSG->setEnabled(autoload);
3523+
if (!sAutoSG) {
3524+
sAutoSG = &DyLib.addGenerator(std::make_unique<AutoLoadLibrarySearchGenerator>());
3525+
}
3526+
sAutoSG->setEnabled(autoload);
35273527

3528-
LLVM_DEBUG(dbgs() << "Autoload=" << (AutoSG->isEnabled() ? "ON" : "OFF"));
3528+
LLVM_DEBUG(dbgs() << "Autoload=" << (sAutoSG->isEnabled() ? "ON" : "OFF"));
35293529
}
35303530

35313531
bool GetLibrariesAutoload() {
3532-
LLVM_DEBUG(dbgs() << "Autoload is " << (AutoSG && AutoSG->isEnabled() ? "ON" : "OFF"));
3533-
return AutoSG && AutoSG->isEnabled();
3532+
LLVM_DEBUG(dbgs() << "Autoload is " << (sAutoSG && sAutoSG->isEnabled() ? "ON" : "OFF"));
3533+
return sAutoSG && sAutoSG->isEnabled();
35343534
}
35353535

35363536
#undef DEBUG_TYPE

unittests/CppInterOp/CMakeLists.txt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,24 @@ target_link_libraries(DynamicLibraryManagerTests
3636
PRIVATE
3737
clangCppInterOp
3838
)
39+
set_source_files_properties(DynamicLibraryManagerTest.cpp PROPERTIES COMPILE_DEFINITIONS
40+
"LLVM_BINARY_DIR=\"${LLVM_BINARY_DIR}\""
41+
)
42+
set_source_files_properties(DynamicLibraryManagerTest.cpp PROPERTIES COMPILE_DEFINITIONS
43+
"CPPINTEROP_VERSION=\"${CPPINTEROP_VERSION}\""
44+
)
3945

4046
set_output_directory(DynamicLibraryManagerTests BINARY_DIR ${CMAKE_BINARY_DIR}/unittests/bin/$<CONFIG>/)
4147

4248
add_dependencies(DynamicLibraryManagerTests TestSharedLib)
4349
#export_executable_symbols_for_plugins(TestSharedLib)
4450
add_subdirectory(TestSharedLib)
45-
# TODO: Remove when libraryunload work correctly
4651
add_dependencies(DynamicLibraryManagerTests TestSharedLib1)
4752
#export_executable_symbols_for_plugins(TestSharedLib1)
4853
add_subdirectory(TestSharedLib1)
54+
add_dependencies(DynamicLibraryManagerTests TestSharedLib2)
55+
#export_executable_symbols_for_plugins(TestSharedLib2)
56+
add_subdirectory(TestSharedLib2)
57+
add_dependencies(DynamicLibraryManagerTests TestSharedLib3)
58+
#export_executable_symbols_for_plugins(TestSharedLib3)
59+
add_subdirectory(TestSharedLib3)

unittests/CppInterOp/DynamicLibraryManagerTest.cpp

Lines changed: 104 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@
22

33
#include "clang/Interpreter/CppInterOp.h"
44

5+
6+
#include "llvm/ExecutionEngine/Orc/Core.h"
7+
#include "llvm/ExecutionEngine/Orc/DebugUtils.h"
58
#include "llvm/Support/FileSystem.h"
69
#include "llvm/Support/Path.h"
7-
10+
#include "llvm/Support/raw_ostream.h"
811

912
// Helper functions
1013

@@ -13,13 +16,14 @@
1316
// GetMainExecutable (since some platforms don't support taking the
1417
// address of main, and some platforms can't implement GetMainExecutable
1518
// without being given the address of a function in the main executable).
16-
std::string GetExecutablePath(const char* Argv0) {
19+
std::string GetExecutablePath(const char* Argv0, void* mainAddr = nullptr) {
1720
// This just needs to be some symbol in the binary; C++ doesn't
1821
// allow taking the address of ::main however.
19-
void* MainAddr = (void*)intptr_t(GetExecutablePath);
22+
void* MainAddr = mainAddr ? mainAddr : (void*)intptr_t(GetExecutablePath);
2023
return llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
2124
}
2225

26+
// Mangle symbol name
2327
static inline std::string MangleNameForDlsym(const std::string& name) {
2428
std::string nameForDlsym = name;
2529
#if defined(R__MACOSX) || defined(R__WIN32)
@@ -29,30 +33,33 @@ static inline std::string MangleNameForDlsym(const std::string& name) {
2933
return nameForDlsym;
3034
}
3135

36+
#include "../../lib/Interpreter/CppInterOp.cpp"
37+
38+
3239
// Tests
3340

3441
TEST(DynamicLibraryManagerTest, Sanity) {
3542
EXPECT_TRUE(Cpp::CreateInterpreter());
3643
EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_zero").c_str()));
3744

38-
std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr);
45+
std::string BinaryPath = GetExecutablePath(nullptr, nullptr);
3946
llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath);
4047
Cpp::AddSearchPath(Dir.str().c_str());
4148

49+
// Find and load library with "ret_zero" symbol defined and exported
50+
//
4251
// FIXME: dlsym on mach-o takes the C-level name, however, the macho-o format
4352
// adds an additional underscore (_) prefix to the lowered names. Figure out
4453
// how to harmonize that API.
45-
4654
std::string PathToTestSharedLib =
4755
Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_zero").c_str(), /*system_search=*/false);
48-
4956
EXPECT_STRNE("", PathToTestSharedLib.c_str())
50-
<< "Cannot find: '" << PathToTestSharedLib << "' in '" << Dir.str()
51-
<< "'";
52-
57+
<< "Cannot find: '" << PathToTestSharedLib << "' in '" << Dir.str() << "'";
5358
EXPECT_TRUE(Cpp::LoadLibrary(PathToTestSharedLib.c_str()));
59+
5460
// Force ExecutionEngine to be created.
5561
Cpp::Process("");
62+
5663
// FIXME: Conda returns false to run this code on osx.
5764
EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_zero").c_str()));
5865

@@ -70,19 +77,18 @@ TEST(DynamicLibraryManagerTest, LibrariesAutoload) {
7077
EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str()));
7178

7279
// Libraries Search path by default is set to main executable directory.
73-
std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr);
80+
std::string BinaryPath = GetExecutablePath(nullptr, nullptr);
7481
llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath);
7582
Cpp::AddSearchPath(Dir.str().c_str());
7683

77-
// Find library with "rec_one" symbol defined and exported
84+
// Find library with "ret_one" symbol defined and exported
7885
//
7986
// FIXME: dlsym on mach-o takes the C-level name, however, the macho-o format
8087
// adds an additional underscore (_) prefix to the lowered names. Figure out
8188
// how to harmonize that API. For now we use out minimal implementation of
8289
// helper function.
8390
std::string PathToTestSharedLib1 =
8491
Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_one").c_str(), /*system_search=*/false);
85-
8692
// If result is "" then we cannot find this library.
8793
EXPECT_STRNE("", PathToTestSharedLib1.c_str())
8894
<< "Cannot find: '" << PathToTestSharedLib1 << "' in '" << Dir.str() << "'";
@@ -117,13 +123,95 @@ TEST(DynamicLibraryManagerTest, LibrariesAutoload) {
117123
Cpp::SetLibrariesAutoload(false);
118124
// Check autorum status (must be turned OFF)
119125
EXPECT_FALSE(Cpp::GetLibrariesAutoload());
120-
//EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); // if unload works
121-
//EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str()));
126+
EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); // false if unload works
122127

123128
// Autoload turn ON again
124129
Cpp::SetLibrariesAutoload(true);
125130
// Check autorum status (must be turned ON)
126131
EXPECT_TRUE(Cpp::GetLibrariesAutoload());
127-
//EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str())); // if unload works
128-
//EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str()));
132+
EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_one").c_str()));
133+
134+
// Find library with "ret_1" symbol defined and exported
135+
std::string PathToTestSharedLib2 =
136+
Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_1").c_str(), /*system_search=*/false);
137+
// If result is "" then we cannot find this library.
138+
EXPECT_STRNE("", PathToTestSharedLib2.c_str())
139+
<< "Cannot find: '" << PathToTestSharedLib2 << "' in '" << Dir.str() << "'";
140+
// Find symbol must success (when auotload=on)
141+
EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_1").c_str()));
142+
// Find symbol must success (when auotload=on)
143+
EXPECT_TRUE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_2").c_str()));
144+
}
145+
146+
TEST(DynamicLibraryManagerTest, LibrariesAutoloadExtraCoverage) {
147+
EXPECT_TRUE(Cpp::CreateInterpreter());
148+
149+
// Libraries Search path by default is set to main executable directory.
150+
std::string BinaryPath = GetExecutablePath(nullptr, nullptr);
151+
llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath);
152+
Cpp::AddSearchPath(Dir.str().c_str());
153+
154+
// Force ExecutionEngine to be created.
155+
Cpp::Process("");
156+
157+
// Autoload turn ON
158+
Cpp::SetLibrariesAutoload(true);
159+
// Check autorum status (must be turned ON)
160+
EXPECT_TRUE(Cpp::GetLibrariesAutoload());
161+
162+
// Cover: MU getName()
163+
std::string res;
164+
llvm::raw_string_ostream rss(res);
165+
const llvm::orc::SymbolNameVector syms;
166+
Cpp::AutoLoadLibrarySearchGenerator::AutoloadLibraryMU MU("test", syms);
167+
rss << MU;
168+
EXPECT_STRNE("", rss.str().c_str()) << "MU problem!";
169+
170+
// Cover: LoadLibrary error
171+
// if (DLM->loadLibrary(lib, false) != DynamicLibraryManager::LoadLibResult::kLoadLibSuccess) {
172+
// LLVM_DEBUG(dbgs() << "MU: Failed to load library " << lib);
173+
// string err = "MU: Failed to load library! " + lib;
174+
// perror(err.c_str());
175+
// } else {
176+
// Find library with "ret_value" symbol defined and exported
177+
std::string PathToTestSharedLib3 =
178+
Cpp::SearchLibrariesForSymbol(MangleNameForDlsym("ret_val").c_str(), /*system_search=*/false);
179+
// If result is "" then we cannot find this library.
180+
EXPECT_STRNE("", PathToTestSharedLib3.c_str())
181+
<< "Cannot find: '" << PathToTestSharedLib3 << "' in '" << Dir.str() << "'";
182+
// Remove library for simulate load error
183+
llvm::sys::fs::remove(PathToTestSharedLib3, true);
184+
EXPECT_TRUE(Cpp::GetLibrariesAutoload());
185+
// FIXME: Conda returns false to run this code on osx.
186+
EXPECT_FALSE(Cpp::GetFunctionAddress(MangleNameForDlsym("ret_val").c_str()));
187+
188+
// Cover
189+
// } else {
190+
// // Collect all failing symbols, delegate their responsibility and then
191+
// // fail their materialization. R->defineNonExistent() sounds like it
192+
// // should do that, but it's not implemented?!
193+
// failedSymbols.insert(symbol);
194+
// TODO: implement test this case
195+
196+
// Cover
197+
// if (!failedSymbols.empty()) {
198+
// auto failingMR = R->delegate(failedSymbols);
199+
// if (failingMR) {
200+
// (*failingMR)->failMaterialization();
201+
// TODO: implement test this case
202+
203+
// Cover
204+
// void discard(const llvm::orc::JITDylib &JD, const llvm::orc::SymbolStringPtr &Name) override {}
205+
// TODO: implement test this case
206+
207+
// Cover
208+
// if (Path.empty()) {
209+
// LLVM_DEBUG(dbgs() << "DynamicLibraryManager::lookupLibMaybeAddExt(): "
210+
// TODO: implement test this case
211+
212+
// Cover
213+
// platform::DLClose(dyLibHandle, &errMsg);
214+
// if (!errMsg.empty()) {
215+
// LLVM_DEBUG(dbgs() << "DynamicLibraryManager::unloadLibrary(): "
216+
// TODO: implement test this case
129217
}

0 commit comments

Comments
 (0)