Skip to content

fix GetFunctionAddress resolution with explicit name change #793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
conrade-ctc wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix GetFunctionAddress resolution with explicit name change #793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion include/CppInterOp/CppInterOp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,8 @@ CPPINTEROP_API bool IsStaticMethod(TCppConstFunction_t method);
CPPINTEROP_API bool IsExplicit(TCppConstFunction_t method);

///\returns the address of the function given its potentially mangled name.
CPPINTEROP_API TCppFuncAddr_t GetFunctionAddress(const char* mangled_name);
CPPINTEROP_API TCppFuncAddr_t
GetFunctionAddressFromMangledName(const char* mangled_name);

///\returns the address of the function given its function declaration.
CPPINTEROP_API TCppFuncAddr_t GetFunctionAddress(TCppFunction_t method);
Expand Down
3 changes: 1 addition & 2 deletions include/CppInterOp/Dispatch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,7 @@ extern "C" CPPINTEROP_API CppFnPtrTy CppGetProcAddress(const char* procname);
DISPATCH_API(IsExplicit, decltype(&CppImpl::IsExplicit)) \
DISPATCH_API(MakeFunctionCallable, \
CppImpl::JitCall (*)(CppImpl::TCppConstFunction_t)) \
DISPATCH_API(GetFunctionAddress, \
CppImpl::TCppFuncAddr_t (*)(CppImpl::TCppFunction_t)) \
DISPATCH_API(GetFunctionAddress, decltype(&CppImpl::GetFunctionAddress)) \
/*DISPATCH_API(API_name, fnptr_ty)*/

// TODO: implement overload that takes an existing opened DL handle
Expand Down
4 changes: 2 additions & 2 deletions lib/CppInterOp/CppInterOp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1430,7 +1430,7 @@ bool IsExplicit(TCppConstFunction_t method) {
return false;
}

TCppFuncAddr_t GetFunctionAddress(const char* mangled_name) {
TCppFuncAddr_t GetFunctionAddressFromMangledName(const char* mangled_name) {
auto& I = getInterp();
auto FDAorErr = compat::getSymbolAddress(I, mangled_name);
if (llvm::Error Err = FDAorErr.takeError())
Expand Down Expand Up @@ -1462,7 +1462,7 @@ static TCppFuncAddr_t GetFunctionAddress(const FunctionDecl* FD) {

// Constructor and Destructors needs to be handled differently
if (!llvm::isa<CXXConstructorDecl>(FD) && !llvm::isa<CXXDestructorDecl>(FD))
return GetFunctionAddress(get_mangled_name(FD).c_str());
return GetFunctionAddressFromMangledName(get_mangled_name(FD).c_str());

return 0;
}
Expand Down
10 changes: 5 additions & 5 deletions unittests/CppInterOp/DynamicLibraryManagerTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, DynamicLibraryManager_Sanity) {
GTEST_SKIP() << "Test fails for OOP JIT builds";

EXPECT_TRUE(TestFixture::CreateInterpreter());
EXPECT_FALSE(Cpp::GetFunctionAddress("ret_zero"));
EXPECT_FALSE(Cpp::GetFunctionAddressFromMangledName("ret_zero"));

std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr);
llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath);
Expand All @@ -59,14 +59,14 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, DynamicLibraryManager_Sanity) {
Cpp::Process("");
// FIXME: Conda returns false to run this code on osx.
#ifndef __APPLE__
EXPECT_TRUE(Cpp::GetFunctionAddress("ret_zero"));
EXPECT_TRUE(Cpp::GetFunctionAddressFromMangledName("ret_zero"));
#endif //__APPLE__

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::GetFunctionAddressFromMangledName("ret_zero"));
}

TYPED_TEST(CPPINTEROP_TEST_MODE, DynamicLibraryManager_BasicSymbolLookup) {
Expand All @@ -81,15 +81,15 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, DynamicLibraryManager_BasicSymbolLookup) {
GTEST_SKIP() << "Test fails for OOP JIT builds";

ASSERT_TRUE(TestFixture::CreateInterpreter());
EXPECT_FALSE(Cpp::GetFunctionAddress("ret_zero"));
EXPECT_FALSE(Cpp::GetFunctionAddressFromMangledName("ret_zero"));

// Load the library manually. Use known preload path (MEMFS path)
const char* wasmLibPath = "libTestSharedLib.so"; // Preloaded path in MEMFS
EXPECT_TRUE(Cpp::LoadLibrary(wasmLibPath, false));

Cpp::Process("");

void* Addr = Cpp::GetFunctionAddress("ret_zero");
void* Addr = Cpp::GetFunctionAddressFromMangledName("ret_zero");
EXPECT_NE(Addr, nullptr) << "Symbol 'ret_zero' not found after dlopen.";

using RetZeroFn = int (*)();
Expand Down
Loading