[Tarantool-patches] [PATCH v5 luajit 2/3] cmake: run tests with Valgrind

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

From: Maksim Tiushev <mandesero@gmail.com>

This patch enables running tests with Valgrind. There is a
`VALGRIND_OPTS` variable [1] that we can set -- it makes the usage of
valgrind more flexible -- we can define any necessary flags in the
command line (not at the building stage). By default, the suppression
files are set to <src/lj.supp> (original suppression file in LuaJIT) and
an additional one <src/lj_extra.supp> (maintained by us).

Also, this patch disables the following tests when running with Valgrind
due to failures:

Disabled due tarantool/tarantool#10803:
  - tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
  - tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
  - tarantool-tests/lj-726-profile-flush-close.test.lua
  - tarantool-tests/misclib-sysprof-lapi.test.lua
  - tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua

Timed out due to running under Valgrind:
  - tarantool-tests/gh-7745-oom-on-trace.test.lua
  - tarantool-tests/lj-1034-tabov-error-frame.test.lua
  - tarantool-c-tests/gh-8594-sysprof-ffunc-crash.c_test

[1]: https://valgrind.org/docs/manual/manual-core.html#manual-core.defopts

Part of tarantool/tarantool#3705
---
 CMakeLists.txt                                |  5 +++++
 src/lj_extra.supp                             | 19 +++++++++++++++++++
 test/CMakeLists.txt                           | 19 +++++++++++++++++++
 test/tarantool-c-tests/CMakeLists.txt         | 11 ++++++++++-
 .../gh-8594-sysprof-ffunc-crash.test.c        |  9 +++++++++
 test/tarantool-tests/CMakeLists.txt           |  6 ++++++
 .../gh-7745-oom-on-trace.test.lua             |  1 +
 .../lj-1034-tabov-error-frame.test.lua        |  1 +
 .../lj-512-profiler-hook-finalizers.test.lua  |  5 ++++-
 .../lj-726-profile-flush-close.test.lua       |  5 ++++-
 .../profilers/gh-5688-tool-cli-flag.test.lua  |  2 ++
 ...4-add-proto-trace-sysprof-default.test.lua |  2 ++
 .../profilers/misclib-sysprof-lapi.test.lua   |  2 ++
 13 files changed, 84 insertions(+), 3 deletions(-)
 create mode 100644 src/lj_extra.supp

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/src/lj_extra.supp b/src/lj_extra.supp
new file mode 100644
index 00000000..ac205f03
--- /dev/null
+++ b/src/lj_extra.supp
@@ -0,0 +1,19 @@
+# Valgrind suppression file maintained by the Tarantool team.
+
+# Missed from the original suppression file.
+# Unaligned access to the string, see <lj_str.c> for the details.
+{
+   Optimized string compare
+   Memcheck:Addr4
+   fun:lj_getu32
+   fun:str_fastcmp
+}
+
+# Missed from the original suppression file.
+# `lj_str_cmp()` may read up to 3 extra bytes from the end of the
+# string. It is OK due to the aligned allocations.
+{
+   Optimized string compare
+   Memcheck:Cond
+   fun:lj_str_cmp
+}
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 0db2dd8b..bda85ec1 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -69,6 +69,25 @@ 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)
+  list(APPEND LUAJIT_TEST_VALGRIND_SUPP
+    --suppressions=${LUAJIT_SOURCE_DIR}/lj.supp
+    --suppressions=${LUAJIT_SOURCE_DIR}/lj_extra.supp
+  )
+  set(LUAJIT_TEST_COMMAND
+    "${VALGRIND} --error-exitcode=1 "
+    "${LUAJIT_TEST_VALGRIND_SUPP} ${LUAJIT_TEST_COMMAND}")
+endif()
+
 separate_arguments(LUAJIT_TEST_COMMAND)
 
 set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt
index 30d174bb..5f6c45da 100644
--- a/test/tarantool-c-tests/CMakeLists.txt
+++ b/test/tarantool-c-tests/CMakeLists.txt
@@ -56,10 +56,19 @@ foreach(test_source ${tests})
 
   # Generate CMake tests.
   set(test_title "test/${TEST_SUITE_NAME}/${exe}${C_TEST_SUFFIX}")
+  set(test_command ${CMAKE_CURRENT_BINARY_DIR}/${exe}${C_TEST_SUFFIX})
+
+  if(LUAJIT_USE_VALGRIND)
+    set(test_command ${VALGRIND} --error-exitcode=1 ${LUAJIT_TEST_VALGRIND_SUPP}
+      ${test_command}
+    )
+  endif()
+
   add_test(NAME ${test_title}
-    COMMAND ${CMAKE_CURRENT_BINARY_DIR}/${exe}${C_TEST_SUFFIX}
+    COMMAND ${test_command}
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
   )
+
   set_tests_properties(${test_title} PROPERTIES
     LABELS ${TEST_SUITE_NAME}
     DEPENDS tarantool-c-tests-deps
diff --git a/test/tarantool-c-tests/gh-8594-sysprof-ffunc-crash.test.c b/test/tarantool-c-tests/gh-8594-sysprof-ffunc-crash.test.c
index cf1d815a..35108e77 100644
--- a/test/tarantool-c-tests/gh-8594-sysprof-ffunc-crash.test.c
+++ b/test/tarantool-c-tests/gh-8594-sysprof-ffunc-crash.test.c
@@ -46,6 +46,8 @@
  * * https://github.com/tarantool/tarantool/issues/9387
  */
 
+#define UNUSED(x) ((void)(x))
+
 #define MESSAGE "Canary is alive"
 #define LUACALL "local a = tostring('" MESSAGE "') return a"
 
@@ -248,6 +250,12 @@ static int tracer(pid_t chpid)
 
 static int test_tostring_call(void *ctx)
 {
+#if LUAJIT_USE_VALGRIND
+	UNUSED(ctx);
+	UNUSED(tracer);
+	UNUSED(tracee);
+	return skip("Disabled with Valgrind (Timeout)");
+#else
 	pid_t chpid = fork();
 	switch(chpid) {
 	case -1:
@@ -264,6 +272,7 @@ static int test_tostring_call(void *ctx)
 	default:
 		return tracer(chpid);
 	}
+#endif
 }
 
 #else /* LUAJIT_OS == LUAJIT_OS_LINUX && LUAJIT_TARGET == LUAJIT_ARCH_X64 */
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 8bdb2cf3..848c4cab 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -89,6 +89,12 @@ if(LUAJIT_ENABLE_TABLE_BUMP)
   list(APPEND LUA_TEST_ENV_MORE LUAJIT_TABLE_BUMP=1)
 endif()
 
+# Needed to skip some long tests or tests using signals.
+# See also https://github.com/tarantool/tarantool/issues/10803.
+if(LUAJIT_USE_VALGRIND)
+  list(APPEND LUA_TEST_ENV_MORE LUAJIT_TEST_USE_VALGRIND=1)
+endif()
+
 set(TEST_SUITE_NAME "tarantool-tests")
 
 # XXX: The call produces both test and target
diff --git a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
index fe320251..c481da24 100644
--- a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
+++ b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
@@ -6,6 +6,7 @@ local test = tap.test('OOM on trace'):skipcond({
                                                    (jit.os == 'OSX'),
   ['Disabled on MacOS due to #8652'] = jit.os == 'OSX',
   ['Test requires JIT enabled'] = not jit.status(),
+  ['Disabled with Valgrind (Timeout)'] = os.getenv("LUAJIT_TEST_USE_VALGRIND"),
 })
 
 test:plan(1)
diff --git a/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua b/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
index 0e23fdb2..de63b6d2 100644
--- a/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
+++ b/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
@@ -4,6 +4,7 @@ local test = tap.test('lj-1034-tabov-error-frame'):skipcond({
   ['Test requires JIT enabled'] = not jit.status(),
   ['Test requires GC64 mode enabled'] = not ffi.abi('gc64'),
   ['Disabled on MacOS due to #8652'] = jit.os == 'OSX',
+  ['Disabled with Valgrind (Timeout)'] = os.getenv("LUAJIT_TEST_USE_VALGRIND"),
 })
 
 -- XXX: The test for the problem uses the table of GC
diff --git a/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua b/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
index f02bd05f..32b12c65 100644
--- a/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
+++ b/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
@@ -1,7 +1,10 @@
 local tap = require('tap')
 local profile = require('jit.profile')
 
-local test = tap.test('lj-512-profiler-hook-finalizers')
+local test = tap.test('lj-512-profiler-hook-finalizers'):skipcond({
+  -- See also https://github.com/tarantool/tarantool/issues/10803.
+  ['Disabled due to #10803'] = os.getenv("LUAJIT_TEST_USE_VALGRIND"),
+})
 test:plan(1)
 
 -- Sampling interval in ms.
diff --git a/test/tarantool-tests/lj-726-profile-flush-close.test.lua b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
index 36cca43d..b26f0603 100644
--- a/test/tarantool-tests/lj-726-profile-flush-close.test.lua
+++ b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
@@ -1,6 +1,9 @@
 local tap = require('tap')
 
-local test = tap.test('lj-726-profile-flush-close')
+local test = tap.test('lj-726-profile-flush-close'):skipcond({
+  -- See also https://github.com/tarantool/tarantool/issues/10803.
+  ['Disabled due to #10803'] = os.getenv("LUAJIT_TEST_USE_VALGRIND"),
+})
 test:plan(1)
 
 local TEST_FILE = 'lj-726-profile-flush-close.profile'
diff --git a/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
index f935dc5b..ad0d6caf 100644
--- a/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
+++ b/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
@@ -7,6 +7,8 @@ local test = tap.test('gh-5688-tool-cli-flag'):skipcond({
   ['No profile tools CLI option integration'] = _TARANTOOL,
   -- See also https://github.com/LuaJIT/LuaJIT/issues/606.
   ['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
+  -- See also https://github.com/tarantool/tarantool/issues/10803.
+  ['Disabled due to #10803'] = os.getenv("LUAJIT_TEST_USE_VALGRIND"),
 })
 
 test:plan(3)
diff --git a/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
index 176c1a15..d9a3080e 100644
--- a/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
+++ b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
@@ -6,6 +6,8 @@ local test = tap.test('gh-7264-add-proto-trace-sysprof-default'):skipcond({
   ['Sysprof is implemented for Linux only'] = jit.os ~= 'Linux',
   -- See also https://github.com/LuaJIT/LuaJIT/issues/606.
   ['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
+  -- See also https://github.com/tarantool/tarantool/issues/10803.
+  ['Disabled due to #10803'] = os.getenv("LUAJIT_TEST_USE_VALGRIND"),
 })
 
 test:plan(2)
diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
index 26c277cd..605ff91c 100644
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
@@ -5,6 +5,8 @@ local test = tap.test("misc-sysprof-lapi"):skipcond({
   ["Sysprof is implemented for Linux only"] = jit.os ~= "Linux",
   -- See also https://github.com/LuaJIT/LuaJIT/issues/606.
   ["Disabled due to LuaJIT/LuaJIT#606"] = os.getenv("LUAJIT_TABLE_BUMP"),
+  -- See also https://github.com/tarantool/tarantool/issues/10803.
+  ['Disabled due to #10803'] = os.getenv("LUAJIT_TEST_USE_VALGRIND"),
 })
 
 test:plan(19)
-- 
2.47.0