Improve rs_port tests and improve memory leak detection for valgrind.

metacall · Dec 12, 2024 · 125d2e1 · 125d2e1
1 parent d901405
commit 125d2e1
Show file tree

Hide file tree

Showing 7 changed files with 98 additions and 39 deletions.
diff --git a/cmake/CompileOptions.cmake b/cmake/CompileOptions.cmake
@@ -73,8 +73,40 @@ if(OPTION_TEST_MEMORYCHECK)
 	set(MEMORYCHECK_COMPILE_DEFINITIONS
 		"__MEMORYCHECK__=1"
 	)
+
+	set(MEMORYCHECK_COMMAND_OPTIONS "--leak-check=full")
+	# set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --show-leak-kinds=all")
+	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --trace-children=yes")
+	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --show-reachable=yes")
+	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --track-origins=yes")
+	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --num-callers=100")
+	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --smc-check=all-non-file") # for JITs
+	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=${CMAKE_SOURCE_DIR}/source/tests/memcheck/valgrind-dl.supp")
+	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=${CMAKE_SOURCE_DIR}/source/tests/memcheck/valgrind-python.supp")
+	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=${CMAKE_SOURCE_DIR}/source/tests/memcheck/valgrind-node.supp")
+	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=${CMAKE_SOURCE_DIR}/source/tests/memcheck/valgrind-wasm.supp")
+	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=${CMAKE_SOURCE_DIR}/source/tests/memcheck/valgrind-wasm.supp")
+
+	# TODO: Implement automatic detection for valgrind suppressions and create a proper test suite for the CI
+	set(MEMORYCHECK_ADDITIONAL_SUPPRESSIONS
+		"/usr/lib/valgrind/python3.supp"
+		"/usr/lib/valgrind/debian.supp"
+	)
+
+	foreach(SUPPRESSION ${MEMORYCHECK_ADDITIONAL_SUPPRESSIONS})
+		if(EXISTS "${SUPPRESSION}")
+			set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=${SUPPRESSION}")
+		endif()
+	endforeach()
+
+	# This is needed in order to allow valgrind to properly track malloc in Python
+	set(TESTS_MEMCHECK_ENVIRONMENT_VARIABLES
+		"PYTHONMALLOC=malloc"
+	)
 else()
 	set(MEMORYCHECK_COMPILE_DEFINITIONS)
+	set(MEMORYCHECK_COMMAND_OPTIONS)
+	set(TESTS_MEMCHECK_ENVIRONMENT_VARIABLES)
 endif()
 
 # ThreadSanitizer is incompatible with AddressSanitizer and LeakSanitizer

diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt
@@ -65,6 +65,7 @@ set(TESTS_ENVIRONMENT_VARIABLES
 	${TESTS_DETOUR_ENVIRONMENT_VARIABLES}
 	${TESTS_PORT_ENVIRONMENT_VARIABLES}
 	${TESTS_SANITIZER_ENVIRONMENT_VARIABLES}
+	${TESTS_MEMCHECK_ENVIRONMENT_VARIABLES}
 	${EXTRA_ENVIRONMENT_VARIABLES}
 )
 

diff --git a/source/loaders/c_loader/source/c_loader_impl.cpp b/source/loaders/c_loader/source/c_loader_impl.cpp
@@ -1121,6 +1121,7 @@ static int c_loader_impl_discover_ast(loader_impl impl, loader_impl_c_handle_bas
 		if (unit == nullptr)
 		{
 			log_write("metacall", LOG_LEVEL_ERROR, "Unable to parse translation unit of: %s", file.c_str());
+			clang_disposeIndex(index);
 			return -1;
 		}
 

diff --git a/source/ports/rs_port/CMakeLists.txt b/source/ports/rs_port/CMakeLists.txt
@@ -80,10 +80,7 @@ if(OPTION_BUILD_ADDRESS_SANITIZER AND "${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang"
 		"${Rust_TOOLCHAIN_TRIPLET}" STREQUAL "x86_64-unknown-linux-gnu")
 		set(RS_PORT_RUN_INSTRUMENTED_TEST ON)
 		set(RUSTUP_NIGTHLY_INSTALL_SRC ON)
-		set(SANITIZER_FLAGS
-			CMAKE_ADDRESS_SANITIZER=1
-			RUSTFLAGS=-Zsanitizer=address
-		)
+		set(SANITIZER_FLAGS "RUSTFLAGS=-Zsanitizer=address")
 		set(NIGHTLY_FLAGS
 			+nightly-${RS_PORT_INSTRUMENTATION_NIGHTLY_VERSION}
 		)
@@ -100,10 +97,7 @@ elseif(OPTION_BUILD_THREAD_SANITIZER AND "${CMAKE_CXX_COMPILER_ID}" MATCHES "Cla
 		"${Rust_TOOLCHAIN_TRIPLET}" STREQUAL "x86_64-unknown-linux-gnu")
 		set(RS_PORT_RUN_INSTRUMENTED_TEST ON)
 		set(RUSTUP_NIGTHLY_INSTALL_SRC ON)
-		set(SANITIZER_FLAGS
-			CMAKE_THREAD_SANITIZER=1
-			RUSTFLAGS=-Zsanitizer=thread
-		)
+		set(SANITIZER_FLAGS "RUSTFLAGS=-Zsanitizer=thread")
 		set(NIGHTLY_FLAGS
 			+nightly-${RS_PORT_INSTRUMENTATION_NIGHTLY_VERSION}
 		)
@@ -154,8 +148,22 @@ if((OPTION_BUILD_ADDRESS_SANITIZER OR OPTION_BUILD_THREAD_SANITIZER) AND NOT RS_
 	return()
 endif()
 
+# If we have cargo-valgrind, run the tests with it
+if(OPTION_TEST_MEMORYCHECK AND NOT (OPTION_BUILD_ADDRESS_SANITIZER OR OPTION_BUILD_THREAD_SANITIZER))
+	# Check if cargo-valgrind is installed
+	execute_process(
+		COMMAND ${Rust_CARGO_EXECUTABLE} install --list
+		OUTPUT_VARIABLE CARGO_VALGRIND_INSTALL
+	)
+	string(FIND "${CARGO_VALGRIND_INSTALL}" "cargo-valgrind" CARGO_VALGRIND_INSTALLED)
+	if(NOT ${CARGO_VALGRIND_INSTALLED} EQUAL -1)
+		set(CARGO_VALGRIND valgrind)
+		set(CARGO_VALGRIND_FLAGS "VALGRINDFLAGS=${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=${CMAKE_CURRENT_SOURCE_DIR}/valgrind-rust.supp")
+	endif()
+endif()
+
 add_test(NAME ${target}
-	COMMAND ${Rust_CARGO_EXECUTABLE} ${NIGHTLY_FLAGS} test ${BUILD_STD_FLAGS}
+	COMMAND ${Rust_CARGO_EXECUTABLE} ${CARGO_VALGRIND} ${NIGHTLY_FLAGS} test ${BUILD_STD_FLAGS}
 	WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
 )
 
@@ -177,5 +185,6 @@ test_environment_variables(${target}
 	"CMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}"
 	"PROJECT_OUTPUT_DIR=${PROJECT_OUTPUT_DIR}"
 	"RUST_BACKTRACE=1"
-	${SANITIZER_FLAGS}
+	"${SANITIZER_FLAGS}"
+	"${CARGO_VALGRIND_FLAGS}"
 )
diff --git a/source/ports/rs_port/tests/metacall_test.rs b/source/ports/rs_port/tests/metacall_test.rs
@@ -347,6 +347,12 @@ fn metacall() {
         test_class();
         test_object();
         test_pointer();
+        test_array();
+        test_bool();
+        test_function();
+        test_map();
+        test_string();
+        test_null();
     }
     if loaders::from_single_file("c", c_test_file).is_ok() {
         test_char();
@@ -357,13 +363,7 @@ fn metacall() {
         test_short();
     }
     if loaders::from_single_file("node", js_test_file).is_ok() {
-        test_array();
-        test_bool();
         test_exception();
-        test_function();
-        test_map();
-        test_null();
-        test_string();
         test_throwable();
         test_future();
     }

diff --git a/source/ports/rs_port/valgrind-rust.supp b/source/ports/rs_port/valgrind-rust.supp
@@ -0,0 +1,39 @@
+{
+    Ignore memory leaked from lang_start_internal.
+    Memcheck:Leak
+    ...
+    fun:new_inner
+    ...
+}
+
+# Ignore leaks from loaders depenecies for now, we want to debug the port only
+{
+    Ignore leaks from C Loader.
+    Memcheck:Leak
+    ...
+    obj:*/libc_loader*.so
+}
+{
+    Ignore leaks from C Loader depends Clang.
+    Memcheck:Leak
+    ...
+    obj:*/libclang-*.so.*
+}
+{
+    Ignore leaks from C Loader depends LLVM.
+    Memcheck:Leak
+    ...
+    obj:*/libLLVM-*.so.*
+}
+{
+    Ignore leaks from NodeJS Loader.
+    Memcheck:Leak
+    ...
+    obj:*/libnode_loader*.so
+}
+{
+    Ignore leaks from Python Loader depends CPython.
+    Memcheck:Leak
+    ...
+    obj:*/libpython*.so.*
+}
diff --git a/source/tests/CMakeLists.txt b/source/tests/CMakeLists.txt
@@ -53,23 +53,6 @@ if(OPTION_TEST_MEMORYCHECK AND (OPTION_BUILD_ADDRESS_SANITIZER OR OPTION_BUILD_T
 endif()
 
 if(OPTION_TEST_MEMORYCHECK AND NOT (OPTION_BUILD_ADDRESS_SANITIZER OR OPTION_BUILD_THREAD_SANITIZER OR OPTION_BUILD_MEMORY_SANITIZER))
-	# set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --show-leak-kinds=all")
-	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --leak-check=full")
-	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --trace-children=yes")
-	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --show-reachable=yes")
-	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --track-origins=yes")
-	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --num-callers=100")
-	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --smc-check=all-non-file") # for JITs
-	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=${CMAKE_CURRENT_SOURCE_DIR}/memcheck/valgrind-dl.supp")
-	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=${CMAKE_CURRENT_SOURCE_DIR}/memcheck/valgrind-python.supp")
-	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=${CMAKE_CURRENT_SOURCE_DIR}/memcheck/valgrind-node.supp")
-	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=${CMAKE_CURRENT_SOURCE_DIR}/memcheck/valgrind-wasm.supp")
-	set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=${CMAKE_CURRENT_SOURCE_DIR}/memcheck/valgrind-wasm.supp")
-
-	# TODO: Implement automatic detection for valgrind suppressions and create a proper test suite for the CI
-	# set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=/usr/lib/valgrind/python3.supp")
-	# set(MEMORYCHECK_COMMAND_OPTIONS "${MEMORYCHECK_COMMAND_OPTIONS} --suppressions=/usr/lib/valgrind/debian.supp")
-
 	# TODO: Memory check does not work properly with CoreCLR
 	#
 	# 	Remove MEMCHECK_IGNORE label from the following tests:
@@ -93,12 +76,6 @@ if(OPTION_TEST_MEMORYCHECK AND NOT (OPTION_BUILD_ADDRESS_SANITIZER OR OPTION_BUI
 			--timeout 5400
 		COMMAND ${CMAKE_COMMAND} -E cat "${CMAKE_BINARY_DIR}/Testing/Temporary/MemoryChecker.*.log"
 	)
-
-	# This is needed in order to allow valgrind to properly track malloc in Python
-	set(TESTS_ENVIRONMENT_VARIABLES
-		${TESTS_ENVIRONMENT_VARIABLES}
-		"PYTHONMALLOC=malloc"
-	)
 endif()
 
 #