[Tarantool-patches] [PATCH v2 luajit 1/3] cmake: run tests with Valgrind cmake: run tests with Valgrind
mandesero at gmail.com
mandesero at gmail.com
Mon Sep 16 18:50:57 MSK 2024
From: Maksim Tiushev <mandesero at gmail.com>
Hi, Sergey!
Thanks for the review!
Changes:
1. Requested by Sergey Bronnikov
> realloc -> realloc()
> to highlight that realloc is a function
- This message was dropped by Sergey Kaplun.
> In the commit message, you say that VALGRIND_OPTIONS is an env variable.
> However, it is not:
- You're right, it doesn't work in the scenario you provided. I've
updated the commit message to be more informative:
"Custom Valgrind testing options can be configured by setting the
`VALGRIND_OPTIONS` variable before running `cmake <..params..>`.
To update the value of this variable, modify the environment variable
`VALGRIND_OPTIONS` and rebuild the project using `cmake <..params..>`
again."
> Also, replace "valgrind" in LUAJIT_TEST_COMMAND by ${VALGRIND_BIN} and
> put "find_program(VALGRIND valgrind)" before:
>
> <snipped>
>
> I would add default set() with "LUAJIT_TEST_COMMAND" and remove a second
> branch:
- I kept the default `LUAJIT_TEST_COMMAND` and added a conditional branch
for `LUAJIT_USE_VALGRIND` with the provided changes.
2. Requested by Sergey Kaplun
> It is better to be a warning by analog with the ASAN build.
> <snipped>
> I don't get this comment. Why valgrind depends on x64, and GC64 as well?
> I suppose we can drop the comment and the check above.
- I changed the message level to warning and removed the last one.
> Please use `LUAJIT_SOURCE_DIR` instead.
- Done.
---
CMakeLists.txt | 5 +++++
test/CMakeLists.txt | 16 ++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index aa2103b3..854b3613 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -289,6 +289,11 @@ endif()
# ASan enabled.
option(LUAJIT_USE_ASAN "Build LuaJIT with AddressSanitizer" OFF)
if(LUAJIT_USE_ASAN)
+ if(LUAJIT_USE_VALGRIND)
+ message(FATAL_ERROR
+ "AddressSanitizer and Valgrind cannot be used simultaneously."
+ )
+ endif()
if(NOT LUAJIT_USE_SYSMALLOC)
message(WARNING
"Unfortunately, internal LuaJIT memory allocator is not instrumented yet,"
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 0db2dd8b..3cc0ddbc 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -69,6 +69,22 @@ add_custom_target(${PROJECT_NAME}-lint DEPENDS
)
set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
+
+if(LUAJIT_USE_VALGRIND)
+ if (NOT LUAJIT_USE_SYSMALLOC)
+ message(WARNING
+ "LUAJIT_USE_SYSMALLOC option is mandatory for Valgrind's memcheck tool"
+ " on x64 and the only way to get useful results from it for all other"
+ " architectures.")
+ endif()
+
+ find_program(VALGRIND valgrind)
+ set(LUAJIT_TEST_VALGRIND_OPTS
+ "--suppressions=${LUAJIT_SOURCE_DIR}/lj.supp $ENV{VALGRIND_OPTIONS}")
+ set(LUAJIT_TEST_COMMAND
+ "${VALGRIND} ${LUAJIT_TEST_VALGRIND_OPTS} ${LUAJIT_TEST_COMMAND}")
+endif()
+
separate_arguments(LUAJIT_TEST_COMMAND)
set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
--
2.34.1
More information about the Tarantool-patches
mailing list