Skip to content

Fix all crashing tests for emscripten build #523

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

Merged
merged 5 commits into from
Mar 17, 2025
Merged

Fix all crashing tests for emscripten build #523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d5850b0
Fix all crashing tests for emscripten build
anutosh491 Mar 14, 2025
ac4f7dd
clang format reviews
anutosh491 Mar 17, 2025
b6e20f5
some reviews
anutosh491 Mar 17, 2025
05b6c46
Apply some suggestions
anutosh491 Mar 17, 2025
15b51c8
Merge branch 'main' into improve_test1
mcbarton Mar 17, 2025
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
10 changes: 6 additions & 4 deletions include/clang/Interpreter/CppInterOp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,15 @@ namespace Cpp {
: m_Kind(K), m_DestructorCall(C), m_FD(Dtor) {}

/// Checks if the passed arguments are valid for the given function.
bool AreArgumentsValid(void* result, ArgList args, void* self) const;
CPPINTEROP_API bool AreArgumentsValid(void* result, ArgList args,
void* self) const;

/// This function is used for debugging, it reports when the function was
/// called.
void ReportInvokeStart(void* result, ArgList args, void* self) const;
void ReportInvokeStart(void* object, unsigned long nary,
int withFree) const;
CPPINTEROP_API void ReportInvokeStart(void* result, ArgList args,
void* self) const;
CPPINTEROP_API void ReportInvokeStart(void* object, unsigned long nary,
Copy link
Collaborator Author

@anutosh491 anutosh491 Mar 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed for tests in FunctionReflectionTest like JitCallAdvanced, GetFunctionCallWrapper etc cause otherwise we see

error: undefined symbol: _ZNK3Cpp7JitCall17AreArgumentsValidEPvNS0_7ArgListES1_ (referenced by root reference (e.g. compiled C/C++ code))
warning: To disable errors for undefined symbols use `-sERROR_ON_UNDEFINED_SYMBOLS=0`
warning: __ZNK3Cpp7JitCall17AreArgumentsValidEPvNS0_7ArgListES1_ may need to be added to EXPORTED_FUNCTIONS if it arrives from a system library
error: undefined symbol: _ZNK3Cpp7JitCall17ReportInvokeStartEPvNS0_7ArgListES1_ (referenced by root reference (e.g. compiled C/C++ code))
warning: __ZNK3Cpp7JitCall17ReportInvokeStartEPvNS0_7ArgListES1_ may need to be added to EXPORTED_FUNCTIONS if it arrives from a system library
error: undefined symbol: _ZNK3Cpp7JitCall17ReportInvokeStartEPvmi (referenced by root reference (e.g. compiled C/C++ code))
warning: __ZNK3Cpp7JitCall17ReportInvokeStartEPvmi may need to be added to EXPORTED_FUNCTIONS if it arrives from a system library
Error: Aborting compilation due to previous errors

int withFree) const;
void ReportInvokeEnd() const;
public:
Kind getKind() const { return m_Kind; }
Expand Down
9 changes: 9 additions & 0 deletions lib/Interpreter/exports.ld
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,12 @@
-Wl,--export=_ZNK5clang7TagType7getDeclEv
-Wl,--export=_ZNK5clang7VarDecl28isThisDeclarationADefinitionERNS_10ASTContextE
-Wl,--export=_ZTVN4llvm18raw_string_ostreamE
-Wl,--export=_ZN4llvm13StringMapImpl11RehashTableEj
-Wl,--export=_ZN4llvm13StringMapImpl15LookupBucketForENS_9StringRefEj
-Wl,--export=_ZN4llvm13StringMapImpl4hashENS_9StringRefE
-Wl,--export=_ZNK5clang10ASTContext14getComplexTypeENS_8QualTypeE
-Wl,--export=_ZNK5clang10ASTContext19getTypeDeclTypeSlowEPKNS_8TypeDeclE
-Wl,--export=_ZNK5clang10RecordDecl19isInjectedClassNameEv
-Wl,--export=_ZNK5clang11DeclContext6lookupENS_15DeclarationNameE
-Wl,--export=_ZNK5clang17ClassTemplateDecl18getSpecializationsEv
-Wl,--export=_ZNK5clang4Sema15getStdNamespaceEv
55 changes: 35 additions & 20 deletions unittests/CppInterOp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,33 @@ add_cppinterop_unittest(CppInterOpTests
)

if(EMSCRIPTEN)
# Explaination of compile and link flags
# To dynamically link to a shared library for Emscripten builds you must use the MAIN_MODULE flag (see both https://github.com/emscripten-core/emscripten/issues/23543#issuecomment-2625334414
# and https://emscripten.org/docs/compiling/Dynamic-Linking.html)
# Without WASM_BIGINT flag then you get fatal errors when trying to run compiled Javascript about trying to convert between BigInt and non BigInt types
# EXPORTED_RUNTIME_METHODS='[\"FS\",\"PATH\",\"LDSO\",\"loadDynamicLibrary\",\"ERRNO_CODES\"]' and --preload-file ${SYSROOT_PATH}/include@/include are not allow the Javascript that is
# compiled to have access to the standard library headers (approach taken from xeus-cpp)
# Without ALLOW_MEMORY_GROWTH=1 tests will fail with aborted(OOM). Approach to fix taken from answers to
# https://stackoverflow.com/questions/67222200/runtimeerror-abortoom-build-with-s-assertions-1-for-more-info
set_target_properties(CppInterOpTests PROPERTIES
LINK_FLAGS "-s MAIN_MODULE=1 -s WASM_BIGINT -s EXPORTED_RUNTIME_METHODS='[\"FS\",\"PATH\",\"LDSO\",\"loadDynamicLibrary\",\"ERRNO_CODES\"]' --preload-file ${SYSROOT_PATH}/include@/include -s ALLOW_MEMORY_GROWTH=1"
# Explanation of Emscripten-specific link flags for CppInterOpTests:
#
# MAIN_MODULE=1:
# Enables building CppInterOpTests.js as the main WebAssembly module, allowing dynamic linking of side modules.
#
# WASM_BIGINT:
# Ensures support for 64-bit integer types by enabling JavaScript BigInt integration in WASM.
#
# ALLOW_MEMORY_GROWTH=1:
# Allows the WebAssembly memory to grow dynamically at runtime to accommodate increasing memory needs.
# Would lead to an abortOnCannotGrowMemory error if memory cannot be grown while running the tests.
#
# STACK_SIZE=32mb: Allocates 32MB of stack space to handle deep recursion or large stack-allocated objects safely.
# INITIAL_MEMORY=128mb: Sets the initial linear memory size to 128MB to reduce the likelihood of early memory expansion and improve performance.
# The STACK_SIZE and INITIAL_MEMORY values are chosen based on what has been put to use for running xeus-cpp-lite.
# Check https://github.com/jupyter-xeus/xeus/blob/main/cmake/WasmBuildOptions.cmake#L35-L36 for more details.
# Not setting these flags would lead to a memory access out of bounds error while running the tests.
#
# --preload-file ${SYSROOT_PATH}/include@/include:
# Preloads the system include directory into the Emscripten virtual filesystem to make headers accessible at runtime.
target_link_options(CppInterOpTests
PUBLIC "SHELL: -s MAIN_MODULE=1"
PUBLIC "SHELL: -s WASM_BIGINT"
PUBLIC "SHELL: -s ALLOW_MEMORY_GROWTH=1"
PUBLIC "SHELL: -s STACK_SIZE=32mb"
PUBLIC "SHELL: -s INITIAL_MEMORY=128mb"
PUBLIC "SHELL: --preload-file ${SYSROOT_PATH}/include@/include"
)
endif()

Expand Down Expand Up @@ -69,16 +86,14 @@ target_link_libraries(DynamicLibraryManagerTests
)

if(EMSCRIPTEN)
# Explaination of compile and link flags
# To dynamically link to a shared library for Emscripten builds you must use the MAIN_MODULE flag (see both https://github.com/emscripten-core/emscripten/issues/23543#issuecomment-2625334414
# and https://emscripten.org/docs/compiling/Dynamic-Linking.html)
# Without WASM_BIGINT flag then you get fatal errors when trying to run compiled Javascript about trying to convert between BigInt and non BigInt types
# EXPORTED_RUNTIME_METHODS='[\"FS\",\"PATH\",\"LDSO\",\"loadDynamicLibrary\",\"ERRNO_CODES\"]' and --preload-file ${SYSROOT_PATH}/include@/include are not allow the Javascript that is
# compiled to have access to the standard library headers (approach taken from xeus-cpp)
# Without ALLOW_MEMORY_GROWTH=1 tests will fail with aborted(OOM). Approach to fix taken from answers to
# https://stackoverflow.com/questions/67222200/runtimeerror-abortoom-build-with-s-assertions-1-for-more-info
set_target_properties(DynamicLibraryManagerTests PROPERTIES
LINK_FLAGS "-s MAIN_MODULE=1 -s WASM_BIGINT -s EXPORTED_RUNTIME_METHODS='[\"FS\",\"PATH\",\"LDSO\",\"loadDynamicLibrary\",\"ERRNO_CODES\"]' --preload-file ${SYSROOT_PATH}/include@/include -s ALLOW_MEMORY_GROWTH=1"
# Check explanation of Emscripten-specific link flags for CppInterOpTests above for DynamicLibraryManagerTests as well.
target_link_options(DynamicLibraryManagerTests
PUBLIC "SHELL: -s MAIN_MODULE=1"
PUBLIC "SHELL: -s WASM_BIGINT"
PUBLIC "SHELL: -s ALLOW_MEMORY_GROWTH=1"
PUBLIC "SHELL: -s STACK_SIZE=32mb"
PUBLIC "SHELL: -s INITIAL_MEMORY=128mb"
PUBLIC "SHELL: --preload-file ${SYSROOT_PATH}/include@/include"
)
endif()

Expand Down
9 changes: 3 additions & 6 deletions unittests/CppInterOp/FunctionReflectionTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,9 +547,6 @@ TEST(FunctionReflectionTest, ExistsFunctionTemplate) {
}

TEST(FunctionReflectionTest, InstantiateTemplateFunctionFromString) {
#ifdef EMSCRIPTEN
GTEST_SKIP() << "Test crashes gtest on Emscipten";
#endif
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";
Cpp::CreateInterpreter();
Expand Down Expand Up @@ -1053,7 +1050,7 @@ TEST(FunctionReflectionTest, IsStaticMethod) {

TEST(FunctionReflectionTest, GetFunctionAddress) {
#ifdef EMSCRIPTEN
GTEST_SKIP() << "Test crashes gtest on Emscipten";
GTEST_SKIP() << "Test fails for Emscipten builds";
#endif
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";
Expand Down Expand Up @@ -1100,7 +1097,7 @@ TEST(FunctionReflectionTest, IsVirtualMethod) {

TEST(FunctionReflectionTest, JitCallAdvanced) {
#ifdef EMSCRIPTEN
GTEST_SKIP() << "Test crashes gtest on Emscipten";
GTEST_SKIP() << "Test fails for Emscipten builds";
#endif
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";
Expand All @@ -1127,7 +1124,7 @@ TEST(FunctionReflectionTest, JitCallAdvanced) {

TEST(FunctionReflectionTest, GetFunctionCallWrapper) {
#ifdef EMSCRIPTEN
GTEST_SKIP() << "Test crashes gtest on Emscipten";
GTEST_SKIP() << "Test fails for Emscipten builds";
#endif
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";
Expand Down
12 changes: 0 additions & 12 deletions unittests/CppInterOp/ScopeReflectionTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,6 @@ TEST(ScopeReflectionTest, SizeOf) {


TEST(ScopeReflectionTest, IsBuiltin) {
#ifdef EMSCRIPTEN
GTEST_SKIP() << "Test crashes gtest on Emscipten";
#endif
// static std::set<std::string> g_builtins =
// {"bool", "char", "signed char", "unsigned char", "wchar_t", "short", "unsigned short",
// "int", "unsigned int", "long", "unsigned long", "long long", "unsigned long long",
Expand Down Expand Up @@ -494,9 +491,6 @@ TEST(ScopeReflectionTest, GetScopefromCompleteName) {
}

TEST(ScopeReflectionTest, GetNamed) {
#ifdef EMSCRIPTEN
GTEST_SKIP() << "Test crashes gtest on Emscipten";
#endif
std::string code = R"(namespace N1 {
namespace N2 {
class C {
Expand Down Expand Up @@ -882,9 +876,6 @@ template<typename T> T TrivialFnTemplate() { return T(); }
}

TEST(ScopeReflectionTest, InstantiateTemplateFunctionFromString) {
#ifdef EMSCRIPTEN
GTEST_SKIP() << "Test crashes gtest on Emscipten";
#endif
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";
Cpp::CreateInterpreter();
Expand Down Expand Up @@ -1025,9 +1016,6 @@ TEST(ScopeReflectionTest, GetClassTemplateInstantiationArgs) {


TEST(ScopeReflectionTest, IncludeVector) {
#ifdef EMSCRIPTEN
GTEST_SKIP() << "Test crashes gtest on Emscipten";
#endif
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";
std::string code = R"(
Expand Down
3 changes: 0 additions & 3 deletions unittests/CppInterOp/TypeReflectionTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,9 +546,6 @@ TEST(TypeReflectionTest, IsPODType) {
}

TEST(TypeReflectionTest, IsSmartPtrType) {
#ifdef EMSCRIPTEN
GTEST_SKIP() << "Test crashes gtest on Emscipten";
#endif
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";
Cpp::CreateInterpreter();
Expand Down
5 changes: 1 addition & 4 deletions unittests/CppInterOp/VariableReflectionTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ CODE

TEST(VariableReflectionTest, GetVariableOffset) {
#ifdef EMSCRIPTEN
GTEST_SKIP() << "Test crashes gtest on Emscipten";
GTEST_SKIP() << "Test fails for Emscipten builds";
#endif
std::vector<Decl *> Decls;
#define Stringify(s) Stringifyx(s)
Expand Down Expand Up @@ -331,9 +331,6 @@ TEST(VariableReflectionTest, GetVariableOffset) {
CODE

TEST(VariableReflectionTest, VariableOffsetsWithInheritance) {
#ifdef EMSCRIPTEN
GTEST_SKIP() << "Test crashes gtest on Emscipten";
#endif
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";

Expand Down
Loading