[Tarantool-patches] [PATCH v2 luajit 1/3] cmake: run tests with Valgrind cmake: run tests with Valgrind

[mandesero at gmail.com]

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