-
Notifications
You must be signed in to change notification settings - Fork 34
[interp] Register runtime symbols for clang-repl #726
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.
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
b912765
9c6be69
e8a6b27
740bcb3
4ef7df9
8d91bcd
e15be64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -90,6 +90,34 @@ | |||||
#include <unistd.h> | ||||||
#endif // WIN32 | ||||||
|
||||||
// Runtime symbols required if the library using JIT (Cpp::Evaluate) does not | ||||||
// link to llvm | ||||||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||||||
struct __clang_Interpreter_NewTag { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: invalid case style for class '__clang_Interpreter_NewTag' [readability-identifier-naming] struct __clang_Interpreter_NewTag {
^ this fix will not be applied because it overlaps with another fix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add the relevant clang-tidy suppression. |
||||||
} __ci_newtag; | ||||||
vgvassilev marked this conversation as resolved.
Show resolved
Hide resolved
vgvassilev marked this conversation as resolved.
Show resolved
Hide resolved
vgvassilev marked this conversation as resolved.
Show resolved
Hide resolved
vgvassilev marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
#if CLANG_VERSION_MAJOR >= 22 | ||||||
aaronj0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
extern "C" void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal, | ||||||
void* OpaqueType) | ||||||
#else | ||||||
void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: declaration uses identifier '__clang_Interpreter_SetValueWithAlloc', which is a reserved identifier [bugprone-reserved-identifier] void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal,
^ this fix will not be applied because it overlaps with another fix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: function '__clang_Interpreter_SetValueWithAlloc' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage]
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: invalid case style for function '__clang_Interpreter_SetValueWithAlloc' [readability-identifier-naming] void* __clang_Interpreter_SetValueWithAlloc(void* This, void* OutVal,
^ this fix will not be applied because it overlaps with another fix |
||||||
void* OpaqueType); | ||||||
#endif | ||||||
|
||||||
#if CLANG_VERSION_MAJOR >= 19 | ||||||
aaronj0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: declaration uses identifier '__clang_Interpreter_SetValueNoAlloc', which is a reserved identifier [bugprone-reserved-identifier] extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This,
^ this fix will not be applied because it overlaps with another fix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: invalid case style for function '__clang_Interpreter_SetValueNoAlloc' [readability-identifier-naming] extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This,
^ this fix will not be applied because it overlaps with another fix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: declaration uses identifier '__clang_Interpreter_SetValueNoAlloc', which is a reserved identifier [bugprone-reserved-identifier] extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This,
^ this fix will not be applied because it overlaps with another fix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: invalid case style for function '__clang_Interpreter_SetValueNoAlloc' [readability-identifier-naming] extern "C" void __clang_Interpreter_SetValueNoAlloc(void* This,
^ this fix will not be applied because it overlaps with another fix |
||||||
void* OutVal, | ||||||
void* OpaqueType, ...); | ||||||
#elif CLANG_VERSION_MAJOR == 18 | ||||||
aaronj0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
void __clang_Interpreter_SetValueNoAlloc(void*, void*, void*); | ||||||
void __clang_Interpreter_SetValueNoAlloc(void*, void*, void*, void*); | ||||||
void __clang_Interpreter_SetValueNoAlloc(void*, void*, void*, float); | ||||||
void __clang_Interpreter_SetValueNoAlloc(void*, void*, void*, double); | ||||||
void __clang_Interpreter_SetValueNoAlloc(void*, void*, void*, long double); | ||||||
void __clang_Interpreter_SetValueNoAlloc(void*, void*, void*, | ||||||
unsigned long long); | ||||||
#endif | ||||||
#endif // CPPINTEROP_USE_CLING | ||||||
|
||||||
namespace Cpp { | ||||||
|
||||||
using namespace clang; | ||||||
|
@@ -3330,6 +3358,37 @@ | |||||
} | ||||||
|
||||||
namespace { | ||||||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||||||
static bool DefineAbsoluteSymbol(compat::Interpreter& I, | ||||||
aaronj0 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
const char* linker_mangled_name, | ||||||
uint64_t address) { | ||||||
|
||||||
using namespace llvm; | ||||||
using namespace llvm::orc; | ||||||
|
||||||
llvm::orc::LLJIT& Jit = *compat::getExecutionEngine(I); | ||||||
llvm::orc::ExecutionSession& ES = Jit.getExecutionSession(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: no header providing "llvm::orc::ExecutionSession" is directly included [misc-include-cleaner] lib/CppInterOp/CppInterOp.cpp:41: - #if CLANG_VERSION_MAJOR >= 19
+ #include <llvm/ExecutionEngine/Orc/Core.h>
+ #if CLANG_VERSION_MAJOR >= 19 |
||||||
JITDylib& DyLib = *Jit.getProcessSymbolsJITDylib().get(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: no header providing "llvm::orc::JITDylib" is directly included [misc-include-cleaner] JITDylib& DyLib = *Jit.getProcessSymbolsJITDylib().get();
^ |
||||||
|
||||||
llvm::orc::SymbolMap InjectedSymbols; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: no header providing "llvm::orc::SymbolMap" is directly included [misc-include-cleaner] lib/CppInterOp/CppInterOp.cpp:41: - #if CLANG_VERSION_MAJOR >= 19
+ #include <llvm/ExecutionEngine/Orc/CoreContainers.h>
+ #if CLANG_VERSION_MAJOR >= 19 |
||||||
auto& DL = compat::getExecutionEngine(I)->getDataLayout(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: 'auto &DL' can be declared as 'const auto &DL' [readability-qualified-auto]
Suggested change
|
||||||
char GlobalPrefix = DL.getGlobalPrefix(); | ||||||
std::string tmp(linker_mangled_name); | ||||||
if (GlobalPrefix != '\0') { | ||||||
tmp = std::string(1, GlobalPrefix) + tmp; | ||||||
} | ||||||
auto Name = ES.intern(tmp); | ||||||
InjectedSymbols[Name] = | ||||||
ExecutorSymbolDef(ExecutorAddr(address), JITSymbolFlags::Exported); | ||||||
|
||||||
if (Error Err = DyLib.define(absoluteSymbols(InjectedSymbols))) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: no header providing "llvm::orc::absoluteSymbols" is directly included [misc-include-cleaner] lib/CppInterOp/CppInterOp.cpp:41: - #if CLANG_VERSION_MAJOR >= 19
+ #include <llvm/ExecutionEngine/Orc/AbsoluteSymbols.h>
+ #if CLANG_VERSION_MAJOR >= 19 |
||||||
logAllUnhandledErrors(std::move(Err), errs(), | ||||||
"DefineAbsoluteSymbol error: "); | ||||||
return true; | ||||||
} | ||||||
return false; | ||||||
} | ||||||
#endif | ||||||
|
||||||
static std::string MakeResourcesPath() { | ||||||
StringRef Dir; | ||||||
#ifdef LLVM_BINARY_DIR | ||||||
|
@@ -3442,6 +3501,85 @@ | |||||
sInterpreters->back().get()}); | ||||||
|
||||||
assert(sInterpreters->size() == sInterpreterASTMap->size()); | ||||||
|
||||||
// Define runtime symbols in the JIT dylib for clang-repl | ||||||
#if !defined(CPPINTEROP_USE_CLING) && !defined(EMSCRIPTEN) | ||||||
DefineAbsoluteSymbol(*I, "__ci_newtag", (uint64_t)&__ci_newtag); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast] DefineAbsoluteSymbol(*I, "__ci_newtag", (uint64_t)&__ci_newtag);
^ |
||||||
// llvm > 22 has this defined as a C symbol that does not require mangling | ||||||
#if CLANG_VERSION_MAJOR >= 22 | ||||||
DefineAbsoluteSymbol(*I, "__clang_Interpreter_SetValueWithAlloc", | ||||||
(uint64_t)&__clang_Interpreter_SetValueWithAlloc); | ||||||
#else | ||||||
// obtain mangled name | ||||||
auto* D = static_cast<clang::Decl*>( | ||||||
Cpp::GetNamed("__clang_Interpreter_SetValueWithAlloc")); | ||||||
if (auto* FD = llvm::dyn_cast<FunctionDecl>(D)) { | ||||||
auto GD = GlobalDecl(FD); | ||||||
std::string mangledName; | ||||||
compat::maybeMangleDeclName(GD, mangledName); | ||||||
DefineAbsoluteSymbol(*I, mangledName.c_str(), | ||||||
(uint64_t)&__clang_Interpreter_SetValueWithAlloc); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast] (uint64_t)&__clang_Interpreter_SetValueWithAlloc);
^ |
||||||
} | ||||||
#endif | ||||||
// llvm < 19 has multiple overloads of __clang_Interpreter_SetValueNoAlloc | ||||||
#if CLANG_VERSION_MAJOR < 19 | ||||||
// obtain all 6 candidates, and obtain the correct Decl for each overload | ||||||
// using BestOverloadFunctionMatch. We then map the decl to the correct | ||||||
// function pointer (force the compiler to find the right declarion by casting | ||||||
// to the corresponding function pointer signature) and then register it. | ||||||
const std::vector<TCppFunction_t> Methods = Cpp::GetFunctionsUsingName( | ||||||
Cpp::GetGlobalScope(), "__clang_Interpreter_SetValueNoAlloc"); | ||||||
std::string mangledName; | ||||||
ASTContext& Ctxt = I->getSema().getASTContext(); | ||||||
auto* TAI = Ctxt.VoidPtrTy.getAsOpaquePtr(); | ||||||
|
||||||
// possible parameter lists for __clang_Interpreter_SetValueNoAlloc overloads | ||||||
// in LLVM 18 | ||||||
const std::vector<std::vector<Cpp::TemplateArgInfo>> a_params = { | ||||||
{TAI, TAI, TAI}, | ||||||
{TAI, TAI, TAI, TAI}, | ||||||
{TAI, TAI, TAI, Ctxt.FloatTy.getAsOpaquePtr()}, | ||||||
{TAI, TAI, TAI, Ctxt.DoubleTy.getAsOpaquePtr()}, | ||||||
{TAI, TAI, TAI, Ctxt.LongDoubleTy.getAsOpaquePtr()}, | ||||||
{TAI, TAI, TAI, Ctxt.UnsignedLongLongTy.getAsOpaquePtr()}}; | ||||||
|
||||||
using FP0 = void (*)(void*, void*, void*); | ||||||
using FP1 = void (*)(void*, void*, void*, void*); | ||||||
using FP2 = void (*)(void*, void*, void*, float); | ||||||
using FP3 = void (*)(void*, void*, void*, double); | ||||||
using FP4 = void (*)(void*, void*, void*, long double); | ||||||
using FP5 = void (*)(void*, void*, void*, unsigned long long); | ||||||
|
||||||
const std::vector<void*> func_pointers = { | ||||||
reinterpret_cast<void*>( | ||||||
static_cast<FP0>(&__clang_Interpreter_SetValueNoAlloc)), | ||||||
reinterpret_cast<void*>( | ||||||
static_cast<FP1>(&__clang_Interpreter_SetValueNoAlloc)), | ||||||
reinterpret_cast<void*>( | ||||||
static_cast<FP2>(&__clang_Interpreter_SetValueNoAlloc)), | ||||||
reinterpret_cast<void*>( | ||||||
static_cast<FP3>(&__clang_Interpreter_SetValueNoAlloc)), | ||||||
reinterpret_cast<void*>( | ||||||
static_cast<FP4>(&__clang_Interpreter_SetValueNoAlloc)), | ||||||
reinterpret_cast<void*>( | ||||||
static_cast<FP5>(&__clang_Interpreter_SetValueNoAlloc))}; | ||||||
|
||||||
// these symbols are not externed, so we need to mangle their names | ||||||
for (size_t i = 0; i < a_params.size(); ++i) { | ||||||
auto* decl = static_cast<clang::Decl*>( | ||||||
Cpp::BestOverloadFunctionMatch(Methods, {}, a_params[i])); | ||||||
if (auto* fd = llvm::dyn_cast<clang::FunctionDecl>(decl)) { | ||||||
auto gd = clang::GlobalDecl(fd); | ||||||
compat::maybeMangleDeclName(gd, mangledName); | ||||||
DefineAbsoluteSymbol(*I, mangledName.c_str(), | ||||||
reinterpret_cast<uint64_t>(func_pointers[i])); | ||||||
} | ||||||
} | ||||||
#else | ||||||
DefineAbsoluteSymbol(*I, "__clang_Interpreter_SetValueNoAlloc", | ||||||
(uint64_t)&__clang_Interpreter_SetValueNoAlloc); | ||||||
aaronj0 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast] (uint64_t)&__clang_Interpreter_SetValueNoAlloc);
^ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast] (uint64_t)&__clang_Interpreter_SetValueNoAlloc);
^ |
||||||
#endif | ||||||
#endif | ||||||
return I; | ||||||
} | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,6 +84,38 @@ TEST(InterpreterTest, Evaluate) { | |
EXPECT_FALSE(HadError) ; | ||
} | ||
|
||
TEST(InterpreterTest, EvaluateExtensive) { | ||
#ifdef EMSCRIPTEN | ||
GTEST_SKIP() << "Test fails for Emscipten builds"; | ||
#endif | ||
Comment on lines
+87
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All the tests at the start of this test are the same as InterpreterTest, Evaluate. Can we not remove that old test, and just replace it with this one? |
||
#ifdef _WIN32 | ||
GTEST_SKIP() << "Disabled on Windows. Needs fixing."; | ||
#endif | ||
if (llvm::sys::RunningOnValgrind()) | ||
GTEST_SKIP() << "XFAIL due to Valgrind report"; | ||
Cpp::CreateInterpreter(); | ||
EXPECT_TRUE(Cpp::Evaluate("__cplusplus") == 201402); | ||
|
||
bool HadError; | ||
EXPECT_TRUE(Cpp::Evaluate("#error", &HadError) == (intptr_t)~0UL); | ||
EXPECT_TRUE(HadError); | ||
// for llvm < 19 this tests all different overloads of __clang_Interpreter_SetValueNoAlloc | ||
EXPECT_EQ(Cpp::Evaluate("int i = 11; ++i", &HadError), 12); | ||
EXPECT_FALSE(HadError) ; | ||
EXPECT_EQ(Cpp::Evaluate("double a = 12.; a", &HadError), 12.); | ||
EXPECT_FALSE(HadError) ; | ||
EXPECT_EQ(Cpp::Evaluate("float b = 13.; b", &HadError), 13.); | ||
EXPECT_FALSE(HadError) ; | ||
EXPECT_EQ(Cpp::Evaluate("long double c = 14.; c", &HadError), 14.); | ||
EXPECT_FALSE(HadError) ; | ||
EXPECT_EQ(Cpp::Evaluate("long double d = 15.; d", &HadError), 15.); | ||
EXPECT_FALSE(HadError); | ||
EXPECT_EQ(Cpp::Evaluate("unsigned long long e = 16; e", &HadError), 16); | ||
EXPECT_FALSE(HadError) ; | ||
EXPECT_NE(Cpp::Evaluate("struct S{} s; s", &HadError), (intptr_t)~0UL); | ||
EXPECT_FALSE(HadError) ; | ||
} | ||
|
||
TEST(InterpreterTest, DeleteInterpreter) { | ||
auto* I1 = Cpp::CreateInterpreter(); | ||
auto* I2 = Cpp::CreateInterpreter(); | ||
|
Uh oh!
There was an error while loading. Please reload this page.