[Tarantool-patches] [PATCH v3 luajit 1/2] cmake: run tests with Valgrind
mandesero at gmail.com
mandesero at gmail.com
Fri Nov 29 18:35:08 MSK 2024
From: Maksim Tiushev <mandesero at gmail.com>
Hi, Sergey!
Thanks for the review!
I rewrote the commit message for this patch. See the updated version below.
> I suppose that we can drop $ENV{VALGRIND_OPTIONS}, since 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).
I have removed the `VALGRIND_OPTIONS` environment variable and replaced it
with the `VALGRIND_OPTS` variable provided by Valgrind. I tested this
change and verified that everything works correctly.
> Therefore, `LUAJIT_TEST_VALGRIND_OPTS` may be renamed to
> `LUAJIT_TEST_VALGRIND_SUPP`.
Agreed. Since this variable specifically handles suppression files (unlike
general Valgrind options), renaming it to `LUAJIT_TEST_VALGRIND_SUPP` is a
good idea.
> It is better to set this variable to the `LUA_TEST_ENV_MORE` list.
> Also, I suggest rename it to the `LUAJIT_TEST_USE_VALGRIND`.
Done, See the updated version below.
> I also get the following error locally:
> | test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua (SIGPROF)
This test is now disabled with Valgrind.
> Also, I've found that tests from LuaJIT-tests suite reports tons of the
> assertions, but still the suite passes. This should be fixed:
>
> I suppose that we need to set `--error-exitcode` option as well.
<snipped>
> Same for PUC-Rio-Lua-5.1-tests, lua-Harness-test.
> ...
I noticed that Valgrind generates a significant number of reports, but it
seems these are merely warnings or informational messages. At the end, the
errors appear to be suppressed, as indicated in the output.
| 1/2 Test #1: LuaJIT-tests-deps ................ Passed 0.50 sec
| test 2
| Start 2: test/LuaJIT-tests
|
| 2: Test command: /usr/bin/valgrind "--suppressions=/home/mandesero/myforks/luajit/src/lj.supp" "/home/mandesero/myforks/luajit/build/src/luajit" "-e" "dofile[[/home/mandesero/myforks/luajit/test/luajit-test-init.lua]]" "/home/mandesero/myforks/luajit/test/LuaJIT-tests/test.lua" "+slow" "+ffi" "+bit" "+jit"
|
| 2: --6813-- used_suppression: 62656 Optimized string compare /home/mandesero/myforks/luajit/src/lj.supp:38
| 2: ==6813==
| 2: ==6813== ERROR SUMMARY: 4707 errors from 27 contexts (suppressed: 62656 from 111)
Same for others tests.
> Also, I confused with the Command content,
> It looks like the valgrind with its options is missed.
I'm also puzzled by the your command content.
As for my test command, as you can see, it works correctly.
> I suppose that we need to set `--error-exitcode` option as well.
I have added the `--error-exitcode` option in the next patch.
> Also, it would be nice to run all tarantool-c-tests under valgrind.
Done. `tarantool-c-tests/gh-8594-sysprof-ffunc-crash.c_test` was
disabled due to timeout.
Branch: https://github.com/tarantool/luajit/tree/mandesero/lj-3705-turn-off-strcmp-opt-in-debug
--
Best regards,
Maksim Tiushev
========================================================================
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 file is set to `src/lj.supp`.
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 +++++
test/CMakeLists.txt | 16 ++++++++++++++++
test/tarantool-c-tests/CMakeLists.txt | 9 ++++++++-
.../gh-8594-sysprof-ffunc-crash.test.c | 7 +++++++
.../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 ++
...7264-add-proto-trace-sysprof-default.test.lua | 2 ++
.../profilers/misclib-sysprof-lapi.test.lua | 2 ++
11 files changed, 52 insertions(+), 3 deletions(-)
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..b93cd0f6 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_SUPP "--suppressions=${LUAJIT_SOURCE_DIR}/lj.supp")
+ set(LUAJIT_TEST_COMMAND
+ "${VALGRIND} ${LUAJIT_TEST_VALGRIND_SUPP} ${LUAJIT_TEST_COMMAND}")
+ list(APPEND LUA_TEST_ENV_MORE LUAJIT_TEST_USE_VALGRIND=1)
+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..264f6740 100644
--- a/test/tarantool-c-tests/CMakeLists.txt
+++ b/test/tarantool-c-tests/CMakeLists.txt
@@ -56,10 +56,17 @@ 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} ${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..569a2e80 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,10 @@ static int tracer(pid_t chpid)
static int test_tostring_call(void *ctx)
{
+#if LUAJIT_USE_VALGRIND
+ UNUSED(ctx);
+ return skip("Disabled with Valgrind (Timeout)");
+#else
pid_t chpid = fork();
switch(chpid) {
case -1:
@@ -264,6 +270,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/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.34.1
More information about the Tarantool-patches
mailing list